ec: Refactor chipset_in_state() |
||||||
Issue descriptionMoved from private tracker - b/35528199 rspangler: chipset_in_state() is a little weird in its implementation, particularly in how it handles returning the state when it's in transition. It probably would be cleaner just to have power_get_state() return the current high-level state (on/suspend/off), with some predictable behavior for what it does if it's in between states (should on->suspend return "on" or "suspend"?) shawnn: One quirk of our implementation is that hook_notify() from power state machines is normally called in transitional states, but chipset_in_state() doesn't consider transitional states to be the new state. Eg. void test_resume(void) { ccprintf("Chipset on? %d\n", chipset_in_state(CHIPSET_STATE_ON)); } DECLARE_HOOK(HOOK_CHIPSET_RESUME, test_resume, HOOK_PRIO_DEFAULT); This will print "Chipset on? 0" on most platforms because we're in POWER_S3S0, not POWER_S0. It would be nice to have chipset_in_state() reflect the transition-to state so this can work elegantly.
,
Sep 18
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/502364851c44f4624c665a11bc305a561847e6f7 commit 502364851c44f4624c665a11bc305a561847e6f7 Author: Jett Rink <jettrink@chromium.org> Date: Tue Sep 18 15:38:08 2018 power: add chipset_in_or_transitioning_to_state We need a method that we can call from the chipset notify hooks that can clearly distinguish which state you are about to be in. This is made evident by the child CL for putting a MUX into low power mode in S5. Without this method, we have to put chipset state into the PD task variable and use that instead (since chipset_in_state won't work because we are in the S3S5 state) BRANCH=none BUG=b:112136208,b:111196155,chromium:736508 TEST=On Phaser the 3300_pd_a drops from 92mW to 32 mW when the charger is plugged into C1 and the SoC is in S5. The rail also says at 32mW after removing and plugging the power back in while the SoC is in S5. Also ensured that power is low upon first insertion and AP does not come on automatically. Change-Id: I93cce2aa319c9689efce222919e5389471001a00 Signed-off-by: Jett Rink <jettrink@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1211368 Reviewed-by: Justin TerAvest <teravest@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1230981 Reviewed-by: Martin Roth <martinroth@chromium.org> Commit-Queue: Martin Roth <martinroth@chromium.org> Tested-by: Martin Roth <martinroth@chromium.org> [modify] https://crrev.com/502364851c44f4624c665a11bc305a561847e6f7/power/common.c [modify] https://crrev.com/502364851c44f4624c665a11bc305a561847e6f7/include/chipset.h
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/5e33f866d4bc8f27a954ccf5fba752832d09667a commit 5e33f866d4bc8f27a954ccf5fba752832d09667a Author: Jett Rink <jettrink@chromium.org> Date: Thu Sep 27 23:20:01 2018 power: add chipset_in_or_transitioning_to_state We need a method that we can call from the chipset notify hooks that can clearly distinguish which state you are about to be in. This is made evident by the child CL for putting a MUX into low power mode in S5. Without this method, we have to put chipset state into the PD task variable and use that instead (since chipset_in_state won't work because we are in the S3S5 state) BRANCH=none BUG=b:112136208,b:111196155,chromium:736508 TEST=On Phaser the 3300_pd_a drops from 92mW to 32 mW when the charger is plugged into C1 and the SoC is in S5. The rail also says at 32mW after removing and plugging the power back in while the SoC is in S5. Also ensured that power is low upon first insertion and AP does not come on automatically. Change-Id: I93cce2aa319c9689efce222919e5389471001a00 Signed-off-by: Jett Rink <jettrink@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1211368 Reviewed-by: Justin TerAvest <teravest@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1247635 Reviewed-by: Furquan Shaikh <furquan@chromium.org> Commit-Queue: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/5e33f866d4bc8f27a954ccf5fba752832d09667a/power/common.c [modify] https://crrev.com/5e33f866d4bc8f27a954ccf5fba752832d09667a/include/chipset.h
,
Oct 1
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/c39081fd324840f24b91f7439b982109c86f1b99 commit c39081fd324840f24b91f7439b982109c86f1b99 Author: Jett Rink <jettrink@chromium.org> Date: Tue Nov 20 06:58:57 2018 power: add chipset_in_or_transitioning_to_state We need a method that we can call from the chipset notify hooks that can clearly distinguish which state you are about to be in. This is made evident by the child CL for putting a MUX into low power mode in S5. Without this method, we have to put chipset state into the PD task variable and use that instead (since chipset_in_state won't work because we are in the S3S5 state) BRANCH=none BUG=b:112136208,b:111196155,chromium:736508 TEST=On Phaser the 3300_pd_a drops from 92mW to 32 mW when the charger is plugged into C1 and the SoC is in S5. The rail also says at 32mW after removing and plugging the power back in while the SoC is in S5. Also ensured that power is low upon first insertion and AP does not come on automatically. Change-Id: I93cce2aa319c9689efce222919e5389471001a00 Signed-off-by: Jett Rink <jettrink@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1211368 Reviewed-by: Justin TerAvest <teravest@chromium.org> (cherry picked from commit 74e613842277cf0d13af3fbad6105055bc039a04) Reviewed-on: https://chromium-review.googlesource.com/c/1342733 Reviewed-by: Philip Chen <philipchen@chromium.org> Commit-Queue: Philip Chen <philipchen@chromium.org> Tested-by: Philip Chen <philipchen@chromium.org> Trybot-Ready: Philip Chen <philipchen@chromium.org> [modify] https://crrev.com/c39081fd324840f24b91f7439b982109c86f1b99/power/common.c [modify] https://crrev.com/c39081fd324840f24b91f7439b982109c86f1b99/include/chipset.h
,
Jan 11
This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Sep 8