[sysrst_ctrl] Misleading documentation and reset default for PIN_ALLOWED_CTL
@rnongie is already working on this.
Since Aug 30, 2024.
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
### Description
The section [Flash Write Protect Output](https://opentitan.org/book/hw/ip/sysrst_ctrl/doc/theory_of_operation.html#flash-write-protect-output) contains the following two sentences:
> [...] The software can release flash_wp_l_o explicitly by setting [PIN_OUT_CTL.FLASH_WP_L](https://opentitan.org/book/hw/ip/sysrst_ctrl/doc/registers.html#pin_out_ctl) to 0 when needed. [...] the value of flash_wp_l_o defaults to logic 0 when it is not explicitly driven via the override function. [...]
Keeping in mind that flash_wp_l is a low-active signal, the first sentence gives the impression that if the override control bit for flash_wp_l is disabled, that the signal will revert to its deasserted high level. However, the second sentence states that in the absense of overriding, the signal will be logic 0, that is, asserted.
In practice, the Z1 silicon seems to behave as the second sentence describes, that is, contradicting the first one.
I think that it would make more sense that the "default" level of flash_wp_l, in absence of any override would be logic 1, that is, deasserted. This would match how the other signal ec_rst_l behaves, in the sense that in absence of override, the level of that signal is controlled by the key combination detection logic, which defaults to logic 1.
Furthermore, the default value for the register PIN_ALLOWED_CTL is to have bits flash_wp_l_0 and ec_rst_l_0 set, and nothing else. This seems consistent with the intention behind the first sentence in the documentation (which is contrary to how Z1 silicon behaves.) If the default level of flash_wp_l is not changed to logic 1, then the PIN_ALLOWED_CTL ought to also have flash_wp_l_1 set by default.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.