-
Notifications
You must be signed in to change notification settings - Fork 32
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
csr.periph: add Peripheral base class. #11
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
A few general quetions:
|
I went with inheritance by lack of knowledge of a better way. 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
Yes, I can decouple the event management logic from the peripheral, and reuse it inside a |
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 |
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.
Yes, something along these lines. In this case, only I have one more proposal here. The 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:
I think you've seen part of my plan (corresponding to items 2 and 3) for the BSP generator in the 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.) |
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 helpersfor 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 callingself.csr_bridge()
. It exposes acsr.Interface
and optionally an IRQ line. The bridge is anElaboratable
, so the subclass must add it as a submodule duringelaborate()
.