Philippe Mathieu-Daudé <phi...@redhat.com> writes: > Hi Markus, Peter. > > On 6/10/20 7:32 AM, Markus Armbruster wrote: >> All remaining conversions to qdev_realize() are for bus-less devices. >> Coccinelle script: >> >> // only correct for bus-less @dev! >> >> @@ >> expression errp; >> expression dev; >> @@ >> - qdev_init_nofail(dev); >> + qdev_realize(dev, NULL, &error_fatal); >> >> @ depends on !(file in "hw/core/qdev.c") && !(file in "hw/core/bus.c")@ >> expression errp; >> expression dev; >> symbol true; >> @@ >> - object_property_set_bool(OBJECT(dev), true, "realized", errp); >> + qdev_realize(DEVICE(dev), NULL, errp); >> >> @ depends on !(file in "hw/core/qdev.c") && !(file in "hw/core/bus.c")@ >> expression errp; >> expression dev; >> symbol true; >> @@ >> - object_property_set_bool(dev, true, "realized", errp); >> + qdev_realize(DEVICE(dev), NULL, errp); > > Finally. Now my ealier suggestion is easier to explain: > Rename qdev_realize() -> sysbus_realize(), extracting the qdev_realize() > part. qdev_realize() doesn't take a Bus argument anymore. > Left for later.
I'm still confused. Cases: * Devices that plug into a bus: use qdev_realize() passing that bus. If there is a bus-specific wrapper, use that, for legibility. In particular, use sysbus_realize() for sysbus devices plugging into the main system bus. * Devices that don't plug into a bus: use qdev_realize() passing a null bus. What would you like me to improve here? > >> >> Note that Coccinelle chokes on ARMSSE typedef vs. macro in >> hw/arm/armsse.c. Worked around by temporarily renaming the macro for >> the spatch run. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> Acked-by: Alistair Francis <alistair.fran...@wdc.com> >> Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> >> --- >> hw/arm/allwinner-a10.c | 2 +- >> hw/arm/allwinner-h3.c | 2 +- >> hw/arm/armsse.c | 20 ++++++--------- >> hw/arm/armv7m.c | 2 +- >> hw/arm/aspeed.c | 3 +-- >> hw/arm/aspeed_ast2600.c | 2 +- >> hw/arm/aspeed_soc.c | 2 +- >> hw/arm/bcm2836.c | 3 +-- >> hw/arm/cubieboard.c | 2 +- >> hw/arm/digic.c | 2 +- >> hw/arm/digic_boards.c | 2 +- >> hw/arm/exynos4210.c | 4 +-- >> hw/arm/fsl-imx25.c | 2 +- >> hw/arm/fsl-imx31.c | 2 +- >> hw/arm/fsl-imx6.c | 2 +- >> hw/arm/fsl-imx6ul.c | 3 +-- >> hw/arm/fsl-imx7.c | 2 +- >> hw/arm/highbank.c | 2 +- >> hw/arm/imx25_pdk.c | 2 +- >> hw/arm/integratorcp.c | 2 +- >> hw/arm/kzm.c | 2 +- >> hw/arm/mcimx6ul-evk.c | 2 +- >> hw/arm/mcimx7d-sabre.c | 2 +- >> hw/arm/mps2-tz.c | 9 +++---- >> hw/arm/mps2.c | 7 +++--- >> hw/arm/musca.c | 6 ++--- >> hw/arm/orangepi.c | 2 +- >> hw/arm/raspi.c | 2 +- >> hw/arm/realview.c | 2 +- >> hw/arm/sabrelite.c | 2 +- >> hw/arm/sbsa-ref.c | 2 +- >> hw/arm/stm32f205_soc.c | 2 +- >> hw/arm/stm32f405_soc.c | 2 +- >> hw/arm/versatilepb.c | 2 +- >> hw/arm/vexpress.c | 2 +- >> hw/arm/virt.c | 2 +- >> hw/arm/xilinx_zynq.c | 2 +- >> hw/arm/xlnx-versal.c | 2 +- >> hw/arm/xlnx-zcu102.c | 2 +- >> hw/arm/xlnx-zynqmp.c | 10 +++----- > > Peter you might want to skim at the changes (other > ARM devices out of hw/arm/ involved) but to resume > basically these types are not SysBusDev: > > - cpu > - soc / container > - or-gate / irq-splitter > > I reviewed all of them. > > Next is for Markus. > >> hw/char/serial-isa.c | 2 +- >> hw/char/serial-pci-multi.c | 2 +- >> hw/char/serial-pci.c | 2 +- >> hw/char/serial.c | 4 +-- > > I need to review again hw/char/serial-isa.c, it is > not clear why it is a container and not a SysBusDevice. TYPE_SERIAL is a bus-less TYPE_DEVICE. TYPE_ISA_SERIAL is its adapter for the ISA bus. It contains one TYPE_SERIAL child. TYPE_SERIAL_MM is its adapter for the sysbus pseudo-bus. It contains one TYPE_SERIAL child. TYPE_PCI_SERIAL, "pci-serial-2x", "pci-serial-4x" are adapters for the PCI bus. They contain one, two and four TYPE_SERIAL respectively. Exemplary use of QOM, I think. >> hw/ide/microdrive.c | 3 ++- > > I never had to look at the PCMCIA devices, they seem > an unfinished attempt to plug a the devices on a bus. > Maybe it is finished, but the code is not clear (and > not documented). I need more time to review. > >> hw/intc/pnv_xive.c | 4 +-- >> hw/intc/spapr_xive.c | 4 +-- >> hw/intc/xics.c | 2 +- >> hw/intc/xive.c | 2 +- >> hw/pci-host/pnv_phb3.c | 6 ++--- >> hw/pci-host/pnv_phb4.c | 2 +- >> hw/pci-host/pnv_phb4_pec.c | 2 +- >> hw/pci-host/prep.c | 3 +-- >> hw/ppc/pnv.c | 32 ++++++++++-------------- >> hw/ppc/pnv_bmc.c | 2 +- >> hw/ppc/pnv_core.c | 2 +- >> hw/ppc/pnv_psi.c | 4 +-- >> hw/ppc/spapr.c | 5 ++-- >> hw/ppc/spapr_cpu_core.c | 2 +- >> hw/ppc/spapr_drc.c | 2 +- >> hw/ppc/spapr_iommu.c | 2 +- >> hw/ppc/spapr_irq.c | 2 +- > > Wow, lot of QOM code for PPC hardware. Not all clear > yet, in particular the pci-host devices. Apparently > a LPC bus in the middle. Need a bit more time. > >> hw/s390x/s390-skeys.c | 2 +- >> hw/s390x/s390-stattrib.c | 2 +- >> hw/s390x/s390-virtio-ccw.c | 4 +-- >> hw/s390x/sclp.c | 2 +- >> hw/s390x/tod.c | 2 +- > > Eh, odd s390x stuff. No clue, I might review it too. > >> target/i386/cpu.c | 3 +-- > > What is this APIC stuff doing burried with TCG?... > > All the rest that is elided and not commented is reviewed. > > When do you plan to send a pullreq? I want to get this wrapped as quickly as possible; rebasing has been quite a time sink. But not at the price of rushed review.