Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lora-e5 plus sdcard does not work #18975

Closed
jdavid opened this issue Nov 25, 2022 · 8 comments
Closed

lora-e5 plus sdcard does not work #18975

jdavid opened this issue Nov 25, 2022 · 8 comments

Comments

@jdavid
Copy link
Contributor

jdavid commented Nov 25, 2022

Description

A SD card attached to the lora-e5-dev board does not work.

Steps to reproduce the issue

To quickly test I added this to the boards/lora-e5-dev/include/board.h file:

+#define SPI_CS GPIO_PIN(PORT_B, 9)              // NSS
+#define SPI_SCLK GPIO_PIN(PORT_B, 13)           // SCK
+#define SPI_MOSI GPIO_PIN(PORT_A, 10)           // MOSI
+#define SPI_MISO GPIO_PIN(PORT_B, 14)           // MISO
+#define CARD_DETECT_PIN (GPIO_PIN(PORT_B, 10))  // D10
+
+#define SDCARD_SPI_PARAM_SPI        SPI_DEV(0)  /**< SPI device */
+#define SDCARD_SPI_PARAM_CS         SPI_CS      /**< Chip Select */
+#define SDCARD_SPI_PARAM_CLK        SPI_SCLK    /**< Serial Clock */
+#define SDCARD_SPI_PARAM_MOSI       SPI_MOSI    /**< Master Output, Slave Input */
+#define SDCARD_SPI_PARAM_MISO       SPI_MISO    /**< Master Input, Slave Output */

Then I tried the test program tests/driver_sdcard_spi

Tried with 2 boards, one LoRa-E5 mini and one LoRa-E5 development. And 2 microSD cards. And 3 different SD card readers. All gave the same error. These are the SD card readers:

For the last one I tried with Arduino and it worked. So I think the sd card modules are fine.

Expected results

I expected the test to work, something like:

2022-08-12 11:26:32,527 # init
2022-08-12 11:26:32,686 # Initializing SD-card at SPI_0...[OK]

Actual results

$ BOARD=lora-e5-dev make -C tests/driver_sdcard_spi all flash term
[...]
> init
2022-11-25 11:35:39,862 # init
2022-11-25 11:35:40,175 # Initializing SD-card at SPI_0...[FAILED]
2022-11-25 11:35:40,180 # enable debugging in sdcard_spi.c for more information!

With debugging enabled this is the output:

2022-11-25 11:37:23,761 # init
2022-11-25 11:37:23,762 # Initializing SD-card at SPI_0...SD_INIT_START
2022-11-25 11:37:23,765 # gpio_init(): [OK]
2022-11-25 11:37:23,766 # SD_INIT_SPI_POWER_SEQ
2022-11-25 11:37:23,773 # SD_INIT_SEND_CMD0
2022-11-25 11:37:23,779 # sdcard_spi_send_cmd: CMD00 (0x00000000) (remaining retry time 89 usec)
2022-11-25 11:37:23,780 # _wait_for_not_busy: [OK]
2022-11-25 11:37:23,787 # CMD00 echo: 0xFF 0xFF 0xFF 0xFF 0xFF 0xFF 
2022-11-25 11:37:23,792 # _wait_for_r1: r1=0xff
2022-11-25 11:37:23,793 # _wait_for_r1: r1=0x01
2022-11-25 11:37:23,793 # _wait_for_r1: R1_VALID
2022-11-25 11:37:23,793 # CMD0: [OK]
2022-11-25 11:37:23,798 # SD_INIT_ENABLE_CRC
2022-11-25 11:37:23,805 # sdcard_spi_send_cmd: CMD59 (0x00000001) (remaining retry time 249988 usec)
2022-11-25 11:37:23,806 # _wait_for_not_busy: [OK]
2022-11-25 11:37:23,810 # CMD59 echo: 0xFF 0xFF 0xFF 0xFF 0xFF 0xFF 
2022-11-25 11:37:23,810 # _wait_for_r1: r1=0xff
2022-11-25 11:37:23,816 # _wait_for_r1: r1=0xff
[...]
2022-11-25 11:37:24,147 # _wait_for_r1: r1=0xff
2022-11-25 11:37:24,147 # _wait_for_r1: r1=0xff
2022-11-25 11:37:24,147 # _wait_for_r1: [TIMEOUT]
2022-11-25 11:37:24,153 # sdcard_spi_send_cmd: R1_TIMEOUT (0xff)
2022-11-25 11:37:24,153 # SD_INIT_CARD_UNKNOWN
2022-11-25 11:37:24,153 # [FAILED]

