👋Hello lbernstone, we appreciate your contribution to this project!

Generated by 🚫dangerJS against4716fc0

Memory usage test (comparing PR against master branch)

The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.

Test Results

 83 files   83 suites   26m 19s ⏱️
 55 tests  54 ✅ 0 💤 1 ❌
598 runs  597 ✅ 0 💤 1 ❌

For more details on these failures, seethis check.

Results for commit4716fc0.

♻️ This comment has been updated with latest results.

Please add a check for IDF versions greater than 5.3:

https://github.com/espressif/arduino-esp32/actions/runs/19442524813/job/55629051409?pr=12038

On P4 EV Board card is no longer mounting with the changes in PR.

@lbernstone If I comment out the UHS-1 flag, the sdmmc starts working again and the speed is improved with the DDR flag only.

@P-R-O-C-H-Y What kind of SD card do you have?
Does it have UHS markings on the card or is it just a SDHC/SDXC card? (or even just SD, which is really a collectors item these days)

@P-R-O-C-H-Y What kind of SD card do you have? Does it have UHS markings on the card or is it just a SDHC/SDXC card? (or even just SD, which is really a collectors item these days)

Its a Kingston 16GB with USH-1 marking on it.

Weird is, that the SD card is not working on the P4-EYE board I got. Even with/without the changes in this PR.

Can you try to format it using some SD-format tool?
Or format it using some non-PC device like a camera.
It might be the partitioning is unconventional and/or the file system already present is not supported by the libraries we use.
Or maybe the "P4 EYE" doesn't use all 4 pins? For example DAT3 is wired as card detection?

The P4-eye have identical SD card schema as the P4-EV board. Will try with formatted card and also with different one.

Maybe also try some SDHC/SDXC card?
Those UHS cards may want to negotiate a lower signal level of 1V8. Not sure if this is expected or required. I know it is a feature of the standard.

in all cases, we want to support more cards, not less. If these options make cards not work, we need to make them optional

I'm not sure if SDHC uses DDR signalling.
If I'm not mistaken, this was initially only a protocol update to support cards > 2 GB.
There is a "high speed" variant for SDHC/XC/UC which doubles the bus speed, so I guess that does look like DDR.

Looking here:https://www.sdcard.org/developers/sd-standard-overview/bus-speed-default-speed-high-speed-uhs-sd-express/
I see there is a 50 MHz and 104 MHz signalling for UHS-I, where there doesn't seem to be a DDR104.
I have no idea what frequency is being used here?

I'll see if I can find some old cards around here and test an array of them. Not sure if I have really small & slow ones around any more.

Weird is, that the SD card is not working on the P4-EYE board I got. Even with/without the changes in this PR.

P4-eye (waveshare) has BOARD_SDMMC_POWER_PIN on 46

Hmm I took another look at the source code of this PR and

host.flags &= ~SDMMC_HOST_FLAG_DDR;

looks like it is disabling DDR?
If so then I am quite curious to why it is improving the speed.

If it is indeed increasing the transfer speed, then it sounds like a timing issue, or maybe some signal is inverted?

Hmm these defines for sure would be a lot more clear with quite a bit more comments...
https://github.com/espressif/esp-idf/blob/8f1e7bc4e08878f5a2aab55fdf302789d3a3eac4/components/sdmmc/include/sd_protocol_types.h#L198-L205

#defineSDMMC_FREQ_DEFAULT20000/*!< SD/MMC Default speed (limited by clock divider)*/#defineSDMMC_FREQ_HIGHSPEED40000/*!< SD High speed (limited by clock divider)*/#defineSDMMC_FREQ_PROBING400/*!< SD/MMC probing speed*/#defineSDMMC_FREQ_52M52000/*!< MMC 52MHz speed*/#defineSDMMC_FREQ_26M26000/*!< MMC 26MHz speed*/#defineSDMMC_FREQ_DDR5050000/*!< MMC 50MHz speed*/#defineSDMMC_FREQ_SDR50100000/*!< MMC 100MHz speed*/#defineSDMMC_FREQ_SDR104200000/*!< MMC 200MHz speed*/

The names of those defines don't appear to match the frequency numbers.
I assume the "DDR50", "SDR50" and "SDR104" refer to the bus speeds onthis page
If so, then both DDR50 and SDR50 refer to 50 MByte/sec.
And since there are 4 bits per transfer on UHS-I cards (not sure about the others as they do have 2 rows of pads, always thought those were for differential signalling, but not 100% sure), you need 2 transfers per byte.
So for SDR50 -> 100 MHz and DDR50 -> 50MHz

Then the most important change of this PR is setting the clock?
And then not disabling DDR and setting the max. freq toSDMMC_FREQ_DDR50 would result in the same speed improvement, right?
Likely with less RF noise and better timings.

Hmm these defines for sure would be a lot more clear with quite a bit more comments...espressif/esp-idf@8f1e7bc/components/sdmmc/include/sd_protocol_types.h#L198-L205

#defineSDMMC_FREQ_DEFAULT20000/*!< SD/MMC Default speed (limited by clock divider)*/#defineSDMMC_FREQ_HIGHSPEED40000/*!< SD High speed (limited by clock divider)*/#defineSDMMC_FREQ_PROBING400/*!< SD/MMC probing speed*/#defineSDMMC_FREQ_52M52000/*!< MMC 52MHz speed*/#defineSDMMC_FREQ_26M26000/*!< MMC 26MHz speed*/#defineSDMMC_FREQ_DDR5050000/*!< MMC 50MHz speed*/#defineSDMMC_FREQ_SDR50100000/*!< MMC 100MHz speed*/#defineSDMMC_FREQ_SDR104200000/*!< MMC 200MHz speed*/

The names of those defines don't appear to match the frequency numbers. I assume the "DDR50", "SDR50" and "SDR104" refer to the bus speeds onthis page If so, then both DDR50 and SDR50 refer to 50 MByte/sec. And since there are 4 bits per transfer on UHS-I cards (not sure about the others as they do have 2 rows of pads, always thought those were for differential signalling, but not 100% sure), you need 2 transfers per byte. So for SDR50 -> 100 MHz and DDR50 -> 50MHz

Then the most important change of this PR is setting the clock? And then not disabling DDR and setting the max. freq toSDMMC_FREQ_DDR50 would result in the same speed improvement, right? Likely with less RF noise and better timings.

Yes, you should be able to force to lower rates by setting a lower frequency, and the PR as written sets SDR (by turning DDR off). But, I do think the process should be backwards compatible, such that if the fastest speed fails, it drops down to try to init at a rate that the card understands, like happens transparently on your PC. I just don't have the hardware available to code & test that.

Ok, the dev board I got doesn't break out the SDIO pins, so I still have no way to test the old (standard SD) cards I have. Does anybody have old microsd cards and a working P4 that can test if the code as written fails back to lower rates automatically? Ideally, it needs to be tested on a non-UHS SDHC (eg 4GB), and then an old 64MB capacity card.

and then an old 64MB capacity card.

That would be extremely hard to find, as I have never seen non-SDHC cards in the micro-SD ("TF") format.

The thing that really needs to be tested is if this works on a device that is not capable of 1V8 operation.