Add optional location to CSRs within a Module

[enjoy-digital]

The CSR locations within a Module are currently determined from the definition order:

class MyModule(Module, AutoCSR):
    def __init__(self):
        self.csr0 = CSRStorage(32)
        self.csr1 = CSRStorage(32)
        self.csr2 = CSRStorage(32)

Inserting a CSR between 2 CSRs moves the following ones:

class MyModule(Module, AutoCSR):
    def __init__(self):
        self.csr0 = CSRStorage(32)
        self.csrx = CSRStorage(32)
        self.csr1 = CSRStorage(32)
        self.csr2 = CSRStorage(32)

Adding csrx will change csr1 and csr2 locations.

We should allow user to use fixed CSR locations within a Module to ease adding/removing a CSR without moving the others.

This could be 1 ) by providing an optional order n in which the CSR should be processed or 2) by providing an optional offset.

With 1) things will still move if the width of the CSR changes and requires more or less bus words. With 2) we should think of the best granularity for the offset to handle the different bus_data_width/csr_data_width cases.

Other suggestions are welcome :)

cc @ozbenh @gsomlo @xobs @mateusz-holenko.

[ozbenh]

What about having a name -> offset map ? That way we can have different map for different "versions" of the devices if necessary and it's nice to keep all the offsets together in one place no ?

[mithro]

Who actually cares about the offset location?

• People using the csr C headers don't? (Bare metal / etc)
• People using the csr.csv or csr.json file don't? (Rust / MicroPython / etc)
• People using the device tree don't? (Linux / Zephyr / etc)

Only people who are hard coding offset locations into their software?

[enjoy-digital]

@mithro: when you start creating systems, you want to keep API compatibility between the different versions of the gateware and software (for all the system or at least some parts, for example a bootloader) and so try to avoid as much as possible CSR mapping changes when it could be avoided or when you are just adding a features. So it's not really a question of using the generated files but a way to be able to guarantee that for any version of the gateware, any version of the software, a feature will work. This is something i want to add since i currently have to work-around in the way the code is described and would like to avoid it.

[ozbenh]

You will need that for Linux drivers too. The device tree will tell you where the CSR bank for the device is, but it won't tell you where each individual CSR is within the bank not the bit layouts.

It would be possible to add that info, at a significant bloat cost, but expect pushback from the community.

Linus himself has expressed very strong opinions in this area in favour of HW interfaces standardization.

What is the drawback anyways ?

[enjoy-digital]

In fact it's already possible to optionally define Memory Regions mapping and CSR locations of the peripherals, adding this is just the next step to be able to create systems with fully static mapping when needed which can have several use cases:

• as @ozbenh describes, to ease having common Linux drivers, we will need to have fixed CSR location within the modules.
• this is required when deploying some systems where just the fact that things are deployed automatically makes you loose some flexibility.
• and just another simple example: if you are asked to create a module that should used an already specified register mapping or be compatible with an existing peripheral (for example because already well supported by software, like a 16550 UART) and you just tell the potential client: the SoC builder i'm using is so powerful that it will generate the mapping for you, no need to worry about specifying it... not sure you'll be taken seriously :)
• ...

So that's more a question of context and use cases. Sometimes it's perfectly fine to just let the SoC automatically generates everything, sometimes at least part of the mapping need to be fixed and in this case, we should not need to use workarounds in the way the gateware is described.

But we need to find something simple, that would be optional. For the peripherals used by Linux, that's possible we won't have to use it or just partially: before upstreaming a driver, we have to already verify that the current mapping offers some flexibility to add new features (spare bits in registers or even spare registers) so that we can just continue to use the automatic CSR locations. But the upstreamed mapping will become the "official" mapping and in case we want to add an extra feature/registrer or refactor the code, we'll need to be able to still generate the module with this "official" mapping, so maybe start to use fixed locations.

[zyp]

Just found this ticket and this is a feature I've been wanting, so here's my two cents.

One of my goals would be to decouple the firmware from the gateware so the gateware can be updated independently of the firmware, i.e. without breaking compatibility with existing firmware builds.

It's possible for a peripheral to have optional features, and for multiple instances of the same peripheral to have different feature sets enabled, which means that each instance might have a different subset of existing registers. All the registers should have the same offsets as in a full-featured instance, allowing for gaps where the unused registers would have been.

Only specifying order would not allow for gaps, so I would strongly prefer being able to specify the offset. For consistency's sake, I propose specifying the offset in bytes regardless of the bus width. I would prefer to get an assertion error if I ask for a misaligned byte offset rather than having word offsets that silently changes if the bus word size changes.