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