-
Notifications
You must be signed in to change notification settings - Fork 1
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
Control/status registers #1
Comments
Here's my proposal for a new CSR design:
|
I've already put together a simple wrapper around CSRs that I initially called It contains things such as values=[
("0b0000", "disable the timer"),
("0b0001", "slow timer"),
("0b1xxx", "fast timer"),
] I made the rule that all Dreg Fields must be valid Python identifiers, except they must not start with "_". Then I expose various values under "_". For example, I also added Finally, For example, with this self.status = status = RegStatus(
Field("have", description="`1` if there is data in the FIFO."),
Field("is_in", description="`1` if an IN stage was detected."),
Field("epno", size=4, description="The destination endpoint for the most recent SETUP token."),
Field("pend", description="`1` if there is an IRQ pending."),
Field("data", description="1` if a DATA stage is expected."),
description="Status and diagnostic information about the SETUP handler",
) You can then access |
Shouldn't all CSRs be reset when the underlying core is reset as well? Having a separate reset just for registers seems odd.
This seems like a desirable feature.
I agree that an instantiated CSR should be " |
Yes, however it's sometimes important to have additional resets. For example, in my current design there is a USB
One issue I'm running into -- in litex, several values are assumed to exist on a CSR. For example, |
I think this might be better addressed with a handshake mechanism. But I will consider this use case.
We can use the same trick as in |
Couple of random thoughts;
Hopefully that was somewhat coherent. |
The code generator will take an abstract model of the registers, yes. It doesn't make any sense to always roundtrip it through JSON, but the abstract model should be bidirectionally convertible to/from canonical JSON representation.
The CSR bus doesn't support atomic updates, which is a significant problem. That would need to be solved.
Of course. Are there any particular considerations for MMIO you have that I'm missing? |
It that because of the "OR-multiplexing" trick done in CSR but not WB, or are there other reasons? |
Why can't the OR-multiplexing trick be done inside the CSR WB adapter, anyway? |
I have no actual proof here, so it would be good for someone to do some real analysis rather than my anecdotes. There might be some useful information @ https://zipcpu.com/zipcpu/2019/08/30/subbus.html The things I have seen are;
|
You can make a 8-bit Wishbone bus. The only real differences between Wishbone and CSR with read-enable signal are:
If that's important enough we can keep CSR, and simply add a read enable signal to support atomic reads (the protocol could be that reading the first byte in a multi-byte CSR makes the target copy the other bytes in that CSR to a buffer that is read from the bus instead of the current value). |
A long time ago, someone said the read/write perspectives in MiSoC's CSRs are inconsistent (link). Would you guys think it's better to call all the read/write data and strobe signals from the CPU perspective? |
Regarding per-bit read/write accessibility, I found an evidence to support such an idea. If we look at this SVD example (please find in page this string: |
I agree that the CPU perspective makes more sense.
Yes. That is one reason why I would like to unify Since no one has shown any objection to this part of proposal, I think it does make sense to do that. |
I've been working on This uses documented CSRs to generate field-level registers. For example, here's a status register https://github.com/im-tomu/valentyusb/blob/tri-fifo/valentyusb/usbcore/cpu/eptri.py#L536: self.status = status = CSRStatus(
fields=[
CSRField("have", description="`1` if there is data in the FIFO."),
CSRField("is_in", description="`1` if an IN stage was detected."),
CSRField("epno", 4, description="The destination endpoint for the most recent SETUP token."),
CSRField("pend", description="`1` if there is an IRQ pending."),
CSRField("data", description="`1` if a DATA stage is expected."),
],
description="Status about the most recent `SETUP` transactions, and the state of the FIFO."
) This gets turned into a table and register map at https://rm.fomu.im/usb.html#usb-setup-status It also gets turned into something functionally useful when it gets used: # Wire up the `STATUS` register
self.comb += [
status.fields.have.eq(buf.readable),
status.fields.is_in.eq(is_in),
status.fields.epno.eq(epno),
status.fields.pend.eq(pending),
status.fields.data.eq(have_data_stage),
] In addition to CSR documentation, we now have With In addition to adding raw Finally, I had a request to allow separating documentation into a different file, to make it easy to edit it in an external editor. So you can pass |
Let's collect requirements for CSRs here.
Overview of oMigen design:
CSRStatus
(R/O) andCSRStorage
(R/W);AutoCSR
collects CSRs from submodules via Python introspection;CSRConstant
allows specifying limited supplementary data in generated code.According to @sbourdeauducq, the primary semantic problem with oMigen CSR design is that it lacks atomicity, and fixing that would involve ditching the CSR bus and instead using Wishbone directly.
According to @mithro and @xobs, a significant problem with oMigen CSR design is that it does not allow generating documentation.
According to @whitequark, a problem with oMigen CSR design is that the code generation is very tightly coupled to MiSoC internals, and is quite hard to maintain.
The text was updated successfully, but these errors were encountered: