EC virtual battery is not a virtual battery |
|||||||
Issue descriptionThe EC "virtual battery" code is supposed to avoid a few problems: 1. Avoids having two "masters" for the battery, the kernel and the EC. In the past we ran into trouble because some battery commands change the state of the battery, like the command that changes us from mA to mW. 2. Avoids having "slow" host commands where we block the whole host command interface waiting to get a lock on the i2c bus, do a transfer, and then return the result. -- The idea was that we'd always be able to "answer" an sbs battery query from the kernel by returning some sort of state that the EC already had or a constant of some sort. If there was some state that the kernel needed that the EC didn't have then the EC should query it at bootup. -- Right now, the virtual battery appears to be a mix between the above and a cached passthrough. It's a little hard for me to follow all the code, but in the very least I see i2c_xfer() calls in "virtual_battery.c" which seems like a bad sign. -- Anyway, maybe someone knows why we need the passthrough mixed behavior, but it seems like everything would be easier to reason about if we got rid of it. It would also mean that we could present "sbs_battery" to the kernel even if we didn't have an SBS battery, which seems like it would be nice.
,
Jun 22 2017
virtual_battery.c is very close to being battery-protocol-agnostic (eg. we can pretend to be a SB regardless of the actual attached battery). We just need to take care of the following two cases: 1) SB_BATTERY_MODE initial read. I think we can replace it with a call to battery_get_mode() (and maybe lie about the mode if we get back EC_ERROR_UNIMPLEMENTED). 2) "Uncached" reads. We need check what exact regs are being asked for here (can check at runtime, but also check sbs-battery.c source to make sure we're not missing any uncommon regs) and implement them. Then, deny all other reads.
,
Jun 22 2017
,
Jun 23 2017
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/67759f3553555a7ea6c3a5296934cbbed2297c68 commit 67759f3553555a7ea6c3a5296934cbbed2297c68 Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Wed Jul 05 21:24:49 2017 virtual_battery: Remove direct i2c access Virtual battery implements a smart battery interface, but the actual battery on the system may speak a different protocol. Support such batteries by removing direct i2c access from the virtual battery driver. Fetch data from storage when available, and call generic battery routines when not. BUG= chromium:717753 BRANCH=None TEST=Manual on kevin, boot and verify "Unhandled VB reg" prints are not seen. Verify by-eye that all regs in cros 4.4 kernel sbs-battery.c are handled (except REG_MANUFACTURER_DATA). Verify that sysfs manufacturer, model name, time_to_full_avg and time_to_empty_avg values are all sane. Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Change-Id: Ia8fc0a80ac7576fa8bdcc3b7dac0609d9d754234 Reviewed-on: https://chromium-review.googlesource.com/547004 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Shawn N <shawnn@chromium.org> [modify] https://crrev.com/67759f3553555a7ea6c3a5296934cbbed2297c68/common/virtual_battery.c [modify] https://crrev.com/67759f3553555a7ea6c3a5296934cbbed2297c68/common/i2c_master.c
,
Jul 11 2017
virtual_battery should now work with any cros-ec-supported battery. Reading POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG / POWER_SUPPLY_PROP_TIME_TO_FULL_AVG may still block on i2c access, but these properties are not commonly used in Chrome OS.
,
Jan 22 2018
,
Oct 12
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/39c9275c4aa77f07d824f4591f7a05095ad8766b commit 39c9275c4aa77f07d824f4591f7a05095ad8766b Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Fri Oct 12 18:20:08 2018 virtual_battery: Remove direct i2c access Virtual battery implements a smart battery interface, but the actual battery on the system may speak a different protocol. Support such batteries by removing direct i2c access from the virtual battery driver. Fetch data from storage when available, and call generic battery routines when not. Conflicts: common/virtual_battery.c (copied from upstream) BUG= chromium:717753 BRANCH=None TEST=Manual on kevin, boot and verify "Unhandled VB reg" prints are not seen. Verify by-eye that all regs in cros 4.4 kernel sbs-battery.c are handled (except REG_MANUFACTURER_DATA). Verify that sysfs manufacturer, model name, time_to_full_avg and time_to_empty_avg values are all sane. Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Change-Id: Ia8fc0a80ac7576fa8bdcc3b7dac0609d9d754234 Reviewed-on: https://chromium-review.googlesource.com/547004 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Shawn N <shawnn@chromium.org> (cherry picked from commit 67759f3553555a7ea6c3a5296934cbbed2297c68) Reviewed-on: https://chromium-review.googlesource.com/c/1277617 Commit-Queue: Vadim Bendebury <vbendeb@chromium.org> Tested-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-by: Vadim Bendebury <vbendeb@chromium.org> [modify] https://crrev.com/39c9275c4aa77f07d824f4591f7a05095ad8766b/common/virtual_battery.c [modify] https://crrev.com/39c9275c4aa77f07d824f4591f7a05095ad8766b/common/i2c_master.c
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/178f87d220cf388f3aa4071a83f4ff85c3137312 commit 178f87d220cf388f3aa4071a83f4ff85c3137312 Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Fri Dec 07 00:18:01 2018 virtual_battery: Remove direct i2c access Virtual battery implements a smart battery interface, but the actual battery on the system may speak a different protocol. Support such batteries by removing direct i2c access from the virtual battery driver. Fetch data from storage when available, and call generic battery routines when not. Conflicts: common/virtual_battery.c (copied from upstream) BUG= chromium:717753 BRANCH=None TEST=Manual on kevin, boot and verify "Unhandled VB reg" prints are not seen. Verify by-eye that all regs in cros 4.4 kernel sbs-battery.c are handled (except REG_MANUFACTURER_DATA). Verify that sysfs manufacturer, model name, time_to_full_avg and time_to_empty_avg values are all sane. Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Change-Id: Ia8fc0a80ac7576fa8bdcc3b7dac0609d9d754234 Reviewed-on: https://chromium-review.googlesource.com/547004 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Shawn N <shawnn@chromium.org> (cherry picked from commit 67759f3553555a7ea6c3a5296934cbbed2297c68) Reviewed-on: https://chromium-review.googlesource.com/c/1277617 Commit-Queue: Vadim Bendebury <vbendeb@chromium.org> Tested-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-by: Vadim Bendebury <vbendeb@chromium.org> (cherry picked from commit 39c9275c4aa77f07d824f4591f7a05095ad8766b) Reviewed-on: https://chromium-review.googlesource.com/c/1366958 [modify] https://crrev.com/178f87d220cf388f3aa4071a83f4ff85c3137312/common/virtual_battery.c [modify] https://crrev.com/178f87d220cf388f3aa4071a83f4ff85c3137312/common/i2c_master.c |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by mruthven@chromium.org
, May 2 2017