| Message ID | 20230808083445.4613-3-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | pnv/lpc: Hook up xscoms for LPC | expand |
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)
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)
On Tue, 29 Aug 2023 at 14:45, Cédric Le Goater <clg@kaod.org> wrote: > > 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. Sorry, I chatted to Fred about this one but forgot to reply on the list. I made a bad assumption about the xscom registers matching between p8 and p9/10. This patch will need to be reworked, so please ignore it. Cheers, Joel
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)
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(+)