[coreboot-gerrit] Change in coreboot[master]: [WIP] common/dptf: Add thermal trip points change event handler

[Sumeet R Pawnikar (Code Review)]

Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/19088 )

Change subject: [WIP] common/dptf: Add thermal trip points change event handler
......................................................................


Patch Set 1:

(3 comments)

https://review.coreboot.org/#/c/19088/1/src/soc/intel/common/acpi/dptf/thermal.asl
File src/soc/intel/common/acpi/dptf/thermal.asl:

Line 64: #ifdef DPTF_TSR0_SENSOR_ID
> So we notify every single sensor when a thermal trip point has occurred? Wh
As per kernel code, only registered 0x81h notification in the INT3403 driver which covers board sensors only. With the current scheme yes it seems we will have to send 0x81h notification to every INT3403 type of device.


Line 104: #ifdef EC_ENABLE_TABLET_EVENT
> By adding this you have to fix up every single other board using this commo
I did not find any file which is using this "/src/soc/intel/common/acpi/dptf/thermal.asl". 
Am I missing any thing ?


Line 109: 			Return (CTOK (DPTF_TSR0_PASSIVE))
> Can't you just leave this at the end of the Method? The If above returns a 
Do you mean to remove 'Else' here i.e. line 107.


-- 
To view, visit https://review.coreboot.org/19088
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I207ed1c415e22e8fccede0f1373af67a072ddea1
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar at intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar at intel.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes