Various RDL Autogen fixes#579
Conversation
Add a none variant with a value of 0 when generating flag enum bits. This is useful to represent "no interrupt(s)" or "no bits" to give to an enable_write/status_write function. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
The SPI Host has two register windows in between other register fields, whereas before this it was assumed that all register windows come after registers. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
fd48c9a to
9252e79
Compare
This includes the auto-generated ethernet driver header.
9252e79 to
e1a42cd
Compare
| mem_width = window["width"] | ||
| mem_width_bytes = 0 | ||
| type_name = None | ||
| if mem_width == 32: |
There was a problem hiding this comment.
Alternatively, in this case I'd actually prefer:
if mem_width not in (32, 64):
raise ValueError(
f"Register window '{window_name}' in device '{device_name}' "
"has entries that are not 32-bits or 64-bits wide"
)
mem_width_bytes = mem_width // 8
type_name = f"uint{mem_width}_t"There was a problem hiding this comment.
That's a lot nicer, thanks!
| end = offset + window["size"] - 4 | ||
| offset_string = f"{hex(offset)}-{hex(end)}" | ||
| end = offset + window["size"] - mem_width_bytes | ||
| offset_string = hex(offset) if num_entries == 1 else f"{hex(offset)}-{hex(end)}" |
There was a problem hiding this comment.
Nit:
| offset_string = hex(offset) if num_entries == 1 else f"{hex(offset)}-{hex(end)}" | |
| offset_string = hex(offset) if num_entries == 1 else f"{offset:x}-{end:x}" |
There was a problem hiding this comment.
Didn't know about this feature, looks much less noisy
There was a problem hiding this comment.
Very useful format specifiers, it's my hex and bin calculator 😄
python -c "print(f'{0x20+0b11:x}')"
python -c "print(f'{0x20+0b11:b}')"| typed_registers, key=lambda reg: min(reg[0]["offsets"]) | ||
| # all the struct declaration fields. Tag each with a boolean value representing whether | ||
| # it is a register or not (a window). | ||
| declaration_fields = [(True, register) for register in typed_registers] + [ |
There was a problem hiding this comment.
The tuple isn't great for readability. The json file has a field "type" can be used instead of the tuple. Or you could simply check if the offset field is an array
There was a problem hiding this comment.
I mentioned this on previous PR(s) but these would really benefit from being a bit more object-oriented with either dataclasses or Pydantic models, it would help to solve this readability a bit. Albeit that's obviously a larger refactoring task.
For now, if nothing else is workable, this might be more readable as its own dict e.g.: {"is_window": False, "field": register}, even if a bit less efficient.
AlexJones0
left a comment
There was a problem hiding this comment.
I've not checked the RDL, only the SW changes.
| reg_name = reg["name"].lower() | ||
| fully_qualified_type_name = "_".join([device_name, reg_name]) | ||
| enum_variants = [] | ||
| enum_variants.append(f"{'_'.join([device_name, reg_name, 'none'])} = (0),") |
There was a problem hiding this comment.
Is there a reason we emit (0)? This is completely subjective, but I'd much prefer just 0.
| mem_width = window["width"] | ||
| mem_width_bytes = 0 | ||
| type_name = None | ||
| if mem_width == 32: |
There was a problem hiding this comment.
Alternatively, in this case I'd actually prefer:
if mem_width not in (32, 64):
raise ValueError(
f"Register window '{window_name}' in device '{device_name}' "
"has entries that are not 32-bits or 64-bits wide"
)
mem_width_bytes = mem_width // 8
type_name = f"uint{mem_width}_t"|
|
||
| # sort the struct fields in ascending order by offset. the specific keys differ based | ||
| # on whether it is a register or a window. | ||
| fields_asceding_by_offset = sorted( |
There was a problem hiding this comment.
Typo:
| fields_asceding_by_offset = sorted( | |
| fields_ascending_by_offset = sorted( |
| typed_registers, key=lambda reg: min(reg[0]["offsets"]) | ||
| # all the struct declaration fields. Tag each with a boolean value representing whether | ||
| # it is a register or not (a window). | ||
| declaration_fields = [(True, register) for register in typed_registers] + [ |
There was a problem hiding this comment.
I mentioned this on previous PR(s) but these would really benefit from being a bit more object-oriented with either dataclasses or Pydantic models, it would help to solve this readability a bit. Albeit that's obviously a larger refactoring task.
For now, if nothing else is workable, this might be more readable as its own dict e.g.: {"is_window": False, "field": register}, even if a bit less efficient.
enable_writeorstatus_writeto clear the register, needed by Update SPI Device HAL to use new autogen interface #387.This also adds the autogenerated SPI Host and Ethernet headers.
Closes #522.