Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

[Alistair Francis]

On Fri, Oct 16, 2020 at 9:31 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> I don't know why it isn't documented in that PDF (or in the register map), 
> but if you check 
> https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.h
>  you'll see the following
>
> ```
> typedef struct
> {
>     volatile uint32_t SOFT_RESET;
>     volatile uint32_t VDETECTOR;
>     volatile uint32_t TVS_CONTROL;
>     volatile uint32_t TVS_TEMP_A;
>     volatile uint32_t TVS_TEMP_B;
>     volatile uint32_t TVS_TEMP_C;
>     volatile uint32_t TVS_VOLT_A;
>     volatile uint32_t TVS_VOLT_B;
>     volatile uint32_t TVS_VOLT_C;
>     volatile uint32_t TVS_OUTPUT0;
>     volatile uint32_t TVS_OUTPUT1;
>     volatile uint32_t TVS_TRIGGER;
>     volatile uint32_t TRIM_VDET1P05;
>     volatile uint32_t TRIM_VDET1P8;
>     volatile uint32_t TRIM_VDET2P5;
>     volatile uint32_t TRIM_TVS;
>     volatile uint32_t TRIM_GDET1P05;
>     volatile uint32_t RESERVED0;
>     volatile uint32_t RESERVED1;
>     volatile uint32_t RESERVED2;
>     volatile uint32_t SERVICES_CR;
>     volatile uint32_t SERVICES_SR;
>     volatile uint32_t USER_DETECTOR_SR;
>     volatile uint32_t USER_DETECTOR_CR;
>     volatile uint32_t MSS_SPI_CR;
>
> } SCBCTRL_TypeDef;
>
> #define MSS_SCBCTRL                    ((SCBCTRL_TypeDef*) (0x37020000UL))
>
> /*2kB bytes long mailbox.*/
> #define MSS_SCBMAILBOX                 ((uint32_t*) (0x37020800UL))
> ```
>
> And in 
> https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.c
>  you'll see MSS_SCB and MSS_SCBMAILBOX used in many places to interact with 
> the FPGA system controller to perform various services.

Ok, so the memory map exists, but not documented in the official documentation.

In this case it's fine to add. Can you add a comment though saying
that it's not documented in the official documentation though?
Otherwise it will confuse developers.

Alistair

>
>
> Cheers,
> Ivan
>
>
> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Friday 16 October 2020 17:08
> To: Ivan Griffin <ivan.griffin@emdalo.com>
> Cc: Bin Meng <bin.meng@windriver.com>; QEMU Trivial 
> <qemu-trivial@nongnu.org>; open list:RISC-V <qemu-riscv@nongnu.org>; 
> qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry
>
> On Fri, Oct 16, 2020 at 8:04 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
> >
> > Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU
> > reporting a STORE/AMO Access Fault.
> >
> > This region is used by the PolarFire SoC port of U-Boot to interact
> > with the FPGA system controller.
> >
> > Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>
> > ---
> >  hw/riscv/microchip_pfsoc.c         | 6 ++++++
> >  include/hw/riscv/microchip_pfsoc.h | 1 +
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> > index 4627179cd3..20e1496e3e 100644
> > --- a/hw/riscv/microchip_pfsoc.c
> > +++ b/hw/riscv/microchip_pfsoc.c
> > @@ -97,6 +97,7 @@ static const struct MemmapEntry {
> >      [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
> >      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
> >      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
> > +    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },
>
> I don't see this in the UG0880 "User Guide PolarFire SoC FPGA Microprocessor 
> Sub-System" memory map.
>
> Where is this documented?
>
> Alistair
>
> >      [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
> >      [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
> >  };
> > @@ -341,6 +342,11 @@ static void microchip_pfsoc_soc_realize(DeviceState 
> > *dev, Error **errp)
> >      create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
> >          memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
> >          memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
> > +
> > +    /* IOSCBCTRL */
> > +    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
> > +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
> > +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
> >  }
> >
> >  static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void
> > *data) diff --git a/include/hw/riscv/microchip_pfsoc.h
> > b/include/hw/riscv/microchip_pfsoc.h
> > index 8bfc7e1a85..3f1874b162 100644
> > --- a/include/hw/riscv/microchip_pfsoc.h
> > +++ b/include/hw/riscv/microchip_pfsoc.h
> > @@ -95,6 +95,7 @@ enum {
> >      MICROCHIP_PFSOC_ENVM_CFG,
> >      MICROCHIP_PFSOC_ENVM_DATA,
> >      MICROCHIP_PFSOC_IOSCB_CFG,
> > +    MICROCHIP_PFSOC_IOSCB_CTRL,
> >      MICROCHIP_PFSOC_DRAM,
> >  };
> >
> > --
> > 2.17.1
> >
> >