From: Aryeh Chen <aryeh.chen@...> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4057According to ASL Coding Guidelines - Device Identifiers "A Device should contain either an _ADR or a _HID object, never both." , so remove _ADR due to _HID exist. Signed-off-by: Aryeh Chen <aryeh.chen@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> --- Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl | 1 - 1 file changed, 1 deletion(-) diff --git a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl b/Platf= orm/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl index 0d94472450..4efb8709ac 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl +++ b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl @@ -27,7 +27,6 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 h= ost hierarchy=0D Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't u= nderstand the new HID=0D Name(_SEG, 0)=0D - Name(_ADR, 0x00000000)=0D Method(^BN00, 0){ return(0x0000) } // Returns default Bus number fo= r Peer PCI busses. Name can be overriden with control method placed directl= y under Device scope=0D Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Ro= ot PCI Bus=0D Name(_UID, 0x0000) // Unique Bus ID, optional=0D --=20 2.26.2.windows.1
|
|
On Mon, 12 Sep 2022 13:17:45 +0800 aryeh.chen@... wrote: From: Aryeh Chen <aryeh.chen@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4057
According to ASL Coding Guidelines - Device Identifiers "A Device should contain either an _ADR or a _HID object, never both." , so remove _ADR due to _HID exist. I'm curious where exactly in ACPI spec it's said... Signed-off-by: Aryeh Chen <aryeh.chen@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> --- Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl index 0d94472450..4efb8709ac 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl +++ b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl @@ -27,7 +27,6 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID Name(_SEG, 0) - Name(_ADR, 0x00000000) Spec also says v6.3 6.1.1 _ADR (Address) An _ADR object must be used when specifying the address of any device on a bus that has a standard enumeration algorithm Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Root PCI Bus Name(_UID, 0x0000) // Unique Bus ID, optional
|
|
Hi Igor,
Section 6.1 of the ACPI specification describes this (Last line of the paragraph below states what Aryeh was saying):
For any device that is on a non-enumerable type of bus (for example, an ISA bus), OSPM enumerates the devices' identifier(s) and the ACPI system firmware must supply an _HID object (plus one or more optional objects such as _CID, _CLS, _HRV, _SUB) for each device to enable OSPM to do that. For devices on an enumerable type of bus, such as a PCI bus, the ACPI system must identify which device on the enumerable bus is identified by a particular address; the ACPI system firmware must supply an _ADR object for each device to enable this. A device object must contain either an _HID object or an _ADR object, but must not contain both.
Thank you, Ankit
toggle quoted message
Show quoted text
-----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Igor Mammedov Sent: Monday, September 12, 2022 1:33 AM To: Chen, Aryeh <aryeh.chen@...> Cc: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Sinha, Ankit <ankit.sinha@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: Re: [edk2-devel] [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl
On Mon, 12 Sep 2022 13:17:45 +0800 aryeh.chen@... wrote:
From: Aryeh Chen <aryeh.chen@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4057
According to ASL Coding Guidelines - Device Identifiers "A Device should contain either an _ADR or a _HID object, never both." , so remove _ADR due to _HID exist. I'm curious where exactly in ACPI spec it's said...
Signed-off-by: Aryeh Chen <aryeh.chen@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> --- Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl index 0d94472450..4efb8709ac 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl +++ b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl @@ -27,7 +27,6 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy
Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID
Name(_SEG, 0) - Name(_ADR, 0x00000000) Spec also says v6.3 6.1.1 _ADR (Address) An _ADR object must be used when specifying the address of any device on a bus that has a standard enumeration algorithm
Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope
Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Root PCI Bus
Name(_UID, 0x0000) // Unique Bus ID, optional
|
|
Hi Aryeh,
Can you please post the test results for this change. It would be good to note that there is no change in behavior in the OS.
Thank you, Ankit
toggle quoted message
Show quoted text
-----Original Message----- From: Chen, Aryeh <aryeh.chen@...> Sent: Sunday, September 11, 2022 10:18 PM To: devel@edk2.groups.io Cc: Chen, Aryeh <aryeh.chen@...>; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Sinha, Ankit <ankit.sinha@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl
From: Aryeh Chen <aryeh.chen@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4057
According to ASL Coding Guidelines - Device Identifiers "A Device should contain either an _ADR or a _HID object, never both." , so remove _ADR due to _HID exist.
Signed-off-by: Aryeh Chen <aryeh.chen@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> --- Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl index 0d94472450..4efb8709ac 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl +++ b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl @@ -27,7 +27,6 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID Name(_SEG, 0)- Name(_ADR, 0x00000000) Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Root PCI Bus Name(_UID, 0x0000) // Unique Bus ID, optional-- 2.26.2.windows.1
|
|
Hi Ankit,
Add a change to remove Name(_ADR, 0x00000000) from edk2-platforms\Platform\Intel\MinPlatformPkg\Acpi\MinDsdt\MinDsdt.asl It can boot to OS success on Win10 21H2 and Ubuntu 22.4.0 on TGLU openboard bios.
=== Acpidump === Signature "DSDT" Length 0x000000FD (253) Revision 0x01 (1) Checksum 0x33 (51) OEM ID "INTEL " OEM Table ID "MIN " OEM Revision 0x00000000 (0) Creator ID "INTL" Creator Revision 0x20210930 (539035952) DefinitionBlock ("DSDT.AML", "DSDT", 0x01, "INTEL ", "MIN ", 0x00000000) { Scope(_SB) { Device(PCI0) { Name(_HID, EISAID("PNP0A08")) Name(_CID, EISAID("PNP0A03")) Name(_SEG, Zero) Method(^BN00, 0, NotSerialized) { Return(Zero) }
=== Original mindsdt.asl === Scope(\_SB) { //--------------------------------------------------------------------------- // Begin PCI tree object scope //--------------------------------------------------------------------------- Device(PCI0) { // PCI Bridge "Host Bridge" Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID Name(_SEG, 0) Name(_ADR, 0x00000000) Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Root PCI Bus
Thanks, Aryeh
toggle quoted message
Show quoted text
-----Original Message----- From: Sinha, Ankit <ankit.sinha@...> Sent: Tuesday, September 13, 2022 4:54 AM To: Chen, Aryeh <aryeh.chen@...>; devel@edk2.groups.io Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: RE: [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl Hi Aryeh, Can you please post the test results for this change. It would be good to note that there is no change in behavior in the OS. Thank you, Ankit -----Original Message----- From: Chen, Aryeh <aryeh.chen@...> Sent: Sunday, September 11, 2022 10:18 PM To: devel@edk2.groups.io Cc: Chen, Aryeh <aryeh.chen@...>; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Sinha, Ankit <ankit.sinha@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl
From: Aryeh Chen <aryeh.chen@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4057
According to ASL Coding Guidelines - Device Identifiers "A Device should contain either an _ADR or a _HID object, never both." , so remove _ADR due to _HID exist.
Signed-off-by: Aryeh Chen <aryeh.chen@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> --- Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl index 0d94472450..4efb8709ac 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl +++ b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl @@ -27,7 +27,6 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID Name(_SEG, 0)- Name(_ADR, 0x00000000) Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Root PCI Bus Name(_UID, 0x0000) // Unique Bus ID, optional-- 2.26.2.windows.1
|
|
Hi Aryeh,
Thank you for testing.
Reviewed-by: Ankit Sinha <ankit.sinha@...>
toggle quoted message
Show quoted text
-----Original Message----- From: Chen, Aryeh <aryeh.chen@...> Sent: Wednesday, September 21, 2022 12:34 AM To: Sinha, Ankit <ankit.sinha@...>; devel@edk2.groups.io Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: RE: [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl
Hi Ankit,
Add a change to remove Name(_ADR, 0x00000000) from edk2- platforms\Platform\Intel\MinPlatformPkg\Acpi\MinDsdt\MinDsdt.asl It can boot to OS success on Win10 21H2 and Ubuntu 22.4.0 on TGLU openboard bios.
=== Acpidump === Signature "DSDT" Length 0x000000FD (253) Revision 0x01 (1) Checksum 0x33 (51) OEM ID "INTEL " OEM Table ID "MIN " OEM Revision 0x00000000 (0) Creator ID "INTL" Creator Revision 0x20210930 (539035952) DefinitionBlock ("DSDT.AML", "DSDT", 0x01, "INTEL ", "MIN ", 0x00000000) { Scope(_SB) { Device(PCI0) { Name(_HID, EISAID("PNP0A08")) Name(_CID, EISAID("PNP0A03")) Name(_SEG, Zero) Method(^BN00, 0, NotSerialized) { Return(Zero) }
=== Original mindsdt.asl === Scope(\_SB) { //--------------------------------------------------------------------------- // Begin PCI tree object scope //--------------------------------------------------------------------------- Device(PCI0) { // PCI Bridge "Host Bridge" Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID Name(_SEG, 0) Name(_ADR, 0x00000000) Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Root PCI Bus
Thanks, Aryeh
-----Original Message----- From: Sinha, Ankit <ankit.sinha@...> Sent: Tuesday, September 13, 2022 4:54 AM To: Chen, Aryeh <aryeh.chen@...>; devel@edk2.groups.io Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: RE: [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl
Hi Aryeh,
Can you please post the test results for this change. It would be good to note that there is no change in behavior in the OS.
Thank you, Ankit
-----Original Message----- From: Chen, Aryeh <aryeh.chen@...> Sent: Sunday, September 11, 2022 10:18 PM To: devel@edk2.groups.io Cc: Chen, Aryeh <aryeh.chen@...>; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Sinha, Ankit <ankit.sinha@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl
From: Aryeh Chen <aryeh.chen@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4057
According to ASL Coding Guidelines - Device Identifiers "A Device should contain either an _ADR or a _HID object, never both." , so remove _ADR due to _HID exist.
Signed-off-by: Aryeh Chen <aryeh.chen@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> --- Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl index 0d94472450..4efb8709ac 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl +++ b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl @@ -27,7 +27,6 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID Name(_SEG, 0)- Name(_ADR, 0x00000000) Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope Method(_BBN, 0){ return(BN00()) } //
Bus number, optional for the Root PCI Bus Name(_UID, 0x0000) // Unique
Bus ID, optional-- 2.26.2.windows.1
|
|
toggle quoted message
Show quoted text
-----Original Message----- From: Chen, Aryeh <aryeh.chen@...> Sent: Sunday, September 11, 2022 10:18 PM To: devel@edk2.groups.io Cc: Chen, Aryeh <aryeh.chen@...>; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Oram, Isaac W <isaac.w.oram@...>; Sinha, Ankit <ankit.sinha@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...> Subject: [PATCH v3] MinPlatformPkg: Remove _ADR from MinDsdt.asl
From: Aryeh Chen <aryeh.chen@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4057
According to ASL Coding Guidelines - Device Identifiers "A Device should contain either an _ADR or a _HID object, never both." , so remove _ADR due to _HID exist.
Signed-off-by: Aryeh Chen <aryeh.chen@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> --- Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl index 0d94472450..4efb8709ac 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl +++ b/Platform/Intel/MinPlatformPkg/Acpi/MinDsdt/MinDsdt.asl @@ -27,7 +27,6 @@ DefinitionBlock ( Name(_HID, EISAID("PNP0A08")) // Indicates PCI Express/PCI-X Mode2 host hierarchy Name(_CID, EISAID("PNP0A03")) // To support legacy OS that doesn't understand the new HID Name(_SEG, 0)- Name(_ADR, 0x00000000) Method(^BN00, 0){ return(0x0000) } // Returns default Bus number for Peer PCI busses. Name can be overriden with control method placed directly under Device scope Method(_BBN, 0){ return(BN00()) } // Bus number, optional for the Root PCI Bus Name(_UID, 0x0000) // Unique Bus ID, optional-- 2.26.2.windows.1
|
|