Re: [Qemu-devel] [PATCH 1/2] riscv: sifive_u: Do not create hard-coded phandles in DT

[Alistair Francis]

On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
> At present the cpu, plic and ethclk nodes' phandles are hard-coded
> to 1/2/3 in DT. If we configure more than 1 cpu for the machine,
> all cpu nodes' phandles conflict with each other as they are all 1.
> Fix it by removing the hardcode.
> 
> Signed-off-by: Bin Meng <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Alistair

> ---
> 
>  hw/riscv/sifive_u.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 5ecc47c..e2120ac 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -86,7 +86,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      uint32_t *cells;
>      char *nodename;
>      char ethclk_names[] = "pclk\0hclk\0tx_clk";
> -    uint32_t plic_phandle, ethclk_phandle;
> +    uint32_t plic_phandle, ethclk_phandle, phandle = 1;
>  
>      fdt = s->fdt = create_device_tree(&s->fdt_size);
>      if (!fdt) {
> @@ -121,6 +121,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
>  
>      for (cpu = s->soc.cpus.num_harts - 1; cpu >= 0; cpu--) {
> +        int cpu_phandle = phandle++;
>          nodename = g_strdup_printf("/cpus/address@hidden", cpu);
>          char *intc = g_strdup_printf("/cpus/address@hidden/interrupt-
> controller", cpu);
>          char *isa = riscv_isa_string(&s->soc.cpus.harts[cpu]);
> @@ -134,8 +135,8 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>          qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu);
>          qemu_fdt_setprop_string(fdt, nodename, "device_type",
> "cpu");
>          qemu_fdt_add_subnode(fdt, intc);
> -        qemu_fdt_setprop_cell(fdt, intc, "phandle", 1);
> -        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle", 1);
> +        qemu_fdt_setprop_cell(fdt, intc, "phandle", cpu_phandle);
> +        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle",
> cpu_phandle);
>          qemu_fdt_setprop_string(fdt, intc, "compatible", "riscv,cpu-
> intc");
>          qemu_fdt_setprop(fdt, intc, "interrupt-controller", NULL,
> 0);
>          qemu_fdt_setprop_cell(fdt, intc, "#interrupt-cells", 1);
> @@ -167,6 +168,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      g_free(cells);
>      g_free(nodename);
>  
> +    plic_phandle = phandle++;
>      cells =  g_new0(uint32_t, s->soc.cpus.num_harts * 4);
>      for (cpu = 0; cpu < s->soc.cpus.num_harts; cpu++) {
>          nodename =
> @@ -192,20 +194,21 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      qemu_fdt_setprop_string(fdt, nodename, "reg-names", "control");
>      qemu_fdt_setprop_cell(fdt, nodename, "riscv,max-priority", 7);
>      qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", 0x35);
> -    qemu_fdt_setprop_cells(fdt, nodename, "phandle", 2);
> -    qemu_fdt_setprop_cells(fdt, nodename, "linux,phandle", 2);
> +    qemu_fdt_setprop_cells(fdt, nodename, "phandle", plic_phandle);
> +    qemu_fdt_setprop_cells(fdt, nodename, "linux,phandle",
> plic_phandle);
>      plic_phandle = qemu_fdt_get_phandle(fdt, nodename);
>      g_free(cells);
>      g_free(nodename);
>  
> +    ethclk_phandle = phandle++;
>      nodename = g_strdup_printf("/soc/ethclk");
>      qemu_fdt_add_subnode(fdt, nodename);
>      qemu_fdt_setprop_string(fdt, nodename, "compatible", "fixed-
> clock");
>      qemu_fdt_setprop_cell(fdt, nodename, "#clock-cells", 0x0);
>      qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
>          SIFIVE_U_GEM_CLOCK_FREQ);
> -    qemu_fdt_setprop_cell(fdt, nodename, "phandle", 3);
> -    qemu_fdt_setprop_cell(fdt, nodename, "linux,phandle", 3);
> +    qemu_fdt_setprop_cell(fdt, nodename, "phandle", ethclk_phandle);
> +    qemu_fdt_setprop_cell(fdt, nodename, "linux,phandle",
> ethclk_phandle);
>      ethclk_phandle = qemu_fdt_get_phandle(fdt, nodename);
>      g_free(nodename);
>