[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-arm] [PATCH 2/2] hw/arm/virt: Silence dtc /memory
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [Qemu-arm] [PATCH 2/2] hw/arm/virt: Silence dtc /memory warning |
Date: |
Sat, 9 Jun 2018 11:37:09 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
Hi Eric,
On 06/09/2018 11:23 AM, Eric Auger wrote:
> When running dtc on the guest /proc/device-tree we get the
> following warning: Warning (unit_address_vs_reg): Node /memory
> has a reg or ranges property, but no unit name".
>
> Let's fix that by adding the unit address to the node name. We also
> don't create the /memory node anymore in create_fdt(). We directly
> create it in load_dtb. /chosen still needs to be created in create_fdt
> as the uart needs it. In case the user provided his own dtb, either
> the bank is added to the existing /memory node or if this latter is
> not found we create a new separate memory node.
>
> Signed-off-by: Eric Auger <address@hidden>
> ---
> hw/arm/boot.c | 20 ++++++++++++++------
> hw/arm/virt.c | 6 ------
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 1e2be20..2054670 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -593,24 +593,32 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo,
> g_free(nodename);
> }
> } else {
> + char *nodename = g_strdup("/memory");
> Error *err = NULL;
>
> - rc = fdt_path_offset(fdt, "/memory");
> + /* If there is an existing /memory node (user provided dtb), we add
> the
> + * new bank into it, otherwise we create a /address@hidden node
> + */
> + rc = fdt_path_offset(fdt, nodename);
> if (rc < 0) {
> - qemu_fdt_add_subnode(fdt, "/memory");
> + g_free(nodename);
> + nodename = g_strdup_printf("/address@hidden" PRIx64,
> binfo->loader_start);
> +
> + qemu_fdt_add_subnode(fdt, nodename);
> }
>
> - if (!qemu_fdt_getprop(fdt, "/memory", "device_type", NULL, &err)) {
> - qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory");
> + if (!qemu_fdt_getprop(fdt, nodename, "device_type", NULL, &err)) {
> + qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");
> }
>
> - rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",
> + rc = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg",
> acells, binfo->loader_start,
> scells, binfo->ram_size);
> if (rc < 0) {
> - fprintf(stderr, "couldn't set /memory/reg\n");
> + fprintf(stderr, "couldn't set %s reg\n", nodename);
> goto fail;
> }
> + g_free(nodename);
> }
This part looks good.
>
> rc = fdt_path_offset(fdt, "/chosen");
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b1e9a6b..46c5c3f 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -203,13 +203,7 @@ static void create_fdt(VirtMachineState *vms)
> qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
> qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
>
> - /*
> - * /chosen and /memory nodes must exist for load_dtb
> - * to fill in necessary properties later
> - */
> qemu_fdt_add_subnode(fdt, "/chosen");
Here I think you should keep the comment, but update it (info from your
commit message).
> - qemu_fdt_add_subnode(fdt, "/memory");
> - qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory");
>
> /* Clock node, for the benefit of the UART. The kernel device tree
> * binding documentation claims the PL011 node clock properties are
>