carlk3 / no-OS-FatFS-SD-SDIO-SPI-RPi-Pico

A FAT filesystem with SDIO and SPI drivers for SD card on Raspberry Pi Pico
Apache License 2.0
76 stars 15 forks source link

New object model seems to break all examples in main branch #5

Open ianarchbell opened 9 months ago

ianarchbell commented 9 months ago

'sd_card_t' {aka 'struct sd_card_t'} has no member named 'spi_if'; did you mean 'spi_if_p'

Changed examples to reflect that but got more problems and left it...

ianarchbell commented 9 months ago

This seemed to work:

static spi_t spis[] = {  // One for each SPI.
    {
        .hw_inst = spi1,  // SPI component
        .miso_gpio = 12,  // GPIO number (not Pico pin number)
        .mosi_gpio = 11,
        .sck_gpio = 10,

        // .baud_rate = 1000 * 1000
        .baud_rate = 12500 * 1000
        // .baud_rate = 25 * 1000 * 1000 // Actual frequency: 20833333.
    }};

static sd_spi_if_t spi_ifs[] = {
    {
        .spi = &spis[0],  // Pointer to the SPI driving this card
        .ss_gpio = 15
        }                   // Slave select for this SD card
};

// Hardware Configuration of the SD Card "objects"
static sd_card_t sd_cards[] = {  // One for each SD card
    { 
        .pcName = "0:",          // Name used to mount device
        .type = SD_IF_SPI,
        .spi_if_p = &spi_ifs[0],
        //.spi_if.spi = &spis[0],  // Pointer to the SPI driving this card
    //    .spi_if_p.ss_gpio = 15,     // The SPI slave select GPIO for this SD card

        .use_card_detect = false,
        .card_detect_gpio = 13,  // Card detect
        .card_detected_true = 1  // What the GPIO read returns when a card is
                                 // present.
    }
};
carlk3 commented 9 months ago

I need to fix up the examples. The command_line example and its various config files should be OK.

The reason for this change is in order to support multiple SDIO-attached cards sdio_if objects now carry a lot of state information, including things like data buffers. The union of two types has the size of the larger type, so the spi_if was much larger than necessary. I solved this by moving the _if objects out of sd_card_t and replacing them with pointers.

ianarchbell commented 9 months ago

Carl, Command Line has similar problem, but changing it to use the config above worked in SPI mode. Unfortunately switching to SDIO mode does not. I get an assertion assertion "0 == gpio_get(sd_card_p->spi_if_p->ss_gpio)" failed if I add another card to the array. If I remove the first card and only use one SDIO the assertion doesn't fail, but I get a timeout.

It's possible it's the hardware as it's a new card I have, but flagging this. It's also possible I'm not using the configuration correctly (would value your input).

I think you've done a lot of very good work that I'd like to reuse. Ian.

// Hardware Configuration of SPI "objects"
// Note: multiple SD cards can be driven by one SPI if they use different slave
// selects.
static spi_t spis[] = {  // One for each SPI.
    {
        .hw_inst = spi1,  // SPI component
        .miso_gpio = 12,  // GPIO number (not Pico pin number)
        .mosi_gpio = 11,
        .sck_gpio = 10,
        .set_drive_strength = true,
        .mosi_gpio_drive_strength = GPIO_DRIVE_STRENGTH_4MA,
        .sck_gpio_drive_strength = GPIO_DRIVE_STRENGTH_2MA,
        .no_miso_gpio_pull_up = true,

        // .baud_rate = 1000 * 1000
        .baud_rate = 12500 * 1000
        // .baud_rate = 25 * 1000 * 1000 // Actual frequency: 20833333.
    }
};

static sd_spi_if_t spi_ifs[] = {
    {
        .spi = &spis[0],  // Pointer to the SPI driving this card
        .ss_gpio = 15
        }                 
};

