<Lord_Nightmare>
@whitequark[cis]: sean riddle figured out a way, with some glitching, to dump the HD6301Y0 chip, info is on his site, i cant link it this second though since i don't have a working browser here
<_whitenotifier-3>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 1b1be38 - Deploying to main from @ amaranth-lang/amaranth@4014aef0330ea459d35a9995bb8d7f49ae0cba11 🚀
<_whitenotifier-3>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 1ad09b5 - Deploying to main from @ amaranth-lang/amaranth@ded84fe9d640c1fffa4a089fcb7233319da5ea93 🚀
<Hoernchen>
are there any known obvious pitfalls when using FSM()s? my design simulates fine and looks the same, but breaks after adding one new fsm state that just does next =..
vegard_e[m] has joined #amaranth-lang
<vegard_e[m]>
next = … or m.next = …?
<Hoernchen>
m.next
<whitequark[cis]>
breaks how?
<Hoernchen>
as in stops working in practice
<Hoernchen>
i don't know what to debug, to be honest
<whitequark[cis]>
do you have a waveform dump?
<whitequark[cis]>
can you share source code?
<Hoernchen>
yeah let me just quickly copy that part..
<vegard_e[m]>
I don't know about common pitfalls, but I were reorganizing the order of some states the other day and forgot to update all the transitions so it got stuck in a loop
<whitequark[cis]>
also, note that the first state you define becomes the reset state
<Hoernchen>
well it has to be 5 instead of with the intermediate state because one clock cycle does not get counted
<whitequark[cis]>
sorry, I'm really struggling to understand what exactly goes wrong
<whitequark[cis]>
could you please show me two code snippets, both in the exact form that you are running, their corresponding waveforms, and the point on the waveform where you want the values to be different?
<Hoernchen>
there should be no difference
<Hoernchen>
the fsm just clocks in a few bits and outputs it
<Hoernchen>
well at least it sounds like there is nothing obvously wrong, so that is a relief, and i'll tie that fsm to a few debug leds
<tpw_rules>
is an SoC meeting planned today?
jfng[m] has joined #amaranth-lang
<jfng[m]>
yes
<tpw_rules>
okay, see you soon
<Hoernchen>
ah the problem is apparently not really my fsm, just timing related issues, the design works as expected when using diamond (lse instead of synplify)
<vipqualitypost[m>
is there a difference in the simulation between the two?
<whitequark[cis]>
I gather that Hoernchen is not simulating the design
<Hoernchen>
yeah i'm running it on the device, the whole contraption drives an asycnfifo and then luna usb, which makes debugging the part that gets stuck/glitches/<dragons here> difficult..
<tpw_rules>
it's fine to not try to debug/simulate luna usb, but putting in at least a cut at the asyncfifo so you can simulate before has immense value
<whitequark[cis]>
is luna updated yet for amaranth 0.4?
<whitequark[cis]>
oh it is, neato
<jfng[m]>
i'm going to be 10min late for the SoC meet, sorry
<cr1901>
I'm only sort of here... still dealing with hard disk failure and finishing tying up loose ends
<jfng[m]>
you may notice that the `access` parameter has been moved from `__init__()` to `__init_subclass__()`
<jfng[m]>
in addition, it can no longer have a default value, users must explicitly provide it
<whitequark[cis]>
tpw_rules: it specifies designer intent. usually some registers are planned to be read-only upfront, and adding an RW field to these later must fail, as it violates a design expectation
<jfng[m]>
@whitequark:matrix.org if `Register` was directly instantiated, then we wouldn't have a value for `access`
<jfng[m]>
this is why it is now required to subclass it
<whitequark[cis]>
but you can provide it as a constructor argument?
<tpw_rules>
okay, that makes sense. is it documented? can it not be assumed as "rw" if not provided either?
<whitequark[cis]>
if you are generating registers from metadata for some reason, it would be pretty annoying to have to generate these subclasses
<cr1901>
>and adding an RW field to these later must fail, as it violates a design expectation <-- what stops someone from changing access to "rw" to bypass that?
<whitequark[cis]>
cr1901: nothing, like nothing stops you from wrapping your entire rust program in a huge `unsafe {}` block
<jfng[m]>
i'm not confortable with a parameter that can both be set from `__init__` and `__init_subclass__`
<whitequark[cis]>
jfng: sure, but what should be done for the use case I highlight?
<tpw_rules>
is it an error to have an RW field on an access="r" register?
<whitequark[cis]>
tpw_rules: yes
<jfng[m]>
could these be generated programmatically, using `type("MyRegister", (Register, ...), {....})` ?
<whitequark[cis]>
yes, but that's awful
<jfng[m]>
yes
<galibert[m]>
Not 100% sure I see the point of the redundancy
<tpw_rules>
the RFC only talks about field access, not register access.
<tpw_rules>
maybe i'm supposed to understand element access first?
<whitequark[cis]>
galibert[m]: it replaces a comment saying something like "all fields in all registers in this block must be R/O", which is somewhat common in IP, with a mechanical check
<tpw_rules>
does field access share anything in common with register access? i.e. is that what access="r" checks?
<jfng[m]>
register access constrains field access
<jfng[m]>
e.g. if your register is read-only, you can't have writable fields
<jfng[m]>
if your register is write-only, you can't have readable fields
<jfng[m]>
(see the `Register.__init__()` docstring, for now)
<galibert[m]>
Are we going to have to make a new class for every register?
<jfng[m]>
a placeholder for reserved fields is neither readable or writable, and has access="n"
<jfng[m]>
every register *definition*, yes
<jfng[m]>
if you replicate the same definition for an array of registers, no
<whitequark[cis]>
please don't take me arguing for making it possible to instantiate a Register wrongly; creating a register class will be the vast majority of uses of this API. however, some important edge cases should be covered too
<cr1901>
Array-of-reg support built-in is cool
<galibert[m]>
I'm going to find really annoying to have the code-level information about the fields for a device exploded over a bunch of classes
<whitequark[cis]>
how so?
<whitequark[cis]>
(I'm not sure what you mean by "code-level information")
<jfng[m]>
@whitequark:matrix.org i wonder if we can just add support for direct instantiation later, once the need for it arises ?
<tpw_rules>
maybe we should rename csr.reg.Field like csr.reg.Container or .Slot or something?
<whitequark[cis]>
jfng[m]: I'm not very comfortable with the current API and the workaround you suggest
<whitequark[cis]>
I can't think of any other place in Amaranth where we require the use of type() to override a restriction we place
<tpw_rules>
^
<cr1901>
tpw_rules: Field is the terminology I'm used to personally. YMMV.
<whitequark[cis]>
and almost nobody even knows how to use type(), so people will surely get confused and frustrated
<tpw_rules>
cr1901: the problem is the new indirection between the field and the component that implements it
<whitequark[cis]>
tpw_rules: any objection to `Field(csr.mode.R, ...)`?
<tpw_rules>
i thought last time we posited csr.action.R which i liked
<whitequark[cis]>
oh, I forgot. if we have already decided on this why rehash it now?
<jfng[m]>
`csr.mode` is redundant with the `access` parameter of `FieldPort`, imo
<galibert[m]>
here's for instance the first serial controller on a h8s-2655
<jfng[m]>
action is somewhat better, but i'm not sure
<whitequark[cis]>
as someone who influenced the design of the CSR API, I will say that this kind of register map is exactly what the current API is made for
<whitequark[cis]>
* galibert: as someone
<tpw_rules>
cr1901: and the objection i raised is that the things in csr.field are not Fields and you'll get weird error messages if you try to interchange them
<galibert[m]>
That's what I understand too. But I'm afraid of the 7 classes that are going to hide whether you got it right
* cr1901
nods
<whitequark[cis]>
galibert: how is that different from the 7 `Register({...})` instantiations? it's even almost the same amount of lines, if you leave newlines in between
<jfng[m]>
whitequark[cis]: then let's support it in this RFC
<galibert[m]>
dispersion. The instanciations would be in the same place and in order
<whitequark[cis]>
you define the 7 classes one after one
<tpw_rules>
they are that way in the RFC now
<whitequark[cis]>
actually, you might notice that the current CSR RFC does not guarantee specific addresses for each register, because the CSR bus width is variable
<whitequark[cis]>
if you want specific addresses you have to pass in an explicit register address map
<cr1901>
action is fine. Ty tpw_rules for elaboration
<jfng[m]>
tpw_rules: yep, i actually discussed it w/ catherine this afternoon; there will be a `from .action import *` statement at the end of `csr.__init__.py`
<jfng[m]>
sorry that i haven't got around to it yet
<tpw_rules>
do you mean from . import action?
<jfng[m]>
oops, yes
<jfng[m]>
in the same fashion `amaranth.lib.crc` handles its `catalog` module
<whitequark[cis]>
is the meeting over?
<tpw_rules>
i don't have anything further, i'm awaiting the revised name and RFC
<jfng[m]>
it is now. i'm writing down the minutes
<jfng[m]>
thanks everyone!
nates93[m] has joined #amaranth-lang
<nates93[m]>
Probably missing something simple with this q: I ended up adding a constraint like: `os.environ["AMARANTH_add_preferences"] += f"set_clock_groups -asynchronous -group [get_clocks {{clk100_0__io}}] -group [get_clocks {{clk48_0__io -include_generated_clocks}}]"` --how should I more naturally get the clk signal names (e.g. `clk100_0__io` ) from within my design?
<whitequark[cis]>
for this particular case, you can't easily do that
<tpw_rules>
quartus is really bad at that
<tpw_rules>
(why are you using an environment variable?)
<nates93[m]>
Ok. Actually using Lattice Radiant. What's the alternative to using an environment variable?
<tpw_rules>
i thought there was a method on the Platform object
<whitequark[cis]>
not for creating async clock groups
<tpw_rules>
isn't there a method to just add stuff to the settings file? i guess maybe not for lattice
<whitequark[cis]>
you can override a method to do this