Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751359AbdH2UF1 (ORCPT ); Tue, 29 Aug 2017 16:05:27 -0400 Received: from mail-io0-f172.google.com ([209.85.223.172]:35755 "EHLO mail-io0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751186AbdH2UFZ (ORCPT ); Tue, 29 Aug 2017 16:05:25 -0400 MIME-Version: 1.0 In-Reply-To: References: <38AF5486-7D86-4D98-AD74-AB2FFC81E8D0@goldelico.com> From: Liam Breck Date: Tue, 29 Aug 2017 13:05:24 -0700 X-Google-Sender-Auth: y9mJVGQJ3a1Rds5CErccMqrR5PI Message-ID: Subject: Re: dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation To: "H. Nikolaus Schaller" Cc: Sebastian Reichel , LKML , Discussions about the Letux Kernel , kernel@pyra-handheld.com Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7062 Lines: 190 Hi, On Tue, Aug 29, 2017 at 12:50 PM, H. Nikolaus Schaller wrote: > Hi Liam, > >> Am 29.08.2017 um 20:18 schrieb Liam Breck : >> >> Hi Nikolaus, thanks for the testing report... >> >> On Tue, Aug 29, 2017 at 7:24 AM, H. Nikolaus Schaller 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 : >>>> >>>> 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 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 >>>>> >>> >