On Thu, 23 Nov 2023 at 14:38, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > The QOM API is lower level than the QDev one. When an instance is > QDev and setting the property can not fail (using &error_abort), > prefer qdev_prop_set_bit() over object_property_set_bool(). > > Mechanical transformation using the following coccinelle patch: > > @@ > expression o, p, v; > @@ > - object_property_set_bool(OBJECT(o), p, v, &error_abort) > + qdev_prop_set_bit(DEVICE(o), p, v) > @@@@ > - object_property_set_bool(o, p, v, &error_abort) > + qdev_prop_set_bit(DEVICE(o), p, v) > > manually adding the missing "hw/qdev-properties.h" header. > > In hw/arm/armsse.c we use the available 'cpudev' instead of 'cpuobj'. > > Suggested-by: Markus Armbruster <arm...@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> @@ -1287,8 +1288,7 @@ void arm_load_kernel(ARMCPU *cpu, MachineState *ms, > struct arm_boot_info *info) > * CPU. > */ > if (cs != first_cpu) { > - object_property_set_bool(cpuobj, "start-powered-off", true, > - &error_abort); > + qdev_prop_set_bit(DEVICE(cpuobj), "start-powered-off", true); > } > } > } This makes this code look a bit weird. Currently we have a loop which has an "Object *cpuobj" which it uses to set properties, in both cases using the object_property_* APIs. With this change, we do half the job using a QOM API and the other half using a qdev API. It would be good to follow up by converting the other property-set so we can have a local Device * instead of the Object *. > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index eace854335..6733652120 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -263,20 +263,13 @@ static void pc_init1(MachineState *machine, > size_t i; > > pci_dev = pci_new_multifunction(-1, pcms->south_bridge); > - object_property_set_bool(OBJECT(pci_dev), "has-usb", > - machine_usb(machine), &error_abort); > - object_property_set_bool(OBJECT(pci_dev), "has-acpi", > - x86_machine_is_acpi_enabled(x86ms), > - &error_abort); > - object_property_set_bool(OBJECT(pci_dev), "has-pic", false, > - &error_abort); > - object_property_set_bool(OBJECT(pci_dev), "has-pit", false, > - &error_abort); > - qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100); > - object_property_set_bool(OBJECT(pci_dev), "smm-enabled", > - x86_machine_is_smm_enabled(x86ms), > - &error_abort); > dev = DEVICE(pci_dev); > + qdev_prop_set_bit(dev, "has-usb", machine_usb(machine)); > + qdev_prop_set_bit(dev, "has-acpi", > x86_machine_is_acpi_enabled(x86ms)); > + qdev_prop_set_bit(dev, "has-pic", false); > + qdev_prop_set_bit(dev, "has-pit", false); > + qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100); This line also can just use "dev". > + qdev_prop_set_bit(dev, "smm-enabled", > x86_machine_is_smm_enabled(x86ms)); > for (i = 0; i < ISA_NUM_IRQS; i++) { > qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]); > } Otherwise Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM