[PATCH v1 1/3] riscv/sifive_u: Fix up file ordering

Alistair Francis posted 3 patches 5 years, 8 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Sagar Karandikar <sagark@eecs.berkeley.edu>
There is a newer version of this series
[PATCH v1 1/3] riscv/sifive_u: Fix up file ordering
Posted by Alistair Francis 5 years, 8 months ago
Split the file into clear machine and SoC sections.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/sifive_u.c | 107 ++++++++++++++++++++++----------------------
 1 file changed, 54 insertions(+), 53 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 156a003642..9a0145b5b4 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -399,6 +399,60 @@ static void riscv_sifive_u_init(MachineState *machine)
                           &address_space_memory);
 }
 
+static bool sifive_u_get_start_in_flash(Object *obj, Error **errp)
+{
+    SiFiveUState *s = RISCV_U_MACHINE(obj);
+
+    return s->start_in_flash;
+}
+
+static void sifive_u_set_start_in_flash(Object *obj, bool value, Error **errp)
+{
+    SiFiveUState *s = RISCV_U_MACHINE(obj);
+
+    s->start_in_flash = value;
+}
+
+static void riscv_sifive_u_machine_instance_init(Object *obj)
+{
+    SiFiveUState *s = RISCV_U_MACHINE(obj);
+
+    s->start_in_flash = false;
+    object_property_add_bool(obj, "start-in-flash", sifive_u_get_start_in_flash,
+                             sifive_u_set_start_in_flash, NULL);
+    object_property_set_description(obj, "start-in-flash",
+                                    "Set on to tell QEMU's ROM to jump to " \
+                                    "flash. Otherwise QEMU will jump to DRAM",
+                                    NULL);
+}
+
+
+static void riscv_sifive_u_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "RISC-V Board compatible with SiFive U SDK";
+    mc->init = riscv_sifive_u_init;
+    mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
+    mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
+    mc->default_cpus = mc->min_cpus;
+}
+
+static const TypeInfo riscv_sifive_u_machine_typeinfo = {
+    .name       = MACHINE_TYPE_NAME("sifive_u"),
+    .parent     = TYPE_MACHINE,
+    .class_init = riscv_sifive_u_machine_class_init,
+    .instance_init = riscv_sifive_u_machine_instance_init,
+    .instance_size = sizeof(SiFiveUState),
+};
+
+static void riscv_sifive_u_machine_init_register_types(void)
+{
+    type_register_static(&riscv_sifive_u_machine_typeinfo);
+}
+
+type_init(riscv_sifive_u_machine_init_register_types)
+
 static void riscv_sifive_u_soc_init(Object *obj)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
@@ -439,33 +493,6 @@ static void riscv_sifive_u_soc_init(Object *obj)
                           TYPE_CADENCE_GEM);
 }
 
-static bool sifive_u_get_start_in_flash(Object *obj, Error **errp)
-{
-    SiFiveUState *s = RISCV_U_MACHINE(obj);
-
-    return s->start_in_flash;
-}
-
-static void sifive_u_set_start_in_flash(Object *obj, bool value, Error **errp)
-{
-    SiFiveUState *s = RISCV_U_MACHINE(obj);
-
-    s->start_in_flash = value;
-}
-
-static void riscv_sifive_u_machine_instance_init(Object *obj)
-{
-    SiFiveUState *s = RISCV_U_MACHINE(obj);
-
-    s->start_in_flash = false;
-    object_property_add_bool(obj, "start-in-flash", sifive_u_get_start_in_flash,
-                             sifive_u_set_start_in_flash, NULL);
-    object_property_set_description(obj, "start-in-flash",
-                                    "Set on to tell QEMU's ROM to jump to " \
-                                    "flash. Otherwise QEMU will jump to DRAM",
-                                    NULL);
-}
-
 static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
@@ -603,29 +630,3 @@ static void riscv_sifive_u_soc_register_types(void)
 }
 
 type_init(riscv_sifive_u_soc_register_types)
-
-static void riscv_sifive_u_machine_class_init(ObjectClass *oc, void *data)
-{
-    MachineClass *mc = MACHINE_CLASS(oc);
-
-    mc->desc = "RISC-V Board compatible with SiFive U SDK";
-    mc->init = riscv_sifive_u_init;
-    mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
-    mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
-    mc->default_cpus = mc->min_cpus;
-}
-
-static const TypeInfo riscv_sifive_u_machine_typeinfo = {
-    .name       = MACHINE_TYPE_NAME("sifive_u"),
-    .parent     = TYPE_MACHINE,
-    .class_init = riscv_sifive_u_machine_class_init,
-    .instance_init = riscv_sifive_u_machine_instance_init,
-    .instance_size = sizeof(SiFiveUState),
-};
-
-static void riscv_sifive_u_machine_init_register_types(void)
-{
-    type_register_static(&riscv_sifive_u_machine_typeinfo);
-}
-
-type_init(riscv_sifive_u_machine_init_register_types)
-- 
2.25.1


Re: [PATCH v1 1/3] riscv/sifive_u: Fix up file ordering
Posted by Bin Meng 5 years, 8 months ago
On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Split the file into clear machine and SoC sections.
>

Yep, I found functions in this file are a little bit confusing as well ..

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/riscv/sifive_u.c | 107 ++++++++++++++++++++++----------------------
>  1 file changed, 54 insertions(+), 53 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 156a003642..9a0145b5b4 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -399,6 +399,60 @@ static void riscv_sifive_u_init(MachineState *machine)

So while we are here, could we rename this to something like:
sifive_u_machine_init()? ie: drop the riscv_ prefix.

>                            &address_space_memory);
>  }
>
> +static bool sifive_u_get_start_in_flash(Object *obj, Error **errp)

and sifive_u_machine_get_start_in_flash()? and other functions too?

> +{
> +    SiFiveUState *s = RISCV_U_MACHINE(obj);
> +
> +    return s->start_in_flash;
> +}
> +
> +static void sifive_u_set_start_in_flash(Object *obj, bool value, Error **errp)
> +{
> +    SiFiveUState *s = RISCV_U_MACHINE(obj);
> +
> +    s->start_in_flash = value;
> +}
> +
> +static void riscv_sifive_u_machine_instance_init(Object *obj)
> +{
> +    SiFiveUState *s = RISCV_U_MACHINE(obj);
> +
> +    s->start_in_flash = false;
> +    object_property_add_bool(obj, "start-in-flash", sifive_u_get_start_in_flash,
> +                             sifive_u_set_start_in_flash, NULL);
> +    object_property_set_description(obj, "start-in-flash",
> +                                    "Set on to tell QEMU's ROM to jump to " \
> +                                    "flash. Otherwise QEMU will jump to DRAM",
> +                                    NULL);
> +}
> +
> +
> +static void riscv_sifive_u_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->desc = "RISC-V Board compatible with SiFive U SDK";
> +    mc->init = riscv_sifive_u_init;
> +    mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
> +    mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
> +    mc->default_cpus = mc->min_cpus;
> +}
> +
> +static const TypeInfo riscv_sifive_u_machine_typeinfo = {
> +    .name       = MACHINE_TYPE_NAME("sifive_u"),
> +    .parent     = TYPE_MACHINE,
> +    .class_init = riscv_sifive_u_machine_class_init,
> +    .instance_init = riscv_sifive_u_machine_instance_init,
> +    .instance_size = sizeof(SiFiveUState),
> +};
> +
> +static void riscv_sifive_u_machine_init_register_types(void)
> +{
> +    type_register_static(&riscv_sifive_u_machine_typeinfo);
> +}
> +
> +type_init(riscv_sifive_u_machine_init_register_types)
> +
>  static void riscv_sifive_u_soc_init(Object *obj)

Similarly this can be renamed to: sifive_u_soc_init(), and other soc functions.

