[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/3] hw/i386/acpi-build: Resolve redundant attribute
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH v2 2/3] hw/i386/acpi-build: Resolve redundant attribute |
Date: |
Mon, 31 Oct 2022 13:41:59 -0400 |
On Mon, Oct 31, 2022 at 01:45:29PM +0100, Igor Mammedov wrote:
> On Fri, 28 Oct 2022 12:34:18 +0200
> Bernhard Beschow <shentey@gmail.com> wrote:
>
> > The is_piix4 attribute is set once in one location and read once in
> > another. Doing both in one location allows for removing the attribute
> > altogether.
>
> we also test for piix4 in acpi_get_pm_info(),
> Perhaps we should move is_piix4 to AcpiPmInfo
> and reuse it in build_dsdt()?
> Also should use is_piix4 as argument to build_iqcr_method()
> to make its call places self documenting. But that's another patch.
Well I picked this up for pull req since i had to make
a decision before freeze. But sure, a cleanup like
this might even be acceptable before rc1 I think.
Bernhard?
>
> > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Message-Id: <20221026133110.91828-3-shentey@gmail.com>
> > ---
> > hw/i386/acpi-build.c | 20 ++++++--------------
> > 1 file changed, 6 insertions(+), 14 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 1ebf14b899..73d8a59737 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -112,7 +112,6 @@ typedef struct AcpiPmInfo {
> > } AcpiPmInfo;
> >
> > typedef struct AcpiMiscInfo {
> > - bool is_piix4;
> > bool has_hpet;
> > #ifdef CONFIG_TPM
> > TPMVersion tpm_version;
> > @@ -281,17 +280,6 @@ static void acpi_get_pm_info(MachineState *machine,
> > AcpiPmInfo *pm)
> >
> > static void acpi_get_misc_info(AcpiMiscInfo *info)
> > {
> > - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
> > - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
> > - assert(!!piix != !!lpc);
> > -
> > - if (piix) {
> > - info->is_piix4 = true;
> > - }
> > - if (lpc) {
> > - info->is_piix4 = false;
> > - }
>
> ack for this hunk
>
> > info->has_hpet = hpet_find();
> > #ifdef CONFIG_TPM
> > info->tpm_version = tpm_get_version(tpm_find());
> > @@ -1334,6 +1322,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > AcpiPmInfo *pm, AcpiMiscInfo *misc,
> > Range *pci_hole, Range *pci_hole64, MachineState *machine)
> > {
> > + Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
> > + Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
> > CrsRangeEntry *entry;
> > Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> > CrsRangeSet crs_range_set;
> > @@ -1354,11 +1344,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id,
> > .oem_table_id = x86ms->oem_table_id };
> >
> > + assert(!!piix != !!lpc);
> > +
> > acpi_table_begin(&table, table_data);
> > dsdt = init_aml_allocator();
> >
> > build_dbg_aml(dsdt);
> > - if (misc->is_piix4) {
> > + if (piix) {
> > sb_scope = aml_scope("_SB");
> > dev = aml_device("PCI0");
> > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> > @@ -1371,7 +1363,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base);
> > }
> > build_piix4_pci0_int(dsdt);
> > - } else {
> > + } else if (lpc) {
> > sb_scope = aml_scope("_SB");
> > dev = aml_device("PCI0");
> > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
- Re: [PATCH v2 3/3] hw/i386/acpi-build: Resolve north rather than south bridges, (continued)
Re: [PATCH v2 3/3] hw/i386/acpi-build: Resolve north rather than south bridges, Ani Sinha, 2022/10/30
Re: [PATCH v2 3/3] hw/i386/acpi-build: Resolve north rather than south bridges, Igor Mammedov, 2022/10/31
[PATCH v2 1/3] hw/i386/acpi-build: Remove unused struct, Bernhard Beschow, 2022/10/28
[PATCH v2 2/3] hw/i386/acpi-build: Resolve redundant attribute, Bernhard Beschow, 2022/10/28