Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- 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 },
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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.
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 hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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
Hi Liam and Sebastian,
Am 29.08.2017 um 22:20 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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 14800000 [ 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 = <3200000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4000000>; };
?
I will send a patch for enabling both fuel gauges and the omap3-pandora-common.dtsi asap.
BR and thanks, Nikolaus
Hi Nikolaus,
On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam and Sebastian,
Am 29.08.2017 um 22:20 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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 14800000
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 = <3200000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4000000>; };?
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.
Hi Liam,
Am 30.08.2017 um 13:24 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam and Sebastian,
Am 29.08.2017 um 22:20 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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 14800000
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 14800000 [ 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 = <3200000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4000000>; };?
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 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.
Imho, DT can and should describe all properties of the standard OpenPandora battery (and not missing registers of the bq27500).
Using this information or not should be defined by different defconfigs.
Anyways I have now collected my patches into a patch set for further review and cherry-picking.
BR, Nikolaus
On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam,
Am 30.08.2017 um 13:24 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam and Sebastian,
Am 29.08.2017 um 22:20 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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 14800000
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 14800000 [ 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 feature is for hw development and/or production. I don't recommend it for shipped equipment.
Does this look ok for
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3200000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4000000>; };?
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.
No, it failed to set charge-* because energy-* is out of range. Fix that and it should just emit a warning about energy-* "buffer does not match dm spec"
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.
Imho, DT can and should describe all properties of the standard OpenPandora battery (and not missing registers of the bq27500).
Using this information or not should be defined by different defconfigs.
Just omit monitored-battery from the bq27500 node on shipped devices. We shall not program NVM on a stock kernel. And unfortunately you can't send an NVM gauge temporary params.
NB: the DT battery node is new and all the implications have not been discovered.
This BQ27 feature was begun in December of last year, and I'm a little tired of thinking about it. Feel free to suggest improvements, but be aware that we might have already considered and rejected them :-)
Hi Liam,
Am 31.08.2017 um 10:58 schrieb Liam Breck liam@networkimprov.net:
On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam,
Am 30.08.2017 um 13:24 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam and Sebastian,
Am 29.08.2017 um 22:20 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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 14800000
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 14800000 [ 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 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 feature is for hw development and/or production. I don't recommend it for shipped equipment.
Mostly agreed...
Does this look ok for
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3200000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4000000>; };?
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.
No, it failed to set charge-* because energy-* is out of range. Fix that and it should just emit a warning about energy-* "buffer does not match dm spec"
Here I strongly disagree. The DT property should describe the battery and not a limitation of the fuel gauge chip or the value is not in the range of what the chip expects.
IMHO, if the fuel gauge driver can't handle, it should not require to tamper with the battery DT description making it wrong and unuseable for other purposes.
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.
Imho, DT can and should describe all properties of the standard OpenPandora battery (and not missing registers of the bq27500).
Using this information or not should be defined by different defconfigs.
Just omit monitored-battery from the bq27500 node on shipped devices. We shall not program NVM on a stock kernel. And unfortunately you can't send an NVM gauge temporary params.
Isn't this what the defconfigs are for?
A stock kernel should keep CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM disabled and I haven't seen that it is enabled in a stock defconfig.
A production kernel (or user wanting to play with it) can enable. But should not change the DTB.
I have done the same mistake to mis-use DT as "configuration" or ask users to install different DTB several times myself and was heavily criticized, because maintainers clearly said DT should *describe* hardware and not configure kernel features. I.e. there should be only one DTB for each board.
Hence I would expect them to say: if the bq27500 monitors the battery it should be described by the DT. And if the battery has some energy-full-design-microwatt-hours, it should be there and not a "0" or omission.
But that is just *my* 2ct. DT maintainers should finally say something.
NB: the DT battery node is new and all the implications have not been discovered.
This BQ27 feature was begun in December of last year, and I'm a little tired of thinking about it.
I fully understand :) I have some patches in tiresome discussion since ca. 3 years...
Feel free to suggest improvements, but be aware that we might have already considered and rejected them :-)
We had observed this issue for the RAM only fuel gauge bq27421 a while ago (I think even before December) and had started our own patch set. Grazvydas recently had made it working on some older kernel which is now superseded by your patch set. So you were faster in submitting a solution (and yours is even more general than what we had ever thought about).
So I am very pleased with that we have a big step forward for the bq27421 based device. Making the bq27500 in the OpenPandora fool proof isn't that important for the moment so we can postpone it.
BR and thanks for all the work, Nikolaus
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 hns@goldelico.com wrote:
Hi Liam,
Am 31.08.2017 um 10:58 schrieb Liam Breck liam@networkimprov.net:
On Wed, Aug 30, 2017 at 11:58 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam,
Am 30.08.2017 um 13:24 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam and Sebastian,
Am 29.08.2017 um 22:20 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote: > Tested on Pyra prototype with bq27421. > > Signed-off-by: H. Nikolaus Schaller hns@goldelico.com > --- > 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 14800000
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 14800000 [ 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.
In the previous run terminate-voltage appeared after battery_unseal. Did you dmesg | grep bq27 ?
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 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 feature is for hw development and/or production. I don't recommend it for shipped equipment.
Mostly agreed...
Does this look ok for
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3200000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4000000>; };?
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.
No, it failed to set charge-* because energy-* is out of range. Fix that and it should just emit a warning about energy-* "buffer does not match dm spec"
Here I strongly disagree. The DT property should describe the battery and not a limitation of the fuel gauge chip or the value is not in the range of what the chip expects.
IMHO, if the fuel gauge driver can't handle, it should not require to tamper with the battery DT description making it wrong and unuseable for other purposes.
You are right that we use DT:battery as config for the case of NVM hw development/manufacture (requiring custom kernel build). It is expedient since we have DT inputs for non-NVM chips.
But upstream dts cannot use monitored-battery to program NVM chips, as it won't (ever) work on normal kernels. You can argue that monitored-battery should cause a warning if an NVM chip has different settings, and we do that!
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.
Imho, DT can and should describe all properties of the standard OpenPandora battery (and not missing registers of the bq27500).
Using this information or not should be defined by different defconfigs.
Just omit monitored-battery from the bq27500 node on shipped devices. We shall not program NVM on a stock kernel. And unfortunately you can't send an NVM gauge temporary params.
Isn't this what the defconfigs are for?
A stock kernel should keep CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM disabled and I haven't seen that it is enabled in a stock defconfig.
A production kernel (or user wanting to play with it) can enable. But should not change the DTB.
Where I previously said "for hw development/production" the latter term means manufacture, ie a kernel for use on assembly line for config/test. config...dt_updates_nvm shall not be set in a defconfig, distro, or shipped hw.
I have done the same mistake to mis-use DT as "configuration" or ask users to install different DTB several times myself and was heavily criticized, because maintainers clearly said DT should *describe* hardware and not configure kernel features. I.e. there should be only one DTB for each board.
Hence I would expect them to say: if the bq27500 monitors the battery it should be described by the DT. And if the battery has some energy-full-design-microwatt-hours, it should be there and not a "0" or omission.
But that is just *my* 2ct. DT maintainers should finally say something.
NB: the DT battery node is new and all the implications have not been discovered.
This BQ27 feature was begun in December of last year, and I'm a little tired of thinking about it.
I fully understand :) I have some patches in tiresome discussion since ca. 3 years...
Feel free to suggest improvements, but be aware that we might have already considered and rejected them :-)
We had observed this issue for the RAM only fuel gauge bq27421 a while ago (I think even before December) and had started our own patch set. Grazvydas recently had made it working on some older kernel which is now superseded by your patch set. So you were faster in submitting a solution (and yours is even more general than what we had ever thought about).
So I am very pleased with that we have a big step forward for the bq27421 based device. Making the bq27500 in the OpenPandora fool proof isn't that important for the moment so we can postpone it.
BR and thanks for all the work,
Feel free to send me a Pyra prototype or first-production unit :-) We're also working on an omap device, tho not a "handheld"
Hi Liam, I finally continues testing on OpenPandora.
Am 31.08.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi,
This may be a fix that allows >0 input from DT, but won't try to program the register since the first 3 fields aren't compatible:
... bq27500_dm_regs[] = { ... [bq27xxx_dm_design_energy] = { 0, 0, 0, 0, 65535 }, /* missing on chip */ NB: align columns with other rows
I have tried with this DT
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3250000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4100000>; };
and here is the result:
root@letux:~# dmesg|fgrep bq27 [ 10.391235] bq27xxx_battery_setup [ 10.391265] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 10.393798] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.432678] bq27xxx_battery_settings [ 10.432952] bq27xxx_battery_set_config [ 10.432952] bq27xxx_battery_unseal [ 10.485168] bq27xxx_battery_update_dm_block [ 10.485198] bq27xxx-battery 2-0055: update design-capacity to 4100
looks good
[ 10.485229] bq27xxx_battery_update_dm_block [ 10.485229] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec
ok, ignored
[ 10.511718] bq27xxx_battery_update_dm_block [ 10.511749] bq27xxx-battery 2-0055: update terminate-voltage to 3250
looks good
[ 10.826446] bq27xxx_battery_seal [ 12.150939] bq27xxx_battery_platform_probe [ 12.151031] bq27xxx_battery_setup [ 12.151062] bq27xxx_battery_setup: dm_regs= (null) [ 12.153411] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.154327] bq27xxx_battery_settings [ 12.154357] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.154388] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6
... login:
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3892000 POWER_SUPPLY_CURRENT_NOW=-317000 POWER_SUPPLY_CAPACITY=0
oops.
POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=64395 POWER_SUPPLY_HEALTH=Dead
oops. But maybe the bq27500 needs a full cycle first
POWER_SUPPLY_MANUFACTURER=Texas Instruments
vvv after plugging in charger
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Charging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3923000 POWER_SUPPLY_CURRENT_NOW=204000 POWER_SUPPLY_CAPACITY=0 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=249 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=800 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
Now a reboot after removing charger and battery for several minutes:
root@letux:~# dmesg|fgrep bq27 [ 10.482818] bq27xxx_battery_setup [ 12.179687] bq27xxx_battery_platform_probe [ 12.179779] bq27xxx_battery_setup [ 12.179809] bq27xxx_battery_setup: dm_regs= (null) [ 12.182495] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.183502] bq27xxx_battery_settings [ 12.183532] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.183563] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 [ 15.145812] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 15.152618] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 15.346557] bq27xxx_battery_settings [ 15.350585] bq27xxx_battery_set_config [ 15.354553] bq27xxx_battery_unseal [ 15.576538] bq27xxx_battery_update_dm_block [ 15.580993] bq27xxx-battery 2-0055: design-capacity has 4100
^^^ ok, no change needed
[ 15.676818] bq27xxx_battery_update_dm_block [ 15.681243] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 15.798675] bq27xxx_battery_update_dm_block [ 15.803100] bq27xxx-battery 2-0055: update terminate-voltage to 3250
^^^ looks as if this is not stored in NVM. Should it be?
BTW: it would be nice if all "update" messages could tell the old value as well.
[ 16.011169] bq27xxx_battery_seal root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3894000 POWER_SUPPLY_CURRENT_NOW=-291000 POWER_SUPPLY_CAPACITY=71
^^^ looks as if the bq27500 did recover from reprogramming
POWER_SUPPLY_CAPACITY_LEVEL=Normal POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=31200 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3748000 POWER_SUPPLY_CHARGE_NOW=2713000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=9930000 POWER_SUPPLY_POWER_AVG=64407 POWER_SUPPLY_HEALTH=Good
^^^ same here
POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
Pls CC me on patch posts. You left me off your last series.
Sorry, did copy the address list around and dropped off some :(
BR, Nikolaus
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.
Am 08.09.2017 um 19:40 schrieb Pali Rohár pali.rohar@gmail.com:
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
Hi Nikolaus,
On Fri, Sep 8, 2017 at 10:38 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam, I finally continues testing on OpenPandora.
Am 31.08.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi,
This may be a fix that allows >0 input from DT, but won't try to program the register since the first 3 fields aren't compatible:
... bq27500_dm_regs[] = { ... [bq27xxx_dm_design_energy] = { 0, 0, 0, 0, 65535 }, /* missing on chip */ NB: align columns with other rows
I have tried with this DT
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3250000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4100000>; };and here is the result:
root@letux:~# dmesg|fgrep bq27 [ 10.391235] bq27xxx_battery_setup [ 10.391265] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 10.393798] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.432678] bq27xxx_battery_settings [ 10.432952] bq27xxx_battery_set_config [ 10.432952] bq27xxx_battery_unseal [ 10.485168] bq27xxx_battery_update_dm_block [ 10.485198] bq27xxx-battery 2-0055: update design-capacity to 4100
looks good
[ 10.485229] bq27xxx_battery_update_dm_block [ 10.485229] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec
ok, ignored
[ 10.511718] bq27xxx_battery_update_dm_block [ 10.511749] bq27xxx-battery 2-0055: update terminate-voltage to 3250
looks good
[ 10.826446] bq27xxx_battery_seal [ 12.150939] bq27xxx_battery_platform_probe [ 12.151031] bq27xxx_battery_setup [ 12.151062] bq27xxx_battery_setup: dm_regs= (null) [ 12.153411] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.154327] bq27xxx_battery_settings [ 12.154357] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.154388] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6
Is there also a bq27000 chip on this device, or a battery with it embedded? If so, it doesn't appear to have a DT node (correct for embedded), hence that warning, which isn't useful in that case. Prob battery_settings() should do: if (!di->dev->of_node || power_supply_get_battery_info(...) < 0) return; altho that would be wrong once get_battery_info() supports ACPI config...
... login:
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3892000 POWER_SUPPLY_CURRENT_NOW=-317000 POWER_SUPPLY_CAPACITY=0
oops.
POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=64395 POWER_SUPPLY_HEALTH=Dead
oops. But maybe the bq27500 needs a full cycle first
I wouldn't guess that the above state is due to the DM update sequence, but I suppose that's possible. Another update pass might clarify that.
POWER_SUPPLY_MANUFACTURER=Texas Instruments
vvv after plugging in charger
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Charging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3923000 POWER_SUPPLY_CURRENT_NOW=204000 POWER_SUPPLY_CAPACITY=0 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=249 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=800 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
Now a reboot after removing charger and battery for several minutes:
root@letux:~# dmesg|fgrep bq27 [ 10.482818] bq27xxx_battery_setup [ 12.179687] bq27xxx_battery_platform_probe [ 12.179779] bq27xxx_battery_setup [ 12.179809] bq27xxx_battery_setup: dm_regs= (null) [ 12.182495] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.183502] bq27xxx_battery_settings [ 12.183532] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.183563] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 [ 15.145812] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 15.152618] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 15.346557] bq27xxx_battery_settings [ 15.350585] bq27xxx_battery_set_config [ 15.354553] bq27xxx_battery_unseal [ 15.576538] bq27xxx_battery_update_dm_block [ 15.580993] bq27xxx-battery 2-0055: design-capacity has 4100
^^^ ok, no change needed
[ 15.676818] bq27xxx_battery_update_dm_block [ 15.681243] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 15.798675] bq27xxx_battery_update_dm_block [ 15.803100] bq27xxx-battery 2-0055: update terminate-voltage to 3250
^^^ looks as if this is not stored in NVM. Should it be?
Oh dear. I imagine this is due to a flaw in the multi-block DM update logic. It could be that one of the pauses between calls should be longer The terminate-voltage spec comes from this datasheet: page 19, http://www.ti.com/lit/ds/symlink/bq27500.pdf
It's possible that this second pass did update it. What does the next reboot show?
BTW: it would be nice if all "update" messages could tell the old value as well.
We have the orig values in memory since we read the block before updating. Feel free to patch that in :-)
[ 16.011169] bq27xxx_battery_seal root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3894000 POWER_SUPPLY_CURRENT_NOW=-291000 POWER_SUPPLY_CAPACITY=71
^^^ looks as if the bq27500 did recover from reprogramming
POWER_SUPPLY_CAPACITY_LEVEL=Normal POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=31200 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3748000 POWER_SUPPLY_CHARGE_NOW=2713000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=9930000 POWER_SUPPLY_POWER_AVG=64407 POWER_SUPPLY_HEALTH=Good
^^^ same here
POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
For general use on OpenPandora, you wouldn't have config...dt_updates_nvm option set, so the driver should report what the chip settings are and whether they aren't what the DT spec'd. Let's verify that too...
Thanks!
Hi Liam,
Am 08.09.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Fri, Sep 8, 2017 at 10:38 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam, I finally continues testing on OpenPandora.
Am 31.08.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi,
This may be a fix that allows >0 input from DT, but won't try to program the register since the first 3 fields aren't compatible:
... bq27500_dm_regs[] = { ... [bq27xxx_dm_design_energy] = { 0, 0, 0, 0, 65535 }, /* missing on chip */ NB: align columns with other rows
I have tried with this DT
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3250000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4100000>; };and here is the result:
root@letux:~# dmesg|fgrep bq27 [ 10.391235] bq27xxx_battery_setup [ 10.391265] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 10.393798] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.432678] bq27xxx_battery_settings [ 10.432952] bq27xxx_battery_set_config [ 10.432952] bq27xxx_battery_unseal [ 10.485168] bq27xxx_battery_update_dm_block [ 10.485198] bq27xxx-battery 2-0055: update design-capacity to 4100
looks good
[ 10.485229] bq27xxx_battery_update_dm_block [ 10.485229] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec
ok, ignored
[ 10.511718] bq27xxx_battery_update_dm_block [ 10.511749] bq27xxx-battery 2-0055: update terminate-voltage to 3250
looks good
[ 10.826446] bq27xxx_battery_seal [ 12.150939] bq27xxx_battery_platform_probe [ 12.151031] bq27xxx_battery_setup [ 12.151062] bq27xxx_battery_setup: dm_regs= (null) [ 12.153411] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.154327] bq27xxx_battery_settings [ 12.154357] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.154388] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6
Is there also a bq27000 chip on this device, or a battery with it embedded? If so, it doesn't appear to have a DT node (correct for embedded), hence that warning, which isn't useful in that case. Prob battery_settings() should do: if (!di->dev->of_node || power_supply_get_battery_info(...) < 0) return; altho that would be wrong once get_battery_info() supports ACPI config...
No it is not available, but could be. The bq27000 can be connected through HDQ to an OMAP3 and therefore there is no auto-detection. If it is configured the driver will be loaded. Even if there is no battery_info.
In fact we use the same defconfig for several devices and there is one (GTA04) which only has a bq27000 inside the battery.
... login:
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3892000 POWER_SUPPLY_CURRENT_NOW=-317000 POWER_SUPPLY_CAPACITY=0
oops.
POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=64395 POWER_SUPPLY_HEALTH=Dead
oops. But maybe the bq27500 needs a full cycle first
I wouldn't guess that the above state is due to the DM update sequence, but I suppose that's possible. Another update pass might clarify that.
Ok, will do tomorrow.
POWER_SUPPLY_MANUFACTURER=Texas Instruments
vvv after plugging in charger
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Charging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3923000 POWER_SUPPLY_CURRENT_NOW=204000 POWER_SUPPLY_CAPACITY=0 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=249 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=800 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
Now a reboot after removing charger and battery for several minutes:
root@letux:~# dmesg|fgrep bq27 [ 10.482818] bq27xxx_battery_setup [ 12.179687] bq27xxx_battery_platform_probe [ 12.179779] bq27xxx_battery_setup [ 12.179809] bq27xxx_battery_setup: dm_regs= (null) [ 12.182495] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.183502] bq27xxx_battery_settings [ 12.183532] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.183563] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 [ 15.145812] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 15.152618] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 15.346557] bq27xxx_battery_settings [ 15.350585] bq27xxx_battery_set_config [ 15.354553] bq27xxx_battery_unseal [ 15.576538] bq27xxx_battery_update_dm_block [ 15.580993] bq27xxx-battery 2-0055: design-capacity has 4100
^^^ ok, no change needed
[ 15.676818] bq27xxx_battery_update_dm_block [ 15.681243] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 15.798675] bq27xxx_battery_update_dm_block [ 15.803100] bq27xxx-battery 2-0055: update terminate-voltage to 3250
^^^ looks as if this is not stored in NVM. Should it be?
Oh dear. I imagine this is due to a flaw in the multi-block DM update logic. It could be that one of the pauses between calls should be longer The terminate-voltage spec comes from this datasheet: page 19, http://www.ti.com/lit/ds/symlink/bq27500.pdf
It's possible that this second pass did update it. What does the next reboot show?
Will boot tomorrow with exactly the same system and report.
BTW: it would be nice if all "update" messages could tell the old value as well.
We have the orig values in memory since we read the block before updating. Feel free to patch that in :-)
Ok.
[ 16.011169] bq27xxx_battery_seal root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3894000 POWER_SUPPLY_CURRENT_NOW=-291000 POWER_SUPPLY_CAPACITY=71
^^^ looks as if the bq27500 did recover from reprogramming
POWER_SUPPLY_CAPACITY_LEVEL=Normal POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=31200 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3748000 POWER_SUPPLY_CHARGE_NOW=2713000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=9930000 POWER_SUPPLY_POWER_AVG=64407 POWER_SUPPLY_HEALTH=Good
^^^ same here
POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
For general use on OpenPandora, you wouldn't have config...dt_updates_nvm option set,
Yes.
so the driver should report what the chip settings are and whether they aren't what the DT spec'd. Let's verify that too...
Ok.
BR, Nikolaus
Howdy,
On Fri, Sep 8, 2017 at 1:28 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam,
Am 08.09.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Fri, Sep 8, 2017 at 10:38 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam, I finally continues testing on OpenPandora.
Am 31.08.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi,
This may be a fix that allows >0 input from DT, but won't try to program the register since the first 3 fields aren't compatible:
... bq27500_dm_regs[] = { ... [bq27xxx_dm_design_energy] = { 0, 0, 0, 0, 65535 }, /* missing on chip */ NB: align columns with other rows
I have tried with this DT
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3250000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4100000>; };and here is the result:
root@letux:~# dmesg|fgrep bq27 [ 10.391235] bq27xxx_battery_setup [ 10.391265] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 10.393798] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.432678] bq27xxx_battery_settings [ 10.432952] bq27xxx_battery_set_config [ 10.432952] bq27xxx_battery_unseal [ 10.485168] bq27xxx_battery_update_dm_block [ 10.485198] bq27xxx-battery 2-0055: update design-capacity to 4100
looks good
[ 10.485229] bq27xxx_battery_update_dm_block [ 10.485229] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec
ok, ignored
[ 10.511718] bq27xxx_battery_update_dm_block [ 10.511749] bq27xxx-battery 2-0055: update terminate-voltage to 3250
looks good
[ 10.826446] bq27xxx_battery_seal [ 12.150939] bq27xxx_battery_platform_probe [ 12.151031] bq27xxx_battery_setup [ 12.151062] bq27xxx_battery_setup: dm_regs= (null) [ 12.153411] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.154327] bq27xxx_battery_settings [ 12.154357] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.154388] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6
Is there also a bq27000 chip on this device, or a battery with it embedded? If so, it doesn't appear to have a DT node (correct for embedded), hence that warning, which isn't useful in that case. Prob battery_settings() should do: if (!di->dev->of_node || power_supply_get_battery_info(...) < 0) return; altho that would be wrong once get_battery_info() supports ACPI config...
No it is not available, but could be. The bq27000 can be connected through HDQ to an OMAP3 and therefore there is no auto-detection. If it is configured the driver will be loaded. Even if there is no battery_info.
In fact we use the same defconfig for several devices and there is one (GTA04) which only has a bq27000 inside the battery.
Any idea why the driver detects a non-present bq27000 and calls probe() for it? defconfig with battery_bq27xxx=y ?
Am 08.09.2017 um 22:57 schrieb Liam Breck liam@networkimprov.net:
Howdy,
On Fri, Sep 8, 2017 at 1:28 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam,
Am 08.09.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Fri, Sep 8, 2017 at 10:38 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam, I finally continues testing on OpenPandora.
Am 31.08.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi,
This may be a fix that allows >0 input from DT, but won't try to program the register since the first 3 fields aren't compatible:
... bq27500_dm_regs[] = { ... [bq27xxx_dm_design_energy] = { 0, 0, 0, 0, 65535 }, /* missing on chip */ NB: align columns with other rows
I have tried with this DT
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3250000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4100000>; };and here is the result:
root@letux:~# dmesg|fgrep bq27 [ 10.391235] bq27xxx_battery_setup [ 10.391265] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 10.393798] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.432678] bq27xxx_battery_settings [ 10.432952] bq27xxx_battery_set_config [ 10.432952] bq27xxx_battery_unseal [ 10.485168] bq27xxx_battery_update_dm_block [ 10.485198] bq27xxx-battery 2-0055: update design-capacity to 4100
looks good
[ 10.485229] bq27xxx_battery_update_dm_block [ 10.485229] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec
ok, ignored
[ 10.511718] bq27xxx_battery_update_dm_block [ 10.511749] bq27xxx-battery 2-0055: update terminate-voltage to 3250
looks good
[ 10.826446] bq27xxx_battery_seal [ 12.150939] bq27xxx_battery_platform_probe [ 12.151031] bq27xxx_battery_setup [ 12.151062] bq27xxx_battery_setup: dm_regs= (null) [ 12.153411] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.154327] bq27xxx_battery_settings [ 12.154357] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.154388] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6
Is there also a bq27000 chip on this device, or a battery with it embedded? If so, it doesn't appear to have a DT node (correct for embedded), hence that warning, which isn't useful in that case. Prob battery_settings() should do: if (!di->dev->of_node || power_supply_get_battery_info(...) < 0) return; altho that would be wrong once get_battery_info() supports ACPI config...
No it is not available, but could be. The bq27000 can be connected through HDQ to an OMAP3 and therefore there is no auto-detection. If it is configured the driver will be loaded. Even if there is no battery_info.
In fact we use the same defconfig for several devices and there is one (GTA04) which only has a bq27000 inside the battery.
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
Hi Liam,
Am 08.09.2017 um 22:28 schrieb H. Nikolaus Schaller hns@goldelico.com:
Hi Liam,
Am 08.09.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Fri, Sep 8, 2017 at 10:38 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam, I finally continues testing on OpenPandora.
Am 31.08.2017 um 22:19 schrieb Liam Breck liam@networkimprov.net:
Hi,
This may be a fix that allows >0 input from DT, but won't try to program the register since the first 3 fields aren't compatible:
... bq27500_dm_regs[] = { ... [bq27xxx_dm_design_energy] = { 0, 0, 0, 0, 65535 }, /* missing on chip */ NB: align columns with other rows
I have tried with this DT
bat: battery { compatible = "simple-battery", "openpandora-battery"; voltage-min-design-microvolt = <3250000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4100000>; };and here is the result:
root@letux:~# dmesg|fgrep bq27 [ 10.391235] bq27xxx_battery_setup [ 10.391265] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 10.393798] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.432678] bq27xxx_battery_settings [ 10.432952] bq27xxx_battery_set_config [ 10.432952] bq27xxx_battery_unseal [ 10.485168] bq27xxx_battery_update_dm_block [ 10.485198] bq27xxx-battery 2-0055: update design-capacity to 4100
looks good
[ 10.485229] bq27xxx_battery_update_dm_block [ 10.485229] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec
ok, ignored
[ 10.511718] bq27xxx_battery_update_dm_block [ 10.511749] bq27xxx-battery 2-0055: update terminate-voltage to 3250
looks good
[ 10.826446] bq27xxx_battery_seal [ 12.150939] bq27xxx_battery_platform_probe [ 12.151031] bq27xxx_battery_setup [ 12.151062] bq27xxx_battery_setup: dm_regs= (null) [ 12.153411] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.154327] bq27xxx_battery_settings [ 12.154357] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.154388] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6
Is there also a bq27000 chip on this device, or a battery with it embedded? If so, it doesn't appear to have a DT node (correct for embedded), hence that warning, which isn't useful in that case. Prob battery_settings() should do: if (!di->dev->of_node || power_supply_get_battery_info(...) < 0) return; altho that would be wrong once get_battery_info() supports ACPI config...
No it is not available, but could be. The bq27000 can be connected through HDQ to an OMAP3 and therefore there is no auto-detection. If it is configured the driver will be loaded. Even if there is no battery_info.
In fact we use the same defconfig for several devices and there is one (GTA04) which only has a bq27000 inside the battery.
... login:
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3892000 POWER_SUPPLY_CURRENT_NOW=-317000 POWER_SUPPLY_CAPACITY=0
oops.
POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=64395 POWER_SUPPLY_HEALTH=Dead
oops. But maybe the bq27500 needs a full cycle first
I wouldn't guess that the above state is due to the DM update sequence, but I suppose that's possible. Another update pass might clarify that.
Ok, will do tomorrow.
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] bq27xxx-battery 2-0055: terminate-voltage has 3250 [ 10.766754] bq27xxx_battery_seal [ 12.300842] bq27xxx_battery_platform_probe [ 12.300933] bq27xxx_battery_setup [ 12.300964] bq27xxx_battery_setup: dm_regs= (null) [ 12.303314] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.304229] bq27xxx_battery_settings [ 12.304260] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.304290] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 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=3952000 POWER_SUPPLY_CURRENT_NOW=-299000 POWER_SUPPLY_CAPACITY=0 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=220 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3751000 POWER_SUPPLY_CHARGE_NOW=0 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=64398 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
and another time:
root@letux:~# dmesg|fgrep bq27 [ 10.595611] bq27xxx_battery_setup [ 10.599182] bq27xxx_battery_setup: dm_regs=bf05b0e0 [ 10.698791] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.764831] bq27xxx_battery_settings [ 10.765075] bq27xxx_battery_set_config [ 10.765106] bq27xxx_battery_unseal [ 10.819915] bq27xxx_battery_update_dm_block [ 10.819946] bq27xxx-battery 2-0055: design-capacity has 4100 [ 10.819976] bq27xxx_battery_update_dm_block [ 10.819976] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 10.831481] bq27xxx_battery_update_dm_block [ 10.831512] bq27xxx-battery 2-0055: terminate-voltage has 3250 [ 10.831542] bq27xxx_battery_seal [ 13.000305] bq27xxx_battery_platform_probe [ 13.000396] bq27xxx_battery_setup [ 13.000427] bq27xxx_battery_setup: dm_regs= (null) [ 13.002807] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 13.003692] bq27xxx_battery_settings [ 13.003723] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 13.003723] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 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=3996000 POWER_SUPPLY_CURRENT_NOW=211000 POWER_SUPPLY_CAPACITY=0 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=270 POWER_SUPPLY_TIME_TO_FULL_NOW=94740 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3751000 POWER_SUPPLY_CHARGE_NOW=0 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=759 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
So rebooting is not enough to awake it.
POWER_SUPPLY_MANUFACTURER=Texas Instruments
vvv after plugging in charger
root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Charging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3923000 POWER_SUPPLY_CURRENT_NOW=204000 POWER_SUPPLY_CAPACITY=0 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=249 POWER_SUPPLY_TIME_TO_EMPTY_NOW=0 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=1147000 POWER_SUPPLY_CHARGE_NOW=2665000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=800 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
Now a reboot after removing charger and battery for several minutes:
root@letux:~# dmesg|fgrep bq27 [ 10.482818] bq27xxx_battery_setup [ 12.179687] bq27xxx_battery_platform_probe [ 12.179779] bq27xxx_battery_setup [ 12.179809] bq27xxx_battery_setup: dm_regs= (null) [ 12.182495] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 12.183502] bq27xxx_battery_settings [ 12.183532] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 12.183563] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 [ 15.145812] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 15.152618] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 15.346557] bq27xxx_battery_settings [ 15.350585] bq27xxx_battery_set_config [ 15.354553] bq27xxx_battery_unseal [ 15.576538] bq27xxx_battery_update_dm_block [ 15.580993] bq27xxx-battery 2-0055: design-capacity has 4100
^^^ ok, no change needed
[ 15.676818] bq27xxx_battery_update_dm_block [ 15.681243] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 15.798675] bq27xxx_battery_update_dm_block [ 15.803100] bq27xxx-battery 2-0055: update terminate-voltage to 3250
^^^ looks as if this is not stored in NVM. Should it be?
Oh dear. I imagine this is due to a flaw in the multi-block DM update logic. It could be that one of the pauses between calls should be longer The terminate-voltage spec comes from this datasheet: page 19, http://www.ti.com/lit/ds/symlink/bq27500.pdf
It's possible that this second pass did update it. What does the next reboot show?
Will boot tomorrow with exactly the same system and report.
This time it did have remembered the terminate-voltage: (copy from above)
[ 10.766754] bq27xxx-battery 2-0055: terminate-voltage has 3250
BTW: it would be nice if all "update" messages could tell the old value as well.
We have the orig values in memory since we read the block before updating. Feel free to patch that in :-)
Ok.
Is a one-liner. Will submit a PATCH.
[ 16.011169] bq27xxx_battery_seal root@letux:~# cat /sys/class/power_supply/bq27500-1-0/uevent POWER_SUPPLY_NAME=bq27500-1-0 POWER_SUPPLY_STATUS=Discharging POWER_SUPPLY_PRESENT=1 POWER_SUPPLY_VOLTAGE_NOW=3894000 POWER_SUPPLY_CURRENT_NOW=-291000 POWER_SUPPLY_CAPACITY=71
^^^ looks as if the bq27500 did recover from reprogramming
POWER_SUPPLY_CAPACITY_LEVEL=Normal POWER_SUPPLY_TEMP=223 POWER_SUPPLY_TIME_TO_EMPTY_NOW=31200 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3748000 POWER_SUPPLY_CHARGE_NOW=2713000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=9930000 POWER_SUPPLY_POWER_AVG=64407 POWER_SUPPLY_HEALTH=Good
^^^ same here
POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
For general use on OpenPandora, you wouldn't have config...dt_updates_nvm option set,
Yes.
so the driver should report what the chip settings are and whether they aren't what the DT spec'd. Let's verify that too...
Ok.
Now here a reboot with a newly built kernel with CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM disabled and real battery definition:
root@letux:~# dmesg|fgrep bq27 [ 10.339141] bq27xxx_battery_setup [ 10.342712] bq27xxx_battery_setup: dm_regs=bf0520de [ 10.362854] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 11.945251] bq27xxx_battery_platform_probe [ 11.945312] bq27xxx_battery_setup [ 11.945373] bq27xxx_battery_setup: dm_regs= (null) [ 11.947662] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 11.948608] bq27xxx_battery_settings [ 11.948608] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 11.948638] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 [ 15.036163] bq27xxx_battery_settings [ 15.112274] bq27xxx_battery_set_config [ 15.138916] bq27xxx_battery_unseal [ 15.246124] bq27xxx_battery_update_dm_block [ 15.250579] bq27xxx-battery 2-0055: design-capacity has 4100; update to 4000 disallowed for flash/NVM data memory [ 15.418457] bq27xxx_battery_update_dm_block [ 15.423034] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 15.569061] bq27xxx_battery_update_dm_block [ 15.573516] bq27xxx-battery 2-0055: terminate-voltage has 3250; update to 3200 disallowed for flash/NVM data memory [ 15.664703] bq27xxx_battery_seal 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=3996000 POWER_SUPPLY_CURRENT_NOW=166000 POWER_SUPPLY_CAPACITY=1 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=251 POWER_SUPPLY_TIME_TO_FULL_NOW=70560 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3751000 POWER_SUPPLY_CHARGE_NOW=7000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=0 POWER_SUPPLY_POWER_AVG=828 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
And another reboot (powered up in between):
root@letux:~# dmesg|fgrep bq27 [ 10.271545] bq27xxx_battery_setup [ 10.271575] bq27xxx_battery_setup: dm_regs=bf0520de [ 10.274139] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.345062] bq27xxx_battery_settings [ 10.345306] bq27xxx_battery_set_config [ 10.345336] bq27xxx_battery_unseal [ 10.382415] bq27xxx_battery_update_dm_block [ 10.382446] bq27xxx-battery 2-0055: design-capacity has 4100; update to 4000 disallowed for flash/NVM data memory [ 10.382446] bq27xxx_battery_update_dm_block [ 10.382476] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 10.392852] bq27xxx_battery_update_dm_block [ 10.392883] bq27xxx-battery 2-0055: terminate-voltage has 3250; update to 3200 disallowed for flash/NVM data memory [ 10.392883] bq27xxx_battery_seal [ 11.832763] bq27xxx_battery_platform_probe [ 11.832855] bq27xxx_battery_setup [ 11.832885] bq27xxx_battery_setup: dm_regs= (null) [ 11.846038] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 11.846984] bq27xxx_battery_settings [ 11.847015] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 11.847045] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 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=4001000 POWER_SUPPLY_CURRENT_NOW=200000 POWER_SUPPLY_CAPACITY=1 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=270 POWER_SUPPLY_TIME_TO_FULL_NOW=69420 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3751000 POWER_SUPPLY_CHARGE_NOW=32000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4100000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=51000 POWER_SUPPLY_POWER_AVG=756 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
And after reenabling CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM
root@letux:~# dmesg|fgrep bq27 [ 10.316589] bq27xxx_battery_setup [ 10.320129] bq27xxx_battery_setup: dm_regs=bf0520e0 [ 10.440979] (NULL device *): hwmon: 'bq27500-1-0' is not a valid name attribute, please fix [ 10.441986] bq27xxx_battery_settings [ 10.442230] bq27xxx_battery_set_config [ 10.442260] bq27xxx_battery_unseal [ 10.503662] bq27xxx_battery_update_dm_block [ 10.503723] bq27xxx-battery 2-0055: update design-capacity from 4100 to 4000 [ 10.503723] bq27xxx_battery_update_dm_block [ 10.503753] bq27xxx-battery 2-0055: buffer does not match design-energy dm spec [ 10.544769] bq27xxx_battery_update_dm_block [ 10.544799] bq27xxx-battery 2-0055: update terminate-voltage from 3250 to 3200 [ 10.866546] bq27xxx_battery_seal [ 11.892913] bq27xxx_battery_platform_probe [ 11.893005] bq27xxx_battery_setup [ 11.893035] bq27xxx_battery_setup: dm_regs= (null) [ 11.913208] (NULL device *): hwmon: 'bq27000-battery' is not a valid name attribute, please fix [ 11.914154] bq27xxx_battery_settings [ 11.914184] power_supply bq27000-battery: power_supply_get_battery_info currently only supports devicetree [ 11.914215] bq27xxx_battery_settings: power_supply_get_battery_info failed ret=-6 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=4012000 POWER_SUPPLY_CURRENT_NOW=256000 POWER_SUPPLY_CAPACITY=3 POWER_SUPPLY_CAPACITY_LEVEL=Low POWER_SUPPLY_TEMP=284 POWER_SUPPLY_TIME_TO_FULL_NOW=54960 POWER_SUPPLY_TECHNOLOGY=Li-ion POWER_SUPPLY_CHARGE_FULL=3751000 POWER_SUPPLY_CHARGE_NOW=76000 POWER_SUPPLY_CHARGE_FULL_DESIGN=4000000 POWER_SUPPLY_CYCLE_COUNT=6 POWER_SUPPLY_ENERGY_NOW=188000 POWER_SUPPLY_POWER_AVG=1027 POWER_SUPPLY_HEALTH=Dead POWER_SUPPLY_MANUFACTURER=Texas Instruments root@letux:~#
This time reprogramming worked and the capacity level is regenerating.
So I'd say everything works as expected.
I'll send my latest patches for review asap.
BR, Nikolaus
Hi Liam,
Am 30.08.2017 um 13:24 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus,
On Wed, Aug 30, 2017 at 2:30 AM, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Liam and Sebastian,
Am 29.08.2017 um 22:20 schrieb Liam Breck liam@networkimprov.net:
Hi Nikolaus, thanks for the patch...
On Tue, Aug 29, 2017 at 1:10 PM, H. Nikolaus Schaller hns@goldelico.com wrote:
Tested on Pyra prototype with bq27421.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
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 14800000
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 14800000 [ 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 = <3200000>; energy-full-design-microwatt-hours = <14800000>; charge-full-design-microamp-hours = <4000000>; };?
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 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.
Imho, DT can and should describe all properties of the standard OpenPandora battery (and not missing registers of the bq27500).
Using this information or not should be defined by different defconfigs.
Anyways I have now collected my patches into a patch set for further review and cherry-picking.
BR, Nikolaus
-
H. Nikolaus Schaller -
Liam Breck -
Pali Rohár -
Sebastian Reichel