chgstate_set_manual_current incorrectly handles `disable` commands |
||
Issue descriptionFound during a review of charger state machine commands in the EC. common/charge_state_v2.c:chgstate_set_manual_current() does not correctly handle an argument of -1 (disable manual override). When the charger driver picks the closest current limit, it clamps the value to zero instead of passing along the -1. Thus, attempting to clear the manual current limit by just setting this single parameter to -1 fails. Workaround: `ectool chargecontrol normal` from the AP clears manual current and voltage overrides in a way that bypasses this bug.
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/7216341f4be4202df6f0dab493bb51848ed249d5 commit 7216341f4be4202df6f0dab493bb51848ed249d5 Author: Jonathan Brandmeyer <jbrandmeyer@chromium.org> Date: Tue Aug 07 08:51:19 2018 charger: Correctly clear manual current limit override The official way to clear the manual current limit is to set the override value to -1. However, paths through the host command CHARGE_STATE_CMD_SET_PARAM that attempt to set -1 instead select the "nearest" current of zero. This disables the charger instead of disabling the manual current limit override. BUG= chromium:870365 , b:111943872 BRANCH=none TEST=Use ectool to set and clear the current limit via `ectool chargestate param 1 0xffffffff` on Careena clears the limit. Signed-off-by: Jonathan Brandmeyer <jbrandmeyer@chromium.org> Change-Id: Icfd1162300dedc2d8b7bca1c95f1ff0da6aa72a3 Reviewed-on: https://chromium-review.googlesource.com/1163983 Commit-Ready: Jonathan Brandmeyer <jbrandmeyer@chromium.org> Tested-by: Jonathan Brandmeyer <jbrandmeyer@chromium.org> Reviewed-by: Edward Hill <ecgh@chromium.org> [modify] https://crrev.com/7216341f4be4202df6f0dab493bb51848ed249d5/common/charge_state_v2.c
,
Aug 7
|
||
►
Sign in to add a comment |
||
Comment 1 by jbrandmeyer@chromium.org
, Aug 2