Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion ......................................................................
soc/intel/{cnl,icl,skl}: Remove unused SMI opregion
TEST=Able to build and boot Hatch and DE.
Change-Id: I6d63c005873fc5d67b4a44f42bb436628d7c1dc3 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/acpi/platform.asl M src/soc/intel/icelake/acpi/platform.asl M src/soc/intel/skylake/acpi/platform.asl 3 files changed, 0 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/36462/1
diff --git a/src/soc/intel/cannonlake/acpi/platform.asl b/src/soc/intel/cannonlake/acpi/platform.asl index 773cdcb..da61619 100644 --- a/src/soc/intel/cannonlake/acpi/platform.asl +++ b/src/soc/intel/cannonlake/acpi/platform.asl @@ -20,15 +20,6 @@ /* Generic indicator for sleep state */ #include <soc/intel/common/acpi/platform.asl>
-/* The APM port can be used for generating software SMIs */ - -OperationRegion (APMP, SystemIO, 0xb2, 2) -Field (APMP, ByteAcc, NoLock, Preserve) -{ - APMC, 8, // APM command - APMS, 8 // APM status -} - /* * The _PIC method is called by the OS to choose between interrupt * routing via the i8259 interrupt controller or the APIC. diff --git a/src/soc/intel/icelake/acpi/platform.asl b/src/soc/intel/icelake/acpi/platform.asl index 080bf7b..b89fd46 100644 --- a/src/soc/intel/icelake/acpi/platform.asl +++ b/src/soc/intel/icelake/acpi/platform.asl @@ -18,15 +18,6 @@ /* Generic indicator for sleep state */ #include <soc/intel/common/acpi/platform.asl>
-/* The APM port can be used for generating software SMIs */ - -OperationRegion (APMP, SystemIO, 0xb2, 2) -Field (APMP, ByteAcc, NoLock, Preserve) -{ - APMC, 8, // APM command - APMS, 8 // APM status -} - /* * The _PIC method is called by the OS to choose between interrupt * routing via the i8259 interrupt controller or the APIC. diff --git a/src/soc/intel/skylake/acpi/platform.asl b/src/soc/intel/skylake/acpi/platform.asl index 39debe1..19345b2 100644 --- a/src/soc/intel/skylake/acpi/platform.asl +++ b/src/soc/intel/skylake/acpi/platform.asl @@ -18,15 +18,6 @@ /* Enable ACPI _SWS methods */ #include <soc/intel/common/acpi/acpi_wake_source.asl>
-/* The APM port can be used for generating software SMIs */ - -OperationRegion (APMP, SystemIO, 0xb2, 2) -Field (APMP, ByteAcc, NoLock, Preserve) -{ - APMC, 8, // APM command - APMS, 8 // APM status -} - /* Port 80 POST */
OperationRegion (POST, SystemIO, 0x80, 1)
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion ......................................................................
Patch Set 3: Code-Review+2
(2 comments)
Some thought below. Let me know what you think.
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... PS1, Line 38: /* IO-Trap at 0x800. : * This is the ACPI->SMI communication interface. : */ : OperationRegion (IO_T, SystemIO, 0x800, 0x10) : Field (IO_T, ByteAcc, NoLock, Preserve) : { : Offset (0x8), : TRP0, 8 /* IO-Trap at 0x808 */ : } I haven't checked but I don't think any platform in master has an io trap smihandler. Do you think it's worth removing too?
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... PS1, Line 49: TRAP This is needed somewhere in drivers/intel/gma , but it's no-op. Maybe change this function to a stub? Maybe it's time to get rid of the dependency and simply remove support for SMI generation in ACPI altogether?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... PS1, Line 38: /* IO-Trap at 0x800. : * This is the ACPI->SMI communication interface. : */ : OperationRegion (IO_T, SystemIO, 0x800, 0x10) : Field (IO_T, ByteAcc, NoLock, Preserve) : { : Offset (0x8), : TRP0, 8 /* IO-Trap at 0x808 */ : }
I haven't checked but I don't think any platform in master has an io trap smihandler. […]
yes should be good to clean as well
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... PS1, Line 49: TRAP
This is needed somewhere in drivers/intel/gma , but it's no-op. […]
may be i will check with GFX team if they have any question, but in general good to clean both in specific CLs as these are more debug hooks
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... PS1, Line 38: /* IO-Trap at 0x800. : * This is the ACPI->SMI communication interface. : */ : OperationRegion (IO_T, SystemIO, 0x800, 0x10) : Field (IO_T, ByteAcc, NoLock, Preserve) : { : Offset (0x8), : TRP0, 8 /* IO-Trap at 0x808 */ : }
yes should be good to clean as well
Done https://review.coreboot.org/c/coreboot/+/36466
https://review.coreboot.org/c/coreboot/+/36462/1/src/soc/intel/skylake/acpi/... PS1, Line 49: TRAP
may be i will check with GFX team if they have any question, but in general good to clean both in sp […]
Done
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion ......................................................................
Patch Set 4: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion ......................................................................
Patch Set 4: Code-Review+2
Thanks!
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion ......................................................................
soc/intel/{cnl,icl,skl}: Remove unused SMI opregion
TEST=Able to build and boot Hatch and DE.
Change-Id: I6d63c005873fc5d67b4a44f42bb436628d7c1dc3 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36462 Reviewed-by: Michael Niewöhner Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/acpi/platform.asl M src/soc/intel/icelake/acpi/platform.asl M src/soc/intel/skylake/acpi/platform.asl 3 files changed, 0 insertions(+), 27 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, approved Michael Niewöhner: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/cannonlake/acpi/platform.asl b/src/soc/intel/cannonlake/acpi/platform.asl index 773cdcb..da61619 100644 --- a/src/soc/intel/cannonlake/acpi/platform.asl +++ b/src/soc/intel/cannonlake/acpi/platform.asl @@ -20,15 +20,6 @@ /* Generic indicator for sleep state */ #include <soc/intel/common/acpi/platform.asl>
-/* The APM port can be used for generating software SMIs */ - -OperationRegion (APMP, SystemIO, 0xb2, 2) -Field (APMP, ByteAcc, NoLock, Preserve) -{ - APMC, 8, // APM command - APMS, 8 // APM status -} - /* * The _PIC method is called by the OS to choose between interrupt * routing via the i8259 interrupt controller or the APIC. diff --git a/src/soc/intel/icelake/acpi/platform.asl b/src/soc/intel/icelake/acpi/platform.asl index 080bf7b..b89fd46 100644 --- a/src/soc/intel/icelake/acpi/platform.asl +++ b/src/soc/intel/icelake/acpi/platform.asl @@ -18,15 +18,6 @@ /* Generic indicator for sleep state */ #include <soc/intel/common/acpi/platform.asl>
-/* The APM port can be used for generating software SMIs */ - -OperationRegion (APMP, SystemIO, 0xb2, 2) -Field (APMP, ByteAcc, NoLock, Preserve) -{ - APMC, 8, // APM command - APMS, 8 // APM status -} - /* * The _PIC method is called by the OS to choose between interrupt * routing via the i8259 interrupt controller or the APIC. diff --git a/src/soc/intel/skylake/acpi/platform.asl b/src/soc/intel/skylake/acpi/platform.asl index 39debe1..19345b2 100644 --- a/src/soc/intel/skylake/acpi/platform.asl +++ b/src/soc/intel/skylake/acpi/platform.asl @@ -18,15 +18,6 @@ /* Enable ACPI _SWS methods */ #include <soc/intel/common/acpi/acpi_wake_source.asl>
-/* The APM port can be used for generating software SMIs */ - -OperationRegion (APMP, SystemIO, 0xb2, 2) -Field (APMP, ByteAcc, NoLock, Preserve) -{ - APMC, 8, // APM command - APMS, 8 // APM status -} - /* Port 80 POST */
OperationRegion (POST, SystemIO, 0x80, 1)
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Michael Niewöhner (Code Review)
-
Subrata Banik (Code Review)