Re: [PATCH 08/41] opentitan: Rename memmap enum constants

[Bin Meng]

On Sat, Aug 15, 2020 at 1:56 AM Philippe Mathieu-Daudé <f4bug@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 <ehabkost@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?

Regards,
Bin