| Message ID | 1565163924-18621-12-git-send-email-bmeng.cn@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: sifive_u: Improve the emulation fidelity of sifive_u machine | expand |
On Wed, Aug 7, 2019 at 3:49 PM Bin Meng <bmeng.cn@gmail.com> wrote: > Current SiFive PRCI model only works with sifive_e machine, as it > only emulates registers or PRCI block in the FE310 SoC. > > Rename the file name to make it clear that it is for sifive_e. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > > Changes in v2: None > > hw/riscv/Makefile.objs | 2 +- > hw/riscv/sifive_e.c | 4 ++-- > hw/riscv/{sifive_prci.c => sifive_e_prci.c} | 14 +++++++------- > include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} | 14 +++++++------- > 4 files changed, 17 insertions(+), 17 deletions(-) > rename hw/riscv/{sifive_prci.c => sifive_e_prci.c} (90%) > rename include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} (82%) > > diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs > index eb9d4f9..c859697 100644 > --- a/hw/riscv/Makefile.objs > +++ b/hw/riscv/Makefile.objs > @@ -2,9 +2,9 @@ obj-y += boot.o > obj-$(CONFIG_SPIKE) += riscv_htif.o > obj-$(CONFIG_HART) += riscv_hart.o > obj-$(CONFIG_SIFIVE_E) += sifive_e.o > +obj-$(CONFIG_SIFIVE_E) += sifive_e_prci.o > obj-$(CONFIG_SIFIVE) += sifive_clint.o > obj-$(CONFIG_SIFIVE) += sifive_gpio.o > -obj-$(CONFIG_SIFIVE) += sifive_prci.o > obj-$(CONFIG_SIFIVE) += sifive_plic.o > obj-$(CONFIG_SIFIVE) += sifive_test.o > obj-$(CONFIG_SIFIVE_U) += sifive_u.o > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > index 2a499d8..2d67670 100644 > --- a/hw/riscv/sifive_e.c > +++ b/hw/riscv/sifive_e.c > @@ -41,9 +41,9 @@ > #include "hw/riscv/riscv_hart.h" > #include "hw/riscv/sifive_plic.h" > #include "hw/riscv/sifive_clint.h" > -#include "hw/riscv/sifive_prci.h" > #include "hw/riscv/sifive_uart.h" > #include "hw/riscv/sifive_e.h" > +#include "hw/riscv/sifive_e_prci.h" > #include "hw/riscv/boot.h" > #include "chardev/char.h" > #include "sysemu/arch_init.h" > @@ -174,7 +174,7 @@ static void riscv_sifive_e_soc_realize(DeviceState > *dev, Error **errp) > SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE); > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", > memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); > - sifive_prci_create(memmap[SIFIVE_E_PRCI].base); > + sifive_e_prci_create(memmap[SIFIVE_E_PRCI].base); > > /* GPIO */ > > I think adding infix to function name is sufficient and keeping the filename the same may be better. The U board PRCI or variant implementation in future could be included in the same files with different create function chihmin > diff --git a/hw/riscv/sifive_prci.c b/hw/riscv/sifive_e_prci.c > similarity index 90% > rename from hw/riscv/sifive_prci.c > rename to hw/riscv/sifive_e_prci.c > index f406682..acb914d 100644 > --- a/hw/riscv/sifive_prci.c > +++ b/hw/riscv/sifive_e_prci.c > @@ -1,5 +1,5 @@ > /* > - * QEMU SiFive PRCI (Power, Reset, Clock, Interrupt) > + * QEMU SiFive E PRCI (Power, Reset, Clock, Interrupt) > * > * Copyright (c) 2017 SiFive, Inc. > * > @@ -22,7 +22,7 @@ > #include "hw/sysbus.h" > #include "qemu/module.h" > #include "target/riscv/cpu.h" > -#include "hw/riscv/sifive_prci.h" > +#include "hw/riscv/sifive_e_prci.h" > > static uint64_t sifive_prci_read(void *opaque, hwaddr addr, unsigned int > size) > { > @@ -82,10 +82,10 @@ static const MemoryRegionOps sifive_prci_ops = { > > static void sifive_prci_init(Object *obj) > { > - SiFivePRCIState *s = SIFIVE_PRCI(obj); > + SiFivePRCIState *s = SIFIVE_E_PRCI(obj); > > memory_region_init_io(&s->mmio, obj, &sifive_prci_ops, s, > - TYPE_SIFIVE_PRCI, 0x8000); > + TYPE_SIFIVE_E_PRCI, 0x8000); > sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); > > s->hfrosccfg = (SIFIVE_PRCI_HFROSCCFG_RDY | SIFIVE_PRCI_HFROSCCFG_EN); > @@ -97,7 +97,7 @@ static void sifive_prci_init(Object *obj) > } > > static const TypeInfo sifive_prci_info = { > - .name = TYPE_SIFIVE_PRCI, > + .name = TYPE_SIFIVE_E_PRCI, > .parent = TYPE_SYS_BUS_DEVICE, > .instance_size = sizeof(SiFivePRCIState), > .instance_init = sifive_prci_init, > @@ -114,9 +114,9 @@ type_init(sifive_prci_register_types) > /* > * Create PRCI device. > */ > -DeviceState *sifive_prci_create(hwaddr addr) > +DeviceState *sifive_e_prci_create(hwaddr addr) > { > - DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_PRCI); > + DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_E_PRCI); > qdev_init_nofail(dev); > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > return dev; > diff --git a/include/hw/riscv/sifive_prci.h > b/include/hw/riscv/sifive_e_prci.h > similarity index 82% > rename from include/hw/riscv/sifive_prci.h > rename to include/hw/riscv/sifive_e_prci.h > index bd51c4a..7932fe7 100644 > --- a/include/hw/riscv/sifive_prci.h > +++ b/include/hw/riscv/sifive_e_prci.h > @@ -1,5 +1,5 @@ > /* > - * QEMU SiFive PRCI (Power, Reset, Clock, Interrupt) interface > + * QEMU SiFive E PRCI (Power, Reset, Clock, Interrupt) interface > * > * Copyright (c) 2017 SiFive, Inc. > * > @@ -16,8 +16,8 @@ > * this program. If not, see <http://www.gnu.org/licenses/>. > */ > > -#ifndef HW_SIFIVE_PRCI_H > -#define HW_SIFIVE_PRCI_H > +#ifndef HW_SIFIVE_E_PRCI_H > +#define HW_SIFIVE_E_PRCI_H > > enum { > SIFIVE_PRCI_HFROSCCFG = 0x0, > @@ -47,10 +47,10 @@ enum { > SIFIVE_PRCI_PLLOUTDIV_DIV1 = (1 << 8) > }; > > -#define TYPE_SIFIVE_PRCI "riscv.sifive.prci" > +#define TYPE_SIFIVE_E_PRCI "riscv.sifive.e.prci" > > -#define SIFIVE_PRCI(obj) \ > - OBJECT_CHECK(SiFivePRCIState, (obj), TYPE_SIFIVE_PRCI) > +#define SIFIVE_E_PRCI(obj) \ > + OBJECT_CHECK(SiFivePRCIState, (obj), TYPE_SIFIVE_E_PRCI) > > typedef struct SiFivePRCIState { > /*< private >*/ > @@ -64,6 +64,6 @@ typedef struct SiFivePRCIState { > uint32_t plloutdiv; > } SiFivePRCIState; > > -DeviceState *sifive_prci_create(hwaddr addr); > +DeviceState *sifive_e_prci_create(hwaddr addr); > > #endif > -- > 2.7.4 > > >
On Wed, Aug 7, 2019 at 4:54 PM Chih-Min Chao <chihmin.chao@sifive.com> wrote: > > > > On Wed, Aug 7, 2019 at 3:49 PM Bin Meng <bmeng.cn@gmail.com> wrote: >> >> Current SiFive PRCI model only works with sifive_e machine, as it >> only emulates registers or PRCI block in the FE310 SoC. >> >> Rename the file name to make it clear that it is for sifive_e. >> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> --- >> >> Changes in v2: None >> >> hw/riscv/Makefile.objs | 2 +- >> hw/riscv/sifive_e.c | 4 ++-- >> hw/riscv/{sifive_prci.c => sifive_e_prci.c} | 14 +++++++------- >> include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} | 14 +++++++------- >> 4 files changed, 17 insertions(+), 17 deletions(-) >> rename hw/riscv/{sifive_prci.c => sifive_e_prci.c} (90%) >> rename include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} (82%) >> >> diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs >> index eb9d4f9..c859697 100644 >> --- a/hw/riscv/Makefile.objs >> +++ b/hw/riscv/Makefile.objs >> @@ -2,9 +2,9 @@ obj-y += boot.o >> obj-$(CONFIG_SPIKE) += riscv_htif.o >> obj-$(CONFIG_HART) += riscv_hart.o >> obj-$(CONFIG_SIFIVE_E) += sifive_e.o >> +obj-$(CONFIG_SIFIVE_E) += sifive_e_prci.o >> obj-$(CONFIG_SIFIVE) += sifive_clint.o >> obj-$(CONFIG_SIFIVE) += sifive_gpio.o >> -obj-$(CONFIG_SIFIVE) += sifive_prci.o >> obj-$(CONFIG_SIFIVE) += sifive_plic.o >> obj-$(CONFIG_SIFIVE) += sifive_test.o >> obj-$(CONFIG_SIFIVE_U) += sifive_u.o >> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c >> index 2a499d8..2d67670 100644 >> --- a/hw/riscv/sifive_e.c >> +++ b/hw/riscv/sifive_e.c >> @@ -41,9 +41,9 @@ >> #include "hw/riscv/riscv_hart.h" >> #include "hw/riscv/sifive_plic.h" >> #include "hw/riscv/sifive_clint.h" >> -#include "hw/riscv/sifive_prci.h" >> #include "hw/riscv/sifive_uart.h" >> #include "hw/riscv/sifive_e.h" >> +#include "hw/riscv/sifive_e_prci.h" >> #include "hw/riscv/boot.h" >> #include "chardev/char.h" >> #include "sysemu/arch_init.h" >> @@ -174,7 +174,7 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) >> SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE); >> sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", >> memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); >> - sifive_prci_create(memmap[SIFIVE_E_PRCI].base); >> + sifive_e_prci_create(memmap[SIFIVE_E_PRCI].base); >> >> /* GPIO */ >> > > I think adding infix to function name is sufficient and keeping the filename the same may be better. > The U board PRCI or variant implementation in future could be included in the same files with different create function > I considered such approach when working on this one, but later I chose to implement a new file for SiFive U machine. The SiFive U and E PRCI blocks have different register blocks so if we put two variants into one file, their functions don't have much in common and we end up just merely physically put them into one file which does not bring too much benefit IMHO. Regards, Bin
On Wed, Aug 7, 2019 at 6:11 PM Bin Meng <bmeng.cn@gmail.com> wrote: > On Wed, Aug 7, 2019 at 4:54 PM Chih-Min Chao <chihmin.chao@sifive.com> > wrote: > > > > > > > > On Wed, Aug 7, 2019 at 3:49 PM Bin Meng <bmeng.cn@gmail.com> wrote: > >> > >> Current SiFive PRCI model only works with sifive_e machine, as it > >> only emulates registers or PRCI block in the FE310 SoC. > >> > >> Rename the file name to make it clear that it is for sifive_e. > >> > >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > >> --- > >> > >> Changes in v2: None > >> > >> hw/riscv/Makefile.objs | 2 +- > >> hw/riscv/sifive_e.c | 4 ++-- > >> hw/riscv/{sifive_prci.c => sifive_e_prci.c} | 14 +++++++------- > >> include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} | 14 +++++++------- > >> 4 files changed, 17 insertions(+), 17 deletions(-) > >> rename hw/riscv/{sifive_prci.c => sifive_e_prci.c} (90%) > >> rename include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} (82%) > >> > >> diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs > >> index eb9d4f9..c859697 100644 > >> --- a/hw/riscv/Makefile.objs > >> +++ b/hw/riscv/Makefile.objs > >> @@ -2,9 +2,9 @@ obj-y += boot.o > >> obj-$(CONFIG_SPIKE) += riscv_htif.o > >> obj-$(CONFIG_HART) += riscv_hart.o > >> obj-$(CONFIG_SIFIVE_E) += sifive_e.o > >> +obj-$(CONFIG_SIFIVE_E) += sifive_e_prci.o > >> obj-$(CONFIG_SIFIVE) += sifive_clint.o > >> obj-$(CONFIG_SIFIVE) += sifive_gpio.o > >> -obj-$(CONFIG_SIFIVE) += sifive_prci.o > >> obj-$(CONFIG_SIFIVE) += sifive_plic.o > >> obj-$(CONFIG_SIFIVE) += sifive_test.o > >> obj-$(CONFIG_SIFIVE_U) += sifive_u.o > >> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > >> index 2a499d8..2d67670 100644 > >> --- a/hw/riscv/sifive_e.c > >> +++ b/hw/riscv/sifive_e.c > >> @@ -41,9 +41,9 @@ > >> #include "hw/riscv/riscv_hart.h" > >> #include "hw/riscv/sifive_plic.h" > >> #include "hw/riscv/sifive_clint.h" > >> -#include "hw/riscv/sifive_prci.h" > >> #include "hw/riscv/sifive_uart.h" > >> #include "hw/riscv/sifive_e.h" > >> +#include "hw/riscv/sifive_e_prci.h" > >> #include "hw/riscv/boot.h" > >> #include "chardev/char.h" > >> #include "sysemu/arch_init.h" > >> @@ -174,7 +174,7 @@ static void riscv_sifive_e_soc_realize(DeviceState > *dev, Error **errp) > >> SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE); > >> sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", > >> memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); > >> - sifive_prci_create(memmap[SIFIVE_E_PRCI].base); > >> + sifive_e_prci_create(memmap[SIFIVE_E_PRCI].base); > >> > >> /* GPIO */ > >> > > > > I think adding infix to function name is sufficient and keeping the > filename the same may be better. > > The U board PRCI or variant implementation in future could be included > in the same files with different create function > > > > I considered such approach when working on this one, but later I chose > to implement a new file for SiFive U machine. > > The SiFive U and E PRCI blocks have different register blocks so if we > put two variants into one file, their functions don't have much in > common and we end up just merely physically put them into one file > which does not bring too much benefit IMHO. > > Regards, > Bin > agree that the difference between u and e prci are a lot and it make sense to separate it Reviewed-by: Chih-Min Chao <chihmin.chao@sifive.com>
On Wed, Aug 7, 2019 at 12:49 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Current SiFive PRCI model only works with sifive_e machine, as it > only emulates registers or PRCI block in the FE310 SoC. > > Rename the file name to make it clear that it is for sifive_e. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > > Changes in v2: None > > hw/riscv/Makefile.objs | 2 +- > hw/riscv/sifive_e.c | 4 ++-- > hw/riscv/{sifive_prci.c => sifive_e_prci.c} | 14 +++++++------- > include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} | 14 +++++++------- > 4 files changed, 17 insertions(+), 17 deletions(-) > rename hw/riscv/{sifive_prci.c => sifive_e_prci.c} (90%) > rename include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} (82%) > > diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs > index eb9d4f9..c859697 100644 > --- a/hw/riscv/Makefile.objs > +++ b/hw/riscv/Makefile.objs > @@ -2,9 +2,9 @@ obj-y += boot.o > obj-$(CONFIG_SPIKE) += riscv_htif.o > obj-$(CONFIG_HART) += riscv_hart.o > obj-$(CONFIG_SIFIVE_E) += sifive_e.o > +obj-$(CONFIG_SIFIVE_E) += sifive_e_prci.o > obj-$(CONFIG_SIFIVE) += sifive_clint.o > obj-$(CONFIG_SIFIVE) += sifive_gpio.o > -obj-$(CONFIG_SIFIVE) += sifive_prci.o > obj-$(CONFIG_SIFIVE) += sifive_plic.o > obj-$(CONFIG_SIFIVE) += sifive_test.o > obj-$(CONFIG_SIFIVE_U) += sifive_u.o > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > index 2a499d8..2d67670 100644 > --- a/hw/riscv/sifive_e.c > +++ b/hw/riscv/sifive_e.c > @@ -41,9 +41,9 @@ > #include "hw/riscv/riscv_hart.h" > #include "hw/riscv/sifive_plic.h" > #include "hw/riscv/sifive_clint.h" > -#include "hw/riscv/sifive_prci.h" > #include "hw/riscv/sifive_uart.h" > #include "hw/riscv/sifive_e.h" > +#include "hw/riscv/sifive_e_prci.h" > #include "hw/riscv/boot.h" > #include "chardev/char.h" > #include "sysemu/arch_init.h" > @@ -174,7 +174,7 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) > SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE); > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", > memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); > - sifive_prci_create(memmap[SIFIVE_E_PRCI].base); > + sifive_e_prci_create(memmap[SIFIVE_E_PRCI].base); > > /* GPIO */ > > diff --git a/hw/riscv/sifive_prci.c b/hw/riscv/sifive_e_prci.c > similarity index 90% > rename from hw/riscv/sifive_prci.c > rename to hw/riscv/sifive_e_prci.c > index f406682..acb914d 100644 > --- a/hw/riscv/sifive_prci.c > +++ b/hw/riscv/sifive_e_prci.c > @@ -1,5 +1,5 @@ > /* > - * QEMU SiFive PRCI (Power, Reset, Clock, Interrupt) > + * QEMU SiFive E PRCI (Power, Reset, Clock, Interrupt) > * > * Copyright (c) 2017 SiFive, Inc. > * > @@ -22,7 +22,7 @@ > #include "hw/sysbus.h" > #include "qemu/module.h" > #include "target/riscv/cpu.h" > -#include "hw/riscv/sifive_prci.h" > +#include "hw/riscv/sifive_e_prci.h" > > static uint64_t sifive_prci_read(void *opaque, hwaddr addr, unsigned int size) > { > @@ -82,10 +82,10 @@ static const MemoryRegionOps sifive_prci_ops = { > > static void sifive_prci_init(Object *obj) > { > - SiFivePRCIState *s = SIFIVE_PRCI(obj); > + SiFivePRCIState *s = SIFIVE_E_PRCI(obj); Should we not rename the struct as well? Alistair > > memory_region_init_io(&s->mmio, obj, &sifive_prci_ops, s, > - TYPE_SIFIVE_PRCI, 0x8000); > + TYPE_SIFIVE_E_PRCI, 0x8000); > sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); > > s->hfrosccfg = (SIFIVE_PRCI_HFROSCCFG_RDY | SIFIVE_PRCI_HFROSCCFG_EN); > @@ -97,7 +97,7 @@ static void sifive_prci_init(Object *obj) > } > > static const TypeInfo sifive_prci_info = { > - .name = TYPE_SIFIVE_PRCI, > + .name = TYPE_SIFIVE_E_PRCI, > .parent = TYPE_SYS_BUS_DEVICE, > .instance_size = sizeof(SiFivePRCIState), > .instance_init = sifive_prci_init, > @@ -114,9 +114,9 @@ type_init(sifive_prci_register_types) > /* > * Create PRCI device. > */ > -DeviceState *sifive_prci_create(hwaddr addr) > +DeviceState *sifive_e_prci_create(hwaddr addr) > { > - DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_PRCI); > + DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_E_PRCI); > qdev_init_nofail(dev); > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > return dev; > diff --git a/include/hw/riscv/sifive_prci.h b/include/hw/riscv/sifive_e_prci.h > similarity index 82% > rename from include/hw/riscv/sifive_prci.h > rename to include/hw/riscv/sifive_e_prci.h > index bd51c4a..7932fe7 100644 > --- a/include/hw/riscv/sifive_prci.h > +++ b/include/hw/riscv/sifive_e_prci.h > @@ -1,5 +1,5 @@ > /* > - * QEMU SiFive PRCI (Power, Reset, Clock, Interrupt) interface > + * QEMU SiFive E PRCI (Power, Reset, Clock, Interrupt) interface > * > * Copyright (c) 2017 SiFive, Inc. > * > @@ -16,8 +16,8 @@ > * this program. If not, see <http://www.gnu.org/licenses/>. > */ > > -#ifndef HW_SIFIVE_PRCI_H > -#define HW_SIFIVE_PRCI_H > +#ifndef HW_SIFIVE_E_PRCI_H > +#define HW_SIFIVE_E_PRCI_H > > enum { > SIFIVE_PRCI_HFROSCCFG = 0x0, > @@ -47,10 +47,10 @@ enum { > SIFIVE_PRCI_PLLOUTDIV_DIV1 = (1 << 8) > }; > > -#define TYPE_SIFIVE_PRCI "riscv.sifive.prci" > +#define TYPE_SIFIVE_E_PRCI "riscv.sifive.e.prci" > > -#define SIFIVE_PRCI(obj) \ > - OBJECT_CHECK(SiFivePRCIState, (obj), TYPE_SIFIVE_PRCI) > +#define SIFIVE_E_PRCI(obj) \ > + OBJECT_CHECK(SiFivePRCIState, (obj), TYPE_SIFIVE_E_PRCI) > > typedef struct SiFivePRCIState { > /*< private >*/ > @@ -64,6 +64,6 @@ typedef struct SiFivePRCIState { > uint32_t plloutdiv; > } SiFivePRCIState; > > -DeviceState *sifive_prci_create(hwaddr addr); > +DeviceState *sifive_e_prci_create(hwaddr addr); > > #endif > -- > 2.7.4 > >
On Sun, Aug 11, 2019 at 1:00 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Alistair, > > On Sat, Aug 10, 2019 at 9:51 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Wed, Aug 7, 2019 at 12:49 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Current SiFive PRCI model only works with sifive_e machine, as it > > > only emulates registers or PRCI block in the FE310 SoC. > > > > > > Rename the file name to make it clear that it is for sifive_e. > > > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > > --- > > > > > > Changes in v2: None > > > > > > hw/riscv/Makefile.objs | 2 +- > > > hw/riscv/sifive_e.c | 4 ++-- > > > hw/riscv/{sifive_prci.c => sifive_e_prci.c} | 14 +++++++------- > > > include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} | 14 +++++++------- > > > 4 files changed, 17 insertions(+), 17 deletions(-) > > > rename hw/riscv/{sifive_prci.c => sifive_e_prci.c} (90%) > > > rename include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} (82%) > > > > > > diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs > > > index eb9d4f9..c859697 100644 > > > --- a/hw/riscv/Makefile.objs > > > +++ b/hw/riscv/Makefile.objs > > > @@ -2,9 +2,9 @@ obj-y += boot.o > > > obj-$(CONFIG_SPIKE) += riscv_htif.o > > > obj-$(CONFIG_HART) += riscv_hart.o > > > obj-$(CONFIG_SIFIVE_E) += sifive_e.o > > > +obj-$(CONFIG_SIFIVE_E) += sifive_e_prci.o > > > obj-$(CONFIG_SIFIVE) += sifive_clint.o > > > obj-$(CONFIG_SIFIVE) += sifive_gpio.o > > > -obj-$(CONFIG_SIFIVE) += sifive_prci.o > > > obj-$(CONFIG_SIFIVE) += sifive_plic.o > > > obj-$(CONFIG_SIFIVE) += sifive_test.o > > > obj-$(CONFIG_SIFIVE_U) += sifive_u.o > > > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > > > index 2a499d8..2d67670 100644 > > > --- a/hw/riscv/sifive_e.c > > > +++ b/hw/riscv/sifive_e.c > > > @@ -41,9 +41,9 @@ > > > #include "hw/riscv/riscv_hart.h" > > > #include "hw/riscv/sifive_plic.h" > > > #include "hw/riscv/sifive_clint.h" > > > -#include "hw/riscv/sifive_prci.h" > > > #include "hw/riscv/sifive_uart.h" > > > #include "hw/riscv/sifive_e.h" > > > +#include "hw/riscv/sifive_e_prci.h" > > > #include "hw/riscv/boot.h" > > > #include "chardev/char.h" > > > #include "sysemu/arch_init.h" > > > @@ -174,7 +174,7 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) > > > SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE); > > > sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", > > > memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); > > > - sifive_prci_create(memmap[SIFIVE_E_PRCI].base); > > > + sifive_e_prci_create(memmap[SIFIVE_E_PRCI].base); > > > > > > /* GPIO */ > > > > > > diff --git a/hw/riscv/sifive_prci.c b/hw/riscv/sifive_e_prci.c > > > similarity index 90% > > > rename from hw/riscv/sifive_prci.c > > > rename to hw/riscv/sifive_e_prci.c > > > index f406682..acb914d 100644 > > > --- a/hw/riscv/sifive_prci.c > > > +++ b/hw/riscv/sifive_e_prci.c > > > @@ -1,5 +1,5 @@ > > > /* > > > - * QEMU SiFive PRCI (Power, Reset, Clock, Interrupt) > > > + * QEMU SiFive E PRCI (Power, Reset, Clock, Interrupt) > > > * > > > * Copyright (c) 2017 SiFive, Inc. > > > * > > > @@ -22,7 +22,7 @@ > > > #include "hw/sysbus.h" > > > #include "qemu/module.h" > > > #include "target/riscv/cpu.h" > > > -#include "hw/riscv/sifive_prci.h" > > > +#include "hw/riscv/sifive_e_prci.h" > > > > > > static uint64_t sifive_prci_read(void *opaque, hwaddr addr, unsigned int size) > > > { > > > @@ -82,10 +82,10 @@ static const MemoryRegionOps sifive_prci_ops = { > > > > > > static void sifive_prci_init(Object *obj) > > > { > > > - SiFivePRCIState *s = SIFIVE_PRCI(obj); > > > + SiFivePRCIState *s = SIFIVE_E_PRCI(obj); > > > > Should we not rename the struct as well? > > > > I think this is OK given it's only used by sifive_e machine and will > not be mixed with sifive_u. Structs can be public though, so this seems risky. I feel like it should be renamed with the file. Alistair > > Regards, > Bin
diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs index eb9d4f9..c859697 100644 --- a/hw/riscv/Makefile.objs +++ b/hw/riscv/Makefile.objs @@ -2,9 +2,9 @@ obj-y += boot.o obj-$(CONFIG_SPIKE) += riscv_htif.o obj-$(CONFIG_HART) += riscv_hart.o obj-$(CONFIG_SIFIVE_E) += sifive_e.o +obj-$(CONFIG_SIFIVE_E) += sifive_e_prci.o obj-$(CONFIG_SIFIVE) += sifive_clint.o obj-$(CONFIG_SIFIVE) += sifive_gpio.o -obj-$(CONFIG_SIFIVE) += sifive_prci.o obj-$(CONFIG_SIFIVE) += sifive_plic.o obj-$(CONFIG_SIFIVE) += sifive_test.o obj-$(CONFIG_SIFIVE_U) += sifive_u.o diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 2a499d8..2d67670 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -41,9 +41,9 @@ #include "hw/riscv/riscv_hart.h" #include "hw/riscv/sifive_plic.h" #include "hw/riscv/sifive_clint.h" -#include "hw/riscv/sifive_prci.h" #include "hw/riscv/sifive_uart.h" #include "hw/riscv/sifive_e.h" +#include "hw/riscv/sifive_e_prci.h" #include "hw/riscv/boot.h" #include "chardev/char.h" #include "sysemu/arch_init.h" @@ -174,7 +174,7 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp) SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE); sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon", memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size); - sifive_prci_create(memmap[SIFIVE_E_PRCI].base); + sifive_e_prci_create(memmap[SIFIVE_E_PRCI].base); /* GPIO */ diff --git a/hw/riscv/sifive_prci.c b/hw/riscv/sifive_e_prci.c similarity index 90% rename from hw/riscv/sifive_prci.c rename to hw/riscv/sifive_e_prci.c index f406682..acb914d 100644 --- a/hw/riscv/sifive_prci.c +++ b/hw/riscv/sifive_e_prci.c @@ -1,5 +1,5 @@ /* - * QEMU SiFive PRCI (Power, Reset, Clock, Interrupt) + * QEMU SiFive E PRCI (Power, Reset, Clock, Interrupt) * * Copyright (c) 2017 SiFive, Inc. * @@ -22,7 +22,7 @@ #include "hw/sysbus.h" #include "qemu/module.h" #include "target/riscv/cpu.h" -#include "hw/riscv/sifive_prci.h" +#include "hw/riscv/sifive_e_prci.h" static uint64_t sifive_prci_read(void *opaque, hwaddr addr, unsigned int size) { @@ -82,10 +82,10 @@ static const MemoryRegionOps sifive_prci_ops = { static void sifive_prci_init(Object *obj) { - SiFivePRCIState *s = SIFIVE_PRCI(obj); + SiFivePRCIState *s = SIFIVE_E_PRCI(obj); memory_region_init_io(&s->mmio, obj, &sifive_prci_ops, s, - TYPE_SIFIVE_PRCI, 0x8000); + TYPE_SIFIVE_E_PRCI, 0x8000); sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); s->hfrosccfg = (SIFIVE_PRCI_HFROSCCFG_RDY | SIFIVE_PRCI_HFROSCCFG_EN); @@ -97,7 +97,7 @@ static void sifive_prci_init(Object *obj) } static const TypeInfo sifive_prci_info = { - .name = TYPE_SIFIVE_PRCI, + .name = TYPE_SIFIVE_E_PRCI, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(SiFivePRCIState), .instance_init = sifive_prci_init, @@ -114,9 +114,9 @@ type_init(sifive_prci_register_types) /* * Create PRCI device. */ -DeviceState *sifive_prci_create(hwaddr addr) +DeviceState *sifive_e_prci_create(hwaddr addr) { - DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_PRCI); + DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_E_PRCI); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); return dev; diff --git a/include/hw/riscv/sifive_prci.h b/include/hw/riscv/sifive_e_prci.h similarity index 82% rename from include/hw/riscv/sifive_prci.h rename to include/hw/riscv/sifive_e_prci.h index bd51c4a..7932fe7 100644 --- a/include/hw/riscv/sifive_prci.h +++ b/include/hw/riscv/sifive_e_prci.h @@ -1,5 +1,5 @@ /* - * QEMU SiFive PRCI (Power, Reset, Clock, Interrupt) interface + * QEMU SiFive E PRCI (Power, Reset, Clock, Interrupt) interface * * Copyright (c) 2017 SiFive, Inc. * @@ -16,8 +16,8 @@ * this program. If not, see <http://www.gnu.org/licenses/>. */ -#ifndef HW_SIFIVE_PRCI_H -#define HW_SIFIVE_PRCI_H +#ifndef HW_SIFIVE_E_PRCI_H +#define HW_SIFIVE_E_PRCI_H enum { SIFIVE_PRCI_HFROSCCFG = 0x0, @@ -47,10 +47,10 @@ enum { SIFIVE_PRCI_PLLOUTDIV_DIV1 = (1 << 8) }; -#define TYPE_SIFIVE_PRCI "riscv.sifive.prci" +#define TYPE_SIFIVE_E_PRCI "riscv.sifive.e.prci" -#define SIFIVE_PRCI(obj) \ - OBJECT_CHECK(SiFivePRCIState, (obj), TYPE_SIFIVE_PRCI) +#define SIFIVE_E_PRCI(obj) \ + OBJECT_CHECK(SiFivePRCIState, (obj), TYPE_SIFIVE_E_PRCI) typedef struct SiFivePRCIState { /*< private >*/ @@ -64,6 +64,6 @@ typedef struct SiFivePRCIState { uint32_t plloutdiv; } SiFivePRCIState; -DeviceState *sifive_prci_create(hwaddr addr); +DeviceState *sifive_e_prci_create(hwaddr addr); #endif
Current SiFive PRCI model only works with sifive_e machine, as it only emulates registers or PRCI block in the FE310 SoC. Rename the file name to make it clear that it is for sifive_e. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- Changes in v2: None hw/riscv/Makefile.objs | 2 +- hw/riscv/sifive_e.c | 4 ++-- hw/riscv/{sifive_prci.c => sifive_e_prci.c} | 14 +++++++------- include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} | 14 +++++++------- 4 files changed, 17 insertions(+), 17 deletions(-) rename hw/riscv/{sifive_prci.c => sifive_e_prci.c} (90%) rename include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} (82%)