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

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

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

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%3E

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.