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.