Change in ...coreboot[master]: sb/intel/i82801gx: Remove unnecessary/redundant use of ACPI offset op...
HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31672
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant use of ACPI offset operator ......................................................................
sb/intel/i82801gx: Remove unnecessary/redundant use of ACPI offset operator
Change-Id: If53072c6a91dd794c70d1fab8697b1713d400fe8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/i82801gx/acpi/globalnvs.asl M src/southbridge/intel/i82801gx/acpi/ich7.asl 2 files changed, 5 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31672/1
diff --git a/src/southbridge/intel/i82801gx/acpi/globalnvs.asl b/src/southbridge/intel/i82801gx/acpi/globalnvs.asl index 33472b6..3e2b3b0 100644 --- a/src/southbridge/intel/i82801gx/acpi/globalnvs.asl +++ b/src/southbridge/intel/i82801gx/acpi/globalnvs.asl @@ -30,7 +30,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter @@ -48,7 +47,6 @@ LINX, 8, // 0x12 - Linux OS DCKN, 8, // 0x13 - PCIe docking state /* Thermal policy */ - Offset (0x14), ACTT, 8, // 0x14 - active trip point TPSV, 8, // 0x15 - passive trip point TC1V, 8, // 0x16 - passive trip point TC1 @@ -87,7 +85,6 @@ UTIL, 8, // 0x3a - ACIN, 8, // 0x3b - /* Integrated Graphics Device */ - Offset (0x3c), IGDS, 8, // 0x3c - IGD state (primary = 1) TLST, 8, // 0x3d - Display Toggle List pointer CADL, 8, // 0x3e - Currently Attached Devices List diff --git a/src/southbridge/intel/i82801gx/acpi/ich7.asl b/src/southbridge/intel/i82801gx/acpi/ich7.asl index 38ca483..1086e0f 100644 --- a/src/southbridge/intel/i82801gx/acpi/ich7.asl +++ b/src/southbridge/intel/i82801gx/acpi/ich7.asl @@ -43,12 +43,12 @@ OperationRegion(GPIO, SystemIO, DEFAULT_GPIOBASE, 0x3c) Field(GPIO, ByteAcc, NoLock, Preserve) { - Offset(0x00), // GPIO Use Select + // GPIO Use Select GU00, 8, GU01, 8, GU02, 8, GU03, 8, - Offset(0x04), // GPIO IO Select + // GPIO IO Select GIO0, 8, GIO1, 8, GIO2, 8, @@ -96,17 +96,17 @@ GIV1, 8, GIV2, 8, GIV3, 8, - Offset(0x30), // GPIO Use Select 2 + // GPIO Use Select 2 GU04, 8, GU05, 8, GU06, 8, GU07, 8, - Offset(0x34), // GPIO IO Select 2 + // GPIO IO Select 2 GIO4, 8, GIO5, 8, GIO6, 8, GIO7, 8, - Offset(0x38), // GPIO Level 2 + // GPIO Level 2 GP32, 1, GP33, 1, GP34, 1,
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31672 )
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant use of ACPI offset operator ......................................................................
Patch Set 1:
Could you go into more detail, why it’s unnecessary?
Hello Patrick Rudolph, build bot (Jenkins), Lijian Zhao,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31672
to look at the new patch set (#2).
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator
IASL detect Unnecessary/redundant uses of the Offset() operator within a Field Unit list. it then send a remark "^ Unnecessary/redundant use of Offset"
example: OperationRegion (OPR1, SystemMemory, 0x100, 0x100) Field (OPR1) { Offset (0), // Never needed FLD1, 32, Offset (4), // Redundant, offset is already 4 (bytes) FLD2, 8, Offset (64), // OK use of Offset. FLD3, 16, }
We will have those remark: dsdt.asl 14: Offset (0), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
dsdt.asl 16: Offset (4), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
Change-Id: If53072c6a91dd794c70d1fab8697b1713d400fe8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/i82801gx/acpi/globalnvs.asl M src/southbridge/intel/i82801gx/acpi/ich7.asl 2 files changed, 5 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31672/2
Hello Patrick Rudolph, build bot (Jenkins), Lijian Zhao,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31672
to look at the new patch set (#3).
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator
IASL detect Unnecessary/redundant uses of the Offset() operator within a Field Unit list. it then send a remark "^ Unnecessary/redundant use of Offset"
example: OperationRegion (OPR1, SystemMemory, 0x100, 0x100) Field (OPR1) { Offset (0), // Never needed FLD1, 32, Offset (4), // Redundant, offset is already 4 (bytes) FLD2, 8, Offset (64), // OK use of Offset. FLD3, 16, }
We will have those remark: dsdt.asl 14: Offset (0), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
dsdt.asl 16: Offset (4), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
Change-Id: If53072c6a91dd794c70d1fab8697b1713d400fe8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/i82801gx/acpi/globalnvs.asl M src/southbridge/intel/i82801gx/acpi/ich7.asl 2 files changed, 6 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31672/3
Hello Patrick Rudolph, build bot (Jenkins), Lijian Zhao,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31672
to look at the new patch set (#4).
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator
IASL detect Unnecessary/redundant uses of the Offset() operator within a Field Unit list. it then send a remark "^ Unnecessary/redundant use of Offset"
example: OperationRegion (OPR1, SystemMemory, 0x100, 0x100) Field (OPR1) { Offset (0), // Never needed FLD1, 32, Offset (4), // Redundant, offset is already 4 (bytes) FLD2, 8, Offset (64), // OK use of Offset. FLD3, 16, }
We will have those remark: dsdt.asl 14: Offset (0), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
dsdt.asl 16: Offset (4), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
Change-Id: If53072c6a91dd794c70d1fab8697b1713d400fe8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/i82801gx/acpi/globalnvs.asl M src/southbridge/intel/i82801gx/acpi/ich7.asl 2 files changed, 7 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31672/4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31672 )
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/#/c/31672/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31672/4//COMMIT_MSG@9 PS4, Line 9: detect detects
https://review.coreboot.org/#/c/31672/4//COMMIT_MSG@11 PS4, Line 11: send sends
https://review.coreboot.org/#/c/31672/4//COMMIT_MSG@11 PS4, Line 11: it It
https://review.coreboot.org/#/c/31672/4//COMMIT_MSG@8 PS4, Line 8: : IASL detect Unnecessary/redundant uses of the Offset() operator within : a Field Unit list. : it then send a remark "^ Unnecessary/redundant use of Offset" : : example: : OperationRegion (OPR1, SystemMemory, 0x100, 0x100) : Field (OPR1) : { : Offset (0), // Never needed : FLD1, 32, : Offset (4), // Redundant, offset is already 4 (bytes) : FLD2, 8, : Offset (64), // OK use of Offset. : FLD3, 16, : } : : We will have those remark: : dsdt.asl 14: Offset (0), : Remark 2158 - ^ Unnecessary/redundant use of Offset : operator : : dsdt.asl 16: Offset (4), : Remark 2158 - ^ Unnecessary/redundant use of Offset : operator Thank you for adding more details, but do you know why these offsets were added in the first place?
Hello Patrick Rudolph, build bot (Jenkins), Lijian Zhao,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31672
to look at the new patch set (#5).
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator
Using ACPICA version 20180927 or greater, IASL detects Unnecessary/redundant uses of the Offset() operator within a Field Unit list. It then sends a remark "^ Unnecessary/redundant use of Offset".
Offsets refer to the current offset are unnecessary. example: OperationRegion (OPR1, SystemMemory, 0x100, 0x100) Field (OPR1) { Offset (0), // Never needed FLD1, 32, Offset (4), // Redundant, offset is already 4 (bytes) FLD2, 8, Offset (64), // OK use of Offset. FLD3, 16, }
We will have those remarks: dsdt.asl 14: Offset (0), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
dsdt.asl 16: Offset (4), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
Change-Id: If53072c6a91dd794c70d1fab8697b1713d400fe8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/i82801gx/acpi/globalnvs.asl M src/southbridge/intel/i82801gx/acpi/ich7.asl 2 files changed, 7 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31672/5
Hello Patrick Rudolph, build bot (Jenkins), Lijian Zhao,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31672
to look at the new patch set (#6).
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator
Using ACPICA version 20180927 or greater, IASL detects Unnecessary/redundant uses of the Offset() operator within a Field Unit list. It then sends a remark "^ Unnecessary/redundant use of Offset".
Offsets refer to the current offset are unnecessary. example: OperationRegion (OPR1, SystemMemory, 0x100, 0x100) Field (OPR1) { Offset (0), // Never needed FLD1, 32, Offset (4), // Redundant, offset is already 4 (bytes) FLD2, 8, Offset (64), // OK use of Offset. FLD3, 16, }
We will have those remarks: dsdt.asl 14: Offset (0), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
dsdt.asl 16: Offset (4), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
Change-Id: If53072c6a91dd794c70d1fab8697b1713d400fe8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/southbridge/intel/i82801gx/acpi/globalnvs.asl M src/southbridge/intel/i82801gx/acpi/ich7.asl 2 files changed, 7 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/31672/6
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31672 )
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
Patch Set 6:
still have those remarks linked to src/southbridge/intel/i82801gx/acpi/pata.asl :
Intel ACPI Component Architecture ASL+ Optimizing Compiler/Disassembler version 20190215 Copyright (c) 2000 - 2019 Intel Corporation
coreboot toolchain vc3c9afbdf1 2019-02-03 dsdt.asl 1651: CreateDwordField (PBUF, 0, PIO0) Remark 2089 - Object is not referenced ^ (Name [PIO0] is within a method [_GTM])
dsdt.asl 1652: CreateDwordField (PBUF, 4, DMA0) Remark 2089 - Object is not referenced ^ (Name [DMA0] is within a method [_GTM])
dsdt.asl 1653: CreateDwordField (PBUF, 8, PIO1) Remark 2089 - Object is not referenced ^ (Name [PIO1] is within a method [_GTM])
dsdt.asl 1654: CreateDwordField (PBUF, 12, DMA1) Remark 2089 - Object is not referenced ^ (Name [DMA1] is within a method [_GTM])
dsdt.asl 1655: CreateDwordField (PBUF, 16, FLAG) Remark 2089 - Object is not referenced ^ (Name [FLAG] is within a method [_GTM])
dsdt.asl 1660: CreateDwordField (Arg0, 0, PIO0) Remark 2089 - Object is not referenced ^ (Name [PIO0] is within a method [_STM])
dsdt.asl 1661: CreateDwordField (Arg0, 4, DMA0) Remark 2089 - Object is not referenced ^ (Name [DMA0] is within a method [_STM])
dsdt.asl 1662: CreateDwordField (Arg0, 8, PIO1) Remark 2089 - Object is not referenced ^ (Name [PIO1] is within a method [_STM])
dsdt.asl 1663: CreateDwordField (Arg0, 12, DMA1) Remark 2089 - Object is not referenced ^ (Name [DMA1] is within a method [_STM])
dsdt.asl 1664: CreateDwordField (Arg0, 16, FLAG) Remark 2089 - Object is not referenced ^ (Name [FLAG] is within a method [_STM])
dsdt.asl 1688: CreateDwordField (PBUF, 0, PIO0) Remark 2089 - Object is not referenced ^ (Name [PIO0] is within a method [_GTM])
dsdt.asl 1689: CreateDwordField (PBUF, 4, DMA0) Remark 2089 - Object is not referenced ^ (Name [DMA0] is within a method [_GTM])
dsdt.asl 1690: CreateDwordField (PBUF, 8, PIO1) Remark 2089 - Object is not referenced ^ (Name [PIO1] is within a method [_GTM])
dsdt.asl 1691: CreateDwordField (PBUF, 12, DMA1) Remark 2089 - Object is not referenced ^ (Name [DMA1] is within a method [_GTM])
dsdt.asl 1692: CreateDwordField (PBUF, 16, FLAG) Remark 2089 - Object is not referenced ^ (Name [FLAG] is within a method [_GTM])
dsdt.asl 1697: CreateDwordField (Arg0, 0, PIO0) Remark 2089 - Object is not referenced ^ (Name [PIO0] is within a method [_STM])
dsdt.asl 1698: CreateDwordField (Arg0, 4, DMA0) Remark 2089 - Object is not referenced ^ (Name [DMA0] is within a method [_STM])
dsdt.asl 1699: CreateDwordField (Arg0, 8, PIO1) Remark 2089 - Object is not referenced ^ (Name [PIO1] is within a method [_STM])
dsdt.asl 1700: CreateDwordField (Arg0, 12, DMA1) Remark 2089 - Object is not referenced ^ (Name [DMA1] is within a method [_STM])
dsdt.asl 1701: CreateDwordField (Arg0, 16, FLAG) Remark 2089 - Object is not referenced ^ (Name [FLAG] is within a method [_STM])
ASL Input: dsdt.asl - 1725 lines, 32186 bytes, 658 keywords AML Output: dsdt.aml - 9254 bytes, 377 named objects, 281 executable opcodes
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31672 )
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31672 )
Change subject: sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator ......................................................................
sb/intel/i82801gx: Remove unnecessary/redundant ACPI offset operator
Using ACPICA version 20180927 or greater, IASL detects Unnecessary/redundant uses of the Offset() operator within a Field Unit list. It then sends a remark "^ Unnecessary/redundant use of Offset".
Offsets refer to the current offset are unnecessary. example: OperationRegion (OPR1, SystemMemory, 0x100, 0x100) Field (OPR1) { Offset (0), // Never needed FLD1, 32, Offset (4), // Redundant, offset is already 4 (bytes) FLD2, 8, Offset (64), // OK use of Offset. FLD3, 16, }
We will have those remarks: dsdt.asl 14: Offset (0), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
dsdt.asl 16: Offset (4), Remark 2158 - ^ Unnecessary/redundant use of Offset operator
Change-Id: If53072c6a91dd794c70d1fab8697b1713d400fe8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/31672 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Lijian Zhao lijian.zhao@intel.com --- M src/southbridge/intel/i82801gx/acpi/globalnvs.asl M src/southbridge/intel/i82801gx/acpi/ich7.asl 2 files changed, 7 insertions(+), 11 deletions(-)
Approvals: build bot (Jenkins): Verified Lijian Zhao: Looks good to me, approved
diff --git a/src/southbridge/intel/i82801gx/acpi/globalnvs.asl b/src/southbridge/intel/i82801gx/acpi/globalnvs.asl index 33472b6..650b07c 100644 --- a/src/southbridge/intel/i82801gx/acpi/globalnvs.asl +++ b/src/southbridge/intel/i82801gx/acpi/globalnvs.asl @@ -30,7 +30,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter @@ -48,7 +47,6 @@ LINX, 8, // 0x12 - Linux OS DCKN, 8, // 0x13 - PCIe docking state /* Thermal policy */ - Offset (0x14), ACTT, 8, // 0x14 - active trip point TPSV, 8, // 0x15 - passive trip point TC1V, 8, // 0x16 - passive trip point TC1 @@ -87,7 +85,6 @@ UTIL, 8, // 0x3a - ACIN, 8, // 0x3b - /* Integrated Graphics Device */ - Offset (0x3c), IGDS, 8, // 0x3c - IGD state (primary = 1) TLST, 8, // 0x3d - Display Toggle List pointer CADL, 8, // 0x3e - Currently Attached Devices List @@ -95,7 +92,6 @@ CSTE, 16, // 0x40 - Current display state NSTE, 16, // 0x42 - Next display state SSTE, 16, // 0x44 - Set display state - Offset (0x46), NDID, 8, // 0x46 - Number of Device IDs DID1, 32, // 0x47 - Device ID 1 DID2, 32, // 0x4b - Device ID 2 diff --git a/src/southbridge/intel/i82801gx/acpi/ich7.asl b/src/southbridge/intel/i82801gx/acpi/ich7.asl index 38ca483..d24c8af 100644 --- a/src/southbridge/intel/i82801gx/acpi/ich7.asl +++ b/src/southbridge/intel/i82801gx/acpi/ich7.asl @@ -43,12 +43,12 @@ OperationRegion(GPIO, SystemIO, DEFAULT_GPIOBASE, 0x3c) Field(GPIO, ByteAcc, NoLock, Preserve) { - Offset(0x00), // GPIO Use Select + // GPIO Use Select GU00, 8, GU01, 8, GU02, 8, GU03, 8, - Offset(0x04), // GPIO IO Select + // GPIO IO Select GIO0, 8, GIO1, 8, GIO2, 8, @@ -96,17 +96,17 @@ GIV1, 8, GIV2, 8, GIV3, 8, - Offset(0x30), // GPIO Use Select 2 + // GPIO Use Select 2 GU04, 8, GU05, 8, GU06, 8, GU07, 8, - Offset(0x34), // GPIO IO Select 2 + // GPIO IO Select 2 GIO4, 8, GIO5, 8, GIO6, 8, GIO7, 8, - Offset(0x38), // GPIO Level 2 + // GPIO Level 2 GP32, 1, GP33, 1, GP34, 1, @@ -125,7 +125,7 @@ OperationRegion(RCRB, SystemMemory, DEFAULT_RCBA, 0x4000) Field(RCRB, DWordAcc, Lock, Preserve) { - Offset(0x0000), // Backbone + // Backbone Offset(0x1000), // Chipset Offset(0x3000), // Legacy Configuration Registers Offset(0x3404), // High Performance Timer Configuration @@ -148,7 +148,7 @@ , 2, // Reserved LPBD, 1, // LPC bridge disable EHCD, 1, // EHCI disable - Offset(0x341a), // FD Root Ports + // FD Root Ports RP1D, 1, // Root Port 1 disable RP2D, 1, // Root Port 2 disable RP3D, 1, // Root Port 3 disable
-
HAOUAS Elyes (Code Review) -
Lijian Zhao (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)