Versions

Operating System Environment
----------------------------
         Operating System: Gentoo 
                   Kernel: Linux 5.15.69-gentoo x86_64 AMD Ryzen 7 5700U with Radeon Graphics
             System shell: GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)
             make's shell: GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)

Installed compiler toolchains
-----------------------------
               native gcc: gcc (Gentoo 11.3.0 p4) 11.3.0
        arm-none-eabi-gcc: arm-none-eabi-gcc (GNU Arm Embedded Toolchain 10.3-2021.10) 10.3.1 20210824 (release)
                  avr-gcc: avr-gcc (Gentoo 11.3.0 p4) 11.3.0
           msp430-elf-gcc: missing
       riscv-none-elf-gcc: missing
  riscv64-unknown-elf-gcc: missing
     riscv-none-embed-gcc: missing
     xtensa-esp32-elf-gcc: missing
   xtensa-esp8266-elf-gcc: missing
                    clang: clang version 15.0.3

Installed compiler libs
-----------------------
     arm-none-eabi-newlib: "4.1.0"
        msp430-elf-newlib: missing
    riscv-none-elf-newlib: missing
riscv64-unknown-elf-newlib: missing
  riscv-none-embed-newlib: missing
  xtensa-esp32-elf-newlib: missing
xtensa-esp8266-elf-newlib: missing
                 avr-libc: "2.1.0" ("20220128")

Installed development tools
---------------------------
                   ccache: missing
                    cmake: cmake version 3.24.3
                 cppcheck: missing
                  doxygen: 1.9.5
                      git: git version 2.37.4
                     make: GNU Make 4.3
                  openocd: Open On-Chip Debugger 0.11.0
                   python: Python 3.10.8
                  python2: missing
                  python3: Python 3.10.8
                   flake8: error: /usr/lib/python-exec/python3.10/python3: No module named flake8
               coccinelle: missing
@jdavid
Copy link
Contributor Author

jdavid commented Dec 27, 2022

The reason this does not work may be because the SPI bus does not work, see issue #19025

@jdavid
Copy link
Contributor Author

jdavid commented Jan 19, 2023

This actually works... I just had to use SPI_DEV(1) instead of SPI_DEV(0) 🤦

Before closing the issue, I have one question. The board does not include a SDcard reader, it must be an external module.
Yet, would it make sense to include the SPI configuration in RIOT's tree? Right now I have this in boards/lora-e5-dev/include/board.h:

#define SPI_CS GPIO_PIN(PORT_B, 9)              // NSS
#define SPI_SCLK GPIO_PIN(PORT_B, 13)           // SCK
#define SPI_MOSI GPIO_PIN(PORT_A, 10)           // MOSI
#define SPI_MISO GPIO_PIN(PORT_B, 14)           // MISO
#define CARD_DETECT_PIN (GPIO_PIN(PORT_B, 10))  // D10

#define SDCARD_SPI_PARAM_SPI        SPI_DEV(1)  /**< SPI device */
#define SDCARD_SPI_PARAM_CS         SPI_CS      /**< Chip Select */
#define SDCARD_SPI_PARAM_CLK        SPI_SCLK    /**< Serial Clock */
#define SDCARD_SPI_PARAM_MOSI       SPI_MOSI    /**< Master Output, Slave Input */
#define SDCARD_SPI_PARAM_MISO       SPI_MISO    /**< Master Input, Slave Output */

Thanks!

@benpicco
Copy link
Contributor

What kind of module is it?
Self wired or somehow related to the board?

@jdavid
Copy link
Contributor Author

jdavid commented Jan 19, 2023

Self wired, I'm testing one from Adafruit.

@benpicco
Copy link
Contributor

hmm I wonder if that would be generally helpful or rather just confusing.

You can however add the config to sdcard_spi_params.h that you just place in the board include folder, the SD card driver should pick it up.

@jdavid
Copy link
Contributor Author

jdavid commented Jan 19, 2023

Thanks that works.

I think SPI_DEV(0) is used internally for the radio.

