power_supply_info and sysfs contains nonsense for gru variants |
|||||||
Issue descriptionIf you run: power_supply_info On any gru- variant (like gru-kevin), you get nonsense for the energy values. Similarly they are nonsense in sysfs (/sys/class/power_supply/sbs-9-000b) --- Fix incoming
,
May 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/01ceab68cd6b542f8c6355425e6ac6da698e0ebf commit 01ceab68cd6b542f8c6355425e6ac6da698e0ebf Author: Douglas Anderson <dianders@chromium.org> Date: Mon May 01 23:59:43 2017 virtual_battery: Fix energy readings The virtual battery "energy" readings were totally broken. Rather than reporting things in units of "10 mW" they were reporting things in units of "10 uW". That's because they were doing this math: result = mV * mA / 10 Said another way: result = (V / 1000) * (A / 1000) / 10 result = (V * A) / (100000) / 10 result = W / 1000000 / 10 result = uW / 10 Aside from the fact that clients were expecting things in "10 mW" instead of "10 uW", we got even more random results. That's because we return to the client in a 16-bit variable, so we were kinda randomly truncating things. Doh. BRANCH=ToT BUG= chromium:717304 TEST=power_supply_info Unfortunately when you try to report sane values for "10 uA" in a 16-bit result, it doesn't work too well ( Change-Id: I8075dffd7ab6b372be5b8fdf293acc96c5878036 Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/492546 Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/01ceab68cd6b542f8c6355425e6ac6da698e0ebf/common/virtual_battery.c
,
May 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/520b8e53bc6a8c8844e6034d97cadff0c89585db commit 520b8e53bc6a8c8844e6034d97cadff0c89585db Author: Douglas Anderson <dianders@chromium.org> Date: Tue May 02 00:08:28 2017 virtual_battery: Fix energy readings The virtual battery "energy" readings were totally broken. Rather than reporting things in units of "10 mW" they were reporting things in units of "10 uW". That's because they were doing this math: result = mV * mA / 10 Said another way: result = (V / 1000) * (A / 1000) / 10 result = (V * A) / (100000) / 10 result = W / 1000000 / 10 result = uW / 10 Aside from the fact that clients were expecting things in "10 mW" instead of "10 uW", we got even more random results. That's because we return to the client in a 16-bit variable, so we were kinda randomly truncating things. Doh. BRANCH=ToT BUG= chromium:717304 TEST=power_supply_info Unfortunately when you try to report sane values for "10 uA" in a 16-bit result, it doesn't work too well ( Change-Id: I8075dffd7ab6b372be5b8fdf293acc96c5878036 Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/492546 Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> (cherry picked from commit 01ceab68cd6b542f8c6355425e6ac6da698e0ebf) Reviewed-on: https://chromium-review.googlesource.com/492567 Commit-Queue: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Trybot-Ready: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/520b8e53bc6a8c8844e6034d97cadff0c89585db/common/virtual_battery.c
,
May 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/520b8e53bc6a8c8844e6034d97cadff0c89585db commit 520b8e53bc6a8c8844e6034d97cadff0c89585db Author: Douglas Anderson <dianders@chromium.org> Date: Tue May 02 00:08:28 2017 virtual_battery: Fix energy readings The virtual battery "energy" readings were totally broken. Rather than reporting things in units of "10 mW" they were reporting things in units of "10 uW". That's because they were doing this math: result = mV * mA / 10 Said another way: result = (V / 1000) * (A / 1000) / 10 result = (V * A) / (100000) / 10 result = W / 1000000 / 10 result = uW / 10 Aside from the fact that clients were expecting things in "10 mW" instead of "10 uW", we got even more random results. That's because we return to the client in a 16-bit variable, so we were kinda randomly truncating things. Doh. BRANCH=ToT BUG= chromium:717304 TEST=power_supply_info Unfortunately when you try to report sane values for "10 uA" in a 16-bit result, it doesn't work too well ( Change-Id: I8075dffd7ab6b372be5b8fdf293acc96c5878036 Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/492546 Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> (cherry picked from commit 01ceab68cd6b542f8c6355425e6ac6da698e0ebf) Reviewed-on: https://chromium-review.googlesource.com/492567 Commit-Queue: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Trybot-Ready: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/520b8e53bc6a8c8844e6034d97cadff0c89585db/common/virtual_battery.c
,
May 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/f1e98218776de9380141384af4ffc9611f6f4adc commit f1e98218776de9380141384af4ffc9611f6f4adc Author: Douglas Anderson <dianders@chromium.org> Date: Tue May 02 04:54:05 2017 virtual_battery: Fix energy readings The virtual battery "energy" readings were totally broken. Rather than reporting things in units of "10 mW" they were reporting things in units of "10 uW". That's because they were doing this math: result = mV * mA / 10 Said another way: result = (V / 1000) * (A / 1000) / 10 result = (V * A) / (100000) / 10 result = W / 1000000 / 10 result = uW / 10 Aside from the fact that clients were expecting things in "10 mW" instead of "10 uW", we got even more random results. That's because we return to the client in a 16-bit variable, so we were kinda randomly truncating things. Doh. BRANCH=ToT BUG= chromium:717304 TEST=power_supply_info Unfortunately when you try to report sane values for "10 uA" in a 16-bit result, it doesn't work too well ( Change-Id: I8075dffd7ab6b372be5b8fdf293acc96c5878036 Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/492546 Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> (cherry picked from commit 01ceab68cd6b542f8c6355425e6ac6da698e0ebf) Reviewed-on: https://chromium-review.googlesource.com/492568 Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/f1e98218776de9380141384af4ffc9611f6f4adc/common/virtual_battery.c
,
May 2 2017
Fixed in the source code. At some point we'll need a firmware spin. Do folks think this is urgent enough to kick one off, or should we just wait for the next one?
,
May 2 2017
Since this issue doesn't often make noticeable impact to users, IMHO we can wait for the next time.
,
Jun 19 2017
Since it's been over a month and we haven't had any other reason for a firmware update, do we want to reconsider and request a firmware spin for this?
,
Jun 19 2017
SGTM
,
Jun 19 2017
Philip: do you want to kick off this effort? Do we want to target M-61, or M-62?
,
Jun 19 2017
Sure. I will submit a request for RW FW qual today.
,
Jun 19 2017
FW qual request submitted, targeting at M61: https://cros-goldeneye.corp.google.com/chromeos/console/firmwareQualEditDetails?firmwareQualId=289
,
Jul 7 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/overlays/overlay-kevin-private/+/51117f6d938ac542f9e37c4ae763cd876f64996b commit 51117f6d938ac542f9e37c4ae763cd876f64996b Author: Philip Chen <philipchen@google.com> Date: Fri Jul 07 04:13:52 2017
,
Jul 10 2017
Should be verified in M-61.
,
Aug 1 2017
,
Jan 22 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by diand...@chromium.org
, May 1 2017