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

[Bin Meng]

On Wed, Aug 7, 2019 at 4:54 PM Chih-Min Chao <address@hidden> wrote:
>
>
>
> On Wed, Aug 7, 2019 at 3:49 PM Bin Meng <address@hidden> 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 <address@hidden>
>> ---
>>
>> 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