[topgen] Handle default values for array elements correctly
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
The topgen auto-generates tie-off assignments for undriven slots of array inter-module signals. When an `inter_signal_list` entry does not specify an explicit `default` field, the helper [`_get_default_name()`](https://github.com/lowRISC/opentitan/blob/639ccbea825347c67d309a260fbe3070ba2bc34f/util/topgen/intermodule.py#L244) computes one. This computation is wrong for array elements.
For signals which are defined in a package it computes the raw value `my_pkg::_DEFAULT` which is correct (e.g., `kmac_pkg::APP_REQ_DEFAULT`).
The final default value however does not respect whether the default value is for a whole array or only an array element. If the width of the signal is >1, the raw value is enclosed in `'{...}`. This is valid to set a default for a whole array. However, the correct default value for an array element would be the raw value only. To fix this, `_get_default_name()` must be able to differentiate the two cases.
As a workaround, set the default explicitly.
To reproduce:
- Based on e1834934ddd52a8ca80cea258276aa77577f5ef4, before the new OTBN-KMAC connection is merged.
For example, increase the number of app interfaces towards the KMAC HWIP from 3 to 4 [here](https://github.com/lowRISC/opentitan/blob/639ccbea825347c67d309a260fbe3070ba2bc34f/hw/ip/kmac/data/kmac.hjson#L134). Also expand the default app config signal so KMAC still compiles: add an entry [here](https://github.com/lowRISC/opentitan/blob/639ccbea825347c67d309a260fbe3070ba2bc34f/hw/ip/kmac/data/kmac.hjson#L147) (duplicating an existing one works fine).
Then run topgen and observe the generated tie-off assignment. It will be
```
// Assign partial inter-module tie-off
...
assign kmac_app_req[3] = '{kmac_pkg::APP_REQ_DEFAULT};
```
instead of:
```
assign kmac_app_req[3] = kmac_pkg::APP_REQ_DEFAULT;
```
Contributor guide
Assessment
This issue has not been assessed yet.