Re: [Qemu-ppc] [Qemu-devel] [PATCH 07/19] uninorth: move PCI mmio memory region initialisation into init function

[Mark Cave-Ayland]

On 07/03/18 07:02, Mark Cave-Ayland wrote:

> On 06/03/18 23:44, BALATON Zoltan wrote:
>
> > On Tue, 6 Mar 2018, Mark Cave-Ayland wrote:
> > > Whilst we are here, rename the memory regions to better reflect 
> > > whether they
> > > belong to either a PCI or an AGP bus.
> > >
> > > Signed-off-by: Mark Cave-Ayland <address@hidden>
> > > ---
> > > hw/pci-host/uninorth.c | 28 ++++++++++++++++++----------
> > > 1 file changed, 18 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> > > index b081e3c153..5b8fc3aa16 100644
> > > --- a/hw/pci-host/uninorth.c
> > > +++ b/hw/pci-host/uninorth.c
> > > @@ -111,29 +111,39 @@ static const MemoryRegionOps unin_data_ops = {
> > >
> > > static void pci_unin_main_init(Object *obj)
> > > {
> > > +    UNINState *s = UNI_NORTH_PCI_HOST_BRIDGE(obj);
> > >     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> > >     PCIHostState *h = PCI_HOST_BRIDGE(obj);
> > >
> > >     /* Use values found on a real PowerMac */
> > >     /* Uninorth main bus */
> > >     memory_region_init_io(&h->conf_mem, OBJECT(h), 
> > > &pci_host_conf_le_ops,
> > > -                          obj, "pci-conf-idx", 0x1000);
> > > +                          obj, "unin-pci-conf-idx", 0x1000);
> > >     memory_region_init_io(&h->data_mem, OBJECT(h), &unin_data_ops, obj,
> > > -                          "pci-conf-data", 0x1000);
> > > +                          "unin-pci-conf-data", 0x1000);
> > > +
> > > +    memory_region_init(&s->pci_mmio, OBJECT(s), "unin-pci-mmio",
> > > +                       0x100000000ULL);
> > > +
> > >     sysbus_init_mmio(sbd, &h->conf_mem);
> > >     sysbus_init_mmio(sbd, &h->data_mem);
> > > }
> > >
> > > static void pci_u3_agp_init(Object *obj)
> > > {
> > > +    UNINState *s = U3_AGP_HOST_BRIDGE(obj);
> > >     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> > >     PCIHostState *h = PCI_HOST_BRIDGE(obj);
> > >
> > >     /* Uninorth U3 AGP bus */
> > >     memory_region_init_io(&h->conf_mem, OBJECT(h), 
> > > &pci_host_conf_le_ops,
> > > -                          obj, "pci-conf-idx", 0x1000);
> > > +                          obj, "unin-pci-conf-idx", 0x1000);
> > >     memory_region_init_io(&h->data_mem, OBJECT(h), &unin_data_ops, obj,
> > > -                          "pci-conf-data", 0x1000);
> > > +                          "unin-pci-conf-data", 0x1000);
> > > +
> > > +    memory_region_init(&s->pci_mmio, OBJECT(s), "unin-pci-mmio",
> >
> > The name of this function and the above comment both suggest this is 
> > an AGP bus so did you mean to rename these to unin-agp-* instead of 
> > unin-pci-*?
>
> Well this patchset purposely avoids doing anything with the U3 model 
> other than the required refactoring to move the wiring to board level as 
> really the entire U3 model needs some love - I can't even boot Linux 
> without this patchset (I suspect it's probably DT related).
>
> Having said that the wiring changes are such an improvement that I would 
> argue for applying this patchset if possible since any future fixes will 
> be considerably easier based upon it.

Looking in detail I think this naming is still correct: the U3 PCI bus 
address is currently 0xf0000000 which is actually the AGP bus rather 
than the PCI bus at 0xf2000000...


ATB,

Mark.