Re: [PATCH for-8.2 2/3] pnv/lpc: Hook up xscom region for P9/P10

[Cédric Le Goater]

On 8/9/23 16:56, Frederic Barrat wrote:
> Hello Joel,
>
> So we're re-using the same xscom ops as on P8. A quick look at the definition 
> of those 4 registers on P8 (0xb0020) and on P9/P10 (0x00090040) seem to show 
> they are not the same though. Am i missing something?

Joel, are we ok ? Should we grab this patch ? or not.

Thanks,

C.


>    Fred
>
>
> On 08/08/2023 10:34, Joel Stanley wrote:
> >  From P9 on the LPC bus is memory mapped. However the xscom access still
> > is possible, so add it too.
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >   include/hw/ppc/pnv_xscom.h | 6 ++++++
> >   hw/ppc/pnv.c               | 4 ++++
> >   hw/ppc/pnv_lpc.c           | 6 ++++++
> >   3 files changed, 16 insertions(+)
> >
> > diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> > index 9bc64635471e..42601bdf419d 100644
> > --- a/include/hw/ppc/pnv_xscom.h
> > +++ b/include/hw/ppc/pnv_xscom.h
> > @@ -96,6 +96,9 @@ struct PnvXScomInterfaceClass {
> >   #define PNV9_XSCOM_SBE_CTRL_BASE  0x00050008
> >   #define PNV9_XSCOM_SBE_CTRL_SIZE  0x1
> > +#define PNV9_XSCOM_LPC_BASE       0x00090040
> > +#define PNV9_XSCOM_LPC_SIZE       PNV_XSCOM_LPC_SIZE
> > +
> >   #define PNV9_XSCOM_SBE_MBOX_BASE  0x000D0050
> >   #define PNV9_XSCOM_SBE_MBOX_SIZE  0x16
> > @@ -155,6 +158,9 @@ struct PnvXScomInterfaceClass {
> >   #define PNV10_XSCOM_SBE_CTRL_BASE  PNV9_XSCOM_SBE_CTRL_BASE
> >   #define PNV10_XSCOM_SBE_CTRL_SIZE  PNV9_XSCOM_SBE_CTRL_SIZE
> > +#define PNV10_XSCOM_LPC_BASE       PNV9_XSCOM_LPC_BASE
> > +#define PNV10_XSCOM_LPC_SIZE       PNV9_XSCOM_LPC_SIZE
> > +
> >   #define PNV10_XSCOM_SBE_MBOX_BASE  PNV9_XSCOM_SBE_MBOX_BASE
> >   #define PNV10_XSCOM_SBE_MBOX_SIZE  PNV9_XSCOM_SBE_MBOX_SIZE
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index afdaa25c2b26..a5db655b41b6 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1566,6 +1566,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
> > Error **errp)
> >       }
> >       memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
> >                                   &chip9->lpc.mmio_regs);
> > +    pnv_xscom_add_subregion(chip, PNV9_XSCOM_LPC_BASE,
> > +                            &chip9->lpc.xscom_regs);
> >       chip->fw_mr = &chip9->lpc.isa_fw;
> >       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> > @@ -1785,6 +1787,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, 
> > Error **errp)
> >       }
> >       memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
> >                                   &chip10->lpc.mmio_regs);
> > +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_LPC_BASE,
> > +                            &chip10->lpc.xscom_regs);
> >       chip->fw_mr = &chip10->lpc.isa_fw;
> >       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> > index caf5e10a5f96..6c6a3134087f 100644
> > --- a/hw/ppc/pnv_lpc.c
> > +++ b/hw/ppc/pnv_lpc.c
> > @@ -666,6 +666,12 @@ static void pnv_lpc_power9_realize(DeviceState *dev, Error 
> > **errp)
> >       /* P9 uses a MMIO region */
> >       memory_region_init_io(&lpc->mmio_regs, OBJECT(lpc), &pnv_lpc_mmio_ops,
> >                             lpc, "lpcm", PNV9_LPCM_SIZE);
> > +
> > +    /* but the XSCOM region still exists */
> > +    pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(lpc),
> > +                          &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
> > +                          PNV_XSCOM_LPC_SIZE);
> > +
> >   }
> >   static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)