Skip to content

support DigitalInOutProtocol for CS of sdcardio.SDCard #11053

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.

Sign up for GitHub

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

Merged
dhalbert merged 4 commits into from
Jun 23, 2026
+38 −18
Merged

support DigitalInOutProtocol for CS of sdcardio.SDCard #11053

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e5709f
support DigitialInOutProtocol for CS of sdcardio.SDCard
bablokb Jun 10, 2026
18425db
fix sdcard creation from automount_sd_card()
bablokb Jun 13, 2026
dc11d41
bugfix: pass object instead of address to MP_OBJ_TO_PTR()
bablokb Jun 16, 2026
7553fc5
verify CS pin is not None
bablokb Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions shared-bindings/sdcardio/SDCard.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
//| with ``storage.VfsFat`` to allow file I/O to an SD card."""
//|
//| def __init__(
//| self, bus: busio.SPI, cs: microcontroller.Pin, baudrate: int = 8000000
//| self,
//| bus: busio.SPI,
//| cs: Union[microcontroller.Pin, digitalio.DigitalInOutProtocol],
//| baudrate: int = 8000000
//| ) -> None:
//| """Construct an SPI SD Card object with the given properties
//|
Expand Down Expand Up @@ -87,10 +90,9 @@ static mp_obj_t sdcardio_sdcard_make_new(const mp_obj_type_t *type, size_t n_arg
mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);

busio_spi_obj_t *spi = validate_obj_is_spi_bus(args[ARG_spi].u_obj, MP_QSTR_spi);
const mcu_pin_obj_t *cs = validate_obj_is_free_pin(args[ARG_cs].u_obj, MP_QSTR_cs);

sdcardio_sdcard_obj_t *self = mp_obj_malloc_with_finaliser(sdcardio_sdcard_obj_t, &sdcardio_SDCard_type);
common_hal_sdcardio_sdcard_construct(self, spi, cs, args[ARG_baudrate].u_int);
common_hal_sdcardio_sdcard_construct(self, spi, args[ARG_cs].u_obj, args[ARG_baudrate].u_int);

return MP_OBJ_FROM_PTR(self);
}
Expand Down
2 changes: 1 addition & 1 deletion shared-bindings/sdcardio/SDCard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

extern const mp_obj_type_t sdcardio_SDCard_type;

void common_hal_sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *spi, const mcu_pin_obj_t *cs, int baudrate);
void common_hal_sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *spi, mp_obj_t cs, int baudrate);
void common_hal_sdcardio_sdcard_deinit(sdcardio_sdcard_obj_t *self);
bool common_hal_sdcardio_sdcard_deinited(sdcardio_sdcard_obj_t *self);
void common_hal_sdcardio_sdcard_check_for_deinit(sdcardio_sdcard_obj_t *self);
Expand Down
32 changes: 23 additions & 9 deletions shared-module/sdcardio/SDCard.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

#include "py/mperrno.h"
#include "py/mphal.h"
#include "py/gc.h"
#include "supervisor/port.h"

#if 0
#define DEBUG_PRINT(...) ((void)mp_printf(&mp_plat_print,##__VA_ARGS__))
Expand Down Expand Up @@ -78,7 +80,7 @@ static bool lock_and_configure_bus(sdcardio_sdcard_obj_t *self) {
}

common_hal_busio_spi_configure(self->bus, self->baudrate, 0, 0, 8);
common_hal_digitalio_digitalinout_set_value(&self->cs, false);
digitalinout_protocol_set_value(self->cs, false);
return true;
}

Expand All @@ -91,7 +93,7 @@ static void lock_bus_or_throw(sdcardio_sdcard_obj_t *self) {
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);
digitalinout_protocol_set_value(self->cs, true);
common_hal_busio_spi_write(self->bus, buf, bytes);
}

Expand Down Expand Up @@ -260,7 +262,7 @@ static mp_rom_error_text_t init_card(sdcardio_sdcard_obj_t *self) {
// and says 80 bit clocks(10*8) is common. Value below is bytes, not bits.
clock_card(self, 10);

common_hal_digitalio_digitalinout_set_value(&self->cs, false);
digitalinout_protocol_set_value(self->cs, false);

assert(!self->in_cmd25);
self->in_cmd25 = false; // should be false already
Expand Down Expand Up @@ -336,11 +338,17 @@ static mp_rom_error_text_t init_card(sdcardio_sdcard_obj_t *self) {
return NULL;
}

mp_rom_error_text_t sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *bus, const mcu_pin_obj_t *cs, int baudrate, bool persistent_mount) {
mp_rom_error_text_t sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *bus, mp_obj_t cs, int baudrate, bool persistent_mount) {
self->bus = bus;
self->persistent_mount = persistent_mount;
common_hal_digitalio_digitalinout_construct(&self->cs, cs);
common_hal_digitalio_digitalinout_switch_to_output(&self->cs, true, DRIVE_MODE_PUSH_PULL);

// Allocate the pins in the same place as self.
bool use_port_allocation = !gc_alloc_possible() || !gc_ptr_on_heap(self);

self->cs = digitalinout_protocol_from_pin(cs, MP_QSTR_cs, true, use_port_allocation, &self->own_cs);
if (self->cs != mp_const_none) {
digitalinout_protocol_switch_to_output(self->cs, true, DRIVE_MODE_PUSH_PULL);
}

@dhalbert dhalbert Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)


