On 4/24/25 6:51 AM, Joel Stanley wrote:
On Wed, 23 Apr 2025 at 20:37, Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:

Throughout the code we're accessing the board memmap, most of the time,
by accessing it statically via 'virt_memmap'. This static map is also
assigned in the machine state in s->memmap.

We're also passing it as a variable to some fdt functions, which is
unorthodox since we can spare a function argument by accessing it
statically or via the machine state.

All the current forms are valid but not all of the are scalable. In the
future we will version this board, and then all this code will need
rework because it should point to the updated memmap. In this case,
we'll want to assign the adequate versioned memmap once during init,
in s->memmap like it is being done today, and the rest of the code
will access the updated map via s->memmap.

I was writing a patch for a machine and came across the same
inconsistencies. Nice clean up.

Some of the device initlisation code could be refactored out to be
shared by other machines within the riscv directory. Related, parts of
the device tree creation could belong to the model, instead of to the
machine, as the properties are a property (!) of the device.


Yes, delegating the FDT creation to the device, instead of having each machine
to create the (mostly) same FDT code over and over again, is something that
I've considering for awhile.

I keep postponing it mainly because I would like to verify with the DT folks if
there's a guarantee that a given device/CPU DT is always the same, i.e. a device
DT is always the same regardless of the machine. I have a guess that that this 
is
indeed the case but a confirmation would be nice .... Conor, care to comment?


In this refactor we could then create FDTs by passing along a memmap pointer and
a fdt pointer, as you've suggested.


All this said, there's no need to do such FDT refactory all at once. I think 
I'll
start with the most common devices between RISC-V boards and go from there.

Thanks,

Daniel



With that in mind we should consider passing the eg. fdt pointer and
the MemMap pointer instead of machine state, where practical.

We're also enforcing the pattern of using s->memmap instead of assigning
it to a temp variable 'memmap'. Code is copy/pasted around all the time
and being consistent is important.

Reviewed-by: Joel Stanley <j...@jms.id.au>

Cheers,

Joel


Reply via email to