static sd_sdio_if_t sdio_ifs[] = {
    /*
        Pins CLK_gpio, D1_gpio, D2_gpio, and D3_gpio are at offsets from pin D0_gpio.
        The offsets are determined by sd_driver\SDIO\rp2040_sdio.pio.
            CLK_gpio = (D0_gpio + SDIO_CLK_PIN_D0_OFFSET) % 32;
            As of this writing, SDIO_CLK_PIN_D0_OFFSET is 30,
              which is -2 in mod32 arithmetic, so:
            CLK_gpio = D0_gpio -2.
            D1_gpio = D0_gpio + 1;
            D2_gpio = D0_gpio + 2;
            D3_gpio = D0_gpio + 3;
     */
    {
        .CMD_gpio = 11,
        .D0_gpio = 12,
        .SDIO_PIO = pio0,
        .DMA_IRQ_num = DMA_IRQ_0
    }                   // Slave select for this SD card
};

// Hardware Configuration of the SD Card "objects"
static sd_card_t sd_cards[] = {  // One for each SD card
    { 
        .pcName = "0:",          // Name used to mount device
        .type = SD_IF_SPI,
        .spi_if_p = &spi_ifs[0],
        .use_card_detect = false,
        // .card_detect_gpio = 13,  // Card detect
        // .card_detected_true = 1  // What the GPIO read returns when a card is
        //                          // present.
    },
    {        // Socket sd0 SDIO
        .pcName = "1:",  // Name used to mount device
        .type = SD_IF_SDIO,
        .sdio_if_p = &sdio_ifs[0],
        .use_card_detect = false

        // SD Card detect:

        // .card_detect_gpio = 9,  
        // .card_detected_true = 0, // What the GPIO read returns when a card is
        //                          // present.
        // .card_detect_use_pull = true,
        // .card_detect_pull_hi = true
    }

    // assertion "0 == gpio_get(sd_card_p->spi_if_p->ss_gpio)" failed

};
carlk3 commented 9 months ago

Ah, I checked in all the other config files, but I didn't check in hw_config.c. I change that one all the time for various tests, so I generally do not check that one in.

ianarchbell commented 9 months ago

Thanks ! I'll give it a try.

carlk3 commented 9 months ago

Regarding your configuration above, you can't drive the same card with SPI and SPIO simultaneously. It has to be one or the other. The SD specification says that once a card is switched to SPI mode, the only way to return to the SD mode is by entering the power cycle. However, I think it is possible to switch from SDIO to SPI without cycling power. ZuluSCSI does something like this in panic situations; it falls back to SPI mode to take a dump, I think. Currently, this library doesn't directly support that, though. For a static configuration, it's not going to work if you use the same pins in an spi_t definition and an sd_sdio_if_t definition.

ianarchbell commented 9 months ago

Yes, I thought that was probably the case but right now I only have one adapter and wanted to run separate tests SPI vs SDIO. I'll remove one at a time instead. Thanks.

ianarchbell commented 8 months ago

Closing issue but have a question. I have aboard the Pico Expansion Plus S1 and I bought it because it has a pinout that should be suitable for SDIO. I'm in the process of designing my own board.

Here's an extract from the hw_config. There's a pinout in the comments. Works fine in SPI mode, but I get:

mount 1: No SD card detected! f_mount error: The physical drive cannot work (3)

Any advice? Thanks, Ian.

static sd_sdio_if_t sdio_ifs[] = {
    /*
        Pins CLK_gpio, D1_gpio, D2_gpio, and D3_gpio are at offsets from pin D0_gpio.
        The offsets are determined by sd_driver\SDIO\rp2040_sdio.pio.
            CLK_gpio = (D0_gpio + SDIO_CLK_PIN_D0_OFFSET) % 32;
            As of this writing, SDIO_CLK_PIN_D0_OFFSET is 30,
              which is -2 in mod32 arithmetic, so:
            CLK_gpio = D0_gpio -2.
            D1_gpio = D0_gpio + 1;
            D2_gpio = D0_gpio + 2;
            D3_gpio = D0_gpio + 3;
     */
    // SD Card Pins on S1 board
    /*

    SPI     SDIO    GPIO
    CLK     CLK     10
    SI      CMD     11  MOSI
    SO      D0      12  MISO
            D1      13
            D2      14
    CS      D3      15        
            CD      NC

    */
    {
        .CMD_gpio = 10,
        .D0_gpio = 12,
        .SDIO_PIO = pio1,
        .DMA_IRQ_num = DMA_IRQ_1,
        .baud_rate = 15 * 1000 * 1000  // 15 MHz
    }           
};
carlk3 commented 8 months ago

