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

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jun 11, 2020 at 10:31:01AM +0200, Philippe Mathieu-Daudé wrote:
> 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.

Don't see why really.

-- 
MST