On 3/8/19 1:01 AM, David Gibson wrote: > On Thu, Mar 07, 2019 at 11:35:38PM +0100, Cédric Le Goater wrote: >> The ISA bus has a different DT nodename on POWER9. Compute the name >> when the PnvChip is realized, that is before it is used by the machine >> to populate the device tree with the ISA devices. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > > I still tend to think that passing an offset into pnv_dt_isa would > have been a better solution, but this will serve. Applied.
Do you mean something like below possibly ? int isa_offset = fdt_path_offset(fdt, pnv->chips[0]->dt_isa_nodename); /* Populate ISA devices on chip 0 */ pnv_dt_isa(pnv->isa_bus, fdt, isa_offset); pnv_dt_isa() is called at the machine level but we could change it to be called at the chip level for chip0 only. C. >> --- >> include/hw/ppc/pnv.h | 2 ++ >> hw/ppc/pnv.c | 18 +++++------------- >> 2 files changed, 7 insertions(+), 13 deletions(-) >> >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h >> index 8d80cb34eebb..c81f157f41a9 100644 >> --- a/include/hw/ppc/pnv.h >> +++ b/include/hw/ppc/pnv.h >> @@ -58,6 +58,8 @@ typedef struct PnvChip { >> MemoryRegion xscom_mmio; >> MemoryRegion xscom; >> AddressSpace xscom_as; >> + >> + gchar *dt_isa_nodename; >> } PnvChip; >> >> #define TYPE_PNV8_CHIP "pnv8-chip" >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index 922e3ec48bb5..6625562d276d 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -417,24 +417,12 @@ static int pnv_dt_isa_device(DeviceState *dev, void >> *opaque) >> return 0; >> } >> >> -static int pnv_chip_isa_offset(PnvChip *chip, void *fdt) >> -{ >> - char *name; >> - int offset; >> - >> - name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x", >> - (uint64_t) PNV_XSCOM_BASE(chip), >> PNV_XSCOM_LPC_BASE); >> - offset = fdt_path_offset(fdt, name); >> - g_free(name); >> - return offset; >> -} >> - >> /* The default LPC bus of a multichip system is on chip 0. It's >> * recognized by the firmware (skiboot) using a "primary" property. >> */ >> static void pnv_dt_isa(PnvMachineState *pnv, void *fdt) >> { >> - int isa_offset = pnv_chip_isa_offset(pnv->chips[0], fdt); >> + int isa_offset = fdt_path_offset(fdt, pnv->chips[0]->dt_isa_nodename); >> ForeachPopulateArgs args = { >> .fdt = fdt, >> .offset = isa_offset, >> @@ -866,6 +854,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, >> Error **errp) >> &error_fatal); >> pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, >> &chip8->lpc.xscom_regs); >> >> + chip->dt_isa_nodename = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x", >> + (uint64_t) PNV_XSCOM_BASE(chip), >> + PNV_XSCOM_LPC_BASE); >> + >> /* Interrupt Management Area. This is the memory region holding >> * all the Interrupt Control Presenter (ICP) registers */ >> pnv_chip_icp_realize(chip8, &local_err); >