lowRISC / lowRISC/opentitan

[tlul_adapter_sram] Check instantiations for factoring in req_o into gnt_i

Open
#30,950 2 comments 0 reactions 4 assignees Claimed by @h-filali View on GitHub
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

### Description

While working on #30902 and studying https://github.com/lowRISC/opentitan/pull/22588 which broke the path between a_valid and a_ready in the TL-UL SRAM adapter in SRAM_CTRL to enable timing closure for Earl Grey Gen1, I noted that:

- This module is used in quite a few places.
- If the SRAM-side gnt_i signal factors in the req_o signal (not the case in SRAM_CTRL), the a_valid to a_ready path is again introduced.

We should check all instantiations of tlul_adapter_sram if this is the case, and if possible, remove this dependency to further ease timing closure.

We need to check the following instantiations:
- [ ] RRAM_CTRL: This is likely going to be critical for Earl Grey 2 on the host interface, i.e., where Ibex fetches instructions. At the moment, RRAM_CTRL signals the `gnt_i` after checking the permissions (host_req_i/gnt_o in rram_ctrl_mp.sv) and the host_gnt_o depends on the host_req_i via the err_fifo_req signal.
- [ ] OTP_CTRL: This is probably a bit less critical as OTP_CTRL sits in the 24 MHz clock domain, but the `tlul_gnt_o` signal inside `otp_ctrl_part_unbuf.sv` indeed factors in `tlul_req_i`. So the path is there. Fixing it looks rather simple. Because it's actually okay to signal statically assign gnt.
- [ ] FLASH_CTRL: Similar to RRAM_CTRL but definitely not a priority.
- [ ] OTBN: The path is there, e.g., for IMEM we have `imem_gnt_bus = imem_req_bus` and this is likely critical as the block sits in the fast domain. Fixing should be easy as the condition whether a bus access is allowed is actually static (depends on whether OTBN is busy, idle or stopped).
- [ ] HMAC: The path is there: `msg_fifo_gnt` directly factors in `msg_fifo_req`. Critical as HMAC is in the fast domain, too.
- [ ] USBDEV: Looks okay, but usbdev_usbif.sv looks complicated and USBDEV is the fast domain. Maybe we can check a timing report.
- [ ] SPI_DEVICE: The path is there. Fixing looks complicated. Luckily SPI_DEVICE sits in the 24 MHz domain.
- [ ] SPI_HOST: This is okay.
- [ ] KMAC: One instance `kmac_staterd` has the path in there. Fixing this looks simple.
- [ ] ROM_CTRL: This looks good :-)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.