Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs

[BALATON Zoltan]

On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
> On 4/7/23 00:02, BALATON Zoltan wrote:
> > The iomem memory region is better used for the PCI IO space but
> > currently used for registers. Stop using it for that to allow this to
> > be cleaned up in the next patch.
> >
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > ---
> >   hw/ppc/ppc440_pcix.c | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> > index adfecf1e76..ee2dc44f67 100644
> > --- a/hw/ppc/ppc440_pcix.c
> > +++ b/hw/ppc/ppc440_pcix.c
> > @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error 
> > **errp)
> >       SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> >       PPC440PCIXState *s;
> >       PCIHostState *h;
> > +    MemoryRegion *regs = g_new(MemoryRegion, 1);
>
> Why not hold it within PPC440PCIXState?

Because it's never needed after this function.

Regards,
BALATON Zoltan

> >       h = PCI_HOST_BRIDGE(dev);
> >       s = PPC440_PCIX_HOST(dev);
> > @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev, 
> > Error **errp)
> >                             h, "pci-conf-idx", 4);
> >       memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops,
> >                             h, "pci-conf-data", 4);
> > -    memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
> > -                          "pci.reg", PPC440_REG_SIZE);
> > +    memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg",
> > +                          PPC440_REG_SIZE);
> >       memory_region_add_subregion(&s->container, PCIC0_CFGADDR, 
> > &h->conf_mem);
> >       memory_region_add_subregion(&s->container, PCIC0_CFGDATA, 
> > &h->data_mem);
> > -    memory_region_add_subregion(&s->container, PPC440_REG_BASE, 
> > &s->iomem);
> > +    memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs);
> >       sysbus_init_mmio(sbd, &s->container);
> >   }