Here there's a picture with the pinout of the LoRa-E5 mini, https://files.seeedstudio.com/products/317990687/image/3001615286723_.pic_hd.jpg
There is only one obvious way to wire the SPI interface, the same goes for the development version.

Should I open a PR to add the sdcard_spi_params.h file?

@jdavid
Copy link
Contributor Author

jdavid commented Jan 20, 2023

I've added a board outside of RIOTBASE, so I won't do a PR for now.

Just a question, it picks sdcard_spi_params.h, but where to put for example the mtd configuration?

I think it should be done in board.h / board.c, but if I add board.h then it overrides board.h from lora-e5-dev, so I would need to copy that file in its entirety.

This may be somehow related to issue #17059 Will comment there.

@jdavid jdavid closed this as completed Jan 23, 2023
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
@maribu
Copy link
Member

maribu commented May 17, 2023

Sorry for the long delay. You could use the mtd_sdcard_default these days and just provide the SDCARD_SPI_PARAM_<FOOBAR> in the out-of-tree board's board.h.

This is how I create a variant of an existing RIOT board outside of the tree:

Structure

nucleo-f429zi-with-sdcard
├── Makefile
├── Makefile.dep
├── Makefile.features
├── Makefile.include
└── include
    └── board.h

2 directories, 5 files

Makefile

(Copy-paste of original board. An include of the original Makefile would be cleaner instead.)

MODULE = board
DIRS = $(RIOTBOARD)/common/nucleo

include $(RIOTBASE)/Makefile.base

Makefile.dep

ifneq (,$(filter mtd,$(USEMODULE)))
  USEMODULE += mtd_sdcard_default
endif

# default to using fatfs on SD card
ifneq (,$(filter vfs_default,$(USEMODULE)))
  USEMODULE += fatfs_vfs
  USEMODULE += mtd
endif

Add an include of the original boards Makefile.dep if the board has one.

Makefile.include

# reuse periph config from nucleo-f429zi board
INCLUDES += -I$(RIOTBOARD)/nucleo-f429zi/include

# load the common Makefile.include for Nucleo boards
include $(RIOTBOARD)/common/nucleo144/Makefile.include

Note that adding the include folder of the original board is done so that the periph_conf.h can be reused. This would even be needed if the original board's Makefile.include would be included here via an include statement, which again would be cleaner than copy pasting as done here.

Makefile.features

# load the Nucleo-F429ZI features list
include $(RIOTBOARD)/nucleo-f429zi/Makefile.features

This now is the clean include that I should have used above as well.

include/board.h

#ifndef BOARD_H
#define BOARD_H

#include "board_nucleo.h"
#include "arduino_pinmap.h"

#ifdef __cplusplus
extern "C" {
#endif

/**
 * @name    LED pin definitions and handlers
 * @{
 */
#define LED0_PIN_NUM        0
#define LED0_PORT_NUM       PORT_B

#define LED1_PIN_NUM        7
#define LED1_PORT_NUM       PORT_B

#define LED2_PIN_NUM        14
#define LED2_PORT_NUM       PORT_B
/** @} */

/**
 * @name    User button
 * @{
 */
#define BTN0_PIN            GPIO_PIN(PORT_C, 13)
#define BTN0_MODE           GPIO_IN_PD
/** @} */

/**
 * @name SD card configuration
 * @{
 */
#define SDCARD_SPI_PARAM_SPI            SPI_DEV(0)
#define SDCARD_SPI_PARAM_CS             GPIO_PIN(PORT_D, 14)
#define SDCARD_SPI_PARAM_CLK            GPIO_PIN(PORT_A, 5)
#define SDCARD_SPI_PARAM_MISO           GPIO_PIN(PORT_A, 6)
#define SDCARD_SPI_PARAM_MOSI           GPIO_PIN(PORT_A, 7)
//#define SD_CARD_SPI_SPEED_POSTINIT      SPI_CLK_1MHZ
/**
 * @}
 */

#ifdef __cplusplus
}
#endif

#include "stm32_leds.h"

#endif /* BOARD_H */
/** @} */

It theoretically be possible to also include the original boards board.h via an #include_next "board.h", but note that the include guard of the original board.h also used BOARD_H as macro. You could just use a different macro name for that to avoid the conflict. Or copy-paste the original board.h and just add the SPI params as I did above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants