Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421 + bq27500

[H. Nikolaus Schaller]

Hi Liam,

> Am 08.09.2017 um 22:28 schrieb H. Nikolaus Schaller :
> 
> 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 = <325>;
>>>   energy-full-design-microwatt-hours = <1480>;
>>>   charge-full-design-microamp-hours = <410>;
>>>   };
>>> 
>>> 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=410
 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.

It seems as if the bq27500 needs some time (or a reboot) to recover from no 
battery and
powered off. Here right after powering on again (same device, same SD card):

root@letux:~# dmesg|fgrep bq27
[   10.619140] bq27xxx_battery_setup
[   10.619171] bq27xxx_battery_setup: dm_regs=bf0520e0
[   10.621734] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
attribute, please fix
[   10.622680] bq27xxx_battery_settings
[   10.622955] bq27xxx_battery_set_config
[   10.622955] bq27xxx_battery_unseal
[   10.756469] bq27xxx_battery_update_dm_block
[   10.756500] bq27xxx-battery 2-0055: design-capacity has 4100
[   10.756500] bq27xxx_battery_update_dm_block
[   10.756530] bq27xxx-battery 2-0055: buffer does not match 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421 + bq27500

[H. Nikolaus Schaller]

Hi Liam,

> Am 08.09.2017 um 22:28 schrieb H. Nikolaus Schaller :
> 
> 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 = <325>;
>>>   energy-full-design-microwatt-hours = <1480>;
>>>   charge-full-design-microamp-hours = <410>;
>>>   };
>>> 
>>> 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=410
 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.

It seems as if the bq27500 needs some time (or a reboot) to recover from no 
battery and
powered off. Here right after powering on again (same device, same SD card):

root@letux:~# dmesg|fgrep bq27
[   10.619140] bq27xxx_battery_setup
[   10.619171] bq27xxx_battery_setup: dm_regs=bf0520e0
[   10.621734] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
attribute, please fix
[   10.622680] bq27xxx_battery_settings
[   10.622955] bq27xxx_battery_set_config
[   10.622955] bq27xxx_battery_unseal
[   10.756469] bq27xxx_battery_update_dm_block
[   10.756500] bq27xxx-battery 2-0055: design-capacity has 4100
[   10.756500] bq27xxx_battery_update_dm_block
[   10.756530] bq27xxx-battery 2-0055: buffer does not match design-energy dm 
spec
[   10.766723] bq27xxx_battery_update_dm_block
[   10.766754] 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

> Am 08.09.2017 um 22:57 schrieb Liam Breck :
> 
> Howdy,
> 
> On Fri, Sep 8, 2017 at 1:28 PM, H. Nikolaus Schaller  
> wrote:
>> 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 = <325>;
   energy-full-design-microwatt-hours = <1480>;
   charge-full-design-microamp-hours = <410>;
   };
 
 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.
> 
> Any idea why the driver detects a non-present bq27000 and calls
> probe() for it? defconfig with battery_bq27xxx=y ?

Yes.

The key reason is that hdq can't detect presence of a device
without support of the device driver. Which means as soon as
you have hdq and the battery driver it will be present and
create /sys/class/power entry. Which says "battery not present".

It is the best it can do. Especially for removeable batteries
(which might be installed some time after boot and probing).

If you want to study further, there is some code in the hdq
subsystem to load the bq27000.

BR,
Nikolaus

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

> Am 08.09.2017 um 22:57 schrieb Liam Breck :
> 
> Howdy,
> 
> On Fri, Sep 8, 2017 at 1:28 PM, H. Nikolaus Schaller  
> wrote:
>> 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 = <325>;
   energy-full-design-microwatt-hours = <1480>;
   charge-full-design-microamp-hours = <410>;
   };
 
 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.
> 
> Any idea why the driver detects a non-present bq27000 and calls
> probe() for it? defconfig with battery_bq27xxx=y ?

Yes.

The key reason is that hdq can't detect presence of a device
without support of the device driver. Which means as soon as
you have hdq and the battery driver it will be present and
create /sys/class/power entry. Which says "battery not present".

It is the best it can do. Especially for removeable batteries
(which might be installed some time after boot and probing).

If you want to study further, there is some code in the hdq
subsystem to load the bq27000.

BR,
Nikolaus

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

Howdy,

On Fri, Sep 8, 2017 at 1:28 PM, H. Nikolaus Schaller  wrote:
> 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 = <325>;
>>>energy-full-design-microwatt-hours = <1480>;
>>>charge-full-design-microamp-hours = <410>;
>>>};
>>>
>>> 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.

Any idea why the driver detects a non-present bq27000 and calls
probe() for it? defconfig with battery_bq27xxx=y ?

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

Howdy,

On Fri, Sep 8, 2017 at 1:28 PM, H. Nikolaus Schaller  wrote:
> 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 = <325>;
>>>energy-full-design-microwatt-hours = <1480>;
>>>charge-full-design-microamp-hours = <410>;
>>>};
>>>
>>> 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.

Any idea why the driver detects a non-present bq27000 and calls
probe() for it? defconfig with battery_bq27xxx=y ?

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

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 = <325>;
>>energy-full-design-microwatt-hours = <1480>;
>>charge-full-design-microamp-hours = <410>;
>>};
>> 
>> 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=410
>>> 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=410
>>> 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] 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

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 = <325>;
>>energy-full-design-microwatt-hours = <1480>;
>>charge-full-design-microamp-hours = <410>;
>>};
>> 
>> 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=410
>>> 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=410
>>> 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
>>> [   

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[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 = <325>;
> energy-full-design-microwatt-hours = <1480>;
> charge-full-design-microamp-hours = <410>;
> };
>
> 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...

> ... 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=410
>> 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.

>> 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=410
>> 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: 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[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 = <325>;
> energy-full-design-microwatt-hours = <1480>;
> charge-full-design-microamp-hours = <410>;
> };
>
> 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...

> ... 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=410
>> 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.

>> 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=410
>> 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 
>> 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

> Am 08.09.2017 um 19:40 schrieb Pali Rohár :
> 
> On Friday 08 September 2017 19:38:37 H. Nikolaus Schaller wrote:
>> 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
> 
> Hi! IIRC hwmon name cannot contain '-'. Use '_' instead.

Yes. I'd like to see it fixed as well...

AFAIR it is a general problem outside of battery drivers and IMHO there is some 
note somewhere.
Nobody "uses" a '-'. It is some logic that automatically creates node names 
which hwmon rejects.

BR,
Nikolaus

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

> Am 08.09.2017 um 19:40 schrieb Pali Rohár :
> 
> On Friday 08 September 2017 19:38:37 H. Nikolaus Schaller wrote:
>> 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
> 
> Hi! IIRC hwmon name cannot contain '-'. Use '_' instead.

Yes. I'd like to see it fixed as well...

AFAIR it is a general problem outside of battery drivers and IMHO there is some 
note somewhere.
Nobody "uses" a '-'. It is some logic that automatically creates node names 
which hwmon rejects.

BR,
Nikolaus

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Pali Rohár]

