| Message ID | 20230122170724.21868-2-shentey@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI controller cleanup | expand |
On Sun, 22 Jan 2023 18:07:18 +0100 Bernhard Beschow <shentey@gmail.com> wrote: > The QOM properties are accessed after the device models have been > realized. This means that the constants are redundant. Remove them. not sure it above means. Perhaps: subj: use existing fields type::foo instead of static memory. all object_property_add_*_ptr() needs is a pointer to memory storing so RO defaults where provided as pointers to static constants. Instead of keeping static constants around, drop them and initialize use existing type::foo field, which were set set later on to this constant later at ... (also see below: maybe squash ICH9_PMIO_GPE0_LEN cleanup here as well) or something along this lines. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/acpi/ich9.c | 5 ++--- > hw/acpi/piix4.c | 10 ++++------ > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index a93c470e9d..2050af67b9 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -433,7 +433,6 @@ static void ich9_pm_set_keep_pci_slot_hpc(Object *obj, bool value, Error **errp) > > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > { > - static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; you are loosing default value here, (true it's set somewhere later) suggest: pm->acpi_regs.gpe.len = ICH9_PMIO_GPE0_LEN; and in patch on top maybe cleanup other places that use ICH9_PMIO_GPE0_LEN with pm->acpi_regs.gpe.len > pm->acpi_memory_hotplug.is_enabled = true; > pm->cpu_hotplug_legacy = true; > pm->disable_s3 = 0; > @@ -448,8 +447,8 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > object_property_add(obj, ACPI_PM_PROP_GPE0_BLK, "uint32", > ich9_pm_get_gpe0_blk, > NULL, NULL, pm); > - object_property_add_uint32_ptr(obj, ACPI_PM_PROP_GPE0_BLK_LEN, > - &gpe0_len, OBJ_PROP_FLAG_READ); > + object_property_add_uint8_ptr(obj, ACPI_PM_PROP_GPE0_BLK_LEN, > + &pm->acpi_regs.gpe.len, OBJ_PROP_FLAG_READ); > object_property_add_bool(obj, "memory-hotplug-support", > ich9_pm_get_memory_hotplug_support, > ich9_pm_set_memory_hotplug_support); > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 0a81f1ad93..370b34eacf 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -421,18 +421,16 @@ static void piix4_pm_add_properties(PIIX4PMState *s) > { > static const uint8_t acpi_enable_cmd = ACPI_ENABLE; > static const uint8_t acpi_disable_cmd = ACPI_DISABLE; > - static const uint32_t gpe0_blk = GPE_BASE; > - static const uint32_t gpe0_blk_len = GPE_LEN; ditto also maybe split on 2 patches 1 for ich9 another for piix4 > static const uint16_t sci_int = 9; > > object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_ENABLE_CMD, > &acpi_enable_cmd, OBJ_PROP_FLAG_READ); > object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_DISABLE_CMD, > &acpi_disable_cmd, OBJ_PROP_FLAG_READ); > - object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK, > - &gpe0_blk, OBJ_PROP_FLAG_READ); > - object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN, > - &gpe0_blk_len, OBJ_PROP_FLAG_READ); > + object_property_add_uint64_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK, > + &s->io_gpe.addr, OBJ_PROP_FLAG_READ); > + object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN, > + &s->ar.gpe.len, OBJ_PROP_FLAG_READ); > object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT, > &sci_int, OBJ_PROP_FLAG_READ); > object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index a93c470e9d..2050af67b9 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -433,7 +433,6 @@ static void ich9_pm_set_keep_pci_slot_hpc(Object *obj, bool value, Error **errp) void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) { - static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; pm->acpi_memory_hotplug.is_enabled = true; pm->cpu_hotplug_legacy = true; pm->disable_s3 = 0; @@ -448,8 +447,8 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) object_property_add(obj, ACPI_PM_PROP_GPE0_BLK, "uint32", ich9_pm_get_gpe0_blk, NULL, NULL, pm); - object_property_add_uint32_ptr(obj, ACPI_PM_PROP_GPE0_BLK_LEN, - &gpe0_len, OBJ_PROP_FLAG_READ); + object_property_add_uint8_ptr(obj, ACPI_PM_PROP_GPE0_BLK_LEN, + &pm->acpi_regs.gpe.len, OBJ_PROP_FLAG_READ); object_property_add_bool(obj, "memory-hotplug-support", ich9_pm_get_memory_hotplug_support, ich9_pm_set_memory_hotplug_support); diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 0a81f1ad93..370b34eacf 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -421,18 +421,16 @@ static void piix4_pm_add_properties(PIIX4PMState *s) { static const uint8_t acpi_enable_cmd = ACPI_ENABLE; static const uint8_t acpi_disable_cmd = ACPI_DISABLE; - static const uint32_t gpe0_blk = GPE_BASE; - static const uint32_t gpe0_blk_len = GPE_LEN; static const uint16_t sci_int = 9; object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_ENABLE_CMD, &acpi_enable_cmd, OBJ_PROP_FLAG_READ); object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_DISABLE_CMD, &acpi_disable_cmd, OBJ_PROP_FLAG_READ); - object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK, - &gpe0_blk, OBJ_PROP_FLAG_READ); - object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN, - &gpe0_blk_len, OBJ_PROP_FLAG_READ); + object_property_add_uint64_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK, + &s->io_gpe.addr, OBJ_PROP_FLAG_READ); + object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN, + &s->ar.gpe.len, OBJ_PROP_FLAG_READ); object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT, &sci_int, OBJ_PROP_FLAG_READ); object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
The QOM properties are accessed after the device models have been realized. This means that the constants are redundant. Remove them. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/acpi/ich9.c | 5 ++--- hw/acpi/piix4.c | 10 ++++------ 2 files changed, 6 insertions(+), 9 deletions(-)