>  {
>      MachineState *ms = MACHINE(qdev_get_machine());
> @@ -439,33 +493,6 @@ static void riscv_sifive_u_soc_init(Object *obj)
>                            TYPE_CADENCE_GEM);
>  }
>
> -static bool sifive_u_get_start_in_flash(Object *obj, Error **errp)
> -{
> -    SiFiveUState *s = RISCV_U_MACHINE(obj);
> -
> -    return s->start_in_flash;
> -}
> -
> -static void sifive_u_set_start_in_flash(Object *obj, bool value, Error **errp)
> -{
> -    SiFiveUState *s = RISCV_U_MACHINE(obj);
> -
> -    s->start_in_flash = value;
> -}
> -
> -static void riscv_sifive_u_machine_instance_init(Object *obj)
> -{
> -    SiFiveUState *s = RISCV_U_MACHINE(obj);
> -
> -    s->start_in_flash = false;
> -    object_property_add_bool(obj, "start-in-flash", sifive_u_get_start_in_flash,
> -                             sifive_u_set_start_in_flash, NULL);
> -    object_property_set_description(obj, "start-in-flash",
> -                                    "Set on to tell QEMU's ROM to jump to " \
> -                                    "flash. Otherwise QEMU will jump to DRAM",
> -                                    NULL);
> -}
> -
>  static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>  {
>      MachineState *ms = MACHINE(qdev_get_machine());
> @@ -603,29 +630,3 @@ static void riscv_sifive_u_soc_register_types(void)
>  }
>
>  type_init(riscv_sifive_u_soc_register_types)
> -
> -static void riscv_sifive_u_machine_class_init(ObjectClass *oc, void *data)
> -{
> -    MachineClass *mc = MACHINE_CLASS(oc);
> -
> -    mc->desc = "RISC-V Board compatible with SiFive U SDK";
> -    mc->init = riscv_sifive_u_init;
> -    mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
> -    mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
> -    mc->default_cpus = mc->min_cpus;
> -}
> -
> -static const TypeInfo riscv_sifive_u_machine_typeinfo = {
> -    .name       = MACHINE_TYPE_NAME("sifive_u"),
> -    .parent     = TYPE_MACHINE,
> -    .class_init = riscv_sifive_u_machine_class_init,
> -    .instance_init = riscv_sifive_u_machine_instance_init,
> -    .instance_size = sizeof(SiFiveUState),
> -};
> -
> -static void riscv_sifive_u_machine_init_register_types(void)
> -{
> -    type_register_static(&riscv_sifive_u_machine_typeinfo);
> -}
> -
> -type_init(riscv_sifive_u_machine_init_register_types)
> --

Regards,
Bin

Re: [PATCH v1 1/3] riscv/sifive_u: Fix up file ordering
Posted by Alistair Francis 5 years, 8 months ago
On Wed, Mar 4, 2020 at 6:10 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > Split the file into clear machine and SoC sections.
> >
>
> Yep, I found functions in this file are a little bit confusing as well ..
>
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/riscv/sifive_u.c | 107 ++++++++++++++++++++++----------------------
> >  1 file changed, 54 insertions(+), 53 deletions(-)
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index 156a003642..9a0145b5b4 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -399,6 +399,60 @@ static void riscv_sifive_u_init(MachineState *machine)
>
> So while we are here, could we rename this to something like:
> sifive_u_machine_init()? ie: drop the riscv_ prefix.
>
> >                            &address_space_memory);
> >  }
> >
> > +static bool sifive_u_get_start_in_flash(Object *obj, Error **errp)
>
> and sifive_u_machine_get_start_in_flash()? and other functions too?
>
> > +{
> > +    SiFiveUState *s = RISCV_U_MACHINE(obj);
> > +
> > +    return s->start_in_flash;
> > +}
> > +
> > +static void sifive_u_set_start_in_flash(Object *obj, bool value, Error **errp)
> > +{
> > +    SiFiveUState *s = RISCV_U_MACHINE(obj);
> > +
> > +    s->start_in_flash = value;
> > +}
> > +
> > +static void riscv_sifive_u_machine_instance_init(Object *obj)
> > +{
> > +    SiFiveUState *s = RISCV_U_MACHINE(obj);
> > +
> > +    s->start_in_flash = false;
> > +    object_property_add_bool(obj, "start-in-flash", sifive_u_get_start_in_flash,
> > +                             sifive_u_set_start_in_flash, NULL);
> > +    object_property_set_description(obj, "start-in-flash",
> > +                                    "Set on to tell QEMU's ROM to jump to " \
> > +                                    "flash. Otherwise QEMU will jump to DRAM",
> > +                                    NULL);
> > +}
> > +
> > +
> > +static void riscv_sifive_u_machine_class_init(ObjectClass *oc, void *data)
> > +{
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > +    mc->desc = "RISC-V Board compatible with SiFive U SDK";
> > +    mc->init = riscv_sifive_u_init;
> > +    mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
> > +    mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
> > +    mc->default_cpus = mc->min_cpus;
> > +}
> > +
> > +static const TypeInfo riscv_sifive_u_machine_typeinfo = {
> > +    .name       = MACHINE_TYPE_NAME("sifive_u"),
> > +    .parent     = TYPE_MACHINE,
> > +    .class_init = riscv_sifive_u_machine_class_init,
> > +    .instance_init = riscv_sifive_u_machine_instance_init,
> > +    .instance_size = sizeof(SiFiveUState),
> > +};
> > +
> > +static void riscv_sifive_u_machine_init_register_types(void)
> > +{
> > +    type_register_static(&riscv_sifive_u_machine_typeinfo);
> > +}
> > +
> > +type_init(riscv_sifive_u_machine_init_register_types)
> > +
> >  static void riscv_sifive_u_soc_init(Object *obj)
>
> Similarly this can be renamed to: sifive_u_soc_init(), and other soc functions.

