Re: [Qemu-devel] [PATCH v2 11/28] riscv: sifive: Rename sifive_prci.{c, h} to sifive_e_prci.{c, h}

[Chih-Min Chao <chihmin.chao@sifive.com>]

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>