Marcel Apfelbaum <marce...@redhat.com> writes: > On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote: >> From: Markus Armbruster <arm...@redhat.com> >> >> Many PCI host bridges consist of a sysbus device and a PCI device. >> You need both for the thing to work. Arguably, these bridges should >> be modelled as a single, composite devices instead of pairs of >> seemingly independent devices you can only use together, but we're not >> there, yet. >> >> Since the sysbus part can't be instantiated with device_add, yet, >> permitting it with the PCI part is useless. We shouldn't offer >> useless options to the user, so let's set >> cannot_instantiate_with_device_add_yet for them. >> >> It's already set for Bonito, grackle, i440FX, and raven. Document >> why. >> >> Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch, >> pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp, >> uni-north-internal-pci, uni-north-pci, and versatile_pci_host. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> hw/mips/gt64xxx_pci.c | 6 ++++++ >> hw/pci-bridge/dec.c | 6 ++++++ >> hw/pci-host/apb.c | 6 ++++++ >> hw/pci-host/bonito.c | 6 +++++- >> hw/pci-host/grackle.c | 6 +++++- >> hw/pci-host/piix.c | 6 +++++- >> hw/pci-host/ppce500.c | 5 +++++ >> hw/pci-host/prep.c | 6 +++++- >> hw/pci-host/q35.c | 5 +++++ >> hw/pci-host/uninorth.c | 24 ++++++++++++++++++++++++ >> hw/pci-host/versatile.c | 6 ++++++ >> hw/ppc/ppc4xx_pci.c | 5 +++++ >> hw/sh4/sh_pci.c | 6 ++++++ >> 13 files changed, 89 insertions(+), 4 deletions(-) >> >> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c >> index 3da2e67..6398514 100644 >> --- a/hw/mips/gt64xxx_pci.c >> +++ b/hw/mips/gt64xxx_pci.c >> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d) >> static void gt64120_pci_class_init(ObjectClass *klass, void *data) >> { >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> + DeviceClass *dc = DEVICE_CLASS(klass); >> >> k->init = gt64120_pci_init; >> k->vendor_id = PCI_VENDOR_ID_MARVELL; >> k->device_id = PCI_DEVICE_ID_MARVELL_GT6412X; >> k->revision = 0x10; >> k->class_id = PCI_CLASS_BRIDGE_HOST; >> + /* >> + * PCI-facing part of the host bridge, not usable without the >> + * host-facing part, which can't be device_add'ed, yet. >> + */ >> + dc->cannot_instantiate_with_device_add_yet = true; > I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
Correct. > What do you think about a different approach: check class_id > in parent class init func and set the flag according to it? > It corresponds to your idea of changing only sysbus base class. > Here we don't have a "natural" base class, but we can use class_id. > What do you think? My understanding of QOM is rather limited, so take the following with due skepticism. I'm afraid the parent's class_init() runs before the child's, and therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's class_init(). To factor common initialization code, I figure I'd have to splice in an abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host bridge types such as this one. Might make sense, but it's a bit more than I bargained for in this series :)