I missed this in v2, fixed in the next version.

Alistair

>
> >  {
> >      MachineState *ms = MACHINE(qdev_get_machine());
> > @@ -439,33 +493,6 @@ static void riscv_sifive_u_soc_init(Object *obj)
> >                            TYPE_CADENCE_GEM);
> >  }
> >
> > -static bool sifive_u_get_start_in_flash(Object *obj, Error **errp)
> > -{
> > -    SiFiveUState *s = RISCV_U_MACHINE(obj);
> > -
> > -    return s->start_in_flash;
> > -}
> > -
> > -static void sifive_u_set_start_in_flash(Object *obj, bool value, Error **errp)
> > -{
> > -    SiFiveUState *s = RISCV_U_MACHINE(obj);
> > -
> > -    s->start_in_flash = value;
> > -}
> > -
> > -static void riscv_sifive_u_machine_instance_init(Object *obj)
> > -{
> > -    SiFiveUState *s = RISCV_U_MACHINE(obj);
> > -
> > -    s->start_in_flash = false;
> > -    object_property_add_bool(obj, "start-in-flash", sifive_u_get_start_in_flash,
> > -                             sifive_u_set_start_in_flash, NULL);
> > -    object_property_set_description(obj, "start-in-flash",
> > -                                    "Set on to tell QEMU's ROM to jump to " \
> > -                                    "flash. Otherwise QEMU will jump to DRAM",
> > -                                    NULL);
> > -}
> > -
> >  static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
> >  {
> >      MachineState *ms = MACHINE(qdev_get_machine());
> > @@ -603,29 +630,3 @@ static void riscv_sifive_u_soc_register_types(void)
> >  }
> >
> >  type_init(riscv_sifive_u_soc_register_types)
> > -
> > -static void riscv_sifive_u_machine_class_init(ObjectClass *oc, void *data)
> > -{
> > -    MachineClass *mc = MACHINE_CLASS(oc);
> > -
> > -    mc->desc = "RISC-V Board compatible with SiFive U SDK";
> > -    mc->init = riscv_sifive_u_init;
> > -    mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
> > -    mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
> > -    mc->default_cpus = mc->min_cpus;
> > -}
> > -
> > -static const TypeInfo riscv_sifive_u_machine_typeinfo = {
> > -    .name       = MACHINE_TYPE_NAME("sifive_u"),
> > -    .parent     = TYPE_MACHINE,
> > -    .class_init = riscv_sifive_u_machine_class_init,
> > -    .instance_init = riscv_sifive_u_machine_instance_init,
> > -    .instance_size = sizeof(SiFiveUState),
> > -};
> > -
> > -static void riscv_sifive_u_machine_init_register_types(void)
> > -{
> > -    type_register_static(&riscv_sifive_u_machine_typeinfo);
> > -}
> > -
> > -type_init(riscv_sifive_u_machine_init_register_types)
> > --
>
> Regards,
> Bin