dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[H. Nikolaus Schaller]

Hi,
I am trying to get this working on our bq27421.

But the only message I get is:

[    6.086407] bq27xxx-battery 1-0055: data memory update not supported for chip

A little research shows that this message comes from

	http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/power/supply/bq27xxx_battery.c#L1279

So di->dm_regs is NULL.

But doing an fgrep for dm_regs shows no line of code where the pointer is set to a non-null value:

master hns$ fgrep -R dm_regs *
drivers/power/supply/bq27xxx_battery.c:	.class = (di)->dm_regs[i].subclass_id, \
drivers/power/supply/bq27xxx_battery.c:	.block = (di)->dm_regs[i].offset / BQ27XXX_DM_SZ, \
drivers/power/supply/bq27xxx_battery.c:	struct bq27xxx_dm_reg *reg = &di->dm_regs[reg_id];
drivers/power/supply/bq27xxx_battery.c:	if (!di->dm_regs) {
drivers/power/supply/bq27xxx_battery.c:	max = di->dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
drivers/power/supply/bq27xxx_battery.c:	max = di->dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
drivers/power/supply/bq27xxx_battery.c:	min = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
drivers/power/supply/bq27xxx_battery.c:	max = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
Binary file drivers/power/supply/bq27xxx_battery.ko matches
Binary file drivers/power/supply/bq27xxx_battery.o matches
Binary file drivers/power/supply/bq27xxx_battery_i2c.ko matches
Binary file drivers/power/supply/bq27xxx_battery_i2c.o matches
include/linux/power/bq27xxx_battery.h:	struct bq27xxx_dm_reg *dm_regs;
master hns$ 

What am I doing wrong here?

BR and thanks,
Nikolaus

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[Liam Breck]

Hi Nikolaus,

You need the patchset which enables this feature, which was delayed
for further work. It's now ready...

https://patchwork.kernel.org/patch/9918947/
https://patchwork.kernel.org/patch/9918949/
https://patchwork.kernel.org/patch/9918951/
https://patchwork.kernel.org/patch/9918953/
https://patchwork.kernel.org/patch/9918955/

And flip the #if 0 protecting bq27421_dm_regs

I'd love to support that chip in this patchset if you can test
non-default settings for all 3 options this week?

Thanks,
Liam


On Tue, Aug 29, 2017 at 2:43 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Hi,
> I am trying to get this working on our bq27421.
>
> But the only message I get is:
>
> [    6.086407] bq27xxx-battery 1-0055: data memory update not supported for chip
>
> A little research shows that this message comes from
>
>         http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/power/supply/bq27xxx_battery.c#L1279
>
> So di->dm_regs is NULL.
>
> But doing an fgrep for dm_regs shows no line of code where the pointer is set to a non-null value:
>
> master hns$ fgrep -R dm_regs *
> drivers/power/supply/bq27xxx_battery.c: .class = (di)->dm_regs[i].subclass_id, \
> drivers/power/supply/bq27xxx_battery.c: .block = (di)->dm_regs[i].offset / BQ27XXX_DM_SZ, \
> drivers/power/supply/bq27xxx_battery.c: struct bq27xxx_dm_reg *reg = &di->dm_regs[reg_id];
> drivers/power/supply/bq27xxx_battery.c: if (!di->dm_regs) {
> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
> drivers/power/supply/bq27xxx_battery.c: min = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
> Binary file drivers/power/supply/bq27xxx_battery.ko matches
> Binary file drivers/power/supply/bq27xxx_battery.o matches
> Binary file drivers/power/supply/bq27xxx_battery_i2c.ko matches
> Binary file drivers/power/supply/bq27xxx_battery_i2c.o matches
> include/linux/power/bq27xxx_battery.h:  struct bq27xxx_dm_reg *dm_regs;
> master hns$
>
> What am I doing wrong here?
>
> BR and thanks,
> Nikolaus
>

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[H. Nikolaus Schaller]

Hi Liam,
seems to work as expected :)

First boot:

[    6.096336] bq27xxx_battery_settings
[    6.097987] bq27xxx_battery_set_config
[    6.097990] bq27xxx_battery_unseal
[    6.107987] bq27xxx-battery 1-0055: update design-capacity to 6000
[    6.107992] bq27xxx-battery 1-0055: update design-energy to 22200
[    6.107997] bq27xxx-battery 1-0055: terminate-voltage has 3200
[    6.382048] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
[    6.382054] bq27xxx_battery_seal

Second boot (w/o removing battery in between):

[    6.008883] bq27xxx_battery_settings
[    6.013280] bq27xxx_battery_set_config
[    6.018761] bq27xxx_battery_unseal
[    6.050694] bq27xxx-battery 1-0055: design-capacity has 6000
[    6.059913] bq27xxx-battery 1-0055: design-energy has 22200
[    6.067113] bq27xxx-battery 1-0055: terminate-voltage has 3200
[    6.075803] bq27xxx_battery_seal

Third boot (with removing battery in between):

[    6.161085] bq27xxx_battery_settings
[    6.161162] bq27xxx_battery_set_config
[    6.161165] bq27xxx_battery_unseal
[    6.177904] bq27xxx-battery 1-0055: update design-capacity to 6000
[    6.177909] bq27xxx-battery 1-0055: update design-energy to 22200
[    6.177914] bq27xxx-battery 1-0055: terminate-voltage has 3200
[    6.440592] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
[    6.440597] bq27xxx_battery_seal

(I have added some printk to trace seal/unseal etc.)

Values match the 6000mAh specified by DT:

	bat: battery {
		compatible = "simple-battery", "pyra-battery";
		voltage-min-design-microvolt = <3200000>;
		energy-full-design-microwatt-hours = <22200000>;
		charge-full-design-microamp-hours = <6000000>;
	};

root@letux:~# cat /sys/class/power_supply/bq27421-0/uevent 
POWER_SUPPLY_NAME=bq27421-0
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=3943000
POWER_SUPPLY_CURRENT_NOW=-634000
POWER_SUPPLY_CAPACITY=82
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=319
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=5467000
POWER_SUPPLY_CHARGE_NOW=4611000
POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
POWER_SUPPLY_MANUFACTURER=Texas Instruments
root@letux:~# 

Anything else I should test?

BR and thanks,
Nikolaus


> Am 29.08.2017 um 12:40 schrieb Liam Breck <liam@networkimprov.net>:
> 
> Hi Nikolaus,
> 
> You need the patchset which enables this feature, which was delayed
> for further work. It's now ready...
> 
> https://patchwork.kernel.org/patch/9918947/
> https://patchwork.kernel.org/patch/9918949/
> https://patchwork.kernel.org/patch/9918951/
> https://patchwork.kernel.org/patch/9918953/
> https://patchwork.kernel.org/patch/9918955/
> 
> And flip the #if 0 protecting bq27421_dm_regs
> 
> I'd love to support that chip in this patchset if you can test
> non-default settings for all 3 options this week?
> 
> Thanks,
> Liam
> 
> 
> On Tue, Aug 29, 2017 at 2:43 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Hi,
>> I am trying to get this working on our bq27421.
>> 
>> But the only message I get is:
>> 
>> [    6.086407] bq27xxx-battery 1-0055: data memory update not supported for chip
>> 
>> A little research shows that this message comes from
>> 
>>        http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/power/supply/bq27xxx_battery.c#L1279
>> 
>> So di->dm_regs is NULL.
>> 
>> But doing an fgrep for dm_regs shows no line of code where the pointer is set to a non-null value:
>> 
>> master hns$ fgrep -R dm_regs *
>> drivers/power/supply/bq27xxx_battery.c: .class = (di)->dm_regs[i].subclass_id, \
>> drivers/power/supply/bq27xxx_battery.c: .block = (di)->dm_regs[i].offset / BQ27XXX_DM_SZ, \
>> drivers/power/supply/bq27xxx_battery.c: struct bq27xxx_dm_reg *reg = &di->dm_regs[reg_id];
>> drivers/power/supply/bq27xxx_battery.c: if (!di->dm_regs) {
>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
>> drivers/power/supply/bq27xxx_battery.c: min = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
>> Binary file drivers/power/supply/bq27xxx_battery.ko matches
>> Binary file drivers/power/supply/bq27xxx_battery.o matches
>> Binary file drivers/power/supply/bq27xxx_battery_i2c.ko matches
>> Binary file drivers/power/supply/bq27xxx_battery_i2c.o matches
>> include/linux/power/bq27xxx_battery.h:  struct bq27xxx_dm_reg *dm_regs;
>> master hns$
>> 
>> What am I doing wrong here?
>> 
>> BR and thanks,
>> Nikolaus
>> 

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[Michael Mrozek]

Am Dienstag, den 29.08.2017, 15:33 +0200 schrieb H. Nikolaus Schaller:

Yes, they were just quick fixes for us, not to be used directly for
something else.

I just thought those might help finding the issue for you :)

> Hi,
> 
> > Am 29.08.2017 um 15:25 schrieb Michael Mrozek <EvilDragon@openpando
> > ra.org>:
> > 
> > The battery monitor has been fixed by notaz.
> > Find the commits here:
> > 
> > https://dev.pyra-handheld.com/notaz/pyra-kernel/commits/pyra-4.10 <
> > https://dev.pyra-handheld.com/notaz/pyra-kernel/commits/pyra-4.10>
> 
> yes, I have seen it.
> 
> Anyways the patches from Liam seem to work on 4.13-rc7 and are a much
> more general solution.
> I am just starting to test a little.
> 
> BR,
> Nikolaus
> 
> 
> > 
> > On August 29, 2017 11:43:43 AM GMT+02:00, "H. Nikolaus Schaller" <h
> > ns@goldelico.com> wrote:
> > Hi,
> > I am trying to get this working on our bq27421.
> > 
> > But the only message I get is:
> > 
> > [    6.086407] bq27xxx-battery 1-0055: data memory update not
> > supported for chip
> > 
> > A little research shows that this message comes from
> > 
> >  http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/po
> > wer/supply/bq27xxx_battery.c#L1279 <http://elixir.free-electrons.co
> > m/linux/v4.13-
> > rc7/source/drivers/power/supply/bq27xxx_battery.c#L1279>
> > 
> > So di->dm_regs is NULL.
> > 
> > But doing an fgrep for dm_regs shows no line of code where the
> > pointer is set to a non-null value:
> > 
> > master hns$ fgrep -R dm_regs *
> > drivers/power/supply/bq27xxx_battery.c: .class = (di)-
> > >dm_regs[i].subclass_id, \
> > drivers/power/supply/bq27xxx_battery.c: .block = (di)-
> > >dm_regs[i].offset / BQ27XXX_DM_SZ, \
> > drivers/power/supply/bq27xxx_battery.c: struct bq27xxx_dm_reg *reg
> > = &di->dm_regs[reg_id];
> > drivers/power/supply/bq27xxx_battery.c: if (!di->dm_regs) {
> > drivers/power/supply/bq27xxx_battery.c: max = di-
> > >dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
> > drivers/power/supply/bq27xxx_battery.c: max = di-
> > >dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
> > drivers/power/supply/bq27xxx_battery.c: min = di-
> > >dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
> > drivers/power/supply/bq27xxx_battery.c: max = di-
> > >dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
> > Binary file drivers/power/supply/bq27xxx_battery.ko matches
> > Binary file drivers/power/supply/bq27xxx_battery.o matches
> > Binary file drivers/power/supply/bq27xxx_battery_i2c.ko matches
> > Binary file drivers/power/supply/bq27xxx_battery_i2c.o matches
> > include/linux/power/bq27xxx_battery.h: struct bq27xxx_dm_reg
> > *dm_regs;
> > master hns$ 
> > 
> > What am I doing wrong here?
> > 
> > BR and thanks,
> > Nikolaus
> > 
> > 
> > Kernel mailing list
> > Kernel@pyra-handheld.com
> > http://pyra-handheld.com/cgi-bin/mailman/listinfo/kernel <http://py
> > ra-handheld.com/cgi-bin/mailman/listinfo/kernel>
> > 
> > -- 
> > Sent from my Android device with K-9 Mail. Please excuse my
> > brevity.
> 
> 
-- 
Mit freundlichen Grüßen,

Michael Mrozek

-----------------------
OpenPandora GmbH
Geschäftsführer: Michael Mrozek

Schäffbräustr. 11
85049 Ingolstadt
Deutschland
Tel.: 0841 / 990 5548
http://www.openpandora.de/
HRB 4879, Amtsgericht Ingolstadt
-----------------------
eMail: mrozek@openpandora.org

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[Liam Breck]

Hi Nikolaus, thanks for the testing report...

On Tue, Aug 29, 2017 at 7:24 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Hi Liam,
> seems to work as expected :)
>
> First boot:
>
> [    6.096336] bq27xxx_battery_settings
> [    6.097987] bq27xxx_battery_set_config
> [    6.097990] bq27xxx_battery_unseal
> [    6.107987] bq27xxx-battery 1-0055: update design-capacity to 6000
> [    6.107992] bq27xxx-battery 1-0055: update design-energy to 22200
> [    6.107997] bq27xxx-battery 1-0055: terminate-voltage has 3200
> [    6.382048] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
> [    6.382054] bq27xxx_battery_seal
>
> Second boot (w/o removing battery in between):
>
> [    6.008883] bq27xxx_battery_settings
> [    6.013280] bq27xxx_battery_set_config
> [    6.018761] bq27xxx_battery_unseal
> [    6.050694] bq27xxx-battery 1-0055: design-capacity has 6000
> [    6.059913] bq27xxx-battery 1-0055: design-energy has 22200
> [    6.067113] bq27xxx-battery 1-0055: terminate-voltage has 3200
> [    6.075803] bq27xxx_battery_seal

I'd be interested to hear what happens after poweroff & boot (ie does
it retain config)?


> Third boot (with removing battery in between):
>
> [    6.161085] bq27xxx_battery_settings
> [    6.161162] bq27xxx_battery_set_config
> [    6.161165] bq27xxx_battery_unseal
> [    6.177904] bq27xxx-battery 1-0055: update design-capacity to 6000
> [    6.177909] bq27xxx-battery 1-0055: update design-energy to 22200
> [    6.177914] bq27xxx-battery 1-0055: terminate-voltage has 3200
> [    6.440592] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
> [    6.440597] bq27xxx_battery_seal
>
> (I have added some printk to trace seal/unseal etc.)
>
> Values match the 6000mAh specified by DT:
>
>         bat: battery {
>                 compatible = "simple-battery", "pyra-battery";
>                 voltage-min-design-microvolt = <3200000>;
>                 energy-full-design-microwatt-hours = <22200000>;
>                 charge-full-design-microamp-hours = <6000000>;
>         };
>
> root@letux:~# cat /sys/class/power_supply/bq27421-0/uevent
> POWER_SUPPLY_NAME=bq27421-0
> POWER_SUPPLY_STATUS=Discharging
> POWER_SUPPLY_PRESENT=1
> POWER_SUPPLY_VOLTAGE_NOW=3943000
> POWER_SUPPLY_CURRENT_NOW=-634000
> POWER_SUPPLY_CAPACITY=82
> POWER_SUPPLY_CAPACITY_LEVEL=Normal
> POWER_SUPPLY_TEMP=319
> POWER_SUPPLY_TECHNOLOGY=Li-ion
> POWER_SUPPLY_CHARGE_FULL=5467000
> POWER_SUPPLY_CHARGE_NOW=4611000
> POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
> POWER_SUPPLY_MANUFACTURER=Texas Instruments
> root@letux:~#
>
> Anything else I should test?

Could you also test voltage-min-design-microvolt at 3177 to make sure
that's configurable too?

Sebastian just queued my series to -next. Would you be inclined to
submit a new patch enabling bq27421? If so, the sooner the better
since merge window is imminent :-)

Liam


>
>
>> Am 29.08.2017 um 12:40 schrieb Liam Breck <liam@networkimprov.net>:
>>
>> Hi Nikolaus,
>>
>> You need the patchset which enables this feature, which was delayed
>> for further work. It's now ready...
>>
>> https://patchwork.kernel.org/patch/9918947/
>> https://patchwork.kernel.org/patch/9918949/
>> https://patchwork.kernel.org/patch/9918951/
>> https://patchwork.kernel.org/patch/9918953/
>> https://patchwork.kernel.org/patch/9918955/
>>
>> And flip the #if 0 protecting bq27421_dm_regs
>>
>> I'd love to support that chip in this patchset if you can test
>> non-default settings for all 3 options this week?
>>
>> Thanks,
>> Liam
>>
>>
>> On Tue, Aug 29, 2017 at 2:43 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> Hi,
>>> I am trying to get this working on our bq27421.
>>>
>>> But the only message I get is:
>>>
>>> [    6.086407] bq27xxx-battery 1-0055: data memory update not supported for chip
>>>
>>> A little research shows that this message comes from
>>>
>>>        http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/power/supply/bq27xxx_battery.c#L1279
>>>
>>> So di->dm_regs is NULL.
>>>
>>> But doing an fgrep for dm_regs shows no line of code where the pointer is set to a non-null value:
>>>
>>> master hns$ fgrep -R dm_regs *
>>> drivers/power/supply/bq27xxx_battery.c: .class = (di)->dm_regs[i].subclass_id, \
>>> drivers/power/supply/bq27xxx_battery.c: .block = (di)->dm_regs[i].offset / BQ27XXX_DM_SZ, \
>>> drivers/power/supply/bq27xxx_battery.c: struct bq27xxx_dm_reg *reg = &di->dm_regs[reg_id];
>>> drivers/power/supply/bq27xxx_battery.c: if (!di->dm_regs) {
>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
>>> drivers/power/supply/bq27xxx_battery.c: min = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
>>> Binary file drivers/power/supply/bq27xxx_battery.ko matches
>>> Binary file drivers/power/supply/bq27xxx_battery.o matches
>>> Binary file drivers/power/supply/bq27xxx_battery_i2c.ko matches
>>> Binary file drivers/power/supply/bq27xxx_battery_i2c.o matches
>>> include/linux/power/bq27xxx_battery.h:  struct bq27xxx_dm_reg *dm_regs;
>>> master hns$
>>>
>>> What am I doing wrong here?
>>>
>>> BR and thanks,
>>> Nikolaus
>>>
>

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[H. Nikolaus Schaller]

Hi Liam,

> Am 29.08.2017 um 20:18 schrieb Liam Breck <liam@networkimprov.net>:
> 
> Hi Nikolaus, thanks for the testing report...
> 
> On Tue, Aug 29, 2017 at 7:24 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Hi Liam,
>> seems to work as expected :)
>> 
>> First boot:
>> 
>> [    6.096336] bq27xxx_battery_settings
>> [    6.097987] bq27xxx_battery_set_config
>> [    6.097990] bq27xxx_battery_unseal
>> [    6.107987] bq27xxx-battery 1-0055: update design-capacity to 6000
>> [    6.107992] bq27xxx-battery 1-0055: update design-energy to 22200
>> [    6.107997] bq27xxx-battery 1-0055: terminate-voltage has 3200
>> [    6.382048] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
>> [    6.382054] bq27xxx_battery_seal
>> 
>> Second boot (w/o removing battery in between):
>> 
>> [    6.008883] bq27xxx_battery_settings
>> [    6.013280] bq27xxx_battery_set_config
>> [    6.018761] bq27xxx_battery_unseal
>> [    6.050694] bq27xxx-battery 1-0055: design-capacity has 6000
>> [    6.059913] bq27xxx-battery 1-0055: design-energy has 22200
>> [    6.067113] bq27xxx-battery 1-0055: terminate-voltage has 3200
>> [    6.075803] bq27xxx_battery_seal
> 
> I'd be interested to hear what happens after poweroff & boot (ie does
> it retain config)?

second boot was simple poweroff + reboot and it changes from "update design-capacity" to "design-capacity has"
which IMHO means that the bq27421 did retain settings and was therefore not reprogrammed.

> 
> 
>> Third boot (with removing battery in between):
>> 
>> [    6.161085] bq27xxx_battery_settings
>> [    6.161162] bq27xxx_battery_set_config
>> [    6.161165] bq27xxx_battery_unseal
>> [    6.177904] bq27xxx-battery 1-0055: update design-capacity to 6000
>> [    6.177909] bq27xxx-battery 1-0055: update design-energy to 22200
>> [    6.177914] bq27xxx-battery 1-0055: terminate-voltage has 3200
>> [    6.440592] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
>> [    6.440597] bq27xxx_battery_seal

this was with removing all power which makes the bq27421 loosing the settings.
And next reboot does another "update design-capacity".

>> 
>> (I have added some printk to trace seal/unseal etc.)
>> 
>> Values match the 6000mAh specified by DT:
>> 
>>        bat: battery {
>>                compatible = "simple-battery", "pyra-battery";
>>                voltage-min-design-microvolt = <3200000>;
>>                energy-full-design-microwatt-hours = <22200000>;
>>                charge-full-design-microamp-hours = <6000000>;
>>        };
>> 
>> root@letux:~# cat /sys/class/power_supply/bq27421-0/uevent
>> POWER_SUPPLY_NAME=bq27421-0
>> POWER_SUPPLY_STATUS=Discharging
>> POWER_SUPPLY_PRESENT=1
>> POWER_SUPPLY_VOLTAGE_NOW=3943000
>> POWER_SUPPLY_CURRENT_NOW=-634000
>> POWER_SUPPLY_CAPACITY=82
>> POWER_SUPPLY_CAPACITY_LEVEL=Normal
>> POWER_SUPPLY_TEMP=319
>> POWER_SUPPLY_TECHNOLOGY=Li-ion
>> POWER_SUPPLY_CHARGE_FULL=5467000
>> POWER_SUPPLY_CHARGE_NOW=4611000
>> POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
>> POWER_SUPPLY_MANUFACTURER=Texas Instruments
>> root@letux:~#
>> 
>> Anything else I should test?
> 
> Could you also test voltage-min-design-microvolt at 3177 to make sure
> that's configurable too?

Yes:

	bat: battery {
		compatible = "simple-battery", "pyra-battery";
		voltage-min-design-microvolt = <3177000>;
		energy-full-design-microwatt-hours = <22200000>;
		charge-full-design-microamp-hours = <6000000>;
	};

gives

[    6.241009] bq27xxx_battery_settings
[    6.241055] bq27xxx_battery_set_config
[    6.241058] bq27xxx_battery_unseal
[    6.257073] bq27xxx-battery 1-0055: update design-capacity to 6000
[    6.257078] bq27xxx-battery 1-0055: update design-energy to 22200
[    6.257084] bq27xxx-battery 1-0055: update terminate-voltage to 3177
[    6.379230] bq27xxx_battery_seal

So it looks good.

> Sebastian just queued my series to -next. Would you be inclined to
> submit a new patch enabling bq27421? If so, the sooner the better
> since merge window is imminent :-)

I already have a patch for that...

BR and thanks,
Nikolaus

> 
> Liam
> 
> 
>> 
>> 
>>> Am 29.08.2017 um 12:40 schrieb Liam Breck <liam@networkimprov.net>:
>>> 
>>> Hi Nikolaus,
>>> 
>>> You need the patchset which enables this feature, which was delayed
>>> for further work. It's now ready...
>>> 
>>> https://patchwork.kernel.org/patch/9918947/
>>> https://patchwork.kernel.org/patch/9918949/
>>> https://patchwork.kernel.org/patch/9918951/
>>> https://patchwork.kernel.org/patch/9918953/
>>> https://patchwork.kernel.org/patch/9918955/
>>> 
>>> And flip the #if 0 protecting bq27421_dm_regs
>>> 
>>> I'd love to support that chip in this patchset if you can test
>>> non-default settings for all 3 options this week?
>>> 
>>> Thanks,
>>> Liam
>>> 
>>> 
>>> On Tue, Aug 29, 2017 at 2:43 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> Hi,
>>>> I am trying to get this working on our bq27421.
>>>> 
>>>> But the only message I get is:
>>>> 
>>>> [    6.086407] bq27xxx-battery 1-0055: data memory update not supported for chip
>>>> 
>>>> A little research shows that this message comes from
>>>> 
>>>>       http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/power/supply/bq27xxx_battery.c#L1279
>>>> 
>>>> So di->dm_regs is NULL.
>>>> 
>>>> But doing an fgrep for dm_regs shows no line of code where the pointer is set to a non-null value:
>>>> 
>>>> master hns$ fgrep -R dm_regs *
>>>> drivers/power/supply/bq27xxx_battery.c: .class = (di)->dm_regs[i].subclass_id, \
>>>> drivers/power/supply/bq27xxx_battery.c: .block = (di)->dm_regs[i].offset / BQ27XXX_DM_SZ, \
>>>> drivers/power/supply/bq27xxx_battery.c: struct bq27xxx_dm_reg *reg = &di->dm_regs[reg_id];
>>>> drivers/power/supply/bq27xxx_battery.c: if (!di->dm_regs) {
>>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
>>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
>>>> drivers/power/supply/bq27xxx_battery.c: min = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
>>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
>>>> Binary file drivers/power/supply/bq27xxx_battery.ko matches
>>>> Binary file drivers/power/supply/bq27xxx_battery.o matches
>>>> Binary file drivers/power/supply/bq27xxx_battery_i2c.ko matches
>>>> Binary file drivers/power/supply/bq27xxx_battery_i2c.o matches
>>>> include/linux/power/bq27xxx_battery.h:  struct bq27xxx_dm_reg *dm_regs;
>>>> master hns$
>>>> 
>>>> What am I doing wrong here?
>>>> 
>>>> BR and thanks,
>>>> Nikolaus
>>>> 
>> 

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[Liam Breck]

Hi,

On Tue, Aug 29, 2017 at 12:50 PM, H. Nikolaus Schaller
<hns@goldelico.com> wrote:
> Hi Liam,
>
>> Am 29.08.2017 um 20:18 schrieb Liam Breck <liam@networkimprov.net>:
>>
>> Hi Nikolaus, thanks for the testing report...
>>
>> On Tue, Aug 29, 2017 at 7:24 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> Hi Liam,
>>> seems to work as expected :)
>>>
>>> First boot:
>>>
>>> [    6.096336] bq27xxx_battery_settings
>>> [    6.097987] bq27xxx_battery_set_config
>>> [    6.097990] bq27xxx_battery_unseal
>>> [    6.107987] bq27xxx-battery 1-0055: update design-capacity to 6000
>>> [    6.107992] bq27xxx-battery 1-0055: update design-energy to 22200
>>> [    6.107997] bq27xxx-battery 1-0055: terminate-voltage has 3200
>>> [    6.382048] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
>>> [    6.382054] bq27xxx_battery_seal
>>>
>>> Second boot (w/o removing battery in between):
>>>
>>> [    6.008883] bq27xxx_battery_settings
>>> [    6.013280] bq27xxx_battery_set_config
>>> [    6.018761] bq27xxx_battery_unseal
>>> [    6.050694] bq27xxx-battery 1-0055: design-capacity has 6000
>>> [    6.059913] bq27xxx-battery 1-0055: design-energy has 22200
>>> [    6.067113] bq27xxx-battery 1-0055: terminate-voltage has 3200
>>> [    6.075803] bq27xxx_battery_seal
>>
>> I'd be interested to hear what happens after poweroff & boot (ie does
>> it retain config)?
>
> second boot was simple poweroff + reboot and it changes from "update design-capacity" to "design-capacity has"
> which IMHO means that the bq27421 did retain settings and was therefore not reprogrammed.

