Hello Frans Hendriks,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/37560
to review the following change.
Change subject: src: Conditionally include TEVT ......................................................................
src: Conditionally include TEVT
ACPI method TEVT is reported as unused when ChromeEC support is not enabled.
The TEVT code in mainboard or SoC is only enabled when an EC is used that uses this event.
The TEVT code in the EC is only enabled if the mainboard or SoC code implements TEVT.
BUG=N/A TEST=Tested on facebook monolith.
Change-Id: I8d2e14407ae2338e58797cdc7eb7d0cadf3cc26e Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/ec/acpi/Kconfig M src/ec/google/chromeec/Kconfig M src/ec/google/wilco/Kconfig M src/ec/google/wilco/acpi/dptf.asl M src/mainboard/google/cyan/acpi/dptf.asl M src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl M src/soc/intel/baytrail/acpi/dptf/thermal.asl M src/soc/intel/braswell/acpi/dptf/thermal.asl M src/soc/intel/common/acpi/dptf/thermal.asl M src/soc/intel/skylake/acpi/dptf/thermal.asl 10 files changed, 32 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37560/1
diff --git a/src/ec/acpi/Kconfig b/src/ec/acpi/Kconfig index 3081a86..1fa707a 100644 --- a/src/ec/acpi/Kconfig +++ b/src/ec/acpi/Kconfig @@ -2,3 +2,9 @@ bool help ACPI Embedded Controller interface. Mostly found in laptops. + +config EC_SUPPORTS_DPTF_TEVT + bool + help + The EC ASL code supports calling of TEVT method when provided by + SoC or mainboard. diff --git a/src/ec/google/chromeec/Kconfig b/src/ec/google/chromeec/Kconfig index 2eb3b95..0d8c7e2 100644 --- a/src/ec/google/chromeec/Kconfig +++ b/src/ec/google/chromeec/Kconfig @@ -190,3 +190,8 @@ help Enable support for Chrome OS mode switches provided by the Chrome OS EC. + +config EC_SUPPORTS_DPTF_TEVT + bool + depends on EC_GOOGLE_CHROMEEC + default y diff --git a/src/ec/google/wilco/Kconfig b/src/ec/google/wilco/Kconfig index 25d7cfa..feca2f5 100644 --- a/src/ec/google/wilco/Kconfig +++ b/src/ec/google/wilco/Kconfig @@ -49,4 +49,8 @@ with the host command and data registers to drive the EC mailbox interface. This is also the MEC EMI base address.
+config EC_SUPPORTS_DPTF_TEVT + bool + default y + endif # EC_GOOGLE_WILCO diff --git a/src/ec/google/wilco/acpi/dptf.asl b/src/ec/google/wilco/acpi/dptf.asl index 0f1663f..42fc9fd 100644 --- a/src/ec/google/wilco/acpi/dptf.asl +++ b/src/ec/google/wilco/acpi/dptf.asl @@ -115,8 +115,10 @@ /* Handle bits that are set */ While (FindSetRightBit (Local1, Local2)) { +#ifdef HAVE_THERM_EVENT_HANDLER /* DPTF will Notify sensor devices */ _SB.DPTF.TEVT (Local2) +#endif
/* Clear current sensor number */ Local1 &= ~(1 << (Local2 - 1)) diff --git a/src/mainboard/google/cyan/acpi/dptf.asl b/src/mainboard/google/cyan/acpi/dptf.asl index 70ab862..81e9fee 100644 --- a/src/mainboard/google/cyan/acpi/dptf.asl +++ b/src/mainboard/google/cyan/acpi/dptf.asl @@ -15,8 +15,6 @@ * GNU General Public License for more details. */
-#define HAVE_THERM_EVENT_HANDLER - /* Include Variant DPTF */ #include <variant/acpi/dptf.asl>
diff --git a/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl b/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl index 1ff308d..77482a4 100644 --- a/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl +++ b/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl @@ -16,6 +16,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) @@ -36,6 +38,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI) diff --git a/src/soc/intel/baytrail/acpi/dptf/thermal.asl b/src/soc/intel/baytrail/acpi/dptf/thermal.asl index d84ae4b..106cd77 100644 --- a/src/soc/intel/baytrail/acpi/dptf/thermal.asl +++ b/src/soc/intel/baytrail/acpi/dptf/thermal.asl @@ -14,6 +14,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) @@ -34,6 +36,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI) diff --git a/src/soc/intel/braswell/acpi/dptf/thermal.asl b/src/soc/intel/braswell/acpi/dptf/thermal.asl index 1fdbea0..c993b5b 100644 --- a/src/soc/intel/braswell/acpi/dptf/thermal.asl +++ b/src/soc/intel/braswell/acpi/dptf/thermal.asl @@ -15,7 +15,9 @@ */
/* Thermal Threshold Event Handler */ -#ifdef HAVE_THERM_EVENT_HANDLER +define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) diff --git a/src/soc/intel/common/acpi/dptf/thermal.asl b/src/soc/intel/common/acpi/dptf/thermal.asl index d41f623..7058b27 100644 --- a/src/soc/intel/common/acpi/dptf/thermal.asl +++ b/src/soc/intel/common/acpi/dptf/thermal.asl @@ -16,6 +16,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) @@ -41,6 +43,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI) diff --git a/src/soc/intel/skylake/acpi/dptf/thermal.asl b/src/soc/intel/skylake/acpi/dptf/thermal.asl index 5f3548e..742b092 100644 --- a/src/soc/intel/skylake/acpi/dptf/thermal.asl +++ b/src/soc/intel/skylake/acpi/dptf/thermal.asl @@ -16,6 +16,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) {
@@ -40,6 +42,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI)
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 2:
it seems like the easiest/cleanest solution would be to just replace `HAVE_THERM_EVENT_HANDLER` with `EC_SUPPORTS_DPTF_TEVT,` select it automatically when chrome-ec or wilco is selected, and guard the ACPI code accordingly. Any reason you didn't go that route?
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 2:
Patch Set 2:
it seems like the easiest/cleanest solution would be to just replace `HAVE_THERM_EVENT_HANDLER` with `EC_SUPPORTS_DPTF_TEVT,` select it automatically when chrome-ec or wilco is selected, and guard the ACPI code accordingly. Any reason you didn't go that route?
Our conversation did not end up here so I am adding it.
On Sat, Dec 7, 2019 at 4:41 AM Wim Vervoorn wvervoorn@eltan.com wrote:
The reason for not doing this is that is that we have two things to consider. The first is that the ec supports calling the tevt method. The 2nd is that the asl code for the platform actually supports implements the tevt method in asl code.
With the solution we created both dependencies are addressed.
I hope this clarifies the reasoning.
On Sat, Dec 7, 2019 at 18:40 Matt DeVillier wrote: perhaps I am misunderstanding the purpose of the patch. Are you wanting to decouple DPTF >support from TEVT?
The existing code utilized HAVE_THERM_EVENT_HANDLER define to enable calling TEVT from the chromeec dptf asl code. The HAVE_THERM_EVENT_HANDLER define is created in chipset or board asl code at the location where the TEVT event is defined. This is fine for all boards that are using chromeec. None of the other EC code implements calling the TEVT event and you end up with a method that will never be called. To prevent this situation (and the warning generated because of it) we added this dependency.
Additionally we changed the code for the google wilco ec as this called the TEVT method unconditionally. Now this is inline with the chromeec asl code.
Hello Patrick Rudolph, Frans Hendriks, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37560
to look at the new patch set (#3).
Change subject: src: Conditionally include TEVT ......................................................................
src: Conditionally include TEVT
ACPI method TEVT is reported as unused when ChromeEC support is not enabled.
The TEVT code in mainboard or SoC is only enabled when an EC is used that uses this event.
The TEVT code in the EC is only enabled if the mainboard or SoC code implements TEVT.
BUG=N/A TEST=Tested on facebook monolith.
Change-Id: I8d2e14407ae2338e58797cdc7eb7d0cadf3cc26e Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/ec/acpi/Kconfig M src/ec/google/chromeec/Kconfig M src/ec/google/wilco/Kconfig M src/ec/google/wilco/acpi/dptf.asl M src/mainboard/google/cyan/acpi/dptf.asl M src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl M src/soc/intel/baytrail/acpi/dptf/thermal.asl M src/soc/intel/braswell/acpi/dptf/thermal.asl M src/soc/intel/common/acpi/dptf/thermal.asl M src/soc/intel/skylake/acpi/dptf/thermal.asl 10 files changed, 32 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37560/3
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 3:
Patch Set 2:
On Sat, Dec 7, 2019 at 18:40 Matt DeVillier wrote: perhaps I am misunderstanding the purpose of the patch. Are you wanting to decouple DPTF >support from TEVT?
The existing code utilized HAVE_THERM_EVENT_HANDLER define to enable calling TEVT from the chromeec dptf asl code. The HAVE_THERM_EVENT_HANDLER define is created in chipset or board asl code at the location where the TEVT event is defined. This is fine for all boards that are using chromeec. None of the other EC code implements calling the TEVT event and you end up with a method that will never be called. To prevent this situation (and the warning generated because of it) we added this dependency.
Additionally we changed the code for the google wilco ec as this called the TEVT method unconditionally. Now this is inline with the chromeec asl code.
if
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/chromeec/Kcon... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/chromeec/Kcon... PS3, Line 194: EC_SUPPORTS_DPTF_TEVT seems like this should be inserted at ln 3 'select EC_SUPPORTS_DPTF_TEVT' rather than be a separate, non-user selectable config item. Same for Wilco
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/wilco/acpi/dp... File src/ec/google/wilco/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/wilco/acpi/dp... PS3, Line 118: #ifdef HAVE_THERM_EVENT_HANDLER : /* DPTF will Notify sensor devices */ : _SB.DPTF.TEVT (Local2) : #endif seems unnecessary if all wilco EC boards/platforms have TEVT support, which appears to be the case
Hello Patrick Rudolph, Frans Hendriks, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37560
to look at the new patch set (#4).
Change subject: src: Conditionally include TEVT ......................................................................
src: Conditionally include TEVT
ACPI method TEVT is reported as unused when ChromeEC support is not enabled.
The TEVT code in mainboard or SoC is only enabled when an EC is used that uses this event.
The TEVT code in the EC is only enabled if the mainboard or SoC code implements TEVT.
BUG=N/A TEST=Tested on facebook monolith.
Change-Id: I8d2e14407ae2338e58797cdc7eb7d0cadf3cc26e Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/ec/acpi/Kconfig M src/ec/google/chromeec/Kconfig M src/ec/google/wilco/Kconfig M src/ec/google/wilco/acpi/dptf.asl M src/mainboard/google/cyan/acpi/dptf.asl M src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl M src/soc/intel/baytrail/acpi/dptf/thermal.asl M src/soc/intel/braswell/acpi/dptf/thermal.asl M src/soc/intel/common/acpi/dptf/thermal.asl M src/soc/intel/skylake/acpi/dptf/thermal.asl 10 files changed, 25 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37560/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/chromeec/Kcon... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/chromeec/Kcon... PS4, Line 3: select EC_SUPPORTS_DPTF_TEVT trailing whitespace
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/wilco/Kconfig File src/ec/google/wilco/Kconfig:
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/wilco/Kconfig... PS4, Line 6: select EC_SUPPORTS_DPTF_TEVT trailing whitespace
Hello Patrick Rudolph, Frans Hendriks, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37560
to look at the new patch set (#5).
Change subject: src: Conditionally include TEVT ......................................................................
src: Conditionally include TEVT
ACPI method TEVT is reported as unused when ChromeEC support is not enabled.
The TEVT code in mainboard or SoC is only enabled when an EC is used that uses this event.
The TEVT code in the EC is only enabled if the mainboard or SoC code implements TEVT.
BUG=N/A TEST=Tested on facebook monolith.
Change-Id: I8d2e14407ae2338e58797cdc7eb7d0cadf3cc26e Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/ec/acpi/Kconfig M src/ec/google/chromeec/Kconfig M src/ec/google/wilco/Kconfig M src/ec/google/wilco/acpi/dptf.asl M src/mainboard/google/cyan/acpi/dptf.asl M src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl M src/soc/intel/baytrail/acpi/dptf/thermal.asl M src/soc/intel/braswell/acpi/dptf/thermal.asl M src/soc/intel/common/acpi/dptf/thermal.asl M src/soc/intel/skylake/acpi/dptf/thermal.asl 10 files changed, 25 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37560/5
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/chromeec/Kcon... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/chromeec/Kcon... PS3, Line 194: EC_SUPPORTS_DPTF_TEVT
seems like this should be inserted at ln 3 'select EC_SUPPORTS_DPTF_TEVT' rather than be a separate, […]
Done
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/chromeec/Kcon... PS3, Line 194: EC_SUPPORTS_DPTF_TEVT
seems like this should be inserted at ln 3 'select EC_SUPPORTS_DPTF_TEVT' rather than be a separate, […]
You are absolutely right, corrected this.
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/chromeec/Kcon... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/chromeec/Kcon... PS4, Line 3: select EC_SUPPORTS_DPTF_TEVT
trailing whitespace
Done
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/wilco/Kconfig File src/ec/google/wilco/Kconfig:
https://review.coreboot.org/c/coreboot/+/37560/4/src/ec/google/wilco/Kconfig... PS4, Line 6: select EC_SUPPORTS_DPTF_TEVT
trailing whitespace
Done
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/wilco/acpi/dp... File src/ec/google/wilco/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/37560/3/src/ec/google/wilco/acpi/dp... PS3, Line 118: #ifdef HAVE_THERM_EVENT_HANDLER : /* DPTF will Notify sensor devices */ : _SB.DPTF.TEVT (Local2) : #endif
seems unnecessary if all wilco EC boards/platforms have TEVT support, which appears to be the case
It is unnecessary at this moment. We did this to keep the implementation consistent and make sure no issues pop up if someone tries to use this ec on another platform or for some reason doesn't want to support dptf.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG@10 PS5, Line 10: enabled. Which tool reports this? iasl? Add the error message?
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG@11 PS5, Line 11: : The TEVT code in mainboard or SoC is only enabled when an EC is used : that uses this event. : : The TEVT code in the EC is only enabled if the mainboard or SoC code : implements TEVT. That is the description of the fix, and the first paragraph is the problem description?
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG@17 PS5, Line 17: What is the net result? For google/cyan it’s removed?
Hello Patrick Rudolph, Frans Hendriks, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37560
to look at the new patch set (#6).
Change subject: src: Conditionally include TEVT ......................................................................
src: Conditionally include TEVT
ACPI method TEVT is reported as unused by iASL (20190509) when ChromeEC support is not enabled. The message is “Method Argument is never used (Arg0)” on Method (TEVT, 1, NotSerialized), which indicates the TEVT method is empty.
The solution is to only enable the TEVT code in mainboard or SoC when an EC is used that uses this event. The TEVT code in the EC is only enabled if the mainboard or SoC code implements TEVT.
The TEVT method will be removed from the ASL code when the EC does not support TEVT.
BUG=N/A TEST=Tested on facebook monolith.
Change-Id: I8d2e14407ae2338e58797cdc7eb7d0cadf3cc26e Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/ec/acpi/Kconfig M src/ec/google/chromeec/Kconfig M src/ec/google/wilco/Kconfig M src/ec/google/wilco/acpi/dptf.asl M src/mainboard/google/cyan/acpi/dptf.asl M src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl M src/soc/intel/baytrail/acpi/dptf/thermal.asl M src/soc/intel/braswell/acpi/dptf/thermal.asl M src/soc/intel/common/acpi/dptf/thermal.asl M src/soc/intel/skylake/acpi/dptf/thermal.asl 10 files changed, 25 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/37560/6
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG@10 PS5, Line 10: enabled.
Which tool reports this? iasl? Add the error message?
Done
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG@11 PS5, Line 11: : The TEVT code in mainboard or SoC is only enabled when an EC is used : that uses this event. : : The TEVT code in the EC is only enabled if the mainboard or SoC code : implements TEVT.
That is the description of the fix, and the first paragraph is the problem description?
Done
https://review.coreboot.org/c/coreboot/+/37560/5//COMMIT_MSG@17 PS5, Line 17:
What is the net result? For google/cyan it’s removed?
Done
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
Patch Set 7: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37560 )
Change subject: src: Conditionally include TEVT ......................................................................
src: Conditionally include TEVT
ACPI method TEVT is reported as unused by iASL (20190509) when ChromeEC support is not enabled. The message is “Method Argument is never used (Arg0)” on Method (TEVT, 1, NotSerialized), which indicates the TEVT method is empty.
The solution is to only enable the TEVT code in mainboard or SoC when an EC is used that uses this event. The TEVT code in the EC is only enabled if the mainboard or SoC code implements TEVT.
The TEVT method will be removed from the ASL code when the EC does not support TEVT.
BUG=N/A TEST=Tested on facebook monolith.
Change-Id: I8d2e14407ae2338e58797cdc7eb7d0cadf3cc26e Signed-off-by: Wim Vervoorn wvervoorn@eltan.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37560 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Frans Hendriks fhendriks@eltan.com --- M src/ec/acpi/Kconfig M src/ec/google/chromeec/Kconfig M src/ec/google/wilco/Kconfig M src/ec/google/wilco/acpi/dptf.asl M src/mainboard/google/cyan/acpi/dptf.asl M src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl M src/soc/intel/baytrail/acpi/dptf/thermal.asl M src/soc/intel/braswell/acpi/dptf/thermal.asl M src/soc/intel/common/acpi/dptf/thermal.asl M src/soc/intel/skylake/acpi/dptf/thermal.asl 10 files changed, 25 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Frans Hendriks: Looks good to me, approved
diff --git a/src/ec/acpi/Kconfig b/src/ec/acpi/Kconfig index 3081a86..1fa707a 100644 --- a/src/ec/acpi/Kconfig +++ b/src/ec/acpi/Kconfig @@ -2,3 +2,9 @@ bool help ACPI Embedded Controller interface. Mostly found in laptops. + +config EC_SUPPORTS_DPTF_TEVT + bool + help + The EC ASL code supports calling of TEVT method when provided by + SoC or mainboard. diff --git a/src/ec/google/chromeec/Kconfig b/src/ec/google/chromeec/Kconfig index 2eb3b95..b33864f 100644 --- a/src/ec/google/chromeec/Kconfig +++ b/src/ec/google/chromeec/Kconfig @@ -1,5 +1,6 @@ config EC_GOOGLE_CHROMEEC bool + select EC_SUPPORTS_DPTF_TEVT help Google's Chrome EC
diff --git a/src/ec/google/wilco/Kconfig b/src/ec/google/wilco/Kconfig index 25d7cfa..ee7b556 100644 --- a/src/ec/google/wilco/Kconfig +++ b/src/ec/google/wilco/Kconfig @@ -3,6 +3,7 @@ default n select EC_GOOGLE_COMMON_MEC select EC_ACPI + select EC_SUPPORTS_DPTF_TEVT help Google Wilco Embedded Controller interface.
diff --git a/src/ec/google/wilco/acpi/dptf.asl b/src/ec/google/wilco/acpi/dptf.asl index 0f1663f..42fc9fd 100644 --- a/src/ec/google/wilco/acpi/dptf.asl +++ b/src/ec/google/wilco/acpi/dptf.asl @@ -115,8 +115,10 @@ /* Handle bits that are set */ While (FindSetRightBit (Local1, Local2)) { +#ifdef HAVE_THERM_EVENT_HANDLER /* DPTF will Notify sensor devices */ _SB.DPTF.TEVT (Local2) +#endif
/* Clear current sensor number */ Local1 &= ~(1 << (Local2 - 1)) diff --git a/src/mainboard/google/cyan/acpi/dptf.asl b/src/mainboard/google/cyan/acpi/dptf.asl index 70ab862..81e9fee 100644 --- a/src/mainboard/google/cyan/acpi/dptf.asl +++ b/src/mainboard/google/cyan/acpi/dptf.asl @@ -15,8 +15,6 @@ * GNU General Public License for more details. */
-#define HAVE_THERM_EVENT_HANDLER - /* Include Variant DPTF */ #include <variant/acpi/dptf.asl>
diff --git a/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl b/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl index 1ff308d..77482a4 100644 --- a/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl +++ b/src/mainboard/google/cyan/variants/terra/include/variant/acpi/thermal.asl @@ -16,6 +16,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) @@ -36,6 +38,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI) diff --git a/src/soc/intel/baytrail/acpi/dptf/thermal.asl b/src/soc/intel/baytrail/acpi/dptf/thermal.asl index d84ae4b..106cd77 100644 --- a/src/soc/intel/baytrail/acpi/dptf/thermal.asl +++ b/src/soc/intel/baytrail/acpi/dptf/thermal.asl @@ -14,6 +14,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) @@ -34,6 +36,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI) diff --git a/src/soc/intel/braswell/acpi/dptf/thermal.asl b/src/soc/intel/braswell/acpi/dptf/thermal.asl index 1fdbea0..7daa36c 100644 --- a/src/soc/intel/braswell/acpi/dptf/thermal.asl +++ b/src/soc/intel/braswell/acpi/dptf/thermal.asl @@ -15,7 +15,9 @@ */
/* Thermal Threshold Event Handler */ -#ifdef HAVE_THERM_EVENT_HANDLER +#define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) diff --git a/src/soc/intel/common/acpi/dptf/thermal.asl b/src/soc/intel/common/acpi/dptf/thermal.asl index d41f623..7058b27 100644 --- a/src/soc/intel/common/acpi/dptf/thermal.asl +++ b/src/soc/intel/common/acpi/dptf/thermal.asl @@ -16,6 +16,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) { Store (ToInteger (Arg0), Local0) @@ -41,6 +43,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI) diff --git a/src/soc/intel/skylake/acpi/dptf/thermal.asl b/src/soc/intel/skylake/acpi/dptf/thermal.asl index 5f3548e..742b092 100644 --- a/src/soc/intel/skylake/acpi/dptf/thermal.asl +++ b/src/soc/intel/skylake/acpi/dptf/thermal.asl @@ -16,6 +16,8 @@
/* Thermal Threshold Event Handler */ #define HAVE_THERM_EVENT_HANDLER + +#if CONFIG(EC_SUPPORTS_DPTF_TEVT) Method (TEVT, 1, NotSerialized) {
@@ -40,6 +42,7 @@ } #endif } +#endif
/* Thermal device initialization - Disable Aux Trip Points */ Method (TINI)
-
build bot (Jenkins) (Code Review) -
Frans Hendriks (Code Review)
-
Matt DeVillier (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Wim Vervoorn (Code Review)