<adamgreig[m]>
forcing all the parameters to be spelled out makes the lines a bit longer but I guess not ultimately a bad thing
<adamgreig[m]>
presumably it still works after the change
<whitequark[cis]>
it looks like the only unique CRC there is the ONFI one
<whitequark[cis]>
where the initial value is b"ON"
<whitequark[cis]>
for ONFI, presumably
<whitequark[cis]>
it is otherwise a normal CRC16
<adamgreig[m]>
the NRSC-5 is used in a national radio systems committee (NRSC) air interface, apparently
<whitequark[cis]>
yeah but those are CO2 sensors
<adamgreig[m]>
coincidence that sensiron like it
<whitequark[cis]>
yeah
<adamgreig[m]>
slightly unusual choice of polynomial but conceivably effective for their messages
<adamgreig[m]>
it looks like 0x7f polynomial is strictly superior though, so who knows
<whitequark[cis]>
interesting
<adamgreig[m]>
often people don't care or it doesn't matter
<adamgreig[m]>
if you have a particular spec from a datasheet it's maybe no bad thing to just spell it out rather than using a predefined one that happens to have the same parameters, but much of a muchness
<adamgreig[m]>
for the ONFI one it's cute they picked "ON" as the initial value but a bit annoying :p
<jfng[m]>
cr1901: this is could be done using aliased registers (single address, multiple registers), and writing to a bit would switch which register is exposed
key2 has quit [Quit: Connection closed for inactivity]
<jfng[m]>
in this specific case, the selector would be part the same register, but it could also come from elsewhere
<jfng[m]>
this is not part of the RFC yet, but TBD before it leaves draft status
<cr1901>
I think you can set/clear multiple bits6-0 simultaneously; bit 7 determines whether you set/clear.
<galibert[m]>
I don't understand that in the example: self._rx_status.f.err.w_mask.eq(rx_fifo.r_rdy),
<cr1901>
I just saw this behavior in an "in-production" chip. It's old (of course!), but I figure the scheme is worth mentioning
<jfng[m]>
i conflated changing the mode and changing the register, as i believe that if you change the semantics of a field, it can be considered a different register sharing the same address
<jfng[m]>
galibert[m]: this is detailed further down in the RFC: it is the interface between user logic (i.e. peripheral) and the register storage, to set the `err` field of `rx_status`
<jfng[m]>
so, i guess we can start by going over the RFC's definition of a CSR register
<cr1901>
Ooooh, okay. I think it'll fit nicely under aliased registers, except the aliasing isn't from R/W, but rather bit7
<jfng[m]>
currently, amaranth-soc uses csr.Element (a Record) as the interface between a CSR bus and a register
<galibert[m]>
I have problems with that rfc. The main one can be summarized into "where's the domain, what's the timing?"
<jfng[m]>
this RFC changes Element to become a simple object with Signals instead record fields (r_stb, r_data,w_stb,w_data)
<galibert[m]>
(now that I actually read it :-) )
<jfng[m]>
galibert[m]: domains and timing imply storage right ?
<galibert[m]>
yep
<galibert[m]>
Your rfc seems to imply storage too
<galibert[m]>
Especially the "write from peripheral with a mask" aspect
<jfng[m]>
so we have two classes:
<jfng[m]>
- `RegisterInterface`, which inherits from `Element`
<jfng[m]>
- `Register`, which inherits from `RegisterInterface` and `Elaboratable`
<jfng[m]>
Register provides the storage you are mentioning
<jfng[m]>
but, there are cases where you only want a `RegisterInterface`:
<jfng[m]>
- you have custom storage inside the peripheral
<jfng[m]>
- you don't want storage
<galibert[m]>
I'm looking at your very first example, which, as a first example, is not supposed to be kinky
<jfng[m]>
yep
<jfng[m]>
it's just an UART, in the sync domain
<galibert[m]>
Specifically, rx_status
<galibert[m]>
You have the block "Writing to a field from the peripheral side"
<galibert[m]>
which is a pile of eq, so no timing
<galibert[m]>
seems to model a "write masked to something", and masked requires storage
<galibert[m]>
but, well, no timing
<galibert[m]>
so I'm lost
<jfng[m]>
okay, so the rx_status register would actually be a good example of RegisterInterface, as it only exposes some wires
<galibert[m]>
It's a direct derivative of csr.Register
<jfng[m]>
but RegisterInterface wasn't part of the RFC back then, and i forgot to update it
<galibert[m]>
Ah
<jfng[m]>
however
<jfng[m]>
if for example, rx_status.f.err was a sticky bit
<cr1901>
jfng[m]: Just to be clear, I'm here and reading. Basically, "I am fine with the high-level details of your RFC, and am not particularly concerned w/ how you implement it". So no real further comment right now.
<jfng[m]>
it would have to be a Register, but this isn't the case here
<galibert[m]>
If it's not a sticky bit, then what is mask?
<galibert[m]>
(and why is it connected to ready is the other "interesting" question)
<jfng[m]>
`w_mask` provide bit-level granularity if you have a field of flags of width > 1
<jfng[m]>
you want to be able to set them independantly
<galibert[m]>
Sorry, that makes no sense for me. Either it's storage and sure you need a mask, but you also need a domain, or it's direct connection but then a mask makes no sense since you don't know what to return when the mask is not set
<galibert[m]>
Or I'm missing something, which wouldn't surprise me at all
<jfng[m]>
consider, an event manager in a peripheral
<galibert[m]>
Sure
<jfng[m]>
it would have a pending multi-bit field somewhere in its registers, which would be RW1C, right ?
<galibert[m]>
sure, why not
<galibert[m]>
Works for a pending register
<jfng[m]>
software would clear a bit to acknowledge an event, and the peripheral may set it if the event condition happens
<galibert[m]>
okay
<jfng[m]>
then it's storage
<galibert[m]>
sure
<jfng[m]>
you have w_mask, and the domain is sync by default
<jfng[m]>
i don't get your issue
<jfng[m]>
ah
<jfng[m]>
if it's a "direct connection", you want RegisterInterface
<jfng[m]>
and you drive it yourself
<jfng[m]>
no mask here
<cr1901>
(I also think there's potential for a terminology issue... multiple 1-bit flag fields vs "a single multiple-bit field of flags")
<galibert[m]>
Ok, the domain being sync by default is not said anywhere. Also, sync may or may not be the domain of the bus
<jfng[m]>
RegisterInterface inherits from csr.Element, so it only has r_stb, r_data, w_stb, w_data
<jfng[m]>
and the peripheral would directly drive it
<jfng[m]>
but in the case of a Register, those aforementioned signals are the bus initiator interface
<jfng[m]>
when using a Register, a peripheral has to read/write fields individually, using reg.f.foo.w_mask, w_data, etc
<jfng[m]>
galibert[m]: good point, i should clarify that 'sync' is the domain by default; as for matching the bus domain, that's the job of a designer, i think
<galibert[m]>
so w_mask is the write enable bit field on the register, w_data the data to write (if any bit in w_mask is set), synchronized on sync (or more probably on the bus domain, whichever is it, because sync makes no sense there)
<jfng[m]>
it sync
<galibert[m]>
So if I have two busses with different domains I'm fucked?
<jfng[m]>
and if the bus domain is not sync, CDC is the responsibility of the soc designer
<galibert[m]>
You don't need CDC when the two domains are a fast clock with different enables
<jfng[m]>
there are different cases here, that we shouldn't conflate
<galibert[m]>
take some time to think about it. You can't assume the bus runs on sync in a specific module
<jfng[m]>
to me:
<jfng[m]>
- either the periphal uses a different domain than the bus, in which case the designer must provide some form synchronization
<jfng[m]>
- either the peripheral uses the same domain as the bus, in which case a DomainRenamer on the peripheral would be adequate ?
<galibert[m]>
at some point I suspect you won't be even able to assume sync exists :-)
<jfng[m]>
i mean, when i write any module, i can't assume it will run on sync either, no ?
<cr1901>
I'd like to add that CDCs between buses aren't a lot of code, but they're code that has to be developed _very_ carefully. alexforencich has a good WB CDC module, but it's not obvious what each of the 90 or so lines is doing until you study it.
<galibert[m]>
that's your choice in your module design
<cr1901>
You probably do not want users developing their own CDC bus modules
<jfng[m]>
cr1901: agreed, but i think that those are out of scope of this RFC
<galibert[m]>
but what you're currently saying is that csr.Register does its update on sync whatever domain the bus runs on. Which makes the CDC csr.Register's responsability, which you very much do not want
<jfng[m]>
jfng[m]: same for bus width converters, etc
<galibert[m]>
I think the rfc should say the storage update is done synchronous to the bus domain, and you don't have to do anything more
<cr1901>
(Note that I don't really know what SoCs do. Handshake is common to xfer data, but Idk if that's at the bus interface or some adhoc FF-synchronized REQ/ACK thing just before/after the bus interface)
<galibert[m]>
and you don't ned to talk about sync anywhere, that's up to the module owner to declare the bus endpoint correctly
zyp[m] has joined #amaranth-lang
<zyp[m]>
it seems to me that when you've got a peripheral in a different domain than the bus, it's often more convenient to put the CDC between the register and the peripheral logic, not between the bus and the register
<jfng[m]>
galibert[m]: - the bus-side interface of a `Register` (indirectly, through a CSR multiplexer)
<jfng[m]>
- the peripheral-side interface of a `Register`
<jfng[m]>
are assumed to be driven on the same domain, indeed
<jfng[m]>
whether that domain is sync or foobar, is unimportant, as long as the SoC design uses a DomainRenamer if needed
<galibert[m]>
Yeah, but the domain renamer is around the csr.Multiplexer constructor call and, importantly, ourside of your rfc (e.g. Not Your Problem)
<jfng[m]>
zyp[m]: wouldn't that result in a multiplication of synchronization primitives (e.g. memories, flops) vs. centralizing it on the bus interface ?
<jfng[m]>
galibert[m]: this is my current understanding, yes
<zyp[m]>
yeah, it's a tradeoff that depends on how many register you've got in the relevant domain
<jfng[m]>
and even if that synchronization happened per-register, that would still be outside the RFC
<zyp[m]>
the benefit of doing the CDC downstream of the register is that you're typically dealing with unidirectional signals, not a bidirectional bus
<jfng[m]>
Register is just about providing storage (flops), with two ports, one for the bus initiator (through a multiplexer), the other for the peripheral
<jfng[m]>
and some semantics (R,RW,etc) to arbitrate it
<zyp[m]>
also, I have to agree that I'd also consider CDC out of scope for the register itself, that's something you'd either handle downstream or upstream of the register
<jfng[m]>
yes
<jfng[m]>
<galibert[m]> "I think the rfc should say the..." <- agreed, it will be clarified
<jfng[m]>
this meeting is coming to an end, let's resume the discussion next time
<jfng[m]>
thank you for the feedback !
<galibert[m]>
you're welcome :-)
<galibert[m]>
I still have some questions, but I won't be available for the two next meetings (biking in holland :-) ) We'll see after that
<jfng[m]>
you can always write them here, or comment the PR (which would probably be better if you're on vacation, i guess !)
<cr1901>
I know CDCs are beyond the scope, but now I'm actually wondering where in practice they're implemented on SoCs... upstream of the registerinterface, downstream of the register, and maybe even "between the interface and register" seem to have their advantages
<cr1901>
Although the advantage of "between" is probably negligible for most people (CDC is in a single location for a single peripheral, once a read/write has been acked, you _know_ it has reached the peripheral logic downstream of the register's backing store. Placing CDC "downstream" may take two or more cycles to actually reach the peripheral)
<cr1901>
Anyways ignore this, this is a minddump for future-me
<galibert[m]>
yeah, my other question is on r_stb/w_stb, do you get it, when do you get it (w.r.t the bus domain)
<galibert[m]>
but it actually looks documented now that I reread the rfc
<galibert[m]>
it's surprising to get a r_stb as a result of a host write, but yeah, it means "read the damn register"
<galibert[m]>
and you get ack on host read, good
<jfng[m]>
it depends on the context, but i think it makes sense
<jfng[m]>
on the user side, `r_stb` means that `r_data` is updated with valid data
<jfng[m]>
yeah, you want w_ack if you are interfacing with FIFOs, etc
vegard_e[m] has joined #amaranth-lang
<vegard_e[m]>
I also find it confusing that r and w are swapped wrt. the initiator perspective
<galibert[m]>
is it possible on a fast host to write a register at cycle t and read a different one at t+1 and have the read result depend on the write?
<vegard_e[m]>
for me it'd make more sense to use the initiator directions consistently throughout everything
<cr1901>
Is initiator writes, the responder wants to read
<cr1901>
If*
<jfng[m]>
vegard_e[m]: yeah, it's a bit awkward, but in isolation, this is actually what happens on the user port
<vegard_e[m]>
yeah, but the relationship between initiator and target is not symmetric, so it doesn't need to be crossed
<jfng[m]>
i mean, the thought of using "capture/update" has crossed my mind :p
<jfng[m]>
on the other hand, using r_mask/r_data to write a field from the user port is kind of awkward
<jfng[m]>
s//`/, s///`/`/, s//`/`r_ack`/
<jfng[m]>
to me, the bus initiator and the peripheral are two entities concurrently accessing the same ressource
<jfng[m]>
<vegard_e[m]> "yeah, but the relationship..." <- also, the RFC paints this relationship as symmetric, e.g. only one side has write access, except for specific for flags, where one side can set and the other clears
<jfng[m]>
RW1C and RW1S are symmetric between initiator and peripheral
<jfng[m]>
> <@_discord_157944445168254976:catircservices.org> yeah, but the relationship between initiator and target is not symmetric, so it doesn't need to be crossed
<jfng[m]>
* also, the RFC paints this relationship as symmetric, e.g. only one side has write access, except for the specific case of flags, where one side can set and the other clears
<vegard_e[m]>
I still find it unintuitive that a write transaction on the bus becomes «please read me» (r_stb) and not «I just got written» (w_stb) 🙂
<jfng[m]>
from the point of view of a software driver, this is awkward
<jfng[m]>
from the point of view of a peripheral implementation, i think this makes sense
<jfng[m]>
and software drivers would only see what the BSP gives them :)
<vegard_e[m]>
yeah, I don't have any issue with the software perspective of this, it's just unintuitive to me to think of a register as something the peripheral reads and writes
<jfng[m]>
if storage is needed, then the register can be seen as a communication medium between software and peripheral
<jfng[m]>
e.g. a register to track pending IRQs
anubis has joined #amaranth-lang
<jfng[m]>
the relationship isn't fully symmetrical, for sure
<jfng[m]>
e.g. bus writes happen on the entire register, but peripherals can do them at the bit granularity if needed
<galibert[m]>
even then, "full register" can be say 16 bits and a write be only on 8
<galibert[m]>
from the host side
<jfng[m]>
no
<galibert[m]>
no?
<jfng[m]>
the multiplexer will only commit writes if all 16 bits are written
<galibert[m]>
you ignore lane enables or you segfault on access?
<galibert[m]>
ah, you drop writes. Interesting
<jfng[m]>
if you only write the first 8 bits, then the transaction is ignored
<jfng[m]>
from the RegisterInterface point of view, bus writes are done atomically to the full register
<galibert[m]>
You realize you just thrown all the "we want to reproduce existing interfaces" arguments out?
<galibert[m]>
* realize you've just
<galibert[m]>
because logically partial writes are commonly accepted
<vegard_e[m]>
not really
<vegard_e[m]>
it's rather common for peripheral buses to not have byte lanes, which can give pretty surprising results if you attempt a byte write
<vegard_e[m]>
I've experienced at least some platforms where a byte write to a peripheral register ends up with the byte replicated and written into every byte of the register
<jfng[m]>
ouch
<vegard_e[m]>
i.e. a byte write of `0x54` to the second byte lane is effectively a write of `0x54545454 & 0x0000ff00`, and dropping the byte lanes drops the second half of that
<galibert[m]>
Sounds very 680x0
<galibert[m]>
(we have to emulat byte smearing in mame because of that kind of issues)
<galibert[m]>
s/emulat/emulate/
<vegard_e[m]>
this is still what modern microcontrollers do
<jfng[m]>
also, if some design really wants partial writes (instead of say, RMWs), then i guess it could simply implement its own csr.Multiplexer variant
<cr1901>
it's how litex does it as well; wishbone provides SEL lines to determine which writes/reads actually get through
<galibert[m]>
jfng: personally I don't really care, it's just that Catherine seemed to consider replicating existing interfaces important. If that aspect isn't that important, all good
<galibert[m]>
(for emulation I don't use wishbone in the first place for extremely obvious reasons)
<vegard_e[m]>
I also think replicating existing interfaces is important, but the existing interfaces that actually need byte lanes are likely a minority
<vegard_e[m]>
stm32 have some SPI or UART peripheral that makes use of it to implement a fifo mode, where a byte write puts one byte into the fifo, while a larger write puts multiple bytes into the fifo
<jfng[m]>
galibert[m]: iirc, csr.Multiplexer enforces atomic transactions as a result of her experience working on misoc (litex' predecessor) and ARTIQ, ... which didn't
<galibert[m]>
huhuhu
<galibert[m]>
good then
<galibert[m]>
the atrociousest interface w.r.t byte lates is of course IDE
<galibert[m]>
s/lates/lanes/
<galibert[m]>
where accessing byte at +2, byte at +3 or word at +2 does different things (transfer a byte, access a different register, transfer a word)
<vegard_e[m]>
one peripheral I wish had byte lanes is the stm32 usb buffer memory (for the parts that's not using the dwc_otg core)
<vegard_e[m]>
there's three versions of it, and they've all got the buffer memory hooked up differently, which requires incompatible access patterns
<vegard_e[m]>
v1 has a 16b wide buffer memory hooked up to a 32b port, so you've got two bytes of memory and two bytes of nothing interleaved
<vegard_e[m]>
v2 has a 16b wide buffer memory hooked up to a width translator so it's now consecutive, but IIRC still has to be accessed by 16b transfers because it doesn't split writes or merge reads
nak has joined #amaranth-lang
<galibert[m]>
I'm waiting for "v3 accesses the buffer serially" or something
<vegard_e[m]>
and v3 has a 32b wide buffer memory, but without byte lane enables, so even if it's consecutive like v2, 16b writes would trash the neighboring bytes
<galibert[m]>
Nice design
jjsuperpower has quit [Remote host closed the connection]
anubis has quit [Remote host closed the connection]
<josuah>
9/24
<josuah>
my bad!
mindw0rk has quit [Ping timeout: 245 seconds]
mindw0rk has joined #amaranth-lang
mindw0rk has quit [Ping timeout: 250 seconds]
anubis has joined #amaranth-lang
anubis has quit [Remote host closed the connection]
allysonprolisko[ has quit [Ping timeout: 245 seconds]