Re: [PATCH v3 03/13] hw/ppc/pegasos2: Change device tree generation

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 20/10/25 15:06, BALATON Zoltan wrote:
> On Mon, 20 Oct 2025, Philippe Mathieu-Daudé wrote:
>> On 18/10/25 17:11, BALATON Zoltan wrote:
>>> We generate a flattened device tree programmatically for VOF. Change
>>> this to load the static parts from a device tree blob and only
>>> generate the parts that depend on run time conditions such as CPU
>>> type, memory size and PCI devices. Moving the static parts in a dts
>>> makes the board code simpler and more generic.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   hw/ppc/pegasos2.c        | 292 +++++++--------------------------------
>>>   pc-bios/dtb/meson.build  |   1 +
>>>   pc-bios/dtb/pegasos2.dtb | Bin 0 -> 1701 bytes
>>>   pc-bios/dtb/pegasos2.dts | 167 ++++++++++++++++++++++
>>>   4 files changed, 220 insertions(+), 240 deletions(-)
>>>   create mode 100644 pc-bios/dtb/pegasos2.dtb
>>>   create mode 100644 pc-bios/dtb/pegasos2.dts
>>
>>
>>>   #define TYPE_PEGASOS2_MACHINE  MACHINE_TYPE_NAME("pegasos2")
>>>   OBJECT_DECLARE_TYPE(Pegasos2MachineState, MachineClass, 
>>> PEGASOS2_MACHINE)
>>> @@ -411,7 +403,11 @@ static void pegasos2_machine_reset(MachineState 
>>> *machine, ResetType type)
>>>           error_report("Memory for initrd is in use");
>>>           exit(1);
>>>       }
>>> +
>>>       fdt = build_fdt(machine, &sz);
>>> +    if (!fdt) {
>>> +        exit(1);
>>
>> To avoid confusing users, either report an error or abort.
> 
> Errors are reported by build_fdt so no need to report again here.

I'll defer to Markus for error style.

> 
>>> +    }
>>>       /* FIXME: VOF assumes entry is same as load address */
>>>       d[0] = cpu_to_be64(pm->kernel_entry);
>>>       d[1] = cpu_to_be64(pm->kernel_size - (pm->kernel_entry - pm- 
>>> >kernel_addr));
>>
>>
>>> +static void *load_dtb(const char *filename, int *fdt_size)
>>> +{
>>> +    void *fdt;
>>> +    char *name = qemu_find_file(QEMU_FILE_TYPE_DTB, filename);
>>
>> g_autofree? Otherwise,
> 
> This function is simple enough to not need autofree but maybe it would 
> be better to ensure it's not forgotten. I may change that if need to 
> respin for some other reason.
> 
> Regards,
> BALATON Zoltan
> 
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>>> +
>>> +    if (!name) {
>>> +        error_report("Could not find dtb file '%s'", filename);
>>> +        return NULL;
>>> +    }
>>> +    fdt = load_device_tree(name, fdt_size);
>>> +    if (!fdt) {
>>> +        error_report("Could not load dtb file '%s'", name);
>>> +    }
>>> +    g_free(name);
>>> +    return fdt;
>>> +}
>>
>>