self->cdv = 512;
self->sectors = 0;
Expand All @@ -353,7 +361,10 @@ mp_rom_error_text_t sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio
extraclock_and_unlock_bus(self);

if (result != NULL) {
common_hal_digitalio_digitalinout_deinit(&self->cs);
if (self->cs != mp_const_none && self->own_cs) {
digitalinout_protocol_deinit(self->cs);
circuitpy_free_obj(self->cs);
}
return result;
}

Expand All @@ -362,7 +373,7 @@ mp_rom_error_text_t sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio
}


void common_hal_sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *bus, const mcu_pin_obj_t *cs, int baudrate) {
void common_hal_sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *bus, mp_obj_t cs, int baudrate) {
// User mounted, so persistent_mount=false.
mp_rom_error_text_t result = sdcardio_sdcard_construct(self, bus, cs, baudrate, false);
if (result != NULL) {
Expand All @@ -376,7 +387,10 @@ void common_hal_sdcardio_sdcard_deinit(sdcardio_sdcard_obj_t *self) {
}
common_hal_sdcardio_sdcard_sync(self);
common_hal_sdcardio_sdcard_mark_deinit(self);
common_hal_digitalio_digitalinout_deinit(&self->cs);
if (self->cs != mp_const_none && self->own_cs) {
digitalinout_protocol_deinit(self->cs);
circuitpy_free_obj(self->cs);
}
}

int common_hal_sdcardio_sdcard_get_blockcount(sdcardio_sdcard_obj_t *self) {
Expand Down
5 changes: 3 additions & 2 deletions shared-module/sdcardio/SDCard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
typedef struct {
mp_obj_base_t base;
busio_spi_obj_t *bus;
digitalio_digitalinout_obj_t cs;
mp_obj_t cs;
int cdv;
int baudrate;
uint32_t sectors;
Expand All @@ -26,6 +26,7 @@ typedef struct {
// Automounted SD cards are usually persistent across VM's. Note this as needed to allow access
// when the VM is not running.
bool persistent_mount;
bool own_cs;
} sdcardio_sdcard_obj_t;

mp_rom_error_text_t sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *bus, const mcu_pin_obj_t *cs, int baudrate, bool persistent_mount);
mp_rom_error_text_t sdcardio_sdcard_construct(sdcardio_sdcard_obj_t *self, busio_spi_obj_t *bus, mp_obj_t cs, int baudrate, bool persistent_mount);
11 changes: 8 additions & 3 deletions shared-module/sdcardio/__init__.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@

#include "shared-module/sdcardio/__init__.h"

#include "py/obj.h"

#include "extmod/vfs_fat.h"

#include "shared-bindings/microcontroller/Pin.h"
#include "shared-bindings/busio/SPI.h"
#include "shared-bindings/digitalio/DigitalInOut.h"
#include "shared-bindings/sdcardio/SDCard.h"
Expand Down Expand Up @@ -81,7 +84,8 @@ void automount_sd_card(void) {
common_hal_busio_spi_never_reset(spi_obj);
#endif
sdcard.base.type = &sdcardio_SDCard_type;
mp_rom_error_text_t error = sdcardio_sdcard_construct(&sdcard, spi_obj, DEFAULT_SD_CS, 25000000, true);
mp_obj_t cs_obj = MP_OBJ_FROM_PTR(DEFAULT_SD_CS);
mp_rom_error_text_t error = sdcardio_sdcard_construct(&sdcard, spi_obj, cs_obj, 25000000, true);
if (error != NULL) {
// Failed to communicate with the card.
_automounted = false;
Expand All @@ -91,8 +95,9 @@ void automount_sd_card(void) {
#endif
return;
}
common_hal_digitalio_digitalinout_never_reset(&sdcard.cs);

if (mp_obj_is_type(cs_obj, &mcu_pin_type)) {
common_hal_digitalio_digitalinout_never_reset(MP_OBJ_TO_PTR(sdcard.cs));
}
fs_user_mount_t *vfs = &_sdcard_usermount;
vfs->base.type = &mp_fat_vfs_type;
vfs->fatfs.drv = vfs;
Expand Down
Loading