Skip to content

[spi_host] Low-level SD card support#446

Open
elliotb-lowrisc wants to merge 2 commits into
lowRISC:mainfrom
elliotb-lowrisc:spi_sd
Open

[spi_host] Low-level SD card support#446
elliotb-lowrisc wants to merge 2 commits into
lowRISC:mainfrom
elliotb-lowrisc:spi_sd

Conversation

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor

@elliotb-lowrisc elliotb-lowrisc commented Apr 17, 2026

Replace the initial SPI Host loopback connections with connections to the microSD card slot for Genesys2 and an SD card model for Verilator. This includes allocating a GPI pin for card presence detection and a GPO pin for SD card reset (power supply switching).

Also output the SPI Host signals on an otherwise unused Genesys2 PMOD header for easier debugging using a logic analyser.

Add SD card and minimal filesystem helpers for SPI Host, and some tests. These have been ported from sonata-system, which required changes for the different spi_host hardware and translating from C++ to C.

Only a subset of the tests will run in Verilator unless an SD card image is provided.

@elliotb-lowrisc elliotb-lowrisc linked an issue Apr 24, 2026 that may be closed by this pull request
@elliotb-lowrisc elliotb-lowrisc linked an issue Apr 24, 2026 that may be closed by this pull request
@elliotb-lowrisc elliotb-lowrisc force-pushed the spi_sd branch 2 times, most recently from 08ccd27 to db2ad39 Compare May 7, 2026 15:44
@elliotb-lowrisc elliotb-lowrisc changed the title [spi_host] Switch from loopback to SD card access [spi_host] Low-level SD card support May 7, 2026
@elliotb-lowrisc elliotb-lowrisc linked an issue May 7, 2026 that may be closed by this pull request
Comment thread sw/device/lib/runtime/filesys_utils.h Outdated
@elliotb-lowrisc elliotb-lowrisc force-pushed the spi_sd branch 3 times, most recently from c1e2865 to 0510cd1 Compare May 8, 2026 14:25
@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Reduced the level of UART logging when logging = false to avoid timing-out in the Verilator test suite

@elliotb-lowrisc elliotb-lowrisc marked this pull request as ready for review May 8, 2026 15:13
Comment thread hw/top_chip/data/pins_genesys2.xdc Outdated
Comment thread sw/device/lib/hal/uart.c Outdated
Comment thread sw/device/lib/runtime/filesys_utils.c Outdated
#include "runtime/filesys_utils.h"
#include "runtime/print.h"
#include "runtime/sdcard.h"
// #include <assert.h>
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.

drop commented line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Are we not going to be adding support for asserts in the future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@engdoreis do you have any new thoughts on this now that the future of this file is clearer?

Copy link
Copy Markdown
Collaborator

@engdoreis engdoreis May 22, 2026

Choose a reason for hiding this comment

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

I think that any comment code should be removed. A TODO command can be used to remind that asserts should be added.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, I guess if this file is likely to be deleted, then it is unlikely that there will be an opportunity to enable these asserts

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, I've removed the assert stuff from this file, and removed or added TODOs to assert stuff in sdcard.c. Does that work for you?

Comment thread sw/device/lib/runtime/filesys_utils.c Outdated
Comment thread sw/device/lib/runtime/filesys_utils.c Outdated
Comment thread sw/device/lib/runtime/filesys_utils.c Outdated
Comment thread sw/device/lib/runtime/filesys_utils.c
Comment thread sw/device/lib/runtime/sdcard.c
Copy link
Copy Markdown
Collaborator

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I've left some initial comments.

Comment thread hw/top_chip/data/pins_genesys2.xdc
.rst_ni,

.gpio_i (gpio_inputs),
.gpio_i ({gpio_inputs[31:10], microsd_det, gpio_inputs[8:0]}),
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.

This is a little bit ugly because the gpio_input now will just ignore any writes to bit index 9. I'm not sure how to solve it more cleanly though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I'm open to better ideas

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 only thing I can think of is that it is probably better to put the microSD detection in the most significant bit rather than in the middle. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you do change the GPIO, make sure to update this line in the devicetree and regenerate it: https://github.com/lowRISC/mocha/blob/main/sw/device/devicetree/mocha.dts#L76

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Introducing a gap in the range of GPI lines in use might make things trickier on the FPGA-side, where at the moment the input signal is only wide enough for the number of GPI lines assigned to pins. We could change it so that the FPGA GPI input is 32-bits wide and only assign some a subset to pins in the XDC file, or we could adopt #411 and have the mapping of board signals to GPI lines in the RTL rather than constraints, where it would be easier to assign some constant values.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@marnovandermaas your thoughts would be appreciated

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.

I think I would go for a gap where the most significant bits are ones you cannot affect by the DPI and any future general purpose inputs that can be affected by the DPI can be added contiguously at the bottom. I'm happy for you to also implement the #411 at the same time if you think that's not too much work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, I'll move the input to a higher index. I might do the rest of the FPGA pin remapping an a subsequent PR.

@ziuziakowska Is there documentation you could point me to for how to re-generate the device tree?

Comment thread hw/top_chip/dv/verilator/top_chip_verilator.sv Outdated
Comment thread doc/ref/dev_guide.md Outdated
@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Use memcpy, shorten some Verilator signal names, doc fix

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Rework uart printing to use uprintf everywhere as requested

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Rebase

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Lint fixes (I forgot to run the formatter after the last round of changes)

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Rebase and tweak documentation change to better match new dev guide section on additional hardware

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Rework log2 to use builtins

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Rebase to resolve RTL conflicts

Replace the initial SPI Host loopback connections with connections to
the microSD card slot for Genesys2 and an SD card model for Verilator.
This includes allocating a GPI pin for card presence detection
and a GPO pin for SD card reset (power supply switching).

Also output the SPI Host signals on an otherwise unused Genesys2
PMOD header for easier debugging using a logic analyser.
@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Removed commented-out XDC lines (they can always be re-created from the schematic)

Add SD card and minimal filesystem helpers for SPI Host, and some tests.
These have been ported from sonata-system, which required changes for
the different spi_host hardware and translating from C++ to C.

Only a subset of the tests will run in Verilator unless an SD card image
is provided.
@elliotb-lowrisc
Copy link
Copy Markdown
Contributor Author

Removed commented-out assertion lines from filesys code, and removed/TODO-ed in sdcard code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge SD card HW/SW changes to Mocha [hw] SPI host SD card integration

4 participants