Re: [PATCH v8 10/10] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.

[Philippe Mathieu-Daudé]

On 6/11/20 10:27 AM, Philippe Mathieu-Daudé wrote:
> Hi Gerd,
> 
> On 6/11/20 9:29 AM, Gerd Hoffmann wrote:
>> Seems to be unused.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>>  hw/i386/acpi-build.c | 11 -----------
>>  1 file changed, 11 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 02cf4199c2e9..d93ea40c58b9 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -1296,7 +1296,6 @@ static void build_q35_isa_bridge(Aml *table)
>>  {
>>      Aml *dev;
>>      Aml *scope;
>> -    Aml *field;
>>  
>>      scope =  aml_scope("_SB.PCI0");
>>      dev = aml_device("ISA");
>> @@ -1306,16 +1305,6 @@ static void build_q35_isa_bridge(Aml *table)
>>      aml_append(dev, aml_operation_region("PIRQ", AML_PCI_CONFIG,
>>                                           aml_int(0x60), 0x0C));
>>  
>> -    aml_append(dev, aml_operation_region("LPCD", AML_PCI_CONFIG,
>> -                                         aml_int(0x80), 0x02));
>> -    field = aml_field("LPCD", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
>> -    aml_append(field, aml_named_field("COMA", 3));
>> -    aml_append(field, aml_reserved_field(1));
>> -    aml_append(field, aml_named_field("COMB", 3));
>> -    aml_append(field, aml_reserved_field(1));
>> -    aml_append(field, aml_named_field("LPTD", 2));
>> -    aml_append(dev, field);
>> -
>>      aml_append(scope, dev);
>>      aml_append(table, scope);
>>  }
>>
> 
> I'm a bit confused, isn't it use to describe these
> devices?
> 
> (qemu) info qtree
> bus: main-system-bus
>   type System
>   dev: q35-pcihost, id ""
>     bus: pcie.0
>       type PCIE
>       dev: ICH9-LPC, id ""
>         gpio-out "gsi" 24
>         class ISA bridge, addr 00:1f.0, pci id 8086:2918 (sub 1af4:1100)
>         bus: isa.0
>           type ISA
>           dev: port92, id ""
>             gpio-out "a20" 1
>           dev: vmmouse, id ""
>           dev: vmport, id ""
>           dev: isa-parallel, id ""
>             index = 0 (0x0)
>             iobase = 888 (0x378)
>             irq = 7 (0x7)
>             chardev = "parallel0"
>             isa irq 7
>           dev: isa-serial, id ""
>             index = 1 (0x1)
>             iobase = 760 (0x2f8)
>             irq = 3 (0x3)
>             chardev = "serial1"
>             wakeup = 0 (0x0)
>             isa irq 3
>           dev: isa-serial, id ""
>             index = 0 (0x0)
>             iobase = 1016 (0x3f8)
>             irq = 4 (0x4)
>             chardev = "serial0"
>             wakeup = 0 (0x0)
>             isa irq 4
> 

Ah, this patch complements the previous "acpi: drop serial/parallel
enable bits from dsdt", right? Maybe better to include this change
with the build_q35_isa_bridge() part. Like in a single patch:
"acpi: q35: drop lpc/serial/parallel enable bits from dsdt"

Then keep the PIIX part of the patches.