Skip to content

Commit

Permalink
interconnect/wishbone: increase WB address width to 31
Browse files Browse the repository at this point in the history 
This is needed to support memory regions up to 4GB in size (currently
limited to 2GB, or 0x8000_0000).

FIXME: CI complains about assertions re. axi_lite.address_width in
       relationship to len(wishbone.adr) and wishbone_adr_shift, which
       seems to be a problem on the 32bit (vexriscv?) CPU used for CI,
       but seems to work fine on Rocket.

Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>

foo
  • Loading branch information
@gsomlo
gsomlo committed Aug 3, 2020
1 parent b8c9da8 commit 70eae5c
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion litex/soc/interconnect/wishbone.py
View file
Expand Up @@ -35,7 +35,7 @@


class Interface(Record):
def __init__(self, data_width=32, adr_width=30):
def __init__(self, data_width=32, adr_width=31):
self.data_width = data_width
self.adr_width = adr_width
Record.__init__(self, set_layout_parameters(_layout,
Expand Down

5 comments on commit 70eae5c

@bunnie
Copy link
Collaborator

@bunnie bunnie commented on 70eae5c Aug 4, 2020

Choose a reason for hiding this comment

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

This breaks all my simulations. The address bus is now 31 bits wide, but is being connected to a VexriscV cpu core with a 30-bit wide address. This makes the top bit a "Z" which is not decodable in a verilog simulation, and the simulation stops before even running the first instruction.

I would recommend doing this in a way where if you want a VexriscV that can go 4GiB memory, you set an argument in the init() parameters to specify that, and then pass the adr_width=31 field only when that is set. Otherwise you're going to break a lot of other people's infrastructure...

@enjoy-digital
Copy link
Owner

Choose a reason for hiding this comment

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

@bunnie: thanks for the feedback, i'm going to look at this.

@gsomlo
Copy link
Collaborator Author

@gsomlo gsomlo commented on 70eae5c Aug 4, 2020

Choose a reason for hiding this comment

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

Maybe if I revert this change, and try to somehow compute the adequate address width (based on the size of the region available to the caller of addr_adapter) here:

https://github.com/enjoy-digital/litex/blob/master/litex/soc/integration/soc.py#L291

we can keep things from breaking? I'm going to try this, see if it's feasible.

Sorry about the breakage :(

@bunnie
Copy link
Collaborator

@bunnie bunnie commented on 70eae5c Aug 4, 2020

Choose a reason for hiding this comment

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

Thanks for looking into it! Feel free to ditch my pull request once you have found a work-around.

@enjoy-digital
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for trying to fix it for us, the default adr_width has been reverted so it will fix things on your side and we'll find another way to get our use-case working without touching the default adr_width.

Please sign in to comment.