| Message ID | 20231012121857.31873-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/pci-host/designware: QOM shuffling (Host bridge <-> Root function) | expand |
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.
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
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,
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(-)