[spi_host] Low-level SD card support by elliotb-lowrisc · Pull Request #446 · lowRISC/mocha

elliotb-lowrisc · 2026-04-17T15:17:41Z

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.

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 · 2026-05-08T14:26:46Z

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

engdoreis · 2026-05-08T14:45:25Z

+## SPI Host (MicroSD card slot)
+set_property -dict { PACKAGE_PIN R28   IOSTANDARD LVCMOS33 } [get_ports { spi_host_sck_o }]; # SD_SCLK
+set_property -dict { PACKAGE_PIN R26   IOSTANDARD LVCMOS33 } [get_ports { spi_host_sd_i  }]; # SD_DAT0
+# set_property -dict { PACKAGE_PIN R30   IOSTANDARD LVCMOS33 } [get_ports { microsd_dat1 }]; # SD_DAT1 unused in SPI bus mode


Any reason to keep the commented line?

It keeps a record of how to connect up the other SD card pins, should they be needed in future.

engdoreis · 2026-05-08T15:08:42Z

+void uart_dump_bytes(uart_t uart, const uint8_t buf[], size_t len)
+{
+    for (size_t off = 0u; off < len; ++off) {
+        uart_putchar(uart, '0' + (buf[off] >> 4));


Please use the printf functions to implement the dump:
https://github.com/lowRISC/mocha/blob/main/sw/device/lib/runtime/print.h

Wouldn't that create a circular reference?

engdoreis · 2026-05-08T15:09:19Z

+#include "runtime/filesys_utils.h"
+#include "runtime/print.h"
+#include "runtime/sdcard.h"
+// #include <assert.h>


drop commented line.

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

engdoreis · 2026-05-08T15:10:54Z

+
+
+// Copy a sequence of bytes; destination and source must _not_ overlap.
+static void copy_bytes(uint8_t *dst, const uint8_t *src, size_t len)


use memcpy from: https://github.com/lowRISC/mocha/blob/main/sw/device/lib/runtime/string.h

Thanks, I'll give this a go next time I have access to an FPGA

engdoreis · 2026-05-08T15:13:49Z

+// public:
+// Unfortunately FAT stores the literal values for bytes/sector and sectors/cluster but only
+// powers of two are permitted.
+static inline uint8_t floor_log2(uint16_t n)


I think you can use a builtin_ctz here:

mocha/sw/device/lib/builtin.h

Line 14 in 5a99b1d

#define ctz(x) (__builtin_ctz((x)))

I think we want to find the leading set bit, not the trailing set bit

engdoreis · 2026-05-08T15:15:38Z

+    uint8_t *dataBuffer = fs->buf.dataBuffer;
+    if (!read_blocks(fs->spi, 0, dataBuffer, 1u, fs->uart)) {
+        if (fs->uart) {
+            uart_puts(fs->uart, "Unable to read the MBR of the SD card\n");


I would use uprintf everywhere for consistency

Why use a formatting printer just to output a string?

engdoreis · 2026-05-08T15:24:59Z

+}
+
+// Open a directory object that started in the given cluster.
+fs_utils_dir_handle_t dir_open(fs_utils_state_t *fs, uint32_t cluster)


This is a lot of code to maintain, for the purpose of testing the sdcard, handling directories is not important. So I suggest droping the directory handling and keeping the bare minimum flat file handling.
Specially because the real use case of the SDCard will be done in Uboot which already has a filesystem implementation.

I'm confused. This code is used in the SD Card test I added. Are you suggesting performing a lesser test?

In any case, I'd have thought it useful to keep this code in case we want to to use bare-metal SD card access in future, such as for a demo.

engdoreis · 2026-05-08T15:28:54Z

+{
+    // Every card tried seems to be more than capable of keeping up with 12.5Mbps.
+    const unsigned kSpiSpeed = 1u; // 50 MHz / (1 + 1) = 12.5 MHz
+    DEV_WRITE(spi + SPI_HOST_CONFIGOPTS_REG, 0xFFFF & kSpiSpeed);


Why not use the spi_host driver instead of directly accessing the mmios?

The spi_host HAL does not presently support many of the features of the spi_host block that this code needs to make the SD card communications work. Perhaps the sdcard code could be re-written if the HAL is sufficiently improved, but until then I thought it better to just stick to using the spi_host block directly rather than only partially using the HAL.

marnovandermaas

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

marnovandermaas · 2026-05-08T15:39:06Z

-# Bootstrap pin, should be pulled down during boot to enter bootstrap mode.
-set_property -dict { PACKAGE_PIN AB29  IOSTANDARD LVCMOS33 PULLTYPE PULLUP } [get_ports { gpio_i[8] }];
+#   Bootstrap pin, should be pulled down during boot to enter bootstrap mode.
+set_property -dict { PACKAGE_PIN AB29  IOSTANDARD LVCMOS33 PULLTYPE PULLUP } [get_ports { gpio_i[8] }]; # PROG_RXFN


What does "PROG_RXFN" mean?

It's the name in the Genesys2 schematic for the net connected to this pin. Unlike Sonata, we're not using the schematic names for the FPGA-top signal names (see #411), so I'm adding them as a comment instead.

marnovandermaas · 2026-05-08T15:42:25Z

    .rst_ni,

-    .gpio_i    (gpio_inputs),
+    .gpio_i    ({gpio_inputs[31:10], microsd_det, gpio_inputs[8:0]}),


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.

Indeed, I'm open to better ideas

marnovandermaas · 2026-05-08T15:59:20Z

  logic       spi_device_sdi;

+  // SPI host signals
+  logic       spi_host_sck_output,    spi_host_csb_output;


I'm not sure I think the "output" tag is more clear here. If you don't like just calling the signals spi_host_* then maybe we can label it h2d for host to device or something similar to indicate direction.

Ah right, I think I was following something like what I did for GPIO, but this is not bidirectional so things can be simpler

marnovandermaas · 2026-05-08T16:02:05Z

 openFPGALoader -b genesys2 build/lowrisc_mocha_chip_mocha_genesys2_0/synth-vivado/lowrisc_mocha_chip_mocha_genesys2_0.bit
 ```

+For the SD card tests, you will also need to insert a FAT32-formatted microSD card containing the "lorem.ips" file (see sw/device/tests/spi_host/lorem_text.h).


Nit: remove "will"

elliotb-lowrisc mentioned this pull request Apr 17, 2026

[hw] SPI host SD card integration #402

Open

elliotb-lowrisc linked an issue Apr 24, 2026 that may be closed by this pull request

Draft hardware changes for SPI - SD card integration #468

Closed

elliotb-lowrisc removed a link to an issue Apr 24, 2026

Draft hardware changes for SPI - SD card integration #468

Closed

elliotb-lowrisc mentioned this pull request Apr 24, 2026

Draft hardware changes for SPI - SD card integration #468

Closed

elliotb-lowrisc linked an issue Apr 24, 2026 that may be closed by this pull request

[hw] SPI host SD card integration #402

Open

elliotb-lowrisc force-pushed the spi_sd branch 2 times, most recently from 08ccd27 to db2ad39 Compare May 7, 2026 15:44

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 mentioned this pull request May 7, 2026

Merge SD card HW/SW changes to Mocha #534

Open

elliotb-lowrisc linked an issue May 7, 2026 that may be closed by this pull request

Merge SD card HW/SW changes to Mocha #534

Open

engdoreis reviewed May 8, 2026

View reviewed changes

Comment thread sw/device/lib/runtime/filesys_utils.h Outdated

elliotb-lowrisc force-pushed the spi_sd branch 2 times, most recently from a0fdd00 to c1e2865 Compare May 8, 2026 12:36

elliotb-lowrisc force-pushed the spi_sd branch from c1e2865 to 0510cd1 Compare May 8, 2026 14:25

elliotb-lowrisc marked this pull request as ready for review May 8, 2026 15:13

engdoreis reviewed May 8, 2026

View reviewed changes

marnovandermaas reviewed May 8, 2026

View reviewed changes



		// Copy a sequence of bytes; destination and source must _not_ overlap.
		static void copy_bytes(uint8_t dst, const uint8_t src, size_t len)

Conversation

elliotb-lowrisc commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

elliotb-lowrisc commented May 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

elliotb-lowrisc commented Apr 17, 2026 •

edited

Loading