Hi, On Tue, Jun 12, 2018 at 01:20:41PM -0700, Brian Norris wrote: > This driver was originally submitted for the TI BQ20Z75 battery IC > (commit a7640bfa10c5 ("power_supply: Add driver for TI BQ20Z75 gas gauge > IC")) and later renamed to express generic SBS support. While it's > mostly true that this driver implemented a standard SBS command set, it > takes liberties with the REG_MANUFACTURER_DATA register. This register > is specified in the SBS spec, but it doesn't make any mention of what > its actual contents are. > > We've sort of noticed this optionality previously, with commit > 17c6d3979e5b ("sbs-battery: make writes to ManufacturerAccess > optional"), where we found that some batteries NAK writes to this > register. > > What this really means is that so far, we've just been lucky that most > batteries have either been compatible with the TI chip, or else at least > haven't reported highly-unexpected values. > > For instance, one battery I have here seems to report either 0x0000 or > 0x0100 to the MANUFACTURER_ACCESS_STATUS command -- while this seems to > match either Wake Up (bits[11:8] = 0000b) or Normal Discharge > (bits[11:8] = 0001b) status for the TI part [1], they don't seem to > actually correspond to real states (for instance, I never see 0101b = > Charge, even when charging). > > On other batteries, I'm getting apparently random data in return, which > means that occasionally, we interpret this as "battery not present" or > "battery is not healthy". > > All in all, it seems to be a really bad idea to make assumptions about > REG_MANUFACTURER_DATA, unless we already know what battery we're using. > Therefore, this patch reimplements the "present" and "health" checks to > the following on most SBS batteries: > > 1. HEALTH: report "unknown" -- I couldn't find a standard SBS command > that gives us much useful here > 2. PRESENT: just send a REG_STATUS command; if it succeeds, then the > battery is present > > Also, we stop sending MANUFACTURER_ACCESS_SLEEP to non-TI parts. I have > no proof that this is useful and supported. > > If someone explicitly provided a 'ti,bq20z75' compatible property, then > we continue to use the existing TI command behaviors, and we effectively > revert commit 17c6d3979e5b ("sbs-battery: make writes to > ManufacturerAccess optional") to again make these commands required. > > [1] http://www.ti.com/lit/er/sluu265a/sluu265a.pdf > > Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx> > Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx> > Acked-by: Rhyland Klein <rklein@xxxxxxxxxx> > --- Thanks, queued to power-supply-next. -- Sebastian
Attachment:
signature.asc
Description: PGP signature