Re: [PATCH 02/15] hw/riscv/spike: use 'fdt' from MachineState

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

On 12/22/22 11:25, Philippe Mathieu-Daudé wrote:
> On 21/12/22 19:22, Daniel Henrique Barboza wrote:
>> The MachineState object provides a 'fdt' pointer that is already being
>> used by other RISC-V machines, and it's also used by the 'dumpdtb' QMP
>> command.
>>
>> Remove the 'fdt' pointer from SpikeState and use MachineState::fdt
>> instead.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   hw/riscv/spike.c         | 12 +++++-------
>>   include/hw/riscv/spike.h |  2 --
>>   2 files changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
>> index 13946acf0d..d96f013e2e 100644
>> --- a/hw/riscv/spike.c
>> +++ b/hw/riscv/spike.c
>> @@ -52,6 +52,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>>                          uint64_t mem_size, const char *cmdline, bool is_32_bit)
>>   {
>>       void *fdt;
>> +    int fdt_size;
>>       uint64_t addr, size;
>>       unsigned long clint_addr;
>>       int cpu, socket;
>> @@ -64,7 +65,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>>           "sifive,clint0", "riscv,clint0"
>>       };
>>   -    fdt = s->fdt = create_device_tree(&s->fdt_size);
>> +    fdt = mc->fdt = create_device_tree(&fdt_size);
>
> We use 'ms' for MachineState and 'mc' for MachineClass. I first got
> scared while looking at that patch that a class field was used. The
> variable is simply badly named. Possible future cleanup: s/mc/ms/.

Thanks for the ack Phil!


And yeah, I think I'll drop a patch with your suggestion later on.


Daniel

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>