diff mbox

[v2,01/23] RISC-V: Make virt create_fdt interface consistent

Message ID 1520568765-58189-2-git-send-email-mjc@sifive.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michael Clark March 9, 2018, 4:12 a.m. UTC
create_fdt sets the fdt variable on RISCVVirtState and this is
used to access the fdt. This reverts a change introduced in
https://github.com/riscv/riscv-qemu/pull/109 which introduced
a redundant return value, overlooking the RISCVVirtState
structure member that made create_fdt inconsistent with the
other RISC-V machines. The other alternative is to change
the other boards to return the fdt. Note: the RISCVVirtState
also contains fdt_size.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 hw/riscv/virt.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Philippe Mathieu-Daudé March 10, 2018, 8:33 p.m. UTC | #1
On 03/09/2018 05:12 AM, Michael Clark wrote:
> create_fdt sets the fdt variable on RISCVVirtState and this is
> used to access the fdt. This reverts a change introduced in
> https://github.com/riscv/riscv-qemu/pull/109 which introduced
> a redundant return value, overlooking the RISCVVirtState
> structure member that made create_fdt inconsistent with the
> other RISC-V machines. The other alternative is to change
> the other boards to return the fdt. Note: the RISCVVirtState
> also contains fdt_size.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/riscv/virt.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index e2c214e..37968d2 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -108,7 +108,7 @@ static hwaddr load_initrd(const char *filename, uint64_t mem_size,
>      return *start + size;
>  }
>  
> -static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
> +static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
>      uint64_t mem_size, const char *cmdline)
>  {
>      void *fdt;
> @@ -264,8 +264,6 @@ static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
>      qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
>      qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
>      g_free(nodename);
> -
> -    return fdt;
>  }
>  
>  static void riscv_virt_board_init(MachineState *machine)
> @@ -279,7 +277,6 @@ static void riscv_virt_board_init(MachineState *machine)
>      char *plic_hart_config;
>      size_t plic_hart_config_len;
>      int i;
> -    void *fdt;
>  
>      /* Initialize SOC */
>      object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> @@ -299,7 +296,7 @@ static void riscv_virt_board_init(MachineState *machine)
>          main_mem);
>  
>      /* create device tree */
> -    fdt = create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
> +    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>  
>      /* boot rom */
>      memory_region_init_ram(boot_rom, NULL, "riscv_virt_board.bootrom",
> @@ -314,9 +311,9 @@ static void riscv_virt_board_init(MachineState *machine)
>              hwaddr end = load_initrd(machine->initrd_filename,
>                                       machine->ram_size, kernel_entry,
>                                       &start);
> -            qemu_fdt_setprop_cell(fdt, "/chosen",
> -                                  "linux,initrd-start", start);
> -            qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> +            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-start",
> +                                  start);
> +            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
>                                    end);
>          }
>      }
>
diff mbox

Patch

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index e2c214e..37968d2 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -108,7 +108,7 @@  static hwaddr load_initrd(const char *filename, uint64_t mem_size,
     return *start + size;
 }
 
-static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
+static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     uint64_t mem_size, const char *cmdline)
 {
     void *fdt;
@@ -264,8 +264,6 @@  static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
     qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
     g_free(nodename);
-
-    return fdt;
 }
 
 static void riscv_virt_board_init(MachineState *machine)
@@ -279,7 +277,6 @@  static void riscv_virt_board_init(MachineState *machine)
     char *plic_hart_config;
     size_t plic_hart_config_len;
     int i;
-    void *fdt;
 
     /* Initialize SOC */
     object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
@@ -299,7 +296,7 @@  static void riscv_virt_board_init(MachineState *machine)
         main_mem);
 
     /* create device tree */
-    fdt = create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
+    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
 
     /* boot rom */
     memory_region_init_ram(boot_rom, NULL, "riscv_virt_board.bootrom",
@@ -314,9 +311,9 @@  static void riscv_virt_board_init(MachineState *machine)
             hwaddr end = load_initrd(machine->initrd_filename,
                                      machine->ram_size, kernel_entry,
                                      &start);
-            qemu_fdt_setprop_cell(fdt, "/chosen",
-                                  "linux,initrd-start", start);
-            qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-start",
+                                  start);
+            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
                                   end);
         }
     }