qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _H


From: Guoheyi
Subject: Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID
Date: Wed, 15 Jan 2020 09:25:15 +0800
User-agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2


在 2020/1/13 23:26, Andrew Jones 写道:
On Mon, Jan 13, 2020 at 09:57:55PM +0800, Guoheyi wrote:
在 2020/1/13 20:08, Igor Mammedov 写道:
On Thu, 19 Dec 2019 14:47:59 +0800
Heyi Guo <address@hidden> wrote:

According to ACPI spec, _ADR should be used for device which is on a
bus that has a standard enumeration algorithm. It does not make sense
to have a _ADR object for devices which already have _HID and will be
enumerated by OSPM.

Signed-off-by: Heyi Guo <address@hidden>
Are you sure it's does not make sense?
Have you checked commit f264d51d8, that added _ADR?
I searched in SPCR spec and ACPI spec, but didn't find such requirement for
serial port device description.

Hi Andrew,

Could you help to explain the reason?
tl;dr: It was a mistake and I agree with removing _ADR from SPCR.

The long version is that ACPI support for ARM took a long time to get
merged upstream. In the meantime Linaro and Red Hat had ACPI support
in their downstream trees. The initial, never upstreamed implementation
of Linux SPCR support used _ADR to find the console UART (probably
because some vendor hacked their ACPI tables that way). I made the
mistake of using the out-of-tree Linux kernel code as my "specification".

Upstream kernels never had this problem and I don't think we need to
worry about any of those downstream kernels which did. They'd be five
years old by now anyway.

Thanks,
drew

Thanks for your confirmation, Andrew.

Hi Igor,

On arm64 physical machine, we didn't have _ADR for SPCR serial port device attached to system bus either, and we never saw any problem.

Thanks,

Heyi

Thanks,

Heyi



---
Cc: Shannon Zhao <address@hidden>
Cc: Peter Maydell <address@hidden>
Cc: "Michael S. Tsirkin" <address@hidden>
Cc: Igor Mammedov <address@hidden>
Cc: address@hidden
Cc: address@hidden
---
   hw/arm/virt-acpi-build.c          |   8 --------
   tests/data/acpi/virt/DSDT         | Bin 18449 -> 18426 bytes
   tests/data/acpi/virt/DSDT.memhp   | Bin 19786 -> 19763 bytes
   tests/data/acpi/virt/DSDT.numamem | Bin 18449 -> 18426 bytes
   4 files changed, 8 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9f4c7d1889..be752c0ad8 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -78,11 +78,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry 
*uart_memmap,
                                AML_EXCLUSIVE, &uart_irq, 1));
       aml_append(dev, aml_name_decl("_CRS", crs));
-    /* The _ADR entry is used to link this device to the UART described
-     * in the SPCR table, i.e. SPCR.base_address.address == _ADR.
-     */
-    aml_append(dev, aml_name_decl("_ADR", aml_int(uart_memmap->base)));
-
       aml_append(scope, dev);
   }
@@ -170,7 +165,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry 
*memmap,
       aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
       aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
       aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
-    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
       aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
       aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
       aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
@@ -334,7 +328,6 @@ static void acpi_dsdt_add_gpio(Aml *scope, const 
MemMapEntry *gpio_memmap,
   {
       Aml *dev = aml_device("GPO0");
       aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061")));
-    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
       aml_append(dev, aml_name_decl("_UID", aml_int(0)));
       Aml *crs = aml_resource_template();
@@ -364,7 +357,6 @@ static void acpi_dsdt_add_power_button(Aml *scope)
   {
       Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE);
       aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
-    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
       aml_append(dev, aml_name_decl("_UID", aml_int(0)));
       aml_append(scope, dev);
   }
diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT
index 
b5895cb22446860a0b9be3d32ec856feb388be4c..a759ff739a071d5fbf50519a6aea296e5e0f1e0c
 100644
GIT binary patch
delta 72
zcmbO@f$>*ABbQ6COUN&G1_q{66S<_BT5Bh&t1wzk^tIeLL4lL8ZSqD=gU!!5x$Pt+
c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>07w882mk;8

delta 94
zcmey>&p2@cBbQ6CONgKc0|V26iCof5J#`b+RhV2^Ci+-%al|{i1o1F1FmP^cRp4ao
tnY@hCfEg&X`7$S;oxFTNc#soEyoaX?Z-8HbfwO@#16Tu)4E1zj005fm7mWY_

diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp
index 
69ad844f65d047973a3e55198beecd45a35b8fce..6e5cc61977e4cd24f765fec0693f75a528c144c1
 100644
GIT binary patch
delta 72
zcmX>#i*fTTMlP3Nmk?uL1_q|eiCof5eHSLGt1wzk^tIeLL4lL8ZSqD=gU!!5U7RH)
c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>03)CjmjD0&

delta 94
zcmdlyi}BPfMlP3Nmk=*s1_q}3iCof5t(PXMt1!8;O!Tqj;)r*23F2X3VBp-?s=&$E
tGkF=O0W(l&^JPwVXL<R6@E|9Scn?n(-T=P<17`zg2CxPo8S3f6006qZ7#siq

diff --git a/tests/data/acpi/virt/DSDT.numamem 
b/tests/data/acpi/virt/DSDT.numamem
index 
b5895cb22446860a0b9be3d32ec856feb388be4c..a759ff739a071d5fbf50519a6aea296e5e0f1e0c
 100644
GIT binary patch
delta 72
zcmbO@f$>*ABbQ6COUN&G1_q{66S<_BT5Bh&t1wzk^tIeLL4lL8ZSqD=gU!!5x$Pt+
c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>07w882mk;8

delta 94
zcmey>&p2@cBbQ6CONgKc0|V26iCof5J#`b+RhV2^Ci+-%al|{i1o1F1FmP^cRp4ao
tnY@hCfEg&X`7$S;oxFTNc#soEyoaX?Z-8HbfwO@#16Tu)4E1zj005fm7mWY_

.


.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]