Wim Vervoorn has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
src: Remove redundant Offset from ASL
IASL reports unnecessary/redundant use of offset operator. Remove the related offset (0xXX) lines.
BUG=N/A TEST=build
Change-Id: Ie8507d3b3cb6f2e75cb87cd3e4bcc4280df27f77 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/ec/google/chromeec/acpi/ec.asl M src/ec/google/chromeec/acpi/emem.asl M src/soc/amd/picasso/acpi/globalnvs.asl M src/soc/amd/stoneyridge/acpi/globalnvs.asl M src/soc/intel/apollolake/acpi/globalnvs.asl M src/soc/intel/baytrail/acpi/globalnvs.asl M src/soc/intel/broadwell/acpi/globalnvs.asl M src/soc/intel/broadwell/acpi/xhci.asl M src/soc/intel/denverton_ns/acpi/globalnvs.asl M src/soc/intel/fsp_baytrail/acpi/globalnvs.asl M src/soc/intel/icelake/acpi/northbridge.asl M src/soc/intel/skylake/acpi/globalnvs.asl M src/soc/intel/skylake/acpi/pmc.asl M src/soc/intel/skylake/acpi/systemagent.asl M src/soc/intel/skylake/acpi/xhci.asl M src/southbridge/intel/bd82x6x/acpi/globalnvs.asl M src/southbridge/intel/fsp_rangeley/acpi/globalnvs.asl M src/southbridge/intel/i82801ix/acpi/globalnvs.asl M src/southbridge/intel/i82801jx/acpi/globalnvs.asl M src/southbridge/intel/lynxpoint/acpi/globalnvs.asl M src/southbridge/intel/lynxpoint/acpi/usb.asl M src/vendorcode/amd/agesa/f12/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl M src/vendorcode/amd/agesa/f14/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibPciLib.esl 24 files changed, 3 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/36857/1
diff --git a/src/ec/google/chromeec/acpi/ec.asl b/src/ec/google/chromeec/acpi/ec.asl index 962988e..5fdfb9e 100644 --- a/src/ec/google/chromeec/acpi/ec.asl +++ b/src/ec/google/chromeec/acpi/ec.asl @@ -41,7 +41,6 @@ OperationRegion (ERAM, EmbeddedControl, 0x00, EC_ACPI_MEM_MAPPED_BEGIN) Field (ERAM, ByteAcc, Lock, Preserve) { - Offset (0x00), RAMV, 8, // EC RAM Version TSTB, 8, // Test Byte TSTC, 8, // Complement of Test Byte @@ -88,7 +87,6 @@ RWSG, 1, // EC has RWSIG task enabled DEVE, 1, // EC supports device events // make sure we're within our space envelope - Offset (0x0e), Offset (0x12), BTID, 8, // Battery index that host wants to read USPP, 8, // USB Port Power diff --git a/src/ec/google/chromeec/acpi/emem.asl b/src/ec/google/chromeec/acpi/emem.asl index 982ec5b..77b4708 100644 --- a/src/ec/google/chromeec/acpi/emem.asl +++ b/src/ec/google/chromeec/acpi/emem.asl @@ -17,7 +17,6 @@ * EMEM data may be accessed through port 62/66 or through LPC at 900h. */
-Offset (0x00), TIN0, 8, // Temperature 0 TIN1, 8, // Temperature 1 TIN2, 8, // Temperature 2 diff --git a/src/soc/amd/picasso/acpi/globalnvs.asl b/src/soc/amd/picasso/acpi/globalnvs.asl index a373a99..a895d78 100644 --- a/src/soc/amd/picasso/acpi/globalnvs.asl +++ b/src/soc/amd/picasso/acpi/globalnvs.asl @@ -27,7 +27,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), PCNT, 8, // 0x00 - Processor Count PPCM, 8, // 0x01 - Max PPC State LIDS, 8, // 0x02 - LID State diff --git a/src/soc/amd/stoneyridge/acpi/globalnvs.asl b/src/soc/amd/stoneyridge/acpi/globalnvs.asl index 03d205f..22f0f3c 100644 --- a/src/soc/amd/stoneyridge/acpi/globalnvs.asl +++ b/src/soc/amd/stoneyridge/acpi/globalnvs.asl @@ -27,7 +27,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), PCNT, 8, // 0x00 - Processor Count PPCM, 8, // 0x01 - Max PPC State LIDS, 8, // 0x02 - LID State diff --git a/src/soc/intel/apollolake/acpi/globalnvs.asl b/src/soc/intel/apollolake/acpi/globalnvs.asl index 4aad29c..c8d528f 100644 --- a/src/soc/intel/apollolake/acpi/globalnvs.asl +++ b/src/soc/intel/apollolake/acpi/globalnvs.asl @@ -27,7 +27,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), PCNT, 8, // 0x00 - Processor Count PPCM, 8, // 0x01 - Max PPC State LIDS, 8, // 0x02 - LID State diff --git a/src/soc/intel/baytrail/acpi/globalnvs.asl b/src/soc/intel/baytrail/acpi/globalnvs.asl index f33fcf6..a691faa 100644 --- a/src/soc/intel/baytrail/acpi/globalnvs.asl +++ b/src/soc/intel/baytrail/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 diff --git a/src/soc/intel/broadwell/acpi/globalnvs.asl b/src/soc/intel/broadwell/acpi/globalnvs.asl index 9ceeca5..5e3a7fa 100644 --- a/src/soc/intel/broadwell/acpi/globalnvs.asl +++ b/src/soc/intel/broadwell/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 diff --git a/src/soc/intel/broadwell/acpi/xhci.asl b/src/soc/intel/broadwell/acpi/xhci.asl index d638a27..b82ee98 100644 --- a/src/soc/intel/broadwell/acpi/xhci.asl +++ b/src/soc/intel/broadwell/acpi/xhci.asl @@ -26,7 +26,6 @@ OperationRegion (XPRT, PCI_Config, 0x00, 0x100) Field (XPRT, AnyAcc, NoLock, Preserve) { - Offset (0x0), DVID, 16, Offset (0x10), , 16, diff --git a/src/soc/intel/denverton_ns/acpi/globalnvs.asl b/src/soc/intel/denverton_ns/acpi/globalnvs.asl index 5ef029e..85ddf9b 100644 --- a/src/soc/intel/denverton_ns/acpi/globalnvs.asl +++ b/src/soc/intel/denverton_ns/acpi/globalnvs.asl @@ -31,7 +31,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 diff --git a/src/soc/intel/fsp_baytrail/acpi/globalnvs.asl b/src/soc/intel/fsp_baytrail/acpi/globalnvs.asl index c4d91a3..e006fef 100644 --- a/src/soc/intel/fsp_baytrail/acpi/globalnvs.asl +++ b/src/soc/intel/fsp_baytrail/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 diff --git a/src/soc/intel/icelake/acpi/northbridge.asl b/src/soc/intel/icelake/acpi/northbridge.asl index e99e7ed..6a5157d 100644 --- a/src/soc/intel/icelake/acpi/northbridge.asl +++ b/src/soc/intel/icelake/acpi/northbridge.asl @@ -54,9 +54,7 @@
Offset (0xa0), /* Top of Used Memory */ TOM, 64, - - Offset (0xa8), /* Top of Upper Used Memory */ - TUUD, 64, + TUUD, 64, /* Top of Upper Used Memory */
Offset (0xbc), /* Top of Low Used Memory */ TLUD, 32, diff --git a/src/soc/intel/skylake/acpi/globalnvs.asl b/src/soc/intel/skylake/acpi/globalnvs.asl index c4544e8..d4ec901 100644 --- a/src/soc/intel/skylake/acpi/globalnvs.asl +++ b/src/soc/intel/skylake/acpi/globalnvs.asl @@ -32,7 +32,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 diff --git a/src/soc/intel/skylake/acpi/pmc.asl b/src/soc/intel/skylake/acpi/pmc.asl index d097082..0e048a3 100644 --- a/src/soc/intel/skylake/acpi/pmc.asl +++ b/src/soc/intel/skylake/acpi/pmc.asl @@ -34,8 +34,7 @@ PMFS, 1, /* PMC_MSG_FULL_STS */ Offset (0x20), MPMC, 32, /* MTPMC */ - Offset (0x24), /* PCH_PM_STS2 */ - , 20, + , 20, /* PCH_PM_STS2 */ UWAB, 1, /* USB2 Workaround Available */ } } diff --git a/src/soc/intel/skylake/acpi/systemagent.asl b/src/soc/intel/skylake/acpi/systemagent.asl index e7b2d90..9ba5566 100644 --- a/src/soc/intel/skylake/acpi/systemagent.asl +++ b/src/soc/intel/skylake/acpi/systemagent.asl @@ -58,9 +58,7 @@
Offset (0xa0), /* Top of Used Memory */ TOM, 64, - - Offset (0xa8), /* Top of Upper Used Memory */ - TUUD, 64, + TUUD, 64, /* Top of Upper Used Memory */
Offset (0xbc), /* Top of Low Used Memory */ TLUD, 32, diff --git a/src/soc/intel/skylake/acpi/xhci.asl b/src/soc/intel/skylake/acpi/xhci.asl index b5aa412..4b2f29f 100644 --- a/src/soc/intel/skylake/acpi/xhci.asl +++ b/src/soc/intel/skylake/acpi/xhci.asl @@ -94,7 +94,6 @@ OperationRegion (XPRT, PCI_Config, 0x00, 0x100) Field (XPRT, AnyAcc, NoLock, Preserve) { - Offset (0x0), DVID, 16, /* VENDORID */ Offset (0x10), , 16, diff --git a/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl b/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl index f7652ee..db21430 100644 --- a/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl +++ b/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl @@ -31,7 +31,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 @@ -46,7 +45,6 @@ LIDS, 8, // 0x0f - LID state (open = 1) PWRS, 8, // 0x10 - Power State (AC = 1) /* Thermal policy */ - Offset (0x11), TLVL, 8, // 0x11 - Throttle Level Limit FLVL, 8, // 0x12 - Current FAN Level TCRT, 8, // 0x13 - Critical Threshold diff --git a/src/southbridge/intel/fsp_rangeley/acpi/globalnvs.asl b/src/southbridge/intel/fsp_rangeley/acpi/globalnvs.asl index 2c47fe5..12e747b 100644 --- a/src/southbridge/intel/fsp_rangeley/acpi/globalnvs.asl +++ b/src/southbridge/intel/fsp_rangeley/acpi/globalnvs.asl @@ -32,7 +32,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 diff --git a/src/southbridge/intel/i82801ix/acpi/globalnvs.asl b/src/southbridge/intel/i82801ix/acpi/globalnvs.asl index c1be852..67ceb74 100644 --- a/src/southbridge/intel/i82801ix/acpi/globalnvs.asl +++ b/src/southbridge/intel/i82801ix/acpi/globalnvs.asl @@ -31,7 +31,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 @@ -49,7 +48,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 diff --git a/src/southbridge/intel/i82801jx/acpi/globalnvs.asl b/src/southbridge/intel/i82801jx/acpi/globalnvs.asl index 44aa8e4..075a7c9 100644 --- a/src/southbridge/intel/i82801jx/acpi/globalnvs.asl +++ b/src/southbridge/intel/i82801jx/acpi/globalnvs.asl @@ -31,7 +31,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 @@ -49,7 +48,6 @@ LINX, 8, // 0x12 - Linux OS DCKN, 8, // 0x13 - PCIe docking state /* Thermal policy */ - Offset (0x14), ACTT, 8, // 0x14 - active trip point PSVT, 8, // 0x15 - passive trip point TC1V, 8, // 0x16 - passive trip point TC1 diff --git a/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl b/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl index ba9f850..790001d 100644 --- a/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl +++ b/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl @@ -31,7 +31,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 @@ -46,7 +45,6 @@ LIDS, 8, // 0x0f - LID state (open = 1) PWRS, 8, // 0x10 - Power State (AC = 1) /* Thermal policy */ - Offset (0x11), TLVL, 8, // 0x11 - Throttle Level Limit FLVL, 8, // 0x12 - Current FAN Level TCRT, 8, // 0x13 - Critical Threshold @@ -94,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/lynxpoint/acpi/usb.asl b/src/southbridge/intel/lynxpoint/acpi/usb.asl index ee88303..6ed03ed 100644 --- a/src/southbridge/intel/lynxpoint/acpi/usb.asl +++ b/src/southbridge/intel/lynxpoint/acpi/usb.asl @@ -70,7 +70,6 @@ OperationRegion (XPRT, PCI_Config, 0x00, 0x100) Field (XPRT, AnyAcc, NoLock, Preserve) { - Offset (0x0), DVID, 16, Offset (0x10), , 16, diff --git a/src/vendorcode/amd/agesa/f12/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl b/src/vendorcode/amd/agesa/f12/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl index 782a06f..4015158 100644 --- a/src/vendorcode/amd/agesa/f12/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl +++ b/src/vendorcode/amd/agesa/f12/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl @@ -141,7 +141,6 @@ Add (Arg1, Local0, Local0) OperationRegion(varOperationRegionMmio, SystemMemory, Local0, 0x4) Field(varOperationRegionMmio, DWordAcc, NoLock, Preserve) { - Offset (0x0), varPciReg32, 32, } return (varPciReg32) @@ -159,7 +158,6 @@ Add (Arg1, Local0, Local0) OperationRegion(varOperationRegionMmio, SystemMemory, Local0, 0x4) Field(varOperationRegionMmio, DWordAcc, NoLock, Preserve) { - Offset (0x0), varPciReg32, 32, } Store (Arg2, varPciReg32) diff --git a/src/vendorcode/amd/agesa/f14/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl b/src/vendorcode/amd/agesa/f14/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl index 463093c..5d545b1 100644 --- a/src/vendorcode/amd/agesa/f14/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl +++ b/src/vendorcode/amd/agesa/f14/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl @@ -142,7 +142,6 @@ Add (Arg1, Local0, Local0) OperationRegion(varOperationRegionMmio, SystemMemory, Local0, 0x4) Field(varOperationRegionMmio, DWordAcc, NoLock, Preserve) { - Offset (0x0), varPciReg32, 32, } return (varPciReg32) @@ -160,7 +159,6 @@ Add (Arg1, Local0, Local0) OperationRegion(varOperationRegionMmio, SystemMemory, Local0, 0x4) Field(varOperationRegionMmio, DWordAcc, NoLock, Preserve) { - Offset (0x0), varPciReg32, 32, } Store (Arg2, varPciReg32) diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibPciLib.esl b/src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibPciLib.esl index 9dba662..342c646 100644 --- a/src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibPciLib.esl +++ b/src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibPciLib.esl @@ -55,7 +55,6 @@ Add (Arg1, Local0, Local0) OperationRegion(varOperationRegionMmio, SystemMemory, Local0, 0x4) Field(varOperationRegionMmio, DWordAcc, NoLock, Preserve) { - Offset (0x0), varPciReg32, 32, } return (varPciReg32) @@ -73,7 +72,6 @@ Add (Arg1, Local0, Local0) OperationRegion(varOperationRegionMmio, SystemMemory, Local0, 0x4) Field(varOperationRegionMmio, DWordAcc, NoLock, Preserve) { - Offset (0x0), varPciReg32, 32, } Store (Arg2, varPciReg32)
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
Patch Set 1: Code-Review+1
Hello Patrick Rudolph, HAOUAS Elyes, Vanny E, Huang Jin, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36857
to look at the new patch set (#3).
Change subject: src: Remove redundant Offset from ASL ......................................................................
src: Remove redundant Offset from ASL
IASL reports unnecessary/redundant use of offset operator. Remove the related offset (0xXX) lines.
BUG=N/A TEST=build
Change-Id: Ie8507d3b3cb6f2e75cb87cd3e4bcc4280df27f77 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/ec/google/chromeec/acpi/ec.asl M src/ec/google/chromeec/acpi/emem.asl M src/soc/amd/picasso/acpi/globalnvs.asl M src/soc/amd/stoneyridge/acpi/globalnvs.asl M src/soc/intel/apollolake/acpi/globalnvs.asl M src/soc/intel/baytrail/acpi/globalnvs.asl M src/soc/intel/broadwell/acpi/globalnvs.asl M src/soc/intel/broadwell/acpi/xhci.asl M src/soc/intel/denverton_ns/acpi/globalnvs.asl M src/soc/intel/fsp_baytrail/acpi/globalnvs.asl M src/soc/intel/icelake/acpi/northbridge.asl M src/soc/intel/skylake/acpi/globalnvs.asl M src/soc/intel/skylake/acpi/pmc.asl M src/soc/intel/skylake/acpi/systemagent.asl M src/soc/intel/skylake/acpi/xhci.asl M src/southbridge/intel/bd82x6x/acpi/globalnvs.asl M src/southbridge/intel/fsp_rangeley/acpi/globalnvs.asl M src/southbridge/intel/i82801ix/acpi/globalnvs.asl M src/southbridge/intel/i82801jx/acpi/globalnvs.asl M src/southbridge/intel/lynxpoint/acpi/globalnvs.asl M src/southbridge/intel/lynxpoint/acpi/usb.asl M src/vendorcode/amd/agesa/f14/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibPciLib.esl 23 files changed, 3 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/36857/3
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
Patch Set 3: Code-Review+1
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
Patch Set 3:
and this file 'src/vendorcode/amd/agesa/f12/Proc/GNB/Modules/GnbPcieAlibV1/PcieAlibCore.esl' ?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
Patch Set 3:
IMO the redundancy is not bad for sanity checking and for readability.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
Patch Set 3:
Patch Set 3:
IMO the redundancy is not bad for sanity checking and for readability.
[quote] acpi/documents/changes.txt: "when optimization is enabled, the iASL compiler will in fact remove the redundant Offset operators and will not emit any AML code for them."
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
IMO the redundancy is not bad for sanity checking and for readability.
[quote] acpi/documents/changes.txt: "when optimization is enabled, the iASL compiler will in fact remove the redundant Offset operators and will not emit any AML code for them."
They won't be in the resulting binary, but the compiler will complain if an Offset is trying to move backwards. An assert statement (that ensures that we're precisely at a given position) would be better, but so far I haven't managed to explain the acpica maintainers that use case in a way that convinces them.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Remove redundant Offset from ASL ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
Patch Set 3:
IMO the redundancy is not bad for sanity checking and for readability.
[quote] acpi/documents/changes.txt: "when optimization is enabled, the iASL compiler will in fact remove the redundant Offset operators and will not emit any AML code for them."
They won't be in the resulting binary, but the compiler will complain if an Offset is trying to move backwards. An assert statement (that ensures that we're precisely at a given position) would be better, but so far I haven't managed to explain the acpica maintainers that use case in a way that convinces them.
Note that this redundancy is neither a warning nor an error. it is just a remark. So we also can keep using them.
Hello Patrick Rudolph, HAOUAS Elyes, Vanny E, Huang Jin, Frans Hendriks, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36857
to look at the new patch set (#4).
Change subject: src: Ignore Redundant offset remarks in ASL code ......................................................................
src: Ignore Redundant offset remarks in ASL code
IASL reports unnecessary/redundant use of offset operator. These messages are only masking usefull messages. Add -vw 2158 so this message isn reported.
BUG=N/A TEST=build
Change-Id: Ie8507d3b3cb6f2e75cb87cd3e4bcc4280df27f77 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M Makefile.inc 1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/36857/4
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Ignore Redundant offset remarks in ASL code ......................................................................
Patch Set 4:
Note that this redundancy is neither a warning nor an error. it is just a remark. So we also can keep using them.
You all are right, this redundancy isn't a problem and can stay there. I uploaded a new patchset that adds this remark to the list of remarks that are ignored. By doing this we can get a clean output and don't have to look at remarks we don't intend to handle anyhow.
Hello Patrick Rudolph, HAOUAS Elyes, Vanny E, Huang Jin, Frans Hendriks, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36857
to look at the new patch set (#5).
Change subject: src: Ignore Redundant offset remarks in ASL code ......................................................................
src: Ignore Redundant offset remarks in ASL code
IASL reports unnecessary/redundant use of offset operator. These messages are only masking usefull messages. Add -vw 2158 so this message isn't reported.
BUG=N/A TEST=build
Change-Id: Ie8507d3b3cb6f2e75cb87cd3e4bcc4280df27f77 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M Makefile.inc 1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/36857/5
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Ignore Redundant offset remarks in ASL code ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36857/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36857/5//COMMIT_MSG@10 PS5, Line 10: usefull useful
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Ignore Redundant offset remarks in ASL code ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36857/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36857/5//COMMIT_MSG@10 PS5, Line 10: usefull
useful
Done
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Ignore Redundant offset remarks in ASL code ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36857 )
Change subject: src: Ignore Redundant offset remarks in ASL code ......................................................................
src: Ignore Redundant offset remarks in ASL code
IASL reports unnecessary/redundant use of offset operator. These messages are only masking usefull messages. Add -vw 2158 so this message isn't reported.
BUG=N/A TEST=build
Change-Id: Ie8507d3b3cb6f2e75cb87cd3e4bcc4280df27f77 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36857 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M Makefile.inc 1 file changed, 4 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve
diff --git a/Makefile.inc b/Makefile.inc index 8de54a0..d3aa885 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -258,7 +258,10 @@ # ResourceTemplate is the correct code. # As it's valid ASL, disable the warning. EMPTY_RESOURCE_TEMPLATE_WARNING = 3150 -IGNORED_IASL_WARNINGS = -vw $(EMPTY_RESOURCE_TEMPLATE_WARNING) +# Redundant offset remarks are not useful in any way and are masking useful +# ones that might indicate an issue so it is better to hide them. +REDUNDANT_OFFSET_REMARK = 2158 +IGNORED_IASL_WARNINGS = -vw $(EMPTY_RESOURCE_TEMPLATE_WARNING) -vw $(REDUNDANT_OFFSET_REMARK)
define asl_template $(CONFIG_CBFS_PREFIX)/$(1).aml-file = $(obj)/$(1).aml
-
Arthur Heymans (Code Review) -
Frans Hendriks (Code Review)
-
HAOUAS Elyes (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Wim Vervoorn (Code Review)