In theory, you can't get that "No SD card detected!" message unless somewhere, somehow, the use_card_detect field of the sd_card_t struct is getting set to true. If the card detect switch is "NC", then you'd want false, obviously.

ianarchbell commented 8 months ago

I do have false so not sure what’s happening. 

Sent from Yahoo Mail for iPhone

On Wednesday, October 4, 2023, 7:15 PM, Carl J Kugler III @.***> wrote:

In theory, you can't get that "No SD card detected!" message unless somewhere, somehow, the use_card_detect filed of the sd_card_t struct is getting set to true. If the card detect switch is "NC", then you'd want false, obviously.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

carlk3 commented 8 months ago

Could you attach your hw_config?

ianarchbell commented 8 months ago

Here it is attached... On Wednesday, October 4, 2023 at 07:37:42 PM PDT, Carl J Kugler III @.***> wrote:

Could you attach your hw_config?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

carlk3 commented 8 months ago

Hmmm, I'm not seeing it (the attachment).

ianarchbell commented 8 months ago

I've pasted here:

/ hw_config.cCopyright 2021 Carl John Kugler III Licensed under the Apache License, Version 2.0 (the License); you may not usethis file except in compliance with the License. You may obtain a copy of theLicense at http://www.apache.org/licenses/LICENSE-2.0Unless required by applicable law or agreed to in writing, software distributedunder the License is distributed on an AS IS BASIS, WITHOUT WARRANTIES ORCONDITIONS OF ANY KIND, either express or implied. See the License for thespecific language governing permissions and limitations under the License.// This file should be tailored to match the hardware design. There should be one element of the spi[] array for each hardware SPI used. There should be one element of the sd_cards[] array for each SD card slot.The name is should correspond to the FatFs "logical drive" identifier.(See http://elm-chan.org/fsw/ff/doc/filename.html#vol)In general, this should correspond to the (zero origin) array index.The rest of the constants will depend on the type ofsocket, which SPI it is driven by, and how it is wired. /

include //#include "my_debug.h"//#include "hw_config.h"//#include "ff.h" / Obtains integer types ///#include "diskio.h" / Declarations of disk functions /