On Friday 08 September 2017 19:38:37 H. Nikolaus Schaller wrote:
> 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

Hi! IIRC hwmon name cannot contain '-'. Use '_' instead.

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Pali Rohár]

On Friday 08 September 2017 19:38:37 H. Nikolaus Schaller wrote:
> 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

Hi! IIRC hwmon name cannot contain '-'. Use '_' instead.

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

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 = <325>;
energy-full-design-microwatt-hours = <1480>;
charge-full-design-microamp-hours = <410>;
};

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

... 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=410
> 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

> 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=410
> 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?

BTW: it would be nice if all "update" messages could tell the old value as well.

> [   16.011169] 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

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 = <325>;
energy-full-design-microwatt-hours = <1480>;
charge-full-design-microamp-hours = <410>;
};

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

... 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=410
> 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

> 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=410
> 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?

BTW: it would be nice if all "update" messages could tell the old value as well.

> [   16.011169] bq27xxx_battery_seal
> root@letux:~# cat 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[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

Pls CC me on patch posts. You left me off your last series.

Cpl more comments below...

On Thu, Aug 31, 2017 at 2:35 AM, H. Nikolaus Schaller  
wrote:
> Hi Liam,
>
>> Am 31.08.2017 um 10:58 schrieb Liam Breck :
>>
>> On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller
>>  wrote:
>>> Hi Liam,
>>>
 Am 30.08.2017 um 13:24 schrieb Liam Breck :

 Hi Nikolaus,

 On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
 wrote:
> Hi Liam and Sebastian,
>
>> Am 29.08.2017 um 22:20 schrieb Liam Breck :
>>
>> Hi Nikolaus, thanks for the patch...
>>
>> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller 
>>  wrote:
>>> Tested on Pyra prototype with bq27421.
>>>
>>> Signed-off-by: H. Nikolaus Schaller 
>>> ---
>>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>>> b/drivers/power/supply/bq27xxx_battery.c
>>> index b6c3120ca5ad..e3c878ef7c7e 100644
>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>>> #define bq27545_dm_regs 0
>>> #endif
>>>
>>> -#if 0 /* not yet tested */
>>> +#if 1 /* tested on Pyra */
>>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>>
>> I think we can drop the #if and #else...#endif so it's just the
>> dm_regs table, as with bq27425.
>
> I have fixed that and did take the chance to update my OpenPandora
> kernel so that I also could test and make the bq27500 working.

 Is this gauge on the board or in the battery?
>>>
>>> It is on the board.
>>>
 If in the battery,
 monitored-battery should not be used.

 For a gauge on the board, if a user changes the battery to a different
 type, they need to update the dtb.
>>>
>>> Well, that is a little difficult to control, but here we have only
>>> one standard Pandora cell. There may exist replacements with different
>>> capacity, but that should not be our problem...
>>>

 I assume you built with config_battery_bq27xxx_dt_updates_nvm?
>>>
>>> Yes.
>>>

> The biggest hurdle was to find out that I have to change the
> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>
> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
> [   12.771392] bq27xxx_battery_i2c_probe call setup
> [   12.874816] bq27xxx_battery_setup
> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
> attribute, please fix
> [   13.234893] bq27xxx_battery_settings
> [   13.238891] bq27xxx-battery 2-0055: invalid 
> battery:energy-full-design-microwatt-hours 1480

 The chip does not support this param, so it should be zero, as it has
 to be set if charge-full-design-* is set. Can you try that?
>>>
>>> Hm. IMHO the DT should describe what the battery has and the driver should 
>>> simply ignore
>>> values it can't handle or reject them but do the best out of it. Telling 
>>> the driver to ignore
>>> a value by setting to 0 would IMHO be the wrong approach.
>>
>> The driver requires that if either charge- or energy-full-design is
>> set, then the other must be. Setting one and leaving the other at
>> default would be an error. Allowing any value for a field unsupported
>> by the chip, when supported field values must be within a range, isn't
>> useful for hw development or production scenarios. The solution for
>> shipped equipment is to drop the monitored-battery ref; see below.
>>
>>> We are also working on the generic-adc-battery driver that makes use of the 
>>> same battery DT
>>> node so that people can choose if they want to configure a kernel for fuel 
>>> gauge or
>>> g-a-b or even both.
>>>

> [   13.312469] bq27xxx_battery_set_config
> [   13.407745] bq27xxx_battery_unseal
> [   13.455993] bq27xxx_battery_update_dm_block
> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
> [   13.694885] bq27xxx_battery_seal

 We need to see output after a reboot.
