Hard Coded SPI speed in Sd2Card.cpp?

[GryphonR]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: ESP-12
• Core Version: 559a286
• Development Env: Arduino IDE
• Operating System: Windows

Settings in IDE

• Module: Nodemcu
• Flash Mode: qio
• Flash Size: 4MB
• lwip Variant: v2 Lower Memory
• Reset Method: nodemcu
• Flash Frequency: 40Mhz
• CPU Frequency: 80Mhz
• Upload Using: [OTA|SERIAL]
• Upload Speed: 115200

Problem Description

The Sd2Card::init function expects the desired SPI speed to be passed in but then doesn't use it for hardware SPI, instead the speed is hard coded to '250000' (250KHz) on line 286:

settings = SPISettings(250000, MSBFIRST, SPI_MODE0);

This compiles and runs, however at such low speeds the SD.begin() call in the SD library that runs this init times out without exception. I found the issue after scoping the SPI clock speed.

Changing the line to use the passed in, desired SPI speed resulted in the Sd card initialising as expected:

settings = SPISettings(sckRateID, MSBFIRST, SPI_MODE0);

I was going to submit a fix, but as this has been there for the last 4 years I wanted to check if I've just interpreted something wrong before I did...

MCVE Sketch

No compile or runtime errors - just the timeout due to a slow network. I can add code if required.

Debug Messages

[devyte]

Without looking at the details, this sounds valid. But a question before moving on: is the currently hardcoded spi clock speed valid? If so, the timeout should be fixed.

[GryphonR]

My first attempt to fix the problem (before I realised the cause) was to extend the timeout from 2s to 4s. Initialisation still timed out without exception. It would probably succeed given enough time, but it's not a realistic time to wait. It would also result in painfully slow read/write speeds.

For reference, my SD card is now working at an 8MHz SPI clock speed - a 32x increase over the hard coded 250KHz.

[devyte]

@RJPGriffin I don't understand your previous comment.
Is the timeout for the whole operation, I.e. if you're not done writing the requested data within that time then blow up?
About the clock speed, what are the clocks supported by older smaller sd cards?

[GryphonR]

Sorry, timeout relates purely to the SD initialisation function.

Calling SD.begin(cs, speed) initialises communication to the SD card, identifies the volume and opens the root file system. (I don't have much more than a superficial understanding of what these individually do) It is necessary for this to complete before you can read/write to the SD card.

boolean SDClass::begin(uint8_t csPin, uint32_t speed) {
/*
    Performs the initialisation required by the sdfatlib library.
    Return true if initialization succeeds, false otherwise.
*/
  return card.init(speed, csPin) &&
         volume.init(card) &&
         root.openRoot(volume);
}

There is a timeout in the card.init function in Sd2Card.cpp. When using the hardware SPI on the esp8266 it attempts to initialize communication to the card at 250khz, regardless of the speed you specify when calling the begin function. I increased the timeout from 2 seconds to 4 and it still failed. I don't know if it would succeed with a longer timeout. The result of this timing out is that the SD card fails to initialize, so you can't access it for any read/write operations.

From a bit of googling the only SD interface that requires clock speeds below 400KHz is "Low-Speed SDIO" which is an an I/O specification rather than storage and so wouldn't work with the library anyway. From what I can find, even older, small SD cards should be able to operate >4MHz. The smallest card I tried with was a 1Gb, non HC card which needed greater than 250khz to initialize successfully.

In any case, you set the communication speed desired in the SD.begin(cs, speed); function call, so set a speed suitable for your application and you'd expect it to initialize to that speed, instead of running at the hard coded 250KHz as is the current behavior.

[devyte]

Ah, I see it now in the code.
Is your solution to just replace the 250000 value with sckRateID, and then call SD.begin(8000000) ? Or rather SPI_FULL_SPEED.

[GryphonR]

Yep exactly that.

[devyte]

I'm ok with that as a fix. the default is supposed to be half speed anyways, not 250000.
Please make a PR, and I'll review/merge.

[d-a-v]

To not break the api,
I think a third optional parameter defaulted to 250KHz should be added to ::begin() and ::init().

In this old MMC doc (that I think is reliable), it is said that the init frequency should be 400KHz and a delay of 1ms after poweron should be observed.

[devyte]

Ah scrap, per that doc, the 250KHz is needed for powerup, not init (freq should be between 100KHz-400KHz).
Then again, looking at this, I don't see the sending of the 74 bytes which is described in that doc. I don't know what to make of that.
Overall, the doc and the code don't seem to agree.
Also, I see further down here that the clock rate seems to be set per the argument sckRateID, so I now suspect that the 250KHz setting is only for during the init, and not the actual sd card data transfer. At this point I suspect that the current init sequence isn't correct for SPI lib use, and that's the reason for the timing out.

[GryphonR]

The MMC doc is an interesting read, I've not looked at it to that low a level before.

I had missed that the clock rate was later set to sckRateID. From watching the clock on a scope, it never changed from 250khz in practice.

Looking at the setSckRate function, it updates the settings for the spi, but doesn't appear to apply those settings. (I'm on my phone though so may well have missed something)

Going against what the MMC doc says, my 'fix' of running the initialisation at a higher speed does work on both HC and non HC SD cards... (Not saying it's right, just that it works somehow)

[d-a-v]

If your fix works, then make a PR for it.
Doc can wait :)