<somlo>
_florent_: "assert 2**(csr_address_width + 2) <= 0x1000000" was really a way to express "csr_address_width <= cpu_address_width - csr_base_bits - 2", where cpu_address_width is 32, csr_base_bits is 8, and 2 is the number of LSBits that Wishbone ignores because its smallest addressable unit is 32 bits.
<somlo>
What's left to ensure csr_address_width isn't specified as too large now? Would that be handled by "register_mem" throwing an error if the csr region collides with whatever region there is *after* it?
<_florent_>
somlo: i needed that for a design when i was to minimize csr region to what is actually used. I should have a second look to be sure it's working for all configuration, but the error will indeed be catched by register_mem if csr_address_width is too large (there is there is an overlap with another region)
<somlo>
_florent_: makes sense, I'll update my candidate CSR.md in the github issue :)
<somlo>
now all we need to do is assert that the bits required to represent the number of banks plus the bits required to represent the largest bank's subregisters or slices is <= csr_address_width :)
<somlo>
also factoring in the bit lost to 64 bit (or larger) alignment, that is...
<_florent_>
yes, sorry i was just focusing on my usecase here ... :) i wanted to have a closer look when reviewing you CSR.md
<somlo>
I'll update it to reflect changes in your commit fb6b078 in a little bit, right now I have to go to go afk for a while
<_florent_>
ok thanks
m4ssi has quit [Quit: Leaving]
CarlFK has joined #litex
<somlo>
_florent_: done updating CSR.md, so it's worth looking from this point forward (whenever you have time)
<keesj>
somlo: the commit looks a bit strange to me what does " Additionally, ensure csr_data_width <= csr_alignment, and also that
<keesj>
the csr base address is aligned to (at least) csr_address_width+2."
<keesj>
mean
<keesj>
why would an address need to be alligned to a width +2? perhaps you want to a align to a multiple of width?
<keesj>
and this code confused me further ~(2**(csr_address_width + 2) - 1)) == self.soc_mem_map["csr"]
<keesj>
can you explain what it does?
<somlo>
keesj: that particular assert should be done in register_mem for all calls (if it isn't already)
<somlo>
and what I mean by it is that the base address bits shouldn't overlap with the intra-segment bits
<somlo>
e.g. if you have a 32bit address, and want 16 bits for a CSR segment, then the base address can't occupy *more* than the 16 MSBits
<somlo>
if it did, it wouln't be a *base* address, but rather some adres *within* the segment, and some sort of (negative) feedback should be given to the user if that happens :)
<somlo>
but since this isn't specific to the CSR region, I'll respin the PR to leave this part out, and check if it's already done in register_mem, and try to add it there if it isn't
<somlo>
re csr_alignment: if our csr (sub)registers must be aligned at 32 bits, we shouldn't try to have subregisters of size *larger* than 32 bits
<somlo>
on 64bit CPUs, subregisters must be aligned at 64bit boundaries. In that case, an individual subregister MAY be as large as 64 bits (but of course can be less, down to 8 bits as is the default)
<somlo>
although (_florent_, please chime in and correct me if I'm wrong) are memory regions (as defined by (origin, length) when passed into `add_memory_region`) expected to be properly aligned on bit boundaries in the general case?
<somlo>
i.e., is it expected that `(origin & ~(size-1)) == origin` ?
<somlo>
I think that SHOULD be true for the CSR segment, but not sure in the general mem region case
<_florent_>
somlo: i need to check, but it think it's expected (and checked) to simplify the mem_decoder
rohitksingh has quit [Remote host closed the connection]
rohitksingh has joined #litex
CarlFK has quit [Read error: Connection reset by peer]