>>>
>>> This was the value after first boot with the new driver enabled.
>>>
>>> A second boot after removing and reinserting battery shows:
>>>
>>> [   11.256713] bq27xxx_battery_setup
>>> [   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
>>> [  

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[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

Pls CC me on patch posts. You left me off your last series.

Cpl more comments below...

On Thu, Aug 31, 2017 at 2:35 AM, H. Nikolaus Schaller  
wrote:
> Hi Liam,
>
>> Am 31.08.2017 um 10:58 schrieb Liam Breck :
>>
>> On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller
>>  wrote:
>>> Hi Liam,
>>>
 Am 30.08.2017 um 13:24 schrieb Liam Breck :

 Hi Nikolaus,

 On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
 wrote:
> Hi Liam and Sebastian,
>
>> Am 29.08.2017 um 22:20 schrieb Liam Breck :
>>
>> Hi Nikolaus, thanks for the patch...
>>
>> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller 
>>  wrote:
>>> Tested on Pyra prototype with bq27421.
>>>
>>> Signed-off-by: H. Nikolaus Schaller 
>>> ---
>>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>>> b/drivers/power/supply/bq27xxx_battery.c
>>> index b6c3120ca5ad..e3c878ef7c7e 100644
>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>>> #define bq27545_dm_regs 0
>>> #endif
>>>
>>> -#if 0 /* not yet tested */
>>> +#if 1 /* tested on Pyra */
>>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>>
>> I think we can drop the #if and #else...#endif so it's just the
>> dm_regs table, as with bq27425.
>
> I have fixed that and did take the chance to update my OpenPandora
> kernel so that I also could test and make the bq27500 working.

 Is this gauge on the board or in the battery?
>>>
>>> It is on the board.
>>>
 If in the battery,
 monitored-battery should not be used.

 For a gauge on the board, if a user changes the battery to a different
 type, they need to update the dtb.
>>>
>>> Well, that is a little difficult to control, but here we have only
>>> one standard Pandora cell. There may exist replacements with different
>>> capacity, but that should not be our problem...
>>>

 I assume you built with config_battery_bq27xxx_dt_updates_nvm?
>>>
>>> Yes.
>>>

> The biggest hurdle was to find out that I have to change the
> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>
> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
> [   12.771392] bq27xxx_battery_i2c_probe call setup
> [   12.874816] bq27xxx_battery_setup
> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
> attribute, please fix
> [   13.234893] bq27xxx_battery_settings
> [   13.238891] bq27xxx-battery 2-0055: invalid 
> battery:energy-full-design-microwatt-hours 1480

 The chip does not support this param, so it should be zero, as it has
 to be set if charge-full-design-* is set. Can you try that?
>>>
>>> Hm. IMHO the DT should describe what the battery has and the driver should 
>>> simply ignore
>>> values it can't handle or reject them but do the best out of it. Telling 
>>> the driver to ignore
>>> a value by setting to 0 would IMHO be the wrong approach.
>>
>> The driver requires that if either charge- or energy-full-design is
>> set, then the other must be. Setting one and leaving the other at
>> default would be an error. Allowing any value for a field unsupported
>> by the chip, when supported field values must be within a range, isn't
>> useful for hw development or production scenarios. The solution for
>> shipped equipment is to drop the monitored-battery ref; see below.
>>
>>> We are also working on the generic-adc-battery driver that makes use of the 
>>> same battery DT
>>> node so that people can choose if they want to configure a kernel for fuel 
>>> gauge or
>>> g-a-b or even both.
>>>

> [   13.312469] bq27xxx_battery_set_config
> [   13.407745] bq27xxx_battery_unseal
> [   13.455993] bq27xxx_battery_update_dm_block
> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
> [   13.694885] bq27xxx_battery_seal

 We need to see output after a reboot.
>>>
>>> This was the value after first boot with the new driver enabled.
>>>
>>> A second boot after removing and reinserting battery shows:
>>>
>>> [   11.256713] bq27xxx_battery_setup
>>> [   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
>>> [   11.257690] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
>>> attribute, please fix
>>> [   11.258056] bq27xxx_battery_settings
>>> [   11.258300] 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Hi Liam,

> Am 31.08.2017 um 10:58 schrieb Liam Breck :
> 
> On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller
>  wrote:
>> Hi Liam,
>> 
>>> Am 30.08.2017 um 13:24 schrieb Liam Breck :
>>> 
>>> Hi Nikolaus,
>>> 
>>> On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
>>> wrote:
 Hi Liam and Sebastian,
 
> Am 29.08.2017 um 22:20 schrieb Liam Breck :
> 
> Hi Nikolaus, thanks for the patch...
> 
> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller 
>  wrote:
>> Tested on Pyra prototype with bq27421.
>> 
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>> b/drivers/power/supply/bq27xxx_battery.c
>> index b6c3120ca5ad..e3c878ef7c7e 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>> #define bq27545_dm_regs 0
>> #endif
>> 
>> -#if 0 /* not yet tested */
>> +#if 1 /* tested on Pyra */
>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
> 
> I think we can drop the #if and #else...#endif so it's just the
> dm_regs table, as with bq27425.
 
 I have fixed that and did take the chance to update my OpenPandora
 kernel so that I also could test and make the bq27500 working.
>>> 
>>> Is this gauge on the board or in the battery?
>> 
>> It is on the board.
>> 
>>> If in the battery,
>>> monitored-battery should not be used.
>>> 
>>> For a gauge on the board, if a user changes the battery to a different
>>> type, they need to update the dtb.
>> 
>> Well, that is a little difficult to control, but here we have only
>> one standard Pandora cell. There may exist replacements with different
>> capacity, but that should not be our problem...
>> 
>>> 
>>> I assume you built with config_battery_bq27xxx_dt_updates_nvm?
>> 
>> Yes.
>> 
>>> 
 The biggest hurdle was to find out that I have to change the
 compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
 
 [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
 [   12.771392] bq27xxx_battery_i2c_probe call setup
 [   12.874816] bq27xxx_battery_setup
 [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
 [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
 attribute, please fix
 [   13.234893] bq27xxx_battery_settings
 [   13.238891] bq27xxx-battery 2-0055: invalid 
 battery:energy-full-design-microwatt-hours 1480
>>> 
>>> The chip does not support this param, so it should be zero, as it has
>>> to be set if charge-full-design-* is set. Can you try that?
>> 
>> Hm. IMHO the DT should describe what the battery has and the driver should 
>> simply ignore
>> values it can't handle or reject them but do the best out of it. Telling the 
>> driver to ignore
>> a value by setting to 0 would IMHO be the wrong approach.
> 
> The driver requires that if either charge- or energy-full-design is
> set, then the other must be. Setting one and leaving the other at
> default would be an error. Allowing any value for a field unsupported
> by the chip, when supported field values must be within a range, isn't
> useful for hw development or production scenarios. The solution for
> shipped equipment is to drop the monitored-battery ref; see below.
> 
>> We are also working on the generic-adc-battery driver that makes use of the 
>> same battery DT
>> node so that people can choose if they want to configure a kernel for fuel 
>> gauge or
>> g-a-b or even both.
>> 
>>> 
 [   13.312469] bq27xxx_battery_set_config
 [   13.407745] bq27xxx_battery_unseal
 [   13.455993] bq27xxx_battery_update_dm_block
 [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
 [   13.694885] bq27xxx_battery_seal
>>> 
>>> We need to see output after a reboot.
>> 
>> This was the value after first boot with the new driver enabled.
>> 
>> A second boot after removing and reinserting battery shows:
>> 
>> [   11.256713] bq27xxx_battery_setup
>> [   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
>> [   11.257690] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
>> attribute, please fix
>> [   11.258056] bq27xxx_battery_settings
>> [   11.258300] bq27xxx-battery 2-0055: invalid 
>> battery:energy-full-design-microwatt-hours 1480
>> [   11.258300] bq27xxx_battery_set_config
>> [   11.258331] bq27xxx_battery_unseal
> 
> No mention of terminate-voltage in that run?

No, I didn't see it again.

> Or is this truncated?
> Looks like you didn't set energy-* to zero, so I can't tell if charge-* works.

Seems to need more 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Hi Liam,

> Am 31.08.2017 um 10:58 schrieb Liam Breck :
> 
> On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller
>  wrote:
>> Hi Liam,
>> 
>>> Am 30.08.2017 um 13:24 schrieb Liam Breck :
>>> 
>>> Hi Nikolaus,
>>> 
>>> On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
>>> wrote:
 Hi Liam and Sebastian,
 
> Am 29.08.2017 um 22:20 schrieb Liam Breck :
> 
> Hi Nikolaus, thanks for the patch...
> 
> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller 
>  wrote:
>> Tested on Pyra prototype with bq27421.
>> 
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>> b/drivers/power/supply/bq27xxx_battery.c
>> index b6c3120ca5ad..e3c878ef7c7e 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>> #define bq27545_dm_regs 0
>> #endif
>> 
>> -#if 0 /* not yet tested */
>> +#if 1 /* tested on Pyra */
>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
> 
> I think we can drop the #if and #else...#endif so it's just the
> dm_regs table, as with bq27425.
 
 I have fixed that and did take the chance to update my OpenPandora
 kernel so that I also could test and make the bq27500 working.
>>> 
>>> Is this gauge on the board or in the battery?
>> 
>> It is on the board.
>> 
>>> If in the battery,
>>> monitored-battery should not be used.
>>> 
>>> For a gauge on the board, if a user changes the battery to a different
>>> type, they need to update the dtb.
>> 
>> Well, that is a little difficult to control, but here we have only
>> one standard Pandora cell. There may exist replacements with different
>> capacity, but that should not be our problem...
>> 
>>> 
>>> I assume you built with config_battery_bq27xxx_dt_updates_nvm?
>> 
>> Yes.
>> 
>>> 
 The biggest hurdle was to find out that I have to change the
 compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
 
 [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
 [   12.771392] bq27xxx_battery_i2c_probe call setup
 [   12.874816] bq27xxx_battery_setup
 [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
 [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
 attribute, please fix
 [   13.234893] bq27xxx_battery_settings
 [   13.238891] bq27xxx-battery 2-0055: invalid 
 battery:energy-full-design-microwatt-hours 1480
>>> 
>>> The chip does not support this param, so it should be zero, as it has
>>> to be set if charge-full-design-* is set. Can you try that?
>> 
>> Hm. IMHO the DT should describe what the battery has and the driver should 
>> simply ignore
>> values it can't handle or reject them but do the best out of it. Telling the 
>> driver to ignore
>> a value by setting to 0 would IMHO be the wrong approach.
> 
> The driver requires that if either charge- or energy-full-design is
> set, then the other must be. Setting one and leaving the other at
> default would be an error. Allowing any value for a field unsupported
> by the chip, when supported field values must be within a range, isn't
> useful for hw development or production scenarios. The solution for
> shipped equipment is to drop the monitored-battery ref; see below.
> 
>> We are also working on the generic-adc-battery driver that makes use of the 
>> same battery DT
>> node so that people can choose if they want to configure a kernel for fuel 
>> gauge or
>> g-a-b or even both.
>> 
>>> 
 [   13.312469] bq27xxx_battery_set_config
 [   13.407745] bq27xxx_battery_unseal
 [   13.455993] bq27xxx_battery_update_dm_block
 [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
 [   13.694885] bq27xxx_battery_seal
>>> 
>>> We need to see output after a reboot.
>> 
>> This was the value after first boot with the new driver enabled.
>> 
>> A second boot after removing and reinserting battery shows:
>> 
>> [   11.256713] bq27xxx_battery_setup
>> [   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
>> [   11.257690] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
>> attribute, please fix
>> [   11.258056] bq27xxx_battery_settings
>> [   11.258300] bq27xxx-battery 2-0055: invalid 
>> battery:energy-full-design-microwatt-hours 1480
>> [   11.258300] bq27xxx_battery_set_config
>> [   11.258331] bq27xxx_battery_unseal
> 
> No mention of terminate-voltage in that run?

No, I didn't see it again.

> Or is this truncated?
> Looks like you didn't set energy-* to zero, so I can't tell if charge-* works.

Seems to need more experimentation (and more time for it).

> 
>>> Note that this chip has NVM so
>>> these settings persist without power.
>> 
>> Yes I know. Up to now the 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller
 wrote:
> Hi Liam,
>
>> Am 30.08.2017 um 13:24 schrieb Liam Breck :
>>
>> Hi Nikolaus,
>>
>> On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
>> wrote:
>>> Hi Liam and Sebastian,
>>>
 Am 29.08.2017 um 22:20 schrieb Liam Breck :

 Hi Nikolaus, thanks for the patch...

 On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
 wrote:
> Tested on Pyra prototype with bq27421.
>
> Signed-off-by: H. Nikolaus Schaller 
> ---
> drivers/power/supply/bq27xxx_battery.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index b6c3120ca5ad..e3c878ef7c7e 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
> #define bq27545_dm_regs 0
> #endif
>
> -#if 0 /* not yet tested */
> +#if 1 /* tested on Pyra */
> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {

 I think we can drop the #if and #else...#endif so it's just the
 dm_regs table, as with bq27425.
>>>
>>> I have fixed that and did take the chance to update my OpenPandora
>>> kernel so that I also could test and make the bq27500 working.
>>
>> Is this gauge on the board or in the battery?
>
> It is on the board.
>
>> If in the battery,
>> monitored-battery should not be used.
>>
>> For a gauge on the board, if a user changes the battery to a different
>> type, they need to update the dtb.
>
> Well, that is a little difficult to control, but here we have only
> one standard Pandora cell. There may exist replacements with different
> capacity, but that should not be our problem...
>
>>
>> I assume you built with config_battery_bq27xxx_dt_updates_nvm?
>
> Yes.
>
>>
>>> The biggest hurdle was to find out that I have to change the
>>> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>>>
>>> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
>>> [   12.771392] bq27xxx_battery_i2c_probe call setup
>>> [   12.874816] bq27xxx_battery_setup
>>> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
>>> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
>>> attribute, please fix
>>> [   13.234893] bq27xxx_battery_settings
>>> [   13.238891] bq27xxx-battery 2-0055: invalid 
>>> battery:energy-full-design-microwatt-hours 1480
>>
>> The chip does not support this param, so it should be zero, as it has
>> to be set if charge-full-design-* is set. Can you try that?
>
> Hm. IMHO the DT should describe what the battery has and the driver should 
> simply ignore
> values it can't handle or reject them but do the best out of it. Telling the 
> driver to ignore
> a value by setting to 0 would IMHO be the wrong approach.

The driver requires that if either charge- or energy-full-design is
set, then the other must be. Setting one and leaving the other at
default would be an error. Allowing any value for a field unsupported
by the chip, when supported field values must be within a range, isn't
useful for hw development or production scenarios. The solution for
shipped equipment is to drop the monitored-battery ref; see below.

> We are also working on the generic-adc-battery driver that makes use of the 
> same battery DT
> node so that people can choose if they want to configure a kernel for fuel 
> gauge or
> g-a-b or even both.
>
>>
>>> [   13.312469] bq27xxx_battery_set_config
>>> [   13.407745] bq27xxx_battery_unseal
>>> [   13.455993] bq27xxx_battery_update_dm_block
>>> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
>>> [   13.694885] bq27xxx_battery_seal
>>
>> We need to see output after a reboot.
>
> This was the value after first boot with the new driver enabled.
>
> A second boot after removing and reinserting battery shows:
>
> [   11.256713] bq27xxx_battery_setup
> [   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
> [   11.257690] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
> attribute, please fix
> [   11.258056] bq27xxx_battery_settings
> [   11.258300] bq27xxx-battery 2-0055: invalid 
> battery:energy-full-design-microwatt-hours 1480
> [   11.258300] bq27xxx_battery_set_config
> [   11.258331] bq27xxx_battery_unseal

No mention of terminate-voltage in that run? Or is this truncated?
Looks like you didn't set energy-* to zero, so I can't tell if charge-* works.

>> Note that this chip has NVM so
>> these settings persist without power.
>
> Yes I know. Up to now the bq27500 has been programmed during production test
> by some special flashing tool. Now as the kernel driver has this capability,
> we should make (optionally) use of it.

The kernel driver has this 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller
 wrote:
> Hi Liam,
>
>> Am 30.08.2017 um 13:24 schrieb Liam Breck :
>>
>> Hi Nikolaus,
>>
>> On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
>> wrote:
>>> Hi Liam and Sebastian,
>>>
 Am 29.08.2017 um 22:20 schrieb Liam Breck :

 Hi Nikolaus, thanks for the patch...

 On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
 wrote:
> Tested on Pyra prototype with bq27421.
>
> Signed-off-by: H. Nikolaus Schaller 
> ---
> drivers/power/supply/bq27xxx_battery.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index b6c3120ca5ad..e3c878ef7c7e 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
> #define bq27545_dm_regs 0
> #endif
>
> -#if 0 /* not yet tested */
> +#if 1 /* tested on Pyra */
> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {

 I think we can drop the #if and #else...#endif so it's just the
 dm_regs table, as with bq27425.
>>>
>>> I have fixed that and did take the chance to update my OpenPandora
>>> kernel so that I also could test and make the bq27500 working.
>>
>> Is this gauge on the board or in the battery?
>
> It is on the board.
>
>> If in the battery,
>> monitored-battery should not be used.
>>
>> For a gauge on the board, if a user changes the battery to a different
>> type, they need to update the dtb.
>
> Well, that is a little difficult to control, but here we have only
> one standard Pandora cell. There may exist replacements with different
> capacity, but that should not be our problem...
>
>>
>> I assume you built with config_battery_bq27xxx_dt_updates_nvm?
>
> Yes.
>
>>
>>> The biggest hurdle was to find out that I have to change the
>>> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>>>
>>> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
>>> [   12.771392] bq27xxx_battery_i2c_probe call setup
>>> [   12.874816] bq27xxx_battery_setup
>>> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
>>> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
>>> attribute, please fix
>>> [   13.234893] bq27xxx_battery_settings
>>> [   13.238891] bq27xxx-battery 2-0055: invalid 
>>> battery:energy-full-design-microwatt-hours 1480
>>
>> The chip does not support this param, so it should be zero, as it has
>> to be set if charge-full-design-* is set. Can you try that?
>
> Hm. IMHO the DT should describe what the battery has and the driver should 
> simply ignore
> values it can't handle or reject them but do the best out of it. Telling the 
> driver to ignore
> a value by setting to 0 would IMHO be the wrong approach.

The driver requires that if either charge- or energy-full-design is
set, then the other must be. Setting one and leaving the other at
default would be an error. Allowing any value for a field unsupported
by the chip, when supported field values must be within a range, isn't
useful for hw development or production scenarios. The solution for
shipped equipment is to drop the monitored-battery ref; see below.

> We are also working on the generic-adc-battery driver that makes use of the 
> same battery DT
> node so that people can choose if they want to configure a kernel for fuel 
> gauge or
> g-a-b or even both.
>
>>
>>> [   13.312469] bq27xxx_battery_set_config
>>> [   13.407745] bq27xxx_battery_unseal
>>> [   13.455993] bq27xxx_battery_update_dm_block
>>> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
>>> [   13.694885] bq27xxx_battery_seal
>>
>> We need to see output after a reboot.
>
> This was the value after first boot with the new driver enabled.
>
> A second boot after removing and reinserting battery shows:
>
> [   11.256713] bq27xxx_battery_setup
> [   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
> [   11.257690] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
> attribute, please fix
> [   11.258056] bq27xxx_battery_settings
> [   11.258300] bq27xxx-battery 2-0055: invalid 
> battery:energy-full-design-microwatt-hours 1480
> [   11.258300] bq27xxx_battery_set_config
> [   11.258331] bq27xxx_battery_unseal

No mention of terminate-voltage in that run? Or is this truncated?
Looks like you didn't set energy-* to zero, so I can't tell if charge-* works.

>> Note that this chip has NVM so
>> these settings persist without power.
>
> Yes I know. Up to now the bq27500 has been programmed during production test
> by some special flashing tool. Now as the kernel driver has this capability,
> we should make (optionally) use of it.

The kernel driver has this feature primarily for gauges that lack NVM.
It sets those without config_battery_bq27xxx_dt_updates_nvm. The NVM
programming 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Hi Liam,

> Am 30.08.2017 um 13:24 schrieb Liam Breck :
> 
> Hi Nikolaus,
> 
> On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
> wrote:
>> Hi Liam and Sebastian,
>> 
>>> Am 29.08.2017 um 22:20 schrieb Liam Breck :
>>> 
>>> Hi Nikolaus, thanks for the patch...
>>> 
>>> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
>>> wrote:
 Tested on Pyra prototype with bq27421.
 
 Signed-off-by: H. Nikolaus Schaller 
 ---
 drivers/power/supply/bq27xxx_battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/power/supply/bq27xxx_battery.c 
 b/drivers/power/supply/bq27xxx_battery.c
 index b6c3120ca5ad..e3c878ef7c7e 100644
 --- a/drivers/power/supply/bq27xxx_battery.c
 +++ b/drivers/power/supply/bq27xxx_battery.c
 @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
 #define bq27545_dm_regs 0
 #endif
 
 -#if 0 /* not yet tested */
 +#if 1 /* tested on Pyra */
 static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>>> 
>>> I think we can drop the #if and #else...#endif so it's just the
>>> dm_regs table, as with bq27425.
>> 
>> I have fixed that and did take the chance to update my OpenPandora
>> kernel so that I also could test and make the bq27500 working.
> 
> Is this gauge on the board or in the battery?

It is on the board.

> If in the battery,
> monitored-battery should not be used.
> 
> For a gauge on the board, if a user changes the battery to a different
> type, they need to update the dtb.

Well, that is a little difficult to control, but here we have only
one standard Pandora cell. There may exist replacements with different
capacity, but that should not be our problem...

> 
> I assume you built with config_battery_bq27xxx_dt_updates_nvm?

Yes.

> 
>> The biggest hurdle was to find out that I have to change the
>> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>> 
>> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
>> [   12.771392] bq27xxx_battery_i2c_probe call setup
>> [   12.874816] bq27xxx_battery_setup
>> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
>> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
>> attribute, please fix
>> [   13.234893] bq27xxx_battery_settings
>> [   13.238891] bq27xxx-battery 2-0055: invalid 
>> battery:energy-full-design-microwatt-hours 1480
> 
> The chip does not support this param, so it should be zero, as it has
> to be set if charge-full-design-* is set. Can you try that?

Hm. IMHO the DT should describe what the battery has and the driver should 
simply ignore
values it can't handle or reject them but do the best out of it. Telling the 
driver to ignore
a value by setting to 0 would IMHO be the wrong approach.

We are also working on the generic-adc-battery driver that makes use of the 
same battery DT
node so that people can choose if they want to configure a kernel for fuel 
gauge or
g-a-b or even both.

> 
>> [   13.312469] bq27xxx_battery_set_config
>> [   13.407745] bq27xxx_battery_unseal
>> [   13.455993] bq27xxx_battery_update_dm_block
>> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
>> [   13.694885] bq27xxx_battery_seal
> 
> We need to see output after a reboot.

This was the value after first boot with the new driver enabled.

A second boot after removing and reinserting battery shows:

[   11.256713] bq27xxx_battery_setup
[   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
[   11.257690] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
attribute, please fix
[   11.258056] bq27xxx_battery_settings
[   11.258300] bq27xxx-battery 2-0055: invalid 
battery:energy-full-design-microwatt-hours 1480
[   11.258300] bq27xxx_battery_set_config
[   11.258331] bq27xxx_battery_unseal

> Note that this chip has NVM so
> these settings persist without power.

Yes I know. Up to now the bq27500 has been programmed during production test
by some special flashing tool. Now as the kernel driver has this capability,
we should make (optionally) use of it.

> 
>> Does this look ok for
>> 
>>bat: battery {
>>compatible = "simple-battery", "openpandora-battery";
>>voltage-min-design-microvolt = <320>;
>>energy-full-design-microwatt-hours = <1480>;
>>charge-full-design-microamp-hours = <400>;
>>};
>> 
>> ?

I mainly had some initial doubts that it did not tell something like
"update design-capacity" to 4000 or "design-capacity has 4000"

Ah, I see:

/* assume design energy & capacity are in same block */

This means the bq27500 never programs capacity because we can't specify energy.
So I't suggest to revise this rule and coupling of energy and capacity setting.

>> 
>> I will send a patch for enabling both fuel gauges and the 
>> 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Hi Liam,

> Am 30.08.2017 um 13:24 schrieb Liam Breck :
> 
> Hi Nikolaus,
> 
> On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
> wrote:
>> Hi Liam and Sebastian,
>> 
>>> Am 29.08.2017 um 22:20 schrieb Liam Breck :
>>> 
>>> Hi Nikolaus, thanks for the patch...
>>> 
>>> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
>>> wrote:
 Tested on Pyra prototype with bq27421.
 
 Signed-off-by: H. Nikolaus Schaller 
 ---
 drivers/power/supply/bq27xxx_battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/power/supply/bq27xxx_battery.c 
 b/drivers/power/supply/bq27xxx_battery.c
 index b6c3120ca5ad..e3c878ef7c7e 100644
 --- a/drivers/power/supply/bq27xxx_battery.c
 +++ b/drivers/power/supply/bq27xxx_battery.c
 @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
 #define bq27545_dm_regs 0
 #endif
 
 -#if 0 /* not yet tested */
 +#if 1 /* tested on Pyra */
 static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>>> 
>>> I think we can drop the #if and #else...#endif so it's just the
>>> dm_regs table, as with bq27425.
>> 
>> I have fixed that and did take the chance to update my OpenPandora
>> kernel so that I also could test and make the bq27500 working.
> 
> Is this gauge on the board or in the battery?

It is on the board.

> If in the battery,
> monitored-battery should not be used.
> 
> For a gauge on the board, if a user changes the battery to a different
> type, they need to update the dtb.

Well, that is a little difficult to control, but here we have only
one standard Pandora cell. There may exist replacements with different
capacity, but that should not be our problem...

> 
> I assume you built with config_battery_bq27xxx_dt_updates_nvm?

Yes.

> 
>> The biggest hurdle was to find out that I have to change the
>> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>> 
>> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
>> [   12.771392] bq27xxx_battery_i2c_probe call setup
>> [   12.874816] bq27xxx_battery_setup
>> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
>> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
>> attribute, please fix
>> [   13.234893] bq27xxx_battery_settings
>> [   13.238891] bq27xxx-battery 2-0055: invalid 
>> battery:energy-full-design-microwatt-hours 1480
> 
> The chip does not support this param, so it should be zero, as it has
> to be set if charge-full-design-* is set. Can you try that?

Hm. IMHO the DT should describe what the battery has and the driver should 
simply ignore
values it can't handle or reject them but do the best out of it. Telling the 
driver to ignore
a value by setting to 0 would IMHO be the wrong approach.

We are also working on the generic-adc-battery driver that makes use of the 
same battery DT
node so that people can choose if they want to configure a kernel for fuel 
gauge or
g-a-b or even both.

> 
>> [   13.312469] bq27xxx_battery_set_config
>> [   13.407745] bq27xxx_battery_unseal
>> [   13.455993] bq27xxx_battery_update_dm_block
>> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
>> [   13.694885] bq27xxx_battery_seal
> 
> We need to see output after a reboot.

This was the value after first boot with the new driver enabled.

A second boot after removing and reinserting battery shows:

[   11.256713] bq27xxx_battery_setup
[   11.256744] bq27xxx_battery_setup: dm_regs=bf05b0e0
[   11.257690] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
attribute, please fix
[   11.258056] bq27xxx_battery_settings
[   11.258300] bq27xxx-battery 2-0055: invalid 
battery:energy-full-design-microwatt-hours 1480
[   11.258300] bq27xxx_battery_set_config
[   11.258331] bq27xxx_battery_unseal

> Note that this chip has NVM so
> these settings persist without power.

Yes I know. Up to now the bq27500 has been programmed during production test
by some special flashing tool. Now as the kernel driver has this capability,
we should make (optionally) use of it.

> 
>> Does this look ok for
>> 
>>bat: battery {
>>compatible = "simple-battery", "openpandora-battery";
>>voltage-min-design-microvolt = <320>;
>>energy-full-design-microwatt-hours = <1480>;
>>charge-full-design-microamp-hours = <400>;
>>};
>> 
>> ?

I mainly had some initial doubts that it did not tell something like
"update design-capacity" to 4000 or "design-capacity has 4000"

Ah, I see:

/* assume design energy & capacity are in same block */

This means the bq27500 never programs capacity because we can't specify energy.
So I't suggest to revise this rule and coupling of energy and capacity setting.

>> 
>> I will send a patch for enabling both fuel gauges and the 
>> omap3-pandora-common.dtsi asap.
> 
> You probably don't want this in upstream dts, as it only programs the
> gauge 

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

Hi Nikolaus,

On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
wrote:
> Hi Liam and Sebastian,
>
>> Am 29.08.2017 um 22:20 schrieb Liam Breck :
>>
>> Hi Nikolaus, thanks for the patch...
>>
>> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
>> wrote:
>>> Tested on Pyra prototype with bq27421.
>>>
>>> Signed-off-by: H. Nikolaus Schaller 
>>> ---
>>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>>> b/drivers/power/supply/bq27xxx_battery.c
>>> index b6c3120ca5ad..e3c878ef7c7e 100644
>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>>> #define bq27545_dm_regs 0
>>> #endif
>>>
>>> -#if 0 /* not yet tested */
>>> +#if 1 /* tested on Pyra */
>>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>>
>> I think we can drop the #if and #else...#endif so it's just the
>> dm_regs table, as with bq27425.
>
> I have fixed that and did take the chance to update my OpenPandora
> kernel so that I also could test and make the bq27500 working.

Is this gauge on the board or in the battery? If in the battery,
monitored-battery should not be used.

For a gauge on the board, if a user changes the battery to a different
type, they need to update the dtb.

I assume you built with config_battery_bq27xxx_dt_updates_nvm?

> The biggest hurdle was to find out that I have to change the
> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>
> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
> [   12.771392] bq27xxx_battery_i2c_probe call setup
> [   12.874816] bq27xxx_battery_setup
> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
> attribute, please fix
> [   13.234893] bq27xxx_battery_settings
> [   13.238891] bq27xxx-battery 2-0055: invalid 
> battery:energy-full-design-microwatt-hours 1480

The chip does not support this param, so it should be zero, as it has
to be set if charge-full-design-* is set. Can you try that?

> [   13.312469] bq27xxx_battery_set_config
> [   13.407745] bq27xxx_battery_unseal
> [   13.455993] bq27xxx_battery_update_dm_block
> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
> [   13.694885] bq27xxx_battery_seal

We need to see output after a reboot. Note that this chip has NVM so
these settings persist without power.

> Does this look ok for
>
> bat: battery {
> compatible = "simple-battery", "openpandora-battery";
> voltage-min-design-microvolt = <320>;
> energy-full-design-microwatt-hours = <1480>;
> charge-full-design-microamp-hours = <400>;
> };
>
> ?
>
> I will send a patch for enabling both fuel gauges and the 
> omap3-pandora-common.dtsi asap.

You probably don't want this in upstream dts, as it only programs the
gauge if the kernel has the above config option. If the box runs a
stock distro, it won't work. A custom-built kernel with the above dts
programs the gauge unless the user sets the module param
dt_monitored_battery_updates_nvm=0. The requisite dtb should be
packaged with the custom-built kernel, and deployed with awareness of
the actual battery present.

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

Hi Nikolaus,

On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller  
wrote:
> Hi Liam and Sebastian,
>
>> Am 29.08.2017 um 22:20 schrieb Liam Breck :
>>
>> Hi Nikolaus, thanks for the patch...
>>
>> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
>> wrote:
>>> Tested on Pyra prototype with bq27421.
>>>
>>> Signed-off-by: H. Nikolaus Schaller 
>>> ---
>>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>>> b/drivers/power/supply/bq27xxx_battery.c
>>> index b6c3120ca5ad..e3c878ef7c7e 100644
>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>>> #define bq27545_dm_regs 0
>>> #endif
>>>
>>> -#if 0 /* not yet tested */
>>> +#if 1 /* tested on Pyra */
>>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>>
>> I think we can drop the #if and #else...#endif so it's just the
>> dm_regs table, as with bq27425.
>
> I have fixed that and did take the chance to update my OpenPandora
> kernel so that I also could test and make the bq27500 working.

Is this gauge on the board or in the battery? If in the battery,
monitored-battery should not be used.

For a gauge on the board, if a user changes the battery to a different
type, they need to update the dtb.

I assume you built with config_battery_bq27xxx_dt_updates_nvm?

> The biggest hurdle was to find out that I have to change the
> compatible string to "ti,bq27500-1" to get non-NULL dm_regs...
>
> [   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
> [   12.771392] bq27xxx_battery_i2c_probe call setup
> [   12.874816] bq27xxx_battery_setup
> [   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
> [   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
> attribute, please fix
> [   13.234893] bq27xxx_battery_settings
> [   13.238891] bq27xxx-battery 2-0055: invalid 
> battery:energy-full-design-microwatt-hours 1480

The chip does not support this param, so it should be zero, as it has
to be set if charge-full-design-* is set. Can you try that?

> [   13.312469] bq27xxx_battery_set_config
> [   13.407745] bq27xxx_battery_unseal
> [   13.455993] bq27xxx_battery_update_dm_block
> [   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
> [   13.694885] bq27xxx_battery_seal

We need to see output after a reboot. Note that this chip has NVM so
these settings persist without power.

> Does this look ok for
>
> bat: battery {
> compatible = "simple-battery", "openpandora-battery";
> voltage-min-design-microvolt = <320>;
> energy-full-design-microwatt-hours = <1480>;
> charge-full-design-microamp-hours = <400>;
> };
>
> ?
>
> I will send a patch for enabling both fuel gauges and the 
> omap3-pandora-common.dtsi asap.

You probably don't want this in upstream dts, as it only programs the
gauge if the kernel has the above config option. If the box runs a
stock distro, it won't work. A custom-built kernel with the above dts
programs the gauge unless the user sets the module param
dt_monitored_battery_updates_nvm=0. The requisite dtb should be
packaged with the custom-built kernel, and deployed with awareness of
the actual battery present.

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Hi Liam and Sebastian,

> Am 29.08.2017 um 22:20 schrieb Liam Breck :
> 
> Hi Nikolaus, thanks for the patch...
> 
> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
> wrote:
>> Tested on Pyra prototype with bq27421.
>> 
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>> b/drivers/power/supply/bq27xxx_battery.c
>> index b6c3120ca5ad..e3c878ef7c7e 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>> #define bq27545_dm_regs 0
>> #endif
>> 
>> -#if 0 /* not yet tested */
>> +#if 1 /* tested on Pyra */
>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
> 
> I think we can drop the #if and #else...#endif so it's just the
> dm_regs table, as with bq27425.

I have fixed that and did take the chance to update my OpenPandora
kernel so that I also could test and make the bq27500 working.

The biggest hurdle was to find out that I have to change the
compatible string to "ti,bq27500-1" to get non-NULL dm_regs...

[   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
[   12.771392] bq27xxx_battery_i2c_probe call setup
[   12.874816] bq27xxx_battery_setup
[   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
[   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
attribute, please fix
[   13.234893] bq27xxx_battery_settings
[   13.238891] bq27xxx-battery 2-0055: invalid 
battery:energy-full-design-microwatt-hours 1480
[   13.312469] bq27xxx_battery_set_config
[   13.407745] bq27xxx_battery_unseal
[   13.455993] bq27xxx_battery_update_dm_block
[   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
[   13.694885] bq27xxx_battery_seal

Does this look ok for

bat: battery {
compatible = "simple-battery", "openpandora-battery";
voltage-min-design-microvolt = <320>;
energy-full-design-microwatt-hours = <1480>;
charge-full-design-microamp-hours = <400>;
};

?

I will send a patch for enabling both fuel gauges and the 
omap3-pandora-common.dtsi asap.

BR and thanks,
Nikolaus

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Hi Liam and Sebastian,

> Am 29.08.2017 um 22:20 schrieb Liam Breck :
> 
> Hi Nikolaus, thanks for the patch...
> 
> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
> wrote:
>> Tested on Pyra prototype with bq27421.
>> 
>> Signed-off-by: H. Nikolaus Schaller 
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>> b/drivers/power/supply/bq27xxx_battery.c
>> index b6c3120ca5ad..e3c878ef7c7e 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>> #define bq27545_dm_regs 0
>> #endif
>> 
>> -#if 0 /* not yet tested */
>> +#if 1 /* tested on Pyra */
>> static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
> 
> I think we can drop the #if and #else...#endif so it's just the
> dm_regs table, as with bq27425.

I have fixed that and did take the chance to update my OpenPandora
kernel so that I also could test and make the bq27500 working.

The biggest hurdle was to find out that I have to change the
compatible string to "ti,bq27500-1" to get non-NULL dm_regs...

[   12.765930] bq27xxx_battery_i2c_probe name=bq27500-1-0
[   12.771392] bq27xxx_battery_i2c_probe call setup
[   12.874816] bq27xxx_battery_setup
[   12.904266] bq27xxx_battery_setup: dm_regs=bf0520e0
[   12.933380] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name 
attribute, please fix
[   13.234893] bq27xxx_battery_settings
[   13.238891] bq27xxx-battery 2-0055: invalid 
battery:energy-full-design-microwatt-hours 1480
[   13.312469] bq27xxx_battery_set_config
[   13.407745] bq27xxx_battery_unseal
[   13.455993] bq27xxx_battery_update_dm_block
[   13.460418] bq27xxx-battery 2-0055: update terminate-voltage to 3200
[   13.694885] bq27xxx_battery_seal

Does this look ok for

bat: battery {
compatible = "simple-battery", "openpandora-battery";
voltage-min-design-microvolt = <320>;
energy-full-design-microwatt-hours = <1480>;
charge-full-design-microamp-hours = <400>;
};

?

I will send a patch for enabling both fuel gauges and the 
omap3-pandora-common.dtsi asap.

BR and thanks,
Nikolaus

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Sebastian Reichel]

Hi,

On Tue, Aug 29, 2017 at 01:20:27PM -0700, Liam Breck wrote:
> Hi Nikolaus, thanks for the patch...
> 
> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
> wrote:
> > Tested on Pyra prototype with bq27421.
> >
> > Signed-off-by: H. Nikolaus Schaller 
> > ---
> >  drivers/power/supply/bq27xxx_battery.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/supply/bq27xxx_battery.c 
> > b/drivers/power/supply/bq27xxx_battery.c
> > index b6c3120ca5ad..e3c878ef7c7e 100644
> > --- a/drivers/power/supply/bq27xxx_battery.c
> > +++ b/drivers/power/supply/bq27xxx_battery.c
> > @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
> >  #define bq27545_dm_regs 0
> >  #endif
> >
> > -#if 0 /* not yet tested */
> > +#if 1 /* tested on Pyra */
> >  static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>
> I think we can drop the #if and #else...#endif so it's just the
> dm_regs table, as with bq27425.

Yes, please.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Sebastian Reichel]

Hi,

On Tue, Aug 29, 2017 at 01:20:27PM -0700, Liam Breck wrote:
> Hi Nikolaus, thanks for the patch...
> 
> On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
> wrote:
> > Tested on Pyra prototype with bq27421.
> >
> > Signed-off-by: H. Nikolaus Schaller 
> > ---
> >  drivers/power/supply/bq27xxx_battery.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/supply/bq27xxx_battery.c 
> > b/drivers/power/supply/bq27xxx_battery.c
> > index b6c3120ca5ad..e3c878ef7c7e 100644
> > --- a/drivers/power/supply/bq27xxx_battery.c
> > +++ b/drivers/power/supply/bq27xxx_battery.c
> > @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
> >  #define bq27545_dm_regs 0
> >  #endif
> >
> > -#if 0 /* not yet tested */
> > +#if 1 /* tested on Pyra */
> >  static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
>
> I think we can drop the #if and #else...#endif so it's just the
> dm_regs table, as with bq27425.

Yes, please.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

Hi Nikolaus, thanks for the patch...

On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
wrote:
> Tested on Pyra prototype with bq27421.
>
> Signed-off-by: H. Nikolaus Schaller 
> ---
>  drivers/power/supply/bq27xxx_battery.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index b6c3120ca5ad..e3c878ef7c7e 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>  #define bq27545_dm_regs 0
>  #endif
>
> -#if 0 /* not yet tested */
> +#if 1 /* tested on Pyra */
>  static struct bq27xxx_dm_reg bq27421_dm_regs[] = {

I think we can drop the #if and #else...#endif so it's just the
dm_regs table, as with bq27425.

Re: [PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[Liam Breck]

Hi Nikolaus, thanks for the patch...

On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller  
wrote:
> Tested on Pyra prototype with bq27421.
>
> Signed-off-by: H. Nikolaus Schaller 
> ---
>  drivers/power/supply/bq27xxx_battery.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index b6c3120ca5ad..e3c878ef7c7e 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
>  #define bq27545_dm_regs 0
>  #endif
>
> -#if 0 /* not yet tested */
> +#if 1 /* tested on Pyra */
>  static struct bq27xxx_dm_reg bq27421_dm_regs[] = {

I think we can drop the #if and #else...#endif so it's just the
dm_regs table, as with bq27425.

[PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Tested on Pyra prototype with bq27421.

Signed-off-by: H. Nikolaus Schaller 
---
 drivers/power/supply/bq27xxx_battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index b6c3120ca5ad..e3c878ef7c7e 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
 #define bq27545_dm_regs 0
 #endif
 
-#if 0 /* not yet tested */
+#if 1 /* tested on Pyra */
 static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
[BQ27XXX_DM_DESIGN_CAPACITY]   = { 82, 10, 2,0,  8000 },
[BQ27XXX_DM_DESIGN_ENERGY] = { 82, 12, 2,0, 32767 },
-- 
2.12.2

[PATCH resend v2] power: supply: bq27xxx: enable writing capacity values for bq27421

[H. Nikolaus Schaller]

Tested on Pyra prototype with bq27421.

Signed-off-by: H. Nikolaus Schaller 
---
 drivers/power/supply/bq27xxx_battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index b6c3120ca5ad..e3c878ef7c7e 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -688,7 +688,7 @@ static struct bq27xxx_dm_reg bq27545_dm_regs[] = {
 #define bq27545_dm_regs 0
 #endif
 
-#if 0 /* not yet tested */
+#if 1 /* tested on Pyra */
 static struct bq27xxx_dm_reg bq27421_dm_regs[] = {
[BQ27XXX_DM_DESIGN_CAPACITY]   = { 82, 10, 2,0,  8000 },
[BQ27XXX_DM_DESIGN_ENERGY] = { 82, 12, 2,0, 32767 },
-- 
2.12.2