On Wed, Mar 4, 2020 at 6:10 AM Bin Meng <bmeng...@gmail.com> wrote:
>
> On Wed, Mar 4, 2020 at 9:37 AM Alistair Francis
> <alistair.fran...@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.fran...@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
