Re: [Qemu-devel] [PATCH 08/19] uninorth: introduce temporary pic_irqs device property

[David Gibson]

On Tue, Mar 06, 2018 at 08:30:52PM +0000, Mark Cave-Ayland wrote:
> This is in preparation for moving the PCI bus wiring inside the uninorth
> host bridge devices. In the future it will be possible to remove this once the
> PICs have been switched to use qdev GPIOs.
> 
> Signed-off-by: Mark Cave-Ayland <address@hidden>

It's not worth changing now, but for future reference.  For an interim
hack like this, it's probably simpler to just make it a field in the
object and have the "callers" just reach in and set it, rather than
making it a formal property.

Yes, that's an abstraction violation, but as a temporary measure to a
better place I think it's acceptable, and avoids a little bit of extra
churn creating then removing the property.

> ---
>  hw/pci-host/uninorth.c         | 25 +++++++++++++++++++++++--
>  include/hw/pci-host/uninorth.h |  1 +
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index 5b8fc3aa16..fc59698f06 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -188,6 +188,7 @@ UNINState *pci_pmac_init(qemu_irq *pic,
>      /* Use values found on a real PowerMac */
>      /* Uninorth main bus */
>      dev = qdev_create(NULL, TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
> +    qdev_prop_set_ptr(dev, "pic-irqs", pic);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      h = PCI_HOST_BRIDGE(s);
> @@ -199,7 +200,7 @@ UNINState *pci_pmac_init(qemu_irq *pic,
>  
>      h->bus = pci_register_root_bus(dev, NULL,
>                                     pci_unin_set_irq, pci_unin_map_irq,
> -                                   pic,
> +                                   d->pic_irqs,
>                                     &d->pci_mmio,
>                                     address_space_io,
>                                     PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> @@ -220,6 +221,7 @@ UNINState *pci_pmac_init(qemu_irq *pic,
>      /* Uninorth AGP bus */
>      pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north-agp");
>      dev = qdev_create(NULL, TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
> +    qdev_prop_set_ptr(dev, "pic-irqs", pic);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      sysbus_mmio_map(s, 0, 0xf0800000);
> @@ -251,6 +253,7 @@ UNINState *pci_pmac_u3_init(qemu_irq *pic,
>  
>      /* Uninorth AGP bus */
>      dev = qdev_create(NULL, TYPE_U3_AGP_HOST_BRIDGE);
> +    qdev_prop_set_ptr(dev, "pic-irqs", pic);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      h = PCI_HOST_BRIDGE(dev);
> @@ -263,7 +266,7 @@ UNINState *pci_pmac_u3_init(qemu_irq *pic,
>  
>      h->bus = pci_register_root_bus(dev, NULL,
>                                     pci_unin_set_irq, pci_unin_map_irq,
> -                                   pic,
> +                                   d->pic_irqs,
>                                     &d->pci_mmio,
>                                     address_space_io,
>                                     PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> @@ -436,10 +439,16 @@ static const TypeInfo unin_internal_pci_host_info = {
>      },
>  };
>  
> +static Property pci_unin_main_properties[] = {
> +    DEFINE_PROP_PTR("pic-irqs", UNINState, pic_irqs),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void pci_unin_main_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> +    dc->props = pci_unin_main_properties;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  }
>  
> @@ -451,10 +460,16 @@ static const TypeInfo pci_unin_main_info = {
>      .class_init    = pci_unin_main_class_init,
>  };
>  
> +static Property pci_u3_agp_properties[] = {
> +    DEFINE_PROP_PTR("pic-irqs", UNINState, pic_irqs),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void pci_u3_agp_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> +    dc->props = pci_u3_agp_properties;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  }
>  
> @@ -466,10 +481,16 @@ static const TypeInfo pci_u3_agp_info = {
>      .class_init    = pci_u3_agp_class_init,
>  };
>  
> +static Property pci_unin_agp_class_properties[] = {
> +    DEFINE_PROP_PTR("pic-irqs", UNINState, pic_irqs),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void pci_unin_agp_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> +    dc->props = pci_unin_agp_class_properties;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  }
>  
> diff --git a/include/hw/pci-host/uninorth.h b/include/hw/pci-host/uninorth.h
> index 9364c14bdd..12979712d0 100644
> --- a/include/hw/pci-host/uninorth.h
> +++ b/include/hw/pci-host/uninorth.h
> @@ -42,6 +42,7 @@
>  typedef struct UNINState {
>      PCIHostState parent_obj;
>  
> +    void *pic_irqs;
>      MemoryRegion pci_mmio;
>      MemoryRegion pci_hole;
>  } UNINState;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature