There are no root function properties exposed by the host
bridge, so using a 2-step QOM creation isn't really useful.

Simplify by creating the root function when the host bridge
is realized.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/designware.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 304eca1b5c..692e0731cd 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -707,6 +707,10 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
                        "pcie-bus-address-space");
     pci_setup_iommu(pci->bus, designware_pcie_host_set_iommu, s);
 
+    object_initialize_child(OBJECT(dev), "root", &s->root,
+                            TYPE_DESIGNWARE_PCIE_ROOT);
+    qdev_prop_set_int32(DEVICE(&s->root), "addr", PCI_DEVFN(0, 0));
+    qdev_prop_set_bit(DEVICE(&s->root), "multifunction", false);
     qdev_realize(DEVICE(&s->root), BUS(pci->bus), &error_fatal);
 }
 
@@ -736,22 +740,11 @@ static void designware_pcie_host_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_designware_pcie_host;
 }
 
-static void designware_pcie_host_init(Object *obj)
-{
-    DesignwarePCIEHost *s = DESIGNWARE_PCIE_HOST(obj);
-    DesignwarePCIERoot *root = &s->root;
-
-    object_initialize_child(obj, "root", root, TYPE_DESIGNWARE_PCIE_ROOT);
-    qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
-    qdev_prop_set_bit(DEVICE(root), "multifunction", false);
-}
-
 static const TypeInfo designware_pcie_types[] = {
     {
         .name           = TYPE_DESIGNWARE_PCIE_HOST,
         .parent         = TYPE_PCI_HOST_BRIDGE,
         .instance_size  = sizeof(DesignwarePCIEHost),
-        .instance_init  = designware_pcie_host_init,
         .class_init     = designware_pcie_host_class_init,
     }, {
         .name           = TYPE_DESIGNWARE_PCIE_ROOT,
-- 
2.41.0

Hi Phil,

On 10/12/23 9:18 AM, Philippe Mathieu-Daudé wrote:
> There are no root function properties exposed by the host
> bridge, so using a 2-step QOM creation isn't really useful.
> 
> Simplify by creating the root function when the host bridge
> is realized.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/pci-host/designware.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 304eca1b5c..692e0731cd 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -707,6 +707,10 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
>                          "pcie-bus-address-space");
>       pci_setup_iommu(pci->bus, designware_pcie_host_set_iommu, s);
>   
> +    object_initialize_child(OBJECT(dev), "root", &s->root,
> +                            TYPE_DESIGNWARE_PCIE_ROOT);
> +    qdev_prop_set_int32(DEVICE(&s->root), "addr", PCI_DEVFN(0, 0));
> +    qdev_prop_set_bit(DEVICE(&s->root), "multifunction", false);
>       qdev_realize(DEVICE(&s->root), BUS(pci->bus), &error_fatal);
>   }
>   
> @@ -736,22 +740,11 @@ static void designware_pcie_host_class_init(ObjectClass *klass, void *data)
>       dc->vmsd = &vmstate_designware_pcie_host;
>   }
>   
> -static void designware_pcie_host_init(Object *obj)
> -{
> -    DesignwarePCIEHost *s = DESIGNWARE_PCIE_HOST(obj);
> -    DesignwarePCIERoot *root = &s->root;
> -
> -    object_initialize_child(obj, "root", root, TYPE_DESIGNWARE_PCIE_ROOT);
> -    qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
> -    qdev_prop_set_bit(DEVICE(root), "multifunction", false);
> -}
> -
>   static const TypeInfo designware_pcie_types[] = {
>       {
>           .name           = TYPE_DESIGNWARE_PCIE_HOST,
>           .parent         = TYPE_PCI_HOST_BRIDGE,
>           .instance_size  = sizeof(DesignwarePCIEHost),
> -        .instance_init  = designware_pcie_host_init,
>           .class_init     = designware_pcie_host_class_init,
>       }, {
>           .name           = TYPE_DESIGNWARE_PCIE_ROOT,
> 

I could not find any mention in the docs recommending the use of
init/realize pair only when properties could be consumed. Anyways,
you agreed with Peter's comment (I agree too), so I understand this
patch will be drop since it doesn't affect the other patches in the
series.


Cheers,
Gustavo

On Thu, 12 Oct 2023 at 13:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> There are no root function properties exposed by the host
> bridge, so using a 2-step QOM creation isn't really useful.
>
> Simplify by creating the root function when the host bridge
> is realized.

It's not necessary, but on the other hand "init child objects
in init; realize child objects in realize" is the standard
way to do this. Does not moving this to the realize method
block anything in the rest of the patchset?

thanks
-- PMM

Hi Peter,

On 17/10/23 18:32, Peter Maydell wrote:
> On Thu, 12 Oct 2023 at 13:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> There are no root function properties exposed by the host
>> bridge, so using a 2-step QOM creation isn't really useful.
>>
>> Simplify by creating the root function when the host bridge
>> is realized.
> 
> It's not necessary, but on the other hand "init child objects
> in init; realize child objects in realize" is the standard
> way to do this. Does not moving this to the realize method
> block anything in the rest of the patchset?

I thought it was only recommended to use the init/realize
pair when properties could be consumed. Otherwise using
a single handler makes a model simpler.

No problem if I can drop this patch.