Re: [PATCH v5 06/14] hw/ppc: set machine->fdt in sam460ex_load_device_tree()

[BALATON Zoltan]

On Sat, 3 Sep 2022, Daniel Henrique Barboza wrote:
> This will enable support for 'dumpdtb' QMP/HMP command for the sam460ex
> machine.
>
> Cc: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/sam460ex.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 850bb3b817..fa6f125fda 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -137,6 +137,7 @@ static int sam460ex_load_device_tree(hwaddr addr,
>                                      hwaddr initrd_size,
>                                      const char *kernel_cmdline)
> {
> +    MachineState *machine = MACHINE(qdev_get_machine());

Sorry for not noticing this earlier but this is ugly, I think you could 
just change the prototype of this function to take MachineState *machine 
instead of the ramsize and kernel_cmdline params which then can also be 
get from machine so they are not needed as separate params any more.

>     uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
>     char *filename;
>     int fdt_size;
> @@ -208,7 +209,9 @@ static int sam460ex_load_device_tree(hwaddr addr,
>                               EBC_FREQ);
>
>     rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
> -    g_free(fdt);
> +
> +    /* Set machine->fdt for 'dumpdtb' QMP/HMP command */

You've changed this and a few other places to the short comment but left 
the long version in pegasos2 and other machines. Could you change all of 
them to make it more consistent and less lines altogether?

Regards,
BALATON Zoltan

> +    machine->fdt = fdt;
>
>     return fdt_size;
> }