Re: [PATCH v8 10/10] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.
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
Reviewed-by: Igor Mammedov
---
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!
Re: [PATCH v8 10/10] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.
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
> >> Reviewed-by: Igor Mammedov
> >> ---
> >> 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
Re: [PATCH v8 10/10] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.
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
>> Reviewed-by: Igor Mammedov
>> ---
>> 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.
Re: [PATCH v8 10/10] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.
Hi Gerd,
On 6/11/20 9:29 AM, Gerd Hoffmann wrote:
> Seems to be unused.
>
> Signed-off-by: Gerd Hoffmann
> Reviewed-by: Igor Mammedov
> ---
> 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
[PATCH v8 10/10] acpi: q35: drop _SB.PCI0.ISA.LPCD opregion.
Seems to be unused.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Igor Mammedov
---
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);
}
--
2.18.4
