Hi Liam, > Am 29.08.2017 um 20:18 schrieb Liam Breck <l...@networkimprov.net>: > > Hi Nikolaus, thanks for the testing report... > > On Tue, Aug 29, 2017 at 7:24 AM, H. Nikolaus Schaller <h...@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 <l...@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 <h...@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 >>>> >>