/ See https://docs.google.com/spreadsheets/d/1BrzLWTyifongf_VQCc2IpJqXWtsrjmG7KnIbSBy-CPU/edit?usp=sharing,tab "Monster", for pin assignments assumed in this configuration file./ / Tested 0: SPI: OK, 12 MHz only SDIO: OK at 31.25 MHz 1: SPI: OK at 20.8 MHz 2: SPI: OK at 20.8 MHz 3: SDIO: OK at 20.8 MHz (not at 31.25 MHz) 4: SDIO: OK at 20.8 MHz (not at 31.25 MHz)/ // SD Card Pins on S1 board/ SPI SDIO GPIOCLK CLK 10SI CMD 11 MOSISO D0 12 MISO D1 13 D2 14CS D3 15 CD NC / // Hardware Configuration of SPI "objects"// Note: multiple SD cards can be driven by one SPI if they use different slave// selects.static spi_t spis[] = { // One for each SPI. { .hw_inst = spi1, // SPI component .miso_gpio = 12, // GPIO number (not Pico pin number) .mosi_gpio = 11, .sck_gpio = 10, .set_drive_strength = true, .mosi_gpio_drive_strength = GPIO_DRIVE_STRENGTH_4MA, .sck_gpio_drive_strength = GPIO_DRIVE_STRENGTH_2MA, .no_miso_gpio_pull_up = true, // .baud_rate = 1000 1000 .baud_rate = 12500 1000 // .baud_rate = 25 1000 1000 // Actual frequency: 20833333. }}; / SPI Interfaces /// static sd_spi_if_t spi_ifs[] = {// { // spi_ifs[0]// .spi = &spis[0], // Pointer to the SPI driving this card// .ss_gpio = 15, // The SPI slave select GPIO for this SD card// .set_drive_strength = true,// .ss_gpio_drive_strength = GPIO_DRIVE_STRENGTH_4MA// }// }; static sd_sdio_if_t sdio_ifs[] = { / Pins CLK_gpio, D1_gpio, D2_gpio, and D3_gpio are at offsets from pin D0_gpio. The offsets are determined by sd_driver\SDIO\rp2040_sdio.pio. CLK_gpio = (D0_gpio + SDIO_CLK_PIN_D0_OFFSET) % 32; As of this writing, SDIO_CLK_PIN_D0_OFFSET is 30, which is -2 in mod32 arithmetic, so: CLK_gpio = D0_gpio -2. D1_gpio = D0_gpio + 1; D2_gpio = D0_gpio + 2; D3_gpio = D0_gpio + 3; / // SD Card Pins on S1 board / SPI SDIO GPIO CLK CLK 10 SI CMD 11 MOSI SO D0 12 MISO D1 13 D2 14 CS D3 15 CD NC / { .CMD_gpio = 10, .D0_gpio = 12, .SDIO_PIO = pio1, .DMA_IRQ_num = DMA_IRQ_1, .baud_rate = 15 1000 1000 // 15 MHz } }; // static sd_card_t sd_cards[] = { // One for each SD card// { // sd_cards[0]: Socket sd0// .pcName = "0:", // Name used to mount device// .type = SD_IF_SPI,// .spi_if_p = &spi_ifs[0], // Pointer to the SPI interface driving this card// // SD Card detect:// .use_card_detect = false, // }// }; // Hardware Configuration of the SD Card "objects"// static sd_card_t sd_cards[] = { // One for each SD cardstatic sd_card_t sd_cards[] = { // One for each SD card { // sd_cards[0]: Socket sd0 { // Socket sd0 SDIO .pcName = "1:", // Name used to mount device .type = SD_IF_SDIO, .sdio_if_p = &sdio_ifs[0], .use_card_detect = false, // SD Card detect: // .card_detect_gpio = 9, // .card_detected_true = 0, // What the GPIO read returns when a card is // // present. // .card_detect_use_pull = true, // .card_detect_pull_hi = true } // assertion "0 == gpio_get(sd_card_p->spi_if_p->ss_gpio)" failed }; /* ** /size_t sd_get_num() { return count_of(sd_cards); }sd_card_t sd_get_by_num(size_t num) { if (num <= sd_get_num()) { return &sd_cards[num]; } else { return NULL; }}size_t spi_get_num() { return count_of(spis); }spi_t spi_get_by_num(size_t num) { if (num <= spi_get_num()) { return &spis[num]; } else { return NULL; }} / [] END OF FILE */

On Wednesday, October 4, 2023 at 08:53:25 PM PDT, Carl J Kugler III ***@***.***> wrote:  

Hmmm, I'm not seeing it

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

carlk3 commented 8 months ago

Sorry, but I cannot make sense of that formatting.

ianarchbell commented 8 months ago

Sorry, it was late and forgot to put code tag around it. Anyway I did get it working on a different card. I did though have an interesting failure when I used PIO1. PIO0 was fine, but with PIO1 there seems to besome interaction with the Wi-Fi - that is supposed to claim unused PIO but in this case it seems to be using PIO1 at the same time as I am. Could be because I am running under FreeRTOS and have not yet got a mutex around the file system calls?

carlk3 commented 8 months ago

I did though have an interesting failure when I used PIO1. PIO0 was fine, but with PIO1 there seems to besome interaction with the Wi-Fi - that is supposed to claim unused PIO but in this case it seems to be using PIO1 at the same time as I am.

I don't think the SDK provides a way to claim unused PIO. Do you mean pio_claim_unused_sm? That is, claim unused State Machine in a PIO block? The author of the ZuluSCSI implementation (that I stole), "jpa-", said "As a downside, my implementation uses more PIO program space than the pico-extras code, so nothing else will fit on the same PIO block."

Also, have you git pulled the latest code? 5 days ago I checked in a fix for a bug in initialization of PIO0, but maybe it is related? See https://github.com/carlk3/no-OS-FatFS-SD-SDIO-SPI-RPi-Pico/pull/7/commits/08e186ce23f307022845578a57c673dc4a25bffa

