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

Support for Wire Buses to SystemVerilog Structs in cocotb-bus #59

Open
zjiefan opened this issue Sep 9, 2023 · 3 comments
Open

Support for Wire Buses to SystemVerilog Structs in cocotb-bus #59

zjiefan opened this issue Sep 9, 2023 · 3 comments

Comments

@zjiefan
Copy link

zjiefan commented Sep 9, 2023

Currently, cocotb-bus lacks the ability to wire a bus driver or monitor (e.g., AvalonSTPkts) to a SystemVerilog struct. Many users prefer to use structs for their interfaces, and as a result, have to create wrappers specifically for cocotb simulation, which can be cumbersome.

I've developed a fix for this issue. The solution basically passes a handle dict to the Bus class constructor. In this dict, keys represent the signal names like StartOfPacket, while the values are the corresponding cocotb handles.

For example, here is a helper function that generates a handle dict for an Avalon bus:

For example, I have this helpful function for avalon bus:

def avst_handle_dict(bus: HierarchyObject) -> Dict[str, ModifiableObject]:
    ret: Dict[str, ModifiableObject] = {}
    ret['valid'] = bus.valid
    ret['startofpacket'] = bus.startofpacket
    ret['endofpacket'] = bus.endofpacket
    ret['data'] = bus.data
    if hasattr(bus, 'empty'):
        ret['empty'] = bus.empty
    return ret

The fix modifies the Bus class. For example, here are some of the changes:

-    def _add_signal(self, attr_name, signame, array_idx=None, case_insensitive=True):
+    def _add_signal(self, attr_name, signame, array_idx=None, case_insensitive=True, handle_dict=None):
         self._entity._log.debug("Signal name {}, idx {}".format(signame, array_idx))
-        if case_insensitive:
+        if handle_dict:
+            handle = handle_dict[attr_name]
+        elif case_insensitive:
             handle = self._caseInsensGetattr(self._entity, signame)
         else:
             handle = getattr(self._entity, signame)

This modification has worked great in my local tests. Would the maintainers be interested in this addition? If so, may I go ahead and create a PR?

@egk696
Copy link

egk696 commented Jan 6, 2025

Using structs/records for buses is for sure the mainstream way in the industry. Has this been included?
Btw how would this work with a split struct/record type in the scheme of request and response?

@zjiefan
Copy link
Author

zjiefan commented Jan 13, 2025

i don't think it is included.

split struct/record for request and response shouldn't be a problem. You basically pass two struct names, not just one, to the handlers to extract signals of both directions.

Using structs/records for buses is for sure the mainstream way in the industry. Has this been included? Btw how would this work with a split struct/record type in the scheme of request and response?

@egk696
Copy link

egk696 commented Jan 17, 2025

i don't think it is included.

split struct/record for request and response shouldn't be a problem. You basically pass two struct names, not just one, to the handlers to extract signals of both directions.

Using structs/records for buses is for sure the mainstream way in the industry. Has this been included? Btw how would this work with a split struct/record type in the scheme of request and response?

You mean the current release supports split struct/record buses? Sorry for the hijacking, but could you please give an example on this issue I have opened: #90 ?Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants