diff mbox series

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

Message ID 20200611072919.16638-11-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series acpi: i386 tweaks | expand

Commit Message

Gerd Hoffmann June 11, 2020, 7:29 a.m. UTC 
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(-)

Comments

Philippe Mathieu-Daudé June 11, 2020, 8:27 a.m. UTC | #1 
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
Philippe Mathieu-Daudé June 11, 2020, 8:31 a.m. UTC | #2 
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.
Michael S. Tsirkin June 11, 2020, 9:12 a.m. UTC | #3 
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.
Philippe Mathieu-Daudé June 11, 2020, 9:25 a.m. UTC | #4 
On 6/11/20 11:12 AM, Michael S. Tsirkin wrote:
> 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.

OK, no problem on my side. Series fully reviewed!
diff mbox series

Patch

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);
 }