Hi Alistair, On Sat, Aug 22, 2020 at 3:04 AM Alistair Francis <alistai...@gmail.com> wrote: > > On Mon, Aug 17, 2020 at 2:53 AM Bin Meng <bmeng...@gmail.com> wrote: > > > > On Sat, Aug 15, 2020 at 1:56 AM Philippe Mathieu-Daudé <f4...@amsat.org> > > wrote: > > > > > > On 8/14/20 12:25 AM, Eduardo Habkost wrote: > > > > Some of the enum constant names conflict with the QOM type check > > > > macros. This needs to be addressed to allow us to transform the > > > > QOM type check macros into functions generated by > > > > OBJECT_DECLARE_TYPE(). > > > > > > > > Rename all the constants to IBEX_DEV_*, to avoid conflicts. > > > > > > > > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > > > > --- > > > > include/hw/riscv/opentitan.h | 38 ++++++++-------- > > > > hw/riscv/opentitan.c | 84 ++++++++++++++++++------------------ > > > > 2 files changed, 61 insertions(+), 61 deletions(-) > > > > > > > > diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h > > > > index 8f29b9cbbf..835a80f896 100644 > > > > --- a/include/hw/riscv/opentitan.h > > > > +++ b/include/hw/riscv/opentitan.h > > > > @@ -49,25 +49,25 @@ typedef struct OpenTitanState { > > > > } OpenTitanState; > > > > > > > > enum { > > > > - IBEX_ROM, > > > > - IBEX_RAM, > > > > - IBEX_FLASH, > > > > - IBEX_UART, > > > > - IBEX_GPIO, > > > > - IBEX_SPI, > > > > - IBEX_FLASH_CTRL, > > > > - IBEX_RV_TIMER, > > > > - IBEX_AES, > > > > - IBEX_HMAC, > > > > - IBEX_PLIC, > > > > - IBEX_PWRMGR, > > > > - IBEX_RSTMGR, > > > > - IBEX_CLKMGR, > > > > - IBEX_PINMUX, > > > > - IBEX_ALERT_HANDLER, > > > > - IBEX_NMI_GEN, > > > > - IBEX_USBDEV, > > > > - IBEX_PADCTRL, > > > > + IBEX_DEV_ROM, > > > > + IBEX_DEV_RAM, > > > > + IBEX_DEV_FLASH, > > > > + IBEX_DEV_UART, > > > > + IBEX_DEV_GPIO, > > > > + IBEX_DEV_SPI, > > > > + IBEX_DEV_FLASH_CTRL, > > > > + IBEX_DEV_RV_TIMER, > > > > + IBEX_DEV_AES, > > > > + IBEX_DEV_HMAC, > > > > + IBEX_DEV_PLIC, > > > > + IBEX_DEV_PWRMGR, > > > > + IBEX_DEV_RSTMGR, > > > > + IBEX_DEV_CLKMGR, > > > > + IBEX_DEV_PINMUX, > > > > + IBEX_DEV_ALERT_HANDLER, > > > > + IBEX_DEV_NMI_GEN, > > > > + IBEX_DEV_USBDEV, > > > > + IBEX_DEV_PADCTRL, > > > > > > Similarly, why is this enum in a public header and not local > > > to opentitan.c, only place where it is used? > > > > > > > I believe the reason is that opentitan.c implements both SoC and board > > stuff. These enums are helpful to define SoC peripherals hence > > technically another board that uses the same SoC may utilize these > > macros. > > > > Unfortunately this is the case for all RISC-V boards so far. Should we > > clean this up, for example, splitting SoC and board codes into 2 > > files? > > Yeah the hw/riscv directory is a bit of a mess. We need to look at > moving non machine parts out (like sifive_uart for example) and then > it's probably worth splitting SoC and machine. It does result in some > extra files (some of which are very simple) but the current setup is > pretty confusing. > > Are you volunteering Bin? Otherwise I can look at it. >
Yes, I will be working on a series to clean hw/riscv directory. Regards, Bin