Skip to content
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.

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

opentitan: verilator: fixup test build #2963

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

twilfredo
Copy link
Contributor

@twilfredo twilfredo commented Feb 10, 2022

Pull Request Overview

Fixup an issue when make test-verilator is ran, the binary loaded to
Verilator is incorrect. It would load the binary.64.vmem file which
might have been previously created by make BOARD_CONFIGURATION=sim_verilator verilator or not at all (error). Note that
a .vmem created by the previous command will not have been built for tests.

This PR updates the makefile/run.sh for cw130/nexysvideo to ensure
that the .vmem created using the test binary is loaded into Verilator.

Depends

This PR builds on:

All merged.

Testing Strategy

make test-verilator for both boards. The test outputs observed over screen dev/pts/x.

TODO or Help Wanted

To keep things tidy, I output all the Verilator test build files into verilator_build/ within the current board directory. So you could easily just delete that after. What do you guys think about this implementation?

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

Update

17/2 - Implemented OBJCOPY variable to be used for object copying.

@github-actions github-actions bot added the WG-OpenTitan In the purview of the OpenTitan working group. label Feb 10, 2022
@twilfredo
Copy link
Contributor Author

@alistair23

@twilfredo twilfredo mentioned this pull request Feb 16, 2022
2 tasks
Fixup an issue when `make test-verilator` is ran, the binary loaded to
verilator is incorrect. It would load the `binary.64.vmem` file which
might have been previously created by `make
BOARD_CONFIGURATION=sim_verilator verilator` or not at all (error). Note that
a `.vmem` created by the previos command will not include any tests.

This PR updates the `makefile/run.sh` for cw130/nexysvideo  to ensure
that the test binary is correctly loaded into verilator.

Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
@twilfredo twilfredo force-pushed the twilfred-verilator-tests-fixup branch from 3adbf56 to dee4eab Compare February 17, 2022 07:00
@bradjc bradjc merged commit 67b69c6 into tock:master Feb 18, 2022
bors bot added a commit that referenced this pull request Feb 28, 2022
2966: opentitan: add spi_host  r=bradjc a=thulithwilfred

### Pull Request Overview

This pull request adds spi_host implementation to opentitan. The driver has been designed with reference to [1]. Opentitan has both spi_device and spi_host capabilities, this PR implements only the spi_host. The base register mapping can be found [2].

### Testing Strategy

Tested by using Verilator and  basic testing on FPGA with the provided unit tests. For Verilator testing, this PR depends on:
- #2963

Tested on the ongoing `ibex_spi` development branch on qemu.

### TODO 

Mainline qemu still requires a patch to update spi_host/spi_device base addresses. As such, testing on qemu by including the `#cfg...` will cause a store access fault when the board initializes SPI and attempts to write to 'invalid' registers in qemu. To avoid CI failure, these sections have be isolated using `#cfg...`. To be removed when qemu fixup pr is merged. 

### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make prepush`.

### References
[1] https://docs.opentitan.org/hw/ip/spi_host/doc/

[2] https://github.com/lowRISC/opentitan/blob/6c317992fbd646818b34f2a2dbf44bc850e461e4/hw/top_earlgrey/sw/autogen/top_earlgrey_memory.h#L107

Co-authored-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WG-OpenTitan In the purview of the OpenTitan working group.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants