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. 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
