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

[Frederic Barrat <fbarrat@linux.ibm.com>]

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?

   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)