csr.periph: add Peripheral base class.

[jfng]

This PR aims to add support for CSR-capable peripherals.

Related issue: #10

A new base class csr.Peripheral can be subclassed to provide nmigen-soc peripherals with helpers for managing CSR registers and sending interrupt requests to a CPU. Support for interrupts is optional.

The plumbing (multiplexing the registers, managing events, requesting interrupts) between the peripheral and the outside world is done by the PeripheralBridge, which is generated for the user by calling self.csr_bridge(). It exposes a csr.Interface and optionally an IRQ line. The bridge is an Elaboratable, so the subclass must add it as a submodule during elaborate().

[codecov[bot]]

Codecov Report

Merging #11 into master will increase coverage by 0.05%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #11      +/-   ##
==========================================
+ Coverage   99.63%   99.69%   +0.05%     
==========================================
  Files           4        5       +1     
  Lines         552      656     +104     
  Branches      127      145      +18     
==========================================
+ Hits          550      654     +104     
  Misses          1        1              
  Partials        1        1              

Impacted Files	Coverage Δ
nmigen_soc/csr/periph.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 967a65f...b9ffd36. Read the comment docs.

[whitequark]

A few general quetions:

• Is a design based on inheritance the best we can do here? The main problem here is that mixing together nmigen-soc code and user code places constraints on evolution of each. If we ever add new fields (even private) their names can clash with user-defined attributes and will be silently overwritten. This is a major backwards compatibility hazard, and this is why I got rid of Migen's Module in favor of nMigen's Elaboratable (which only ever touches private fields). I think a design based on composition would work better in long term, but I do not right now have a specific proposal.
• Should the events be tightly coupled to peripherals, or would it make more sense to introduce them as a separate mechanism that then becomes a part of the peripheral code? I can think of a few uses for IRQ-like, prioritized events in SoC code that do not directly correspond to peripherals. For example, some USB device cores offer many dozens of events that are then composed into a single IRQ line the USB device peripheral provides to the host.

[jfng]

I think a design based on composition would work better in long term, but I do not right now have a specific proposal.

I went with inheritance by lack of knowledge of a better way. By composition, do you mean something like this ?

class ExamplePeripheral(Elaboratable):
    def __init__(self):
        self._bridge = csr.PeripheralBridge(data_width=8, alignment=0)

        self._data   = self._bridge.csr(8, "w")
        self._rdy    = self._bridge.event(mode="rise")

        self.csr_bus = self._bridge.bus
        self.irq     = self._bridge.irq

    def elaborate(self, platform):
        m = Module()
        m.submodules.bridge = self._bridge
        # ...
        return m

Besides relying on naming conventions such as csr_bus or irq, this should keep the boilerplate low while avoiding inheritance.

Should the events be tightly coupled to peripherals, or would it make more sense to introduce them as a separate mechanism that then becomes a part of the peripheral code? I can think of a few uses for IRQ-like, prioritized events in SoC code that do not directly correspond to peripherals. For example, some USB device cores offer many dozens of events that are then composed into a single IRQ line the USB device peripheral provides to the host.

Yes, I can decouple the event management logic from the peripheral, and reuse it inside a csr.PeripheralBridge afterwards.

[awygle]

Hm, I'm sympathetic to both views re: inheritance. The composition example is somewhat unsatisfying because it's purely based on convention, which seems like a recipe for a lot of not-quite-compatible variants. What about using something analogous to FIFOInterface here? If not, I'd at least prefer to expose self.bridge instead of breaking out specific fields.

[whitequark]

Yes, I can decouple the event management logic from the peripheral, and reuse it inside a csr.PeripheralBridge afterwards.

Let's do that first since it's a small self-contained addition, and I can think more about the design for peripherals in the meantime.

By composition, do you mean something like this ?

Yes, something along these lines. In this case, only csr_bus and irq become part of the "peripheral interface", which means that we do not risk breaking existing code if we change implementation details of nmigen-soc.

I have one more proposal here. The PeripheralBridge class that you suggest here is just an implementation detail of every peripheral, and that is a good thing, since it means peripherals have complete implementation freedom. However, from the logical point of view of the code that uses the peripheral, all resources of a peripheral--CSRs, memories, IRQs, configuration constants, etc--are a part of a single logical group. We currently do not have anything like this.

First, consider this grouping from the perspective of firmware. For the firmware running on any specific core, there is a unified view of the available peripherals that the board support package generator uses, consisting of:

1. static configuration data (generating constants),
2. control/status registers (generating accessor functions),
3. memory windows (generating named address ranges),
4. event numbers (generating interrupt handlers and interrupt number constants).

I think you've seen part of my plan (corresponding to items 2 and 3) for the BSP generator in the MemoryMap class. To recap, if you have a list of resources somewhere, then using a root MemoryMap of a particular core, you can use find_resource to determine where in the address space of that core the resources are located.

Second, the hardware. For the hardware the perspective is actually rather different because the memory is hierarchical, but the events are not; beyond that, the memory topology is more complex than a straightforward range tree.

I think what would make sense here is having metadata classes per logical peripheral that collect together static data, CSRs, memories, and events, and which are incrementally grouped together through the entire interconnect hierarchy. The CPU gateware would (I think) only really use the events from this information, but the BSP generator would use all of it.

(I believe this is a more fleshed out version of @awygle's proposal here, who commented before I finished writing this.)