That implies that the board is keeping the battery gauge powered
during poweroff. That's not a problem per se, but could indicate that
other subsystems are also powered...

>>
>>> Third boot (with removing battery in between):
>>>
>>> [    6.161085] bq27xxx_battery_settings
>>> [    6.161162] bq27xxx_battery_set_config
>>> [    6.161165] bq27xxx_battery_unseal
>>> [    6.177904] bq27xxx-battery 1-0055: update design-capacity to 6000
>>> [    6.177909] bq27xxx-battery 1-0055: update design-energy to 22200
>>> [    6.177914] bq27xxx-battery 1-0055: terminate-voltage has 3200
>>> [    6.440592] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
>>> [    6.440597] bq27xxx_battery_seal
>
> this was with removing all power which makes the bq27421 loosing the settings.
> And next reboot does another "update design-capacity".
>
>>>
>>> (I have added some printk to trace seal/unseal etc.)
>>>
>>> Values match the 6000mAh specified by DT:
>>>
>>>        bat: battery {
>>>                compatible = "simple-battery", "pyra-battery";
>>>                voltage-min-design-microvolt = <3200000>;
>>>                energy-full-design-microwatt-hours = <22200000>;
>>>                charge-full-design-microamp-hours = <6000000>;
>>>        };
>>>
>>> root@letux:~# cat /sys/class/power_supply/bq27421-0/uevent
>>> POWER_SUPPLY_NAME=bq27421-0
>>> POWER_SUPPLY_STATUS=Discharging
>>> POWER_SUPPLY_PRESENT=1
>>> POWER_SUPPLY_VOLTAGE_NOW=3943000
>>> POWER_SUPPLY_CURRENT_NOW=-634000
>>> POWER_SUPPLY_CAPACITY=82
>>> POWER_SUPPLY_CAPACITY_LEVEL=Normal
>>> POWER_SUPPLY_TEMP=319
>>> POWER_SUPPLY_TECHNOLOGY=Li-ion
>>> POWER_SUPPLY_CHARGE_FULL=5467000
>>> POWER_SUPPLY_CHARGE_NOW=4611000
>>> POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
>>> POWER_SUPPLY_MANUFACTURER=Texas Instruments
>>> root@letux:~#
>>>
>>> Anything else I should test?
>>
>> Could you also test voltage-min-design-microvolt at 3177 to make sure
>> that's configurable too?
>
> Yes:
>
>         bat: battery {
>                 compatible = "simple-battery", "pyra-battery";
>                 voltage-min-design-microvolt = <3177000>;
>                 energy-full-design-microwatt-hours = <22200000>;
>                 charge-full-design-microamp-hours = <6000000>;
>         };
>
> gives
>
> [    6.241009] bq27xxx_battery_settings
> [    6.241055] bq27xxx_battery_set_config
> [    6.241058] bq27xxx_battery_unseal
> [    6.257073] bq27xxx-battery 1-0055: update design-capacity to 6000
> [    6.257078] bq27xxx-battery 1-0055: update design-energy to 22200
> [    6.257084] bq27xxx-battery 1-0055: update terminate-voltage to 3177
> [    6.379230] bq27xxx_battery_seal
>
> So it looks good.

 \o/

>> Sebastian just queued my series to -next. Would you be inclined to
>> submit a new patch enabling bq27421? If so, the sooner the better
>> since merge window is imminent :-)
>
> I already have a patch for that...

Peering eagerly at linux-pm for said patch...


