[topgen] Support external TL-UL connections #58
Conversation
|
@LouisTheLuis This PR's name is too verbose: could you change, perhaps to something like "[topgen] Support external TLUL connections": this captures the intent and is more concise. A detailed description doesn't belong in the title. |
| type: uni | ||
| act: rcv | ||
| width: 1 | ||
| default: " " |
There was a problem hiding this comment.
Why do you need to add a space here?
There was a problem hiding this comment.
This is a change also performed to support an ibexless top in #30. When rv_core_ibex is removed, the cpu_dump signal from rstmgr defaults to a rv_core_ibex_pkg parameter. This causes linting issues as removing rv_core_ibex also removes rv_core_ibex_pkg.
The change was made in rstmgr.hjson.tpl, so this is auto-generated.
|
Before I get too far into this review I have a question: should we modify darjeeling to have an ibex-less top attached to it? This appendage could destabilize darjeeling, plus it may not even belong in the ROT, but in some other opentitan derived top. Attaching multiple tops is the role of another flow. |
The changes in |
LouisTheLuis
force-pushed
the
zR-multitop-new-tlul-ports
branch
from
8378bf9 to
75453ce
Compare
LouisTheLuis
changed the title
LouisTheLuis
force-pushed
the
zR-multitop-new-tlul-ports
branch
from
75453ce to
e5b0716
Compare
util/topgen/intermodule.py
Outdated
| 'target module has no matching bus interface.'.format( | ||
| 'host' if is_host else 'device', port['name'])) | ||
| continue | ||
| if "external" in name_to_module[port_base].keys() and \ |
There was a problem hiding this comment.
I have noticed you use this idiom in other places as well. Rather than this it is more pythonic to use if module[port_base].get("external"):.
This is because Dict's get returns None if there is no such a key, otherwise it returns the corresponding value, and None is always False in an if.
|
One more meta-comment. The substance of this PR are the changes in the tools to support the ibex-less tops. The tops you create are really just for testing the new feature. I think we can test the "external" feature with just a simple example along the lines of {repo_top}/util/example/tlgen/xbar_main.hjson, perhaps with an associated pytest to verify specific properties of the generated artifacts? You may also need a simple top.hjson along the xbar? I also apologize: I could have framed this change along these lines before all the work you did with the concrete tops. On the positive side, you understand topgen a lot better now. |
LouisTheLuis
force-pushed
the
zR-multitop-new-tlul-ports
branch
14 times, most recently
from
ab17429 to
804fa0a
Compare
|
Have removed the modified tops just to leave the essential changes to |
LouisTheLuis
force-pushed
the
zR-multitop-new-tlul-ports
branch
4 times, most recently
from
c27073c to
8948247
Compare
LouisTheLuis
force-pushed
the
zR-multitop-new-tlul-ports
branch
from
8948247 to
f94a9ed
Compare
This commit builds upon a previous PR (zerorisc#30). Introduced some attributes that allow for an external TL-UL port to be added to a particular top. For a `host` port, it requires defining the external device (e.g. `top_darjeeling_no_ibex`) as a module in the top configuration file; include the device in the address space for the top and include the following additional fields: - `external`, indicating that it is a external device - `external_size`, indicating the byte size of the address space of the device For a `device` port, adding the node in the corresponding `xbar` with the `external` field will generate the corresponding port connection.
LouisTheLuis
force-pushed
the
zR-multitop-new-tlul-ports
branch
from
f94a9ed to
1fae946
Compare
LouisTheLuis
changed the title
2 participants
This commit builds upon a previous PR (#30). Introduced some attributes that allow for an external TL-UL port to be added to a particular top.
For a
hostport, it requires defining the external device (e.g.top_darjeeling_no_ibex) as a module in the top configuration file; include the device in the address space for the top and include the following additional fields:external, indicating that it is a external deviceexternal_size, indicating the byte size of the address space of the deviceFor a
deviceport, adding the node in the correspondingxbarwith theexternalfield will generate the corresponding port connection.