Could be because I am running under FreeRTOS and have not yet got a mutex around the file system calls?

If you're running FreeRTOS, you might consider using FreeRTOS-FAT-CLI-for-RPi-Pico instead. That is designed from the ground up for FreeRTOS. I haven't yet ported the SDIO driver to that repository, but it should be easy to do.

ianarchbell commented 8 months ago

I don't think the SDK provides a way to claim unused PIO. Do you mean pio_claim_unused_sm? That is, claim unused State Machine in a PIO block? The author of the ZuluSCSI implementation (that I stole), "jpa-", said "As a downside, my implementation uses more PIO program space than the pico-extras code, so nothing else will fit on the same PIO block."

Yes, I meant state machine.

Also, have you git pulled the latest code? 5 days ago I checked in a fix for a bug in initialization of PIO0, but maybe it is related? See 08e186c

Yes, I have. That's when I made better progress. I deleted entire repo and recloned. Got command line working and then my own stuff.

If you're running FreeRTOS, you might consider using FreeRTOS-FAT-CLI-for-RPi-Pico instead. That is designed from the ground up for FreeRTOS. I haven't yet ported the SDIO driver to that repository, but it should be easy to do.

Yes, I saw that repo but hard to give up five times faster reads, especially as I am using it as part of a webserver implementation. Are you planning to offer a OS-FatFS-SD-SDIO-SPI-RPi-Pico for FreeRTOS in the near future?

carlk3 commented 8 months ago

Are you planning to offer a OS-FatFS-SD-SDIO-SPI-RPi-Pico for FreeRTOS in the near future?

I'm not sure when I'll have time to port the SDIO driver over. That will require migrating to the new configuration model, too. I would stick to Lab-Project-FreeRTOS-FAT instead of FatFs for that. FatFs does not have sufficient concurrency controls. With SDIO and FreeRTOS SMP the performance should be excellent for multi-threaded applications.

ianarchbell commented 8 months ago

OK sounds like I should switch to FreeRTOS-FAT-CLI-for-RPi-Pico so it plays nicely with FreeRTOS and hope that you eventually do the SDIO port to get the performance! Thanks very much for your help.

carlk3 commented 8 months ago

While FatFS has some support for “re-entrancy” or thread safety (where "thread" == "task", in FreeRTOS terminology), it is limited to operations such as:

  • f_read
  • f_write
  • f_sync
  • f_close
  • f_lseek
  • f_closedir
  • f_readdir
  • f_truncate

There does not appear to be sufficient FAT and directory locking in FatFS to make operations like f_mkdir, f_chdir and f_getcwd thread safe. If your application has a static directory tree, it should be OK in a multi-tasking application with FatFS. Otherwise, you probably need to add some additional locking.

Then, there are the primitives (if that's the right word) used for mutual exclusion and various ways of waiting (delay, wait for interrupt, etc.). This library uses the Pico SDK primitives (which are really oriented to multi-processing on the two cores), but FreeRTOS-FAT-CLI-for-RPi-Pico uses the FreeRTOS primitives, which put the waiting task into a Blocked state, allowing other, Ready tasks to run.

FreeRTOS-FAT-CLI-for-RPi-Pico is designed to maximize parallelism. So, if you have two cores and multiple SD card buses (SPI or SDIO), multiple FreeRTOS tasks can keep them all busy simultaneously.

ianarchbell commented 8 months ago

Thanks. I got the FreeRTOS version working thank you. Note that in SDK 1.5 spi.h does not include pico/time.h (whereas 1.4 did) so build fails with missing timer functions. I added #include pico/time.h to sd_spi.c (but maybe better in sd_spi.h where I noticed you had the other pico includes) to get it to link cleanly.

ianarchbell commented 8 months ago

I meant spi.h for the best place to include the header.

carlk3 commented 8 months ago

Thanks. I got the FreeRTOS version working thank you. Note that in SDK 1.5 spi.h does not include pico/time.h (whereas 1.4 did) so build fails with missing timer functions.

Ah. That repository is definitely due for some maintenance, as well as enhancement (SDIO).