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

Changed from hardcoded to dinamic io inside memwrap.v related to iob-soc instance. #833

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

Vasco-Luz
Copy link
Contributor

When the IOB SoC was instantiated inside the MemWrap, its IO ports were hardcoded. This caused issues with other cores that required the IOB SoC ports to be dynamic. A modification was implemented to address this. This pull request contains the necessary changes.

@Vasco-Luz Vasco-Luz changed the title changed from hardcoded to dinamic io inside memwrap.v Changed from hardcoded to dinamic io inside memwrap.v related to iob-soc instance. Apr 23, 2024
`ifdef IOB_SOC_USE_EXTMEM
wire [ AXI_ID_W-1:0] axi_awid;
assign axi_awid_o = axi_awid;
wire [ AXI_ADDR_W-1:0] axi_awaddr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are these wires used ?

In any case, they are not needed : there are snippets that use the _o and _i directly , no assignment needed .
Please ask the team.

Is this module only used for synthesis?

Copy link
Contributor Author

@Vasco-Luz Vasco-Luz Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fix so the main can be used in other cores. Just like the iob_memwrap the instance iob_soc needs to use a. Vs for the io's to get the new io's, when the iob has alterations, for example in Iob_soc_tester. The wires are used because the .vs of the iob_soc generates io's for the axi that are like .axi_awid_o(axi_awid). Instead of changing the .Vs the solution found was to temporally use assings. So this is not a change related to synthesis.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure you understand: axi_m_m.vs already uses _i and _o , no need for wires. Talk to team

Copy link
Contributor

@P-Miranda P-Miranda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IOB_SOC_USE_EXTMEM wires are used to connect to axi_m_portmap wires in iob_soc instance.

Current if_gen.py does not support generating a portmap with sufixes for wires.
Removing the wires and connecting directly to iob_soc_mwrap.v ports requires adding option to add sufix to connection_name in portmaps:

# if_gen.py module
def m_portmap(port_prefix, wire_prefix, fout, bus_start=0, bus_size=1):
    for i in range(len(table)):
        if table[i]["master"] == 1:
            port = port_prefix + table[i]["name"] + suffix(table[i]["signal"]) # NOTE: port gets suffix
            connection_name = wire_prefix + table[i]["name"] # NOTE: no option to give sufix to connection_name for portmap
            write_portmap(
                port,
                connection_name,
                table[i]["width"],
                bus_start,
                bus_size,
                table[i]["description"],
                fout,
            )

@AndreMerendeira
Copy link
Contributor

This is a special case of portmap .vs since it mixes internal and external connections, hence the fix

@jjts jjts merged commit 2255d8a into IObundle:main Apr 24, 2024
8 checks passed
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

Successfully merging this pull request may close these issues.

5 participants