>>
>>
>>>
>>>
>>>> Am 29.08.2017 um 12:40 schrieb Liam Breck <liam@networkimprov.net>:
>>>>
>>>> Hi Nikolaus,
>>>>
>>>> You need the patchset which enables this feature, which was delayed
>>>> for further work. It's now ready...
>>>>
>>>> https://patchwork.kernel.org/patch/9918947/
>>>> https://patchwork.kernel.org/patch/9918949/
>>>> https://patchwork.kernel.org/patch/9918951/
>>>> https://patchwork.kernel.org/patch/9918953/
>>>> https://patchwork.kernel.org/patch/9918955/
>>>>
>>>> And flip the #if 0 protecting bq27421_dm_regs
>>>>
>>>> I'd love to support that chip in this patchset if you can test
>>>> non-default settings for all 3 options this week?
>>>>
>>>> Thanks,
>>>> Liam
>>>>
>>>>
>>>> On Tue, Aug 29, 2017 at 2:43 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>>> Hi,
>>>>> I am trying to get this working on our bq27421.
>>>>>
>>>>> But the only message I get is:
>>>>>
>>>>> [    6.086407] bq27xxx-battery 1-0055: data memory update not supported for chip
>>>>>
>>>>> A little research shows that this message comes from
>>>>>
>>>>>       http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/power/supply/bq27xxx_battery.c#L1279
>>>>>
>>>>> So di->dm_regs is NULL.
>>>>>
>>>>> But doing an fgrep for dm_regs shows no line of code where the pointer is set to a non-null value:
>>>>>
>>>>> master hns$ fgrep -R dm_regs *
>>>>> drivers/power/supply/bq27xxx_battery.c: .class = (di)->dm_regs[i].subclass_id, \
>>>>> drivers/power/supply/bq27xxx_battery.c: .block = (di)->dm_regs[i].offset / BQ27XXX_DM_SZ, \
>>>>> drivers/power/supply/bq27xxx_battery.c: struct bq27xxx_dm_reg *reg = &di->dm_regs[reg_id];
>>>>> drivers/power/supply/bq27xxx_battery.c: if (!di->dm_regs) {
>>>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_ENERGY].max;
>>>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_DESIGN_CAPACITY].max;
>>>>> drivers/power/supply/bq27xxx_battery.c: min = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].min;
>>>>> drivers/power/supply/bq27xxx_battery.c: max = di->dm_regs[BQ27XXX_DM_TERMINATE_VOLTAGE].max;
>>>>> Binary file drivers/power/supply/bq27xxx_battery.ko matches
>>>>> Binary file drivers/power/supply/bq27xxx_battery.o matches
>>>>> Binary file drivers/power/supply/bq27xxx_battery_i2c.ko matches
>>>>> Binary file drivers/power/supply/bq27xxx_battery_i2c.o matches
>>>>> include/linux/power/bq27xxx_battery.h:  struct bq27xxx_dm_reg *dm_regs;
>>>>> master hns$
>>>>>
>>>>> What am I doing wrong here?
>>>>>
>>>>> BR and thanks,
>>>>> Nikolaus
>>>>>
>>>
>

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[H. Nikolaus Schaller]

Hi,

> Am 29.08.2017 um 22:05 schrieb Liam Breck <liam@networkimprov.net>:
> 
> Hi,
> 
> On Tue, Aug 29, 2017 at 12:50 PM, H. Nikolaus Schaller
> <hns@goldelico.com> wrote:
>> Hi Liam,
>> 
>>> Am 29.08.2017 um 20:18 schrieb Liam Breck <liam@networkimprov.net>:
>>> 
>>> Hi Nikolaus, thanks for the testing report...
>>> 
>>> On Tue, Aug 29, 2017 at 7:24 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> Hi Liam,
>>>> seems to work as expected :)
>>>> 
>>>> First boot:
>>>> 
>>>> [    6.096336] bq27xxx_battery_settings
>>>> [    6.097987] bq27xxx_battery_set_config
>>>> [    6.097990] bq27xxx_battery_unseal
>>>> [    6.107987] bq27xxx-battery 1-0055: update design-capacity to 6000
>>>> [    6.107992] bq27xxx-battery 1-0055: update design-energy to 22200
>>>> [    6.107997] bq27xxx-battery 1-0055: terminate-voltage has 3200
>>>> [    6.382048] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
>>>> [    6.382054] bq27xxx_battery_seal
>>>> 
>>>> Second boot (w/o removing battery in between):
>>>> 
>>>> [    6.008883] bq27xxx_battery_settings
>>>> [    6.013280] bq27xxx_battery_set_config
>>>> [    6.018761] bq27xxx_battery_unseal
>>>> [    6.050694] bq27xxx-battery 1-0055: design-capacity has 6000
>>>> [    6.059913] bq27xxx-battery 1-0055: design-energy has 22200
>>>> [    6.067113] bq27xxx-battery 1-0055: terminate-voltage has 3200
>>>> [    6.075803] bq27xxx_battery_seal
>>> 
>>> I'd be interested to hear what happens after poweroff & boot (ie does
>>> it retain config)?
>> 
>> second boot was simple poweroff + reboot and it changes from "update design-capacity" to "design-capacity has"
>> which IMHO means that the bq27421 did retain settings and was therefore not reprogrammed.
> 
> That implies that the board is keeping the battery gauge powered
> during poweroff.

Yes, that is how hardware is designed on this system.

> That's not a problem per se, but could indicate that
> other subsystems are also powered...

