* [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver
@ 2015-12-17 10:12 H. Nikolaus Schaller
2015-12-17 10:12 ` [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 H. Nikolaus Schaller
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: H. Nikolaus Schaller @ 2015-12-17 10:12 UTC (permalink / raw)
To: Pali Rohár, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse
Cc: linux-pm, linux-kernel, gta04-owner, marek, H. Nikolaus Schaller
H. Nikolaus Schaller (2):
power:bq27xxx: fix reading for bq27000 and bq27010
power:bq27xxx: fix register numbers of bq27500
drivers/power/bq27xxx_battery.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
--
2.5.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 2015-12-17 10:12 [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver H. Nikolaus Schaller @ 2015-12-17 10:12 ` H. Nikolaus Schaller 2015-12-24 2:46 ` Andrew F. Davis 2015-12-25 13:20 ` Pali Rohár 2015-12-17 10:12 ` [PATCH 2/2] power:bq27xxx: fix register numbers of bq27500 H. Nikolaus Schaller 2015-12-19 8:00 ` [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver Sebastian Reichel 2 siblings, 2 replies; 9+ messages in thread From: H. Nikolaus Schaller @ 2015-12-17 10:12 UTC (permalink / raw) To: Pali Rohár, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, linux-kernel, gta04-owner, marek, H. Nikolaus Schaller bug: the driver reports funny capacity values: root@letux:/sys/class/power_supply/bq27000-battery# cat uevent POWER_SUPPLY_NAME=bq27000-battery POWER_SUPPLY_STATUS=Charging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3702000 POWER_SUPPLY_CURRENT_NOW=-464635 POWER_SUPPLY_CAPACITY=1536 <- over 100% is magic POWER_SUPPLY_CAPACITY_LEVEL=Normal POWER_SUPPLY_TEMP=311 POWER_SUPPLY_TIME_TO_FULL_NOW=10440 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=805450 POWER_SUPPLY_CHARGE_NOW=1068 POWER_SUPPLY_CHARGE_FULL_DESIGN=8844998 <- battery has just 1200 mAh POWER_SUPPLY_CYCLE_COUNT=21 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=0 POWER_SUPPLY_HEALTH=Good POWER_SUPPLY_MANUFACTURER=Texas Instruments reason: the state of charge and the design capacity register are single byte only. The design capacity returns the higer order byte. tested: GTA04 with Openmoko/FIC HF08x battery (using hdq) Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- drivers/power/bq27xxx_battery.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/power/bq27xxx_battery.c b/drivers/power/bq27xxx_battery.c index 880233c..e54a125 100644 --- a/drivers/power/bq27xxx_battery.c +++ b/drivers/power/bq27xxx_battery.c @@ -471,7 +471,10 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di) { int soc; - soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false); + if (di->chip == BQ27000 || di->chip == BQ27010) + soc = bq27xxx_read(di, BQ27XXX_REG_SOC, true); + else + soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false); if (soc < 0) dev_dbg(di->dev, "error reading State-of-Charge\n"); @@ -536,7 +539,10 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di) { int dcap; - dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false); + if (di->chip == BQ27000 || di->chip == BQ27010) + dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, true); + else + dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false); if (dcap < 0) { dev_dbg(di->dev, "error reading initial last measured discharge\n"); @@ -544,7 +550,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di) } if (di->chip == BQ27000 || di->chip == BQ27010) - dcap *= BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS; + dcap = (dcap << 8) * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS; else dcap *= 1000; -- 2.5.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 2015-12-17 10:12 ` [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 H. Nikolaus Schaller @ 2015-12-24 2:46 ` Andrew F. Davis 2015-12-25 13:20 ` Pali Rohár 1 sibling, 0 replies; 9+ messages in thread From: Andrew F. Davis @ 2015-12-24 2:46 UTC (permalink / raw) To: H. Nikolaus Schaller, Pali Rohár, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, linux-kernel, gta04-owner, marek On 12/17/2015 04:12 AM, H. Nikolaus Schaller wrote: > bug: the driver reports funny capacity values: > > root@letux:/sys/class/power_supply/bq27000-battery# cat uevent > POWER_SUPPLY_NAME=bq27000-battery > POWER_SUPPLY_STATUS=Charging > POWER_SUPPLY_PRESENT=1 > POWER_SUPPLY_VOLTAGE_NOW=3702000 > POWER_SUPPLY_CURRENT_NOW=-464635 > POWER_SUPPLY_CAPACITY=1536 <- over 100% is magic > POWER_SUPPLY_CAPACITY_LEVEL=Normal > POWER_SUPPLY_TEMP=311 > POWER_SUPPLY_TIME_TO_FULL_NOW=10440 > POWER_SUPPLY_TECHNOLOGY=Li-ion > POWER_SUPPLY_CHARGE_FULL=805450 > POWER_SUPPLY_CHARGE_NOW=1068 > POWER_SUPPLY_CHARGE_FULL_DESIGN=8844998 <- battery has just 1200 mAh > POWER_SUPPLY_CYCLE_COUNT=21 > POWER_SUPPLY_ENERGY_NOW=0 > POWER_SUPPLY_POWER_AVG=0 > POWER_SUPPLY_HEALTH=Good > POWER_SUPPLY_MANUFACTURER=Texas Instruments > > reason: the state of charge and the design capacity register are single > byte only. The design capacity returns the higer order byte. > > tested: GTA04 with Openmoko/FIC HF08x battery (using hdq) > > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > drivers/power/bq27xxx_battery.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/bq27xxx_battery.c b/drivers/power/bq27xxx_battery.c > index 880233c..e54a125 100644 > --- a/drivers/power/bq27xxx_battery.c > +++ b/drivers/power/bq27xxx_battery.c > @@ -471,7 +471,10 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di) > { > int soc; > > - soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false); > + if (di->chip == BQ27000 || di->chip == BQ27010) > + soc = bq27xxx_read(di, BQ27XXX_REG_SOC, true); > + else > + soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false); > > if (soc < 0) > dev_dbg(di->dev, "error reading State-of-Charge\n"); > @@ -536,7 +539,10 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di) > { > int dcap; > > - dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false); > + if (di->chip == BQ27000 || di->chip == BQ27010) > + dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, true); > + else > + dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false); > I'm wondering how the I2C versions will be effected by this change, will we get the right byte when we only read one? Maybe the fix should be to still read 2-bytes but mask out the right bits from the result? The data-sheet doesn't seem clear on this. Well, it is more right than it was before, I'll look for a bq27200 to test and make changes if needed some other time, but for now: Acked-by: Andrew F. Davis <afd@ti.com> > if (dcap < 0) { > dev_dbg(di->dev, "error reading initial last measured discharge\n"); > @@ -544,7 +550,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di) > } > > if (di->chip == BQ27000 || di->chip == BQ27010) > - dcap *= BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS; > + dcap = (dcap << 8) * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS; > else > dcap *= 1000; > > -- Andrew F. Davis ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 2015-12-17 10:12 ` [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 H. Nikolaus Schaller 2015-12-24 2:46 ` Andrew F. Davis @ 2015-12-25 13:20 ` Pali Rohár 1 sibling, 0 replies; 9+ messages in thread From: Pali Rohár @ 2015-12-25 13:20 UTC (permalink / raw) To: H. Nikolaus Schaller Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, linux-pm, linux-kernel, gta04-owner, marek [-- Attachment #1: Type: Text/Plain, Size: 2978 bytes --] On Thursday 17 December 2015 11:12:53 H. Nikolaus Schaller wrote: > bug: the driver reports funny capacity values: > > root@letux:/sys/class/power_supply/bq27000-battery# cat uevent > POWER_SUPPLY_NAME=bq27000-battery > POWER_SUPPLY_STATUS=Charging > POWER_SUPPLY_PRESENT=1 > POWER_SUPPLY_VOLTAGE_NOW=3702000 > POWER_SUPPLY_CURRENT_NOW=-464635 > POWER_SUPPLY_CAPACITY=1536 <- over 100% is magic > POWER_SUPPLY_CAPACITY_LEVEL=Normal > POWER_SUPPLY_TEMP=311 > POWER_SUPPLY_TIME_TO_FULL_NOW=10440 > POWER_SUPPLY_TECHNOLOGY=Li-ion > POWER_SUPPLY_CHARGE_FULL=805450 > POWER_SUPPLY_CHARGE_NOW=1068 > POWER_SUPPLY_CHARGE_FULL_DESIGN=8844998 <- battery has just 1200 mAh > POWER_SUPPLY_CYCLE_COUNT=21 > POWER_SUPPLY_ENERGY_NOW=0 > POWER_SUPPLY_POWER_AVG=0 > POWER_SUPPLY_HEALTH=Good > POWER_SUPPLY_MANUFACTURER=Texas Instruments > > reason: the state of charge and the design capacity register are > single byte only. The design capacity returns the higer order byte. > > tested: GTA04 with Openmoko/FIC HF08x battery (using hdq) > > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > drivers/power/bq27xxx_battery.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/bq27xxx_battery.c > b/drivers/power/bq27xxx_battery.c index 880233c..e54a125 100644 > --- a/drivers/power/bq27xxx_battery.c > +++ b/drivers/power/bq27xxx_battery.c > @@ -471,7 +471,10 @@ static int bq27xxx_battery_read_soc(struct > bq27xxx_device_info *di) { > int soc; > > - soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false); > + if (di->chip == BQ27000 || di->chip == BQ27010) > + soc = bq27xxx_read(di, BQ27XXX_REG_SOC, true); > + else > + soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false); > > if (soc < 0) > dev_dbg(di->dev, "error reading State-of-Charge\n"); > @@ -536,7 +539,10 @@ static int bq27xxx_battery_read_dcap(struct > bq27xxx_device_info *di) { > int dcap; > > - dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false); > + if (di->chip == BQ27000 || di->chip == BQ27010) > + dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, true); > + else > + dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false); > > if (dcap < 0) { > dev_dbg(di->dev, "error reading initial last measured > discharge\n"); @@ -544,7 +550,7 @@ static int > bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di) } > > if (di->chip == BQ27000 || di->chip == BQ27010) > - dcap *= BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS; > + dcap = (dcap << 8) * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS; > else > dcap *= 1000; Hi! This patch fixes commit d74534c27775857cb09abd0f92ed9539dc8d0a93 (power: bq27xxx_battery: Add support for additional bq27xxx family devices) in which this bug (all 3 patch chunks) were introduced. Before that commit code was very similar after applying this patch. So: Reviewed-by: Pali Rohár <pali.rohar@gmail.com> -- Pali Rohár pali.rohar@gmail.com [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] power:bq27xxx: fix register numbers of bq27500 2015-12-17 10:12 [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver H. Nikolaus Schaller 2015-12-17 10:12 ` [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 H. Nikolaus Schaller @ 2015-12-17 10:12 ` H. Nikolaus Schaller 2015-12-24 2:52 ` Andrew F. Davis 2015-12-19 8:00 ` [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver Sebastian Reichel 2 siblings, 1 reply; 9+ messages in thread From: H. Nikolaus Schaller @ 2015-12-17 10:12 UTC (permalink / raw) To: Pali Rohár, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, linux-kernel, gta04-owner, marek, H. Nikolaus Schaller bug: according to data sheet some register numbers are wrong. tested: no Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- drivers/power/bq27xxx_battery.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/power/bq27xxx_battery.c b/drivers/power/bq27xxx_battery.c index e54a125..f9a5f4f 100644 --- a/drivers/power/bq27xxx_battery.c +++ b/drivers/power/bq27xxx_battery.c @@ -198,10 +198,10 @@ static u8 bq27500_regs[] = { INVALID_REG_ADDR, /* TTECP - NA */ 0x0c, /* NAC */ 0x12, /* LMD(FCC) */ - 0x1e, /* CYCT */ + 0x2a, /* CYCT */ INVALID_REG_ADDR, /* AE - NA */ - 0x20, /* SOC(RSOC) */ - 0x2e, /* DCAP(ILMD) */ + 0x2c, /* SOC(RSOC) */ + 0x3c, /* DCAP(ILMD) */ INVALID_REG_ADDR, /* AP - NA */ }; -- 2.5.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] power:bq27xxx: fix register numbers of bq27500 2015-12-17 10:12 ` [PATCH 2/2] power:bq27xxx: fix register numbers of bq27500 H. Nikolaus Schaller @ 2015-12-24 2:52 ` Andrew F. Davis 0 siblings, 0 replies; 9+ messages in thread From: Andrew F. Davis @ 2015-12-24 2:52 UTC (permalink / raw) To: H. Nikolaus Schaller, Pali Rohár, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, linux-kernel, gta04-owner, marek On 12/17/2015 04:12 AM, H. Nikolaus Schaller wrote: > bug: according to data sheet some register numbers are wrong. > > tested: no > > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> Indeed they are, good catch. Acked-by: Andrew F. Davis <afd@ti.com> > --- > drivers/power/bq27xxx_battery.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/bq27xxx_battery.c b/drivers/power/bq27xxx_battery.c > index e54a125..f9a5f4f 100644 > --- a/drivers/power/bq27xxx_battery.c > +++ b/drivers/power/bq27xxx_battery.c > @@ -198,10 +198,10 @@ static u8 bq27500_regs[] = { > INVALID_REG_ADDR, /* TTECP - NA */ > 0x0c, /* NAC */ > 0x12, /* LMD(FCC) */ > - 0x1e, /* CYCT */ > + 0x2a, /* CYCT */ > INVALID_REG_ADDR, /* AE - NA */ > - 0x20, /* SOC(RSOC) */ > - 0x2e, /* DCAP(ILMD) */ > + 0x2c, /* SOC(RSOC) */ > + 0x3c, /* DCAP(ILMD) */ > INVALID_REG_ADDR, /* AP - NA */ > }; > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver 2015-12-17 10:12 [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver H. Nikolaus Schaller 2015-12-17 10:12 ` [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 H. Nikolaus Schaller 2015-12-17 10:12 ` [PATCH 2/2] power:bq27xxx: fix register numbers of bq27500 H. Nikolaus Schaller @ 2015-12-19 8:00 ` Sebastian Reichel 2015-12-24 13:57 ` Andrew F. Davis 2 siblings, 1 reply; 9+ messages in thread From: Sebastian Reichel @ 2015-12-19 8:00 UTC (permalink / raw) To: Andrew F. Davis Cc: H. Nikolaus Schaller, Pali Rohár, linux-pm, linux-kernel, gta04-owner, marek [-- Attachment #1: Type: text/plain, Size: 501 bytes --] Hi Andrew, On Thu, Dec 17, 2015 at 11:12:52AM +0100, H. Nikolaus Schaller wrote: > H. Nikolaus Schaller (2): > power:bq27xxx: fix reading for bq27000 and bq27010 > power:bq27xxx: fix register numbers of bq27500 > > drivers/power/bq27xxx_battery.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) Can you give me your Acked-By (or Tested-By) for those patches? Also I wonder if you would be available as official reviewer for the driver. -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver 2015-12-19 8:00 ` [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver Sebastian Reichel @ 2015-12-24 13:57 ` Andrew F. Davis 2015-12-25 13:22 ` Pali Rohár 0 siblings, 1 reply; 9+ messages in thread From: Andrew F. Davis @ 2015-12-24 13:57 UTC (permalink / raw) To: Sebastian Reichel Cc: H. Nikolaus Schaller, Pali Rohár, linux-pm, linux-kernel, gta04-owner, marek On 12/19/2015 02:00 AM, Sebastian Reichel wrote: > Hi Andrew, > > On Thu, Dec 17, 2015 at 11:12:52AM +0100, H. Nikolaus Schaller wrote: >> H. Nikolaus Schaller (2): >> power:bq27xxx: fix reading for bq27000 and bq27010 >> power:bq27xxx: fix register numbers of bq27500 >> >> drivers/power/bq27xxx_battery.c | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) > > Can you give me your Acked-By (or Tested-By) for those patches? Done > Also I wonder if you would be available as official reviewer for > the driver. Sure, what's the normal procedure for this, should I submit a patch adding myself to that driver in MAINTAINERS? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver 2015-12-24 13:57 ` Andrew F. Davis @ 2015-12-25 13:22 ` Pali Rohár 0 siblings, 0 replies; 9+ messages in thread From: Pali Rohár @ 2015-12-25 13:22 UTC (permalink / raw) To: Andrew F. Davis Cc: Sebastian Reichel, H. Nikolaus Schaller, linux-pm, linux-kernel, gta04-owner, marek [-- Attachment #1: Type: Text/Plain, Size: 335 bytes --] On Thursday 24 December 2015 14:57:56 Andrew F. Davis wrote: > > Also I wonder if you would be available as official reviewer for > > the driver. > > Sure, what's the normal procedure for this, should I submit a > patch adding myself to that driver in MAINTAINERS? Yes, send patch. -- Pali Rohár pali.rohar@gmail.com [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-12-25 13:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-17 10:12 [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver H. Nikolaus Schaller 2015-12-17 10:12 ` [PATCH 1/2] power:bq27xxx: fix reading for bq27000 and bq27010 H. Nikolaus Schaller 2015-12-24 2:46 ` Andrew F. Davis 2015-12-25 13:20 ` Pali Rohár 2015-12-17 10:12 ` [PATCH 2/2] power:bq27xxx: fix register numbers of bq27500 H. Nikolaus Schaller 2015-12-24 2:52 ` Andrew F. Davis 2015-12-19 8:00 ` [PATCH 0/2] two fixes for new bq27000/10 and bq27500 driver Sebastian Reichel 2015-12-24 13:57 ` Andrew F. Davis 2015-12-25 13:22 ` Pali Rohár
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).