Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757136AbdIHU2w (ORCPT ); Fri, 8 Sep 2017 16:28:52 -0400 Received: from mo4-p00-ob.smtp.rzone.de ([81.169.146.163]:14030 "EHLO mo4-p00-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757102AbdIHU2u (ORCPT ); Fri, 8 Sep 2017 16:28:50 -0400 X-RZG-AUTH: :JGIXVUS7cutRB/49FwqZ7WcecEarQROEYabkiUo6mSAGQ+qKIDwoIDdYJQ== X-RZG-CLASS-ID: mo00 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Subject: Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421 From: "H. Nikolaus Schaller" In-Reply-To: Date: Fri, 8 Sep 2017 22:28:45 +0200 Cc: Sebastian Reichel , =?utf-8?Q?Pali_Roh=C3=A1r?= , "Andrew F. Davis" , Linux PM mailing list , linux-kernel , Discussions about the Letux Kernel , kernel@pyra-handheld.com Message-Id: <13B50F41-D8AC-46E5-B57C-274CFB4DAD75@goldelico.com> References: <7428DD60-FE09-41C4-9DBC-FFDCB9D0B591@goldelico.com> <3A487394-3755-4EA0-86C6-6855DA0AC547@goldelico.com> <9903AE74-6BAA-4051-ADFC-0DB5972AF26E@goldelico.com> To: Liam Breck X-Mailer: Apple Mail (2.3124) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v88KT2jk008551 Content-Length: 7971 Lines: 226 Hi Liam, > Am 08.09.2017 um 22:19 schrieb Liam Breck : > > Hi Nikolaus, > > On Fri, Sep 8, 2017 at 10:38 AM, H. Nikolaus Schaller wrote: >> Hi Liam, >> I finally continues testing on OpenPandora. >> >>> Am 31.08.2017 um 22:19 schrieb Liam Breck : >>> >>> Hi, >>> >>> This may be a fix that allows >0 input from DT, but won't try to >>> program the register since the first 3 fields aren't compatible: >>> >>> ... bq27500_dm_regs[] = { >>> ... >>> [bq27xxx_dm_design_energy] = { 0, 0, 0, 0, 65535 }, /* missing on chip */ >>> NB: align columns with other rows >> >> I have tried with this DT >> >> bat: battery { >> compatible = "simple-battery", "openpandora-battery"; >> voltage-min-design-microvolt = <3250000>; >> energy-full-design-microwatt-hours = <14800000>; >> charge-full-design-microamp-hours = <4100000>; >> }; >> >> and here is the result: >> >>> root@letux:~# dmesg|fgrep bq27 >>> [ 10.391235] bq27xxx_battery_setup >>> [ 10.391265] bq27xxx_battery_setup: dm_regs=bf0520e0 >>> [ 10.393798] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix >>> [ 10.432678] bq27xxx_battery_settings >>> [ 10.432952] bq27xxx_battery_set_config >>> [ 10.432952] bq27xxx_battery_unseal >>> [ 10.485168] bq27xxx_battery_update_dm_block >>> [ 10.485198] bq27xxx-battery 2-0055: update design-capacity to 4100 >> >> looks good >> >>> [ 10.485229] bq27xxx_battery_update_dm_block >>> [ 10.485229] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec >> >> ok, ignored >> >>> [ 10.511718] bq27xxx_battery_update_dm_block >>> [ 10.511749] bq27xxx-battery 2-0055: update terminate-voltage to 3250 >> >> looks good >> >>> [ 10.826446] bq27xxx_battery_seal >>> [ 12.150939] bq27xxx_battery_platform_probe >>> [ 12.151031] bq27xxx_battery_setup >>> [ 12.151062] bq27xxx_battery_setup: dm_regs= (null) >>> [ 12.153411] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix >>> [ 12.154327] bq27xxx_battery_settings >>> [ 12.154357] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree >>> [ 12.154388] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 > > Is there also a bq27000 chip on this device, or a battery with it > embedded? If so, it doesn't appear to have a DT node (correct for > embedded), hence that warning, which isn't useful in that case. Prob > battery_settings() should do: > if (!di->dev->of_node || power_supply_get_battery_info(...) < 0) return; > altho that would be wrong once get_battery_info() supports ACPI config... No it is not available, but could be. The bq27000 can be connected through HDQ to an OMAP3 and therefore there is no auto-detection. If it is configured the driver will be loaded. Even if there is no battery_info. In fact we use the same defconfig for several devices and there is one (GTA04) which only has a bq27000 inside the battery. > >> ... login: >> >>> root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent >>> POWER_SUPPLY_NAME=bq27500-1-0 >>> POWER_SUPPLY_STATUS=Discharging >>> POWER_SUPPLY_PRESENT=1 >>> POWER_SUPPLY_VOLTAGE_NOW=3892000 >>> POWER_SUPPLY_CURRENT_NOW=-317000 >>> POWER_SUPPLY_CAPACITY=0 >> >> oops. >> >>> POWER_SUPPLY_CAPACITY_LEVEL=Low >>> POWER_SUPPLY_TEMP=223 >>> POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 >>> POWER_SUPPLY_TECHNOLOGY=Li-ion >>> POWER_SUPPLY_CHARGE_FULL=1147000 >>> POWER_SUPPLY_CHARGE_NOW=2665000 >>> POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 >>> POWER_SUPPLY_CYCLE_COUNT=6 >>> POWER_SUPPLY_ENERGY_NOW=0 >>> POWER_SUPPLY_POWER_AVG=64395 >>> POWER_SUPPLY_HEALTH=Dead >> >> oops. But maybe the bq27500 needs a full cycle first > > I wouldn't guess that the above state is due to the DM update > sequence, but I suppose that's possible. Another update pass might > clarify that. Ok, will do tomorrow. > >>> POWER_SUPPLY_MANUFACTURER=Texas Instruments >> >> vvv after plugging in charger >> >>> root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent >>> POWER_SUPPLY_NAME=bq27500-1-0 >>> POWER_SUPPLY_STATUS=Charging >>> POWER_SUPPLY_PRESENT=1 >>> POWER_SUPPLY_VOLTAGE_NOW=3923000 >>> POWER_SUPPLY_CURRENT_NOW=204000 >>> POWER_SUPPLY_CAPACITY=0 >>> POWER_SUPPLY_CAPACITY_LEVEL=Low >>> POWER_SUPPLY_TEMP=249 >>> POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 >>> POWER_SUPPLY_TECHNOLOGY=Li-ion >>> POWER_SUPPLY_CHARGE_FULL=1147000 >>> POWER_SUPPLY_CHARGE_NOW=2665000 >>> POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 >>> POWER_SUPPLY_CYCLE_COUNT=6 >>> POWER_SUPPLY_ENERGY_NOW=0 >>> POWER_SUPPLY_POWER_AVG=800 >>> POWER_SUPPLY_HEALTH=Dead >>> POWER_SUPPLY_MANUFACTURER=Texas Instruments >>> root@letux:~# >> >> Now a reboot after removing charger and battery for several minutes: >> >>> root@letux:~# dmesg|fgrep bq27 >>> [ 10.482818] bq27xxx_battery_setup >>> [ 12.179687] bq27xxx_battery_platform_probe >>> [ 12.179779] bq27xxx_battery_setup >>> [ 12.179809] bq27xxx_battery_setup: dm_regs= (null) >>> [ 12.182495] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix >>> [ 12.183502] bq27xxx_battery_settings >>> [ 12.183532] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree >>> [ 12.183563] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 >>> [ 15.145812] bq27xxx_battery_setup: dm_regs=bf0520e0 >>> [ 15.152618] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix >>> [ 15.346557] bq27xxx_battery_settings >>> [ 15.350585] bq27xxx_battery_set_config >>> [ 15.354553] bq27xxx_battery_unseal >>> [ 15.576538] bq27xxx_battery_update_dm_block >>> [ 15.580993] bq27xxx-battery 2-0055: design-capacity has 4100 >> >> ^^^ ok, no change needed >> >>> [ 15.676818] bq27xxx_battery_update_dm_block >>> [ 15.681243] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec >>> [ 15.798675] bq27xxx_battery_update_dm_block >>> [ 15.803100] bq27xxx-battery 2-0055: update terminate-voltage to 3250 >> >> ^^^ looks as if this is not stored in NVM. Should it be? > > Oh dear. I imagine this is due to a flaw in the multi-block DM update > logic. It could be that one of the pauses between calls should be > longer The terminate-voltage spec comes from this datasheet: > page 19, http://www.ti.com/lit/ds/symlink/bq27500.pdf > > It's possible that this second pass did update it. What does the next > reboot show? Will boot tomorrow with exactly the same system and report. > >> BTW: it would be nice if all "update" messages could tell the old value as well. > > We have the orig values in memory since we read the block before > updating. Feel free to patch that in :-) Ok. > >>> [ 16.011169] bq27xxx_battery_seal >>> root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent >>> POWER_SUPPLY_NAME=bq27500-1-0 >>> POWER_SUPPLY_STATUS=Discharging >>> POWER_SUPPLY_PRESENT=1 >>> POWER_SUPPLY_VOLTAGE_NOW=3894000 >>> POWER_SUPPLY_CURRENT_NOW=-291000 >>> POWER_SUPPLY_CAPACITY=71 >> >> ^^^ looks as if the bq27500 did recover from reprogramming >> >>> POWER_SUPPLY_CAPACITY_LEVEL=Normal >>> POWER_SUPPLY_TEMP=223 >>> POWER_SUPPLY_TIME_TO_EMPTY_NOW=31200 >>> POWER_SUPPLY_TECHNOLOGY=Li-ion >>> POWER_SUPPLY_CHARGE_FULL=3748000 >>> POWER_SUPPLY_CHARGE_NOW=2713000 >>> POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 >>> POWER_SUPPLY_CYCLE_COUNT=6 >>> POWER_SUPPLY_ENERGY_NOW=9930000 >>> POWER_SUPPLY_POWER_AVG=64407 >>> POWER_SUPPLY_HEALTH=Good >> >> ^^^ same here >> >>> POWER_SUPPLY_MANUFACTURER=Texas Instruments >>> root@letux:~# > > For general use on OpenPandora, you wouldn't have > config...dt_updates_nvm option set, Yes. > so the driver should report what > the chip settings are and whether they aren't what the DT spec'd. > Let's verify that too... Ok. BR, Nikolaus