Skip to content

[Bootrom] Enable SPI 4 bytes addressing#441

Merged
engdoreis merged 5 commits intolowRISC:mainfrom
engdoreis:bootrom-4b-addr
Apr 16, 2026
Merged

[Bootrom] Enable SPI 4 bytes addressing#441
engdoreis merged 5 commits intolowRISC:mainfrom
engdoreis:bootrom-4b-addr

Conversation

@engdoreis
Copy link
Copy Markdown
Collaborator

@engdoreis engdoreis commented Apr 15, 2026

This will support running tests in DRAM and SRAM.

@engdoreis engdoreis force-pushed the bootrom-4b-addr branch 3 times, most recently from c7252a6 to 4e6f943 Compare April 15, 2026 17:20
Copy link
Copy Markdown

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

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

LGTM, just a few nits.

Comment thread util/fpga_runner.py Outdated
void spi_device_cmd_info_4b_enable_set(spi_device_t spi_device, uint8_t opcode)
{
uint32_t reg = 0;
reg = reg | (opcode << SPI_DEVICE_CMD_OPCODE);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Really minor nit (not necessary to fix in this PR). Subjectively I prefer reg |= (opcode << SPI_DEVICE_CMD_OPCODE); since it removes the needless duplication of reg but I see this is in many places in this file. Maybe we could change it in a different PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I agree, but I don't want to change the spi_device.c because it will replaced by the soon.

// Configure commands
spi_device_cmd_info_set(spi_device, SPI_DEVICE_CMD_INFO_0_REG, SPI_DEVICE_OPCODE_READ_STATUS,
false, 0, false);
spi_device_cmd_info_set(spi_device, SPI_DEVICE_CMD_INFO_0_REG, SPI_DEVICE_OPCODE_READ_STATUS, 0,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Repeated a few different times - even for 0, I think we should use the constant name:

Suggested change
spi_device_cmd_info_set(spi_device, SPI_DEVICE_CMD_INFO_0_REG, SPI_DEVICE_OPCODE_READ_STATUS, 0,
spi_device_cmd_info_set(spi_device, SPI_DEVICE_CMD_INFO_0_REG, SPI_DEVICE_OPCODE_READ_STATUS, SPI_DEVICE_CMD_ADDR_MODE_ADDR_DISABLED,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I agree, but I don't want to change the spi_device.c because it will replaced by the soon.

Comment thread sw/device/bootrom/bootrom.c Outdated
Comment thread sw/device/bootrom/bootrom.c Outdated
Comment thread sw/device/bootrom/bootrom.c Outdated
This a requirement because there will be tests built for DRAM and SRAM.
The bootrom will use this to:
 - Check that there's a valid test loaded in memory before jumping.
 - Discover the address where it should jump to.
@engdoreis engdoreis force-pushed the bootrom-4b-addr branch 2 times, most recently from 190ff9a to 80916f6 Compare April 16, 2026 11:09
@engdoreis engdoreis merged commit fac6f9b into lowRISC:main Apr 16, 2026
5 checks passed
@engdoreis engdoreis deleted the bootrom-4b-addr branch April 16, 2026 13:04
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.

2 participants