Well, the power controller (Palmas) stays powered to respond to the power-on-key...

Therefore I completely removed all power as well.

> 
>>> 
>>>> Third boot (with removing battery in between):
>>>> 
>>>> [    6.161085] bq27xxx_battery_settings
>>>> [    6.161162] bq27xxx_battery_set_config
>>>> [    6.161165] bq27xxx_battery_unseal
>>>> [    6.177904] bq27xxx-battery 1-0055: update design-capacity to 6000
>>>> [    6.177909] bq27xxx-battery 1-0055: update design-energy to 22200
>>>> [    6.177914] bq27xxx-battery 1-0055: terminate-voltage has 3200
>>>> [    6.440592] bq27xxx-battery 1-0055: cfgupdate 0, retries 7
>>>> [    6.440597] bq27xxx_battery_seal
>> 
>> this was with removing all power which makes the bq27421 loosing the settings.
>> And next reboot does another "update design-capacity".
>> 
>>>> 
>>>> (I have added some printk to trace seal/unseal etc.)
>>>> 
>>>> Values match the 6000mAh specified by DT:
>>>> 
>>>>       bat: battery {
>>>>               compatible = "simple-battery", "pyra-battery";
>>>>               voltage-min-design-microvolt = <3200000>;
>>>>               energy-full-design-microwatt-hours = <22200000>;
>>>>               charge-full-design-microamp-hours = <6000000>;
>>>>       };
>>>> 
>>>> root@letux:~# cat /sys/class/power_supply/bq27421-0/uevent
>>>> POWER_SUPPLY_NAME=bq27421-0
>>>> POWER_SUPPLY_STATUS=Discharging
>>>> POWER_SUPPLY_PRESENT=1
>>>> POWER_SUPPLY_VOLTAGE_NOW=3943000
>>>> POWER_SUPPLY_CURRENT_NOW=-634000
>>>> POWER_SUPPLY_CAPACITY=82
>>>> POWER_SUPPLY_CAPACITY_LEVEL=Normal
>>>> POWER_SUPPLY_TEMP=319
>>>> POWER_SUPPLY_TECHNOLOGY=Li-ion
>>>> POWER_SUPPLY_CHARGE_FULL=5467000
>>>> POWER_SUPPLY_CHARGE_NOW=4611000
>>>> POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
>>>> POWER_SUPPLY_MANUFACTURER=Texas Instruments
>>>> root@letux:~#
>>>> 
>>>> Anything else I should test?
>>> 
>>> Could you also test voltage-min-design-microvolt at 3177 to make sure
>>> that's configurable too?
>> 
>> Yes:
>> 
>>        bat: battery {
>>                compatible = "simple-battery", "pyra-battery";
>>                voltage-min-design-microvolt = <3177000>;
>>                energy-full-design-microwatt-hours = <22200000>;
>>                charge-full-design-microamp-hours = <6000000>;
>>        };
>> 
>> gives
>> 
>> [    6.241009] bq27xxx_battery_settings
>> [    6.241055] bq27xxx_battery_set_config
>> [    6.241058] bq27xxx_battery_unseal
>> [    6.257073] bq27xxx-battery 1-0055: update design-capacity to 6000
>> [    6.257078] bq27xxx-battery 1-0055: update design-energy to 22200
>> [    6.257084] bq27xxx-battery 1-0055: update terminate-voltage to 3177
>> [    6.379230] bq27xxx_battery_seal
>> 
>> So it looks good.
> 
> \o/
> 
>>> Sebastian just queued my series to -next. Would you be inclined to
>>> submit a new patch enabling bq27421? If so, the sooner the better
>>> since merge window is imminent :-)
>> 
>> I already have a patch for that...
> 
> Peering eagerly at linux-pm for said patch...

Seems to have been lost on first attempt. Will resend in some minutes.

BR,
Nikolaus

Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation

[Pavel Machek]

Hi!

> > Values match the 6000mAh specified by DT:
> >
> >         bat: battery {
> >                 compatible = "simple-battery", "pyra-battery";
> >                 voltage-min-design-microvolt = <3200000>;
> >                 energy-full-design-microwatt-hours = <22200000>;
> >                 charge-full-design-microamp-hours = <6000000>;
> >         };
> >
> > root@letux:~# cat /sys/class/power_supply/bq27421-0/uevent
> > POWER_SUPPLY_NAME=bq27421-0
> > POWER_SUPPLY_STATUS=Discharging
> > POWER_SUPPLY_PRESENT=1
> > POWER_SUPPLY_VOLTAGE_NOW=3943000
> > POWER_SUPPLY_CURRENT_NOW=-634000
> > POWER_SUPPLY_CAPACITY=82
> > POWER_SUPPLY_CAPACITY_LEVEL=Normal
> > POWER_SUPPLY_TEMP=319
> > POWER_SUPPLY_TECHNOLOGY=Li-ion
> > POWER_SUPPLY_CHARGE_FULL=5467000
> > POWER_SUPPLY_CHARGE_NOW=4611000
> > POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
> > POWER_SUPPLY_MANUFACTURER=Texas Instruments
> > root@letux:~#
> >
> > Anything else I should test?
> 
> Could you also test voltage-min-design-microvolt at 3177 to make sure
> that's configurable too?

That should be 3177000 microvolts, no?
									Pavel