support DigitalInOutProtocol for CS of sdcardio.SDCard

[bablokb]

This fixes #11047. The implementation is basically a copy&paste&edit of what fourwire.FourWire does.

Tested on an upcoming Waveshare board that uses SD_CS behind an ioexpander, and on a Pico with a normal SD breakout attached.

[bablokb]

With my last fix, all builds are now successful.

@dhalbert , @mikeysklar : I don't have the hardware to test the automount feature (I never bothered to route the sd-card-detect pin on my pcbs). It would be great if one of you could do a test on one of the boards with integrated sd-card that use this feature (e.g. adalogger, fruit-jam).

[mikeysklar]

checking now on a Metro RP2350

[mikeysklar]

I tried this PR on a Metro RP2350 and connected it two different hosts macOS and Linux.

CircuitPython 10.3.0-alpha.2-31-g6285efc59e-dirty

I'm not seeing the microSD drive pass through correctly. Tried both exFAT (64GB) and FAT32 (16GB) cards.

On macOS I get this error.

"The disk you attached was not readable by this computer."

The disk device is visible, but the filesystem fails to mount.

It looks like this is passing an address instead of an object.

common_hal_digitalio_digitalinout_never_reset(MP_OBJ_TO_PTR(&sdcard.cs));

should be:

common_hal_digitalio_digitalinout_never_reset(MP_OBJ_TO_PTR(sdcard.cs));

Will confirm.

[mikeysklar]

Yep, that fixed it:

Ubuntu 24.04.4 LTS:

/dev/sdb1          15328     1724     13604  12% /media/sklarm/CIRCUITPY
/dev/sdc1       61794304    14336  61779968   1% /media/sklarm/64GB_SD

macOS Tahoe 26.5.1:

/dev/disk4s1        30660      3452     27208    12%       0          0     -   /Volumes/CIRCUITPY
/dev/disk5s1    123588864     28928 123559936     1%       1          0  100%   /Volumes/64GB_SD

[bablokb]

@mikeysklar thanks for testing and for tracking down the bug. I will update my PR, but that won't happen before Tuesday.

[mikeysklar]

No rush. Let me know if you think it is worth checking on other boards or if the Metro RP2350 is enough. I do have an Adalogger RP2040.

[bablokb]

I think one board is enough - they all use the same automount function. I did test the other codepath (normal mount using storage), so that should be fine too.

[bablokb]

With the latest commit, this should be ready for merging. Thanks again to @mikeysklar for testing and fixing the automount codepath.

[dhalbert]

Looks good, but I have a concern about None for the CS pin.

[dhalbert]

The true third argument to digitalinout_protocol_from_pin() is allowing cs to be None, and the code further down also allows that.

In the original code, cs must be a pin: that's validated in shared-bindings.

So I don't think you want to allow None for CS, unless you want to handle a case where CS is pulled down all the time. (I'm not sure that even works.)

[bablokb]

This is indeed following the implementation of fourwire.FourWire. I am aware of this. And having CS pulled low is bad hardware design IMHO, but it happens. It saves a pin by dedicating a SPI bus to a single device.

The board I am currently bringing up does this for the display CS, so I was happy to see that fourwire.FourWire already supported this. There is no reason to not support this for sdcardio.SDCard.

[dhalbert]

This is indeed following the implementation of fourwire.FourWire. I am aware of this. And having CS pulled low is bad hardware design IMHO, but it happens. It saves a pin by dedicating a SPI bus to a single device.

Some displays can work with CS tied to low But for SD cards, having CS high sometimes is a required part of the spec. For instance, there is a reset sequence for SD cards where you hold CS high, and then clock the card for >= 74 clock cycles to have it go into an init state. See

circuitpython/shared-module/sdcardio/SDCard.c

Lines 91 to 96 in 8a10c8f

	static void clock_card(sdcardio_sdcard_obj_t *self, int bytes) {
	uint8_t buf[bytes];
	memset(buf, 0xff, bytes);
	common_hal_digitalio_digitalinout_set_value(&self->cs, true);
	common_hal_busio_spi_write(self->bus, buf, bytes);
	}

and

circuitpython/shared-module/sdcardio/SDCard.c

Lines 258 to 261 in 8a10c8f

	static mp_rom_error_text_t init_card(sdcardio_sdcard_obj_t *self) {
	// https://nodeloop.org/guides/sd-card-spi-init-guide/ recommends at least 74 bit clocks
	// and says 80 bit clocks(10*8) is common. Value below is bytes, not bits.
	clock_card(self, 10);

As mentioned in the code, https://nodeloop.org/guides/sd-card-spi-init-guide/ talks about this, and cautions about CS. To quote:

Common failure cases

No 0x01 after CMD0

• CS was not high during the dummy clocks.
• Initialization clock is too fast.
• The command frame or CRC byte is wrong.
• The card is not actually powered or level-shifted correctly.

Maybe some cards work without sending the init sequence, but it's out of spec, and one should not design a board without a CS pin for an SD card.

[bablokb]

Good arguments, I am convinced. I will update the implementation accordingly.