The sdcardio library is not working correctly

[vano7209]

CircuitPython version and board name

Adafruit CircuitPython 10.0.0-alpha.1-19-g380b8bda7a-dirty on 2025-04-08; ESP32-S3-DevKitC-1-N8R8 with ESP32S3
Board ID:espressif_esp32s3_devkitc_1_n8r8
UID:468E334483CD

Code/REPL

import busio
import board
import sdcardio

sd_card_cs_pin  = board.IO15
spi0_module     = busio.SPI(clock=board.IO12, MOSI=board.IO11, MISO=board.IO13)
sd_card = sdcardio.SDCard(spi0_module, sd_card_cs_pin, baudrate=int(250000))

buff_write = bytearray([0xAA]*512)
buff_read = bytearray([0]*512)

sd_card.writeblocks(0, buff_write)
sd_card.readblocks(0, buff_read)

if buff_write[0] != buff_read[0]:
    print(buff_read)

Behavior

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

Press any key to enter the REPL. Use CTRL-D to reload.
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.
main.py output:
bytearray(b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02;v'\x00\x00\x00\x01\x01\x00\x0c\xfe\x7f\xe8?\x00\x00\x00\xc1\xffw\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00U\xaa")

Code done running.

Press any key to enter the REPL. Use CTRL-D to reload.

Description

The writeblocks function from the sdcardio library does not work in principle. Any attempts to use it fail. And here's why:

• The writeblocks call is addressed to the file "shared-bindings/sdcardio/SDCard.c", function "_sdcardio_sdcard_writeblocks":

static mp_obj_t _sdcardio_sdcard_writeblocks(mp_obj_t self_in, mp_obj_t start_block_in, mp_obj_t buf_in) {
    uint32_t start_block = mp_obj_get_int(start_block_in);
    mp_buffer_info_t bufinfo;
    mp_get_buffer_raise(buf_in, &bufinfo, MP_BUFFER_READ);
    sdcardio_sdcard_obj_t *self = (sdcardio_sdcard_obj_t *)self_in;
    int result = common_hal_sdcardio_sdcard_writeblocks(self, start_block, &bufinfo);
    if (result < 0) {
        mp_raise_OSError(-result);
    }
    return mp_const_none;
}

• This function forwards the call to "shared-module/sdcardio/SDCard.c", function "common_hal_sdcardio_sdcard_writeblocks":

int common_hal_sdcardio_sdcard_writeblocks(sdcardio_sdcard_obj_t *self, uint32_t start_block, mp_buffer_info_t *buf) {
    // deinit check is in lock_and_configure_bus()
    if (buf->len % 512 != 0) {
        mp_raise_ValueError_varg(MP_ERROR_TEXT("Buffer must be a multiple of %d bytes"), 512);
    }
    lock_and_configure_bus(self);
    int r = sdcardio_sdcard_writeblocks(MP_OBJ_FROM_PTR(self), buf->buf, start_block, buf->len / 512);
    extraclock_and_unlock_bus(self);
    return r;
}

• In this function, the SPI locking and setting is performed and then the "sdcardio_sdcard_writeblocks" function is called in which the locking is called again:

mp_uint_t sdcardio_sdcard_writeblocks(mp_obj_t self_in, uint8_t *buf, uint32_t start_block, uint32_t nblocks) {
    // deinit check is in lock_and_configure_bus()
    sdcardio_sdcard_obj_t *self = MP_OBJ_TO_PTR(self_in);
    if (!lock_and_configure_bus(self)) {
        return MP_EAGAIN;
    }
…

• Since the line is already taken, the lock fails and an MP_EAGAIN error is returned. But besides the fact that the SPI is taken twice, the returned error is greater than zero, which is treated as no error in the "_sdcardio_sdcard_writeblocks" function and is ignored.

• The problem does not occur when using storage|vfsFAT, because in the file "extmod/vfs_blockdev.c", functions "mp_vfs_blockdev_init":

void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
    mp_load_method(bdev, MP_QSTR_readblocks, self->readblocks);
    mp_load_method_maybe(bdev, MP_QSTR_writeblocks, self->writeblocks);
    mp_load_method_maybe(bdev, MP_QSTR_ioctl, self->u.ioctl);

    // CIRCUITPY-CHANGE: Support native SD cards.
    #if CIRCUITPY_SDCARDIO
    if (mp_obj_get_type(bdev) == &sdcardio_SDCard_type) {
        self->flags |= MP_BLOCKDEV_FLAG_NATIVE | MP_BLOCKDEV_FLAG_HAVE_IOCTL;
        self->readblocks[0] = mp_const_none;
        self->readblocks[1] = bdev;
        self->readblocks[2] = (mp_obj_t)sdcardio_sdcard_readblocks; // native version
        self->writeblocks[0] = mp_const_none;
        self->writeblocks[1] = bdev;
        self->writeblocks[2] = (mp_obj_t)sdcardio_sdcard_writeblocks; // native version
        self->u.ioctl[0] = mp_const_none;
        self->u.ioctl[1] = bdev;
        self->u.ioctl[2] = (mp_obj_t)sdcardio_sdcard_ioctl; // native version
    }
    #endif
…

Binds calls without “common_hal_”.

Additional information

It is necessary to replace errors (MP_EAGAIN) with (-MP_EAGAIN), and remove line lock/unlock in the function "common_hal_sdcardio_sdcard_writeblocks", as it is done in "common_hal_sdcardio_sdcard_readblocks".

[RetiredWizard]

Thanks for the detail issue description, I looked through the modules you covered and I could be mistaken but I think there is an unsigned integer issue.

sdcardio_sdcard_writeblocks/readblocks return mp_uint_t values so the negative errors end up as positive 2s-compliments in common_hal versions of the routines. The mp_unit_t write_blocks routine appears to only return EAGAIN, errors with mangled negative codes or a zero so the return value could be forced negative if non-zero in the calling routine.

That feels like a bit of a hack but I don't know if changing the native sdcardio_sdcard routine return value type to mp_int_t would cause problems, the only other place I spotted it being used was "extmod/vfs_blockdev.c". If the native routines return type was changed then I think more consistent error handling could be implemented throughout the routines.

[vano7209]

I'm assuming that all functions that call “sdcardio_sdcard_writeblocks” and “sdcardio_sdcard_readblocks” are of int type, so changing the function type to mp_int_t and negating MP_EAGAIN should not break anything.
If do not change the function type, everything will still work. A negative error will become just a large positive number, for example MP_EAGAIN = 11, but if you change it to -11, the functions will return something like 0xFFFFFFF5. But since all calling functions are of int type, the error will convert to -11 again.
I have already tested sdcardio working if I change only the MP_EAGAIN sign, and it works as expected, giving a busy error to the console.