power: supply: sbs-battery: Silence warning about unknown chemistry
PATCH 1/2 power: supply: sbs-battery: Silence warning about unknown chemistry
Older variants of controller don't support reporting type of the battery. Make warning message about unknown chemistry to be printed only once in order to stop flooding kernel log with the message on each request of the property. This patch fixes the noisy messages on Asus Transformer TF101.
--- drivers/power/supply/sbs-battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/power/supply/sbs-battery.c +1 -1
@@ -814,7 +814,7 @@ static int sbs_get_chemistry(struct i2c_client *client, val->intval =POWER_SUPPLY_TECHNOLOGY_UNKNOWN ;if (val->intval ==POWER_SUPPLY_TECHNOLOGY_UNKNOWN ) -dev_warn (&client->dev,"Unknown chemistry: %s\n" , chemistry); +dev_warn_once (&client->dev,"Unknown chemistry: %s\n" , chemistry);return 0 ; }
Signature · 1 line
2.30.2
PATCH 2/2 power: supply: sbs-battery: Fall back to Li-ion battery type for bq20z75
The older bq20z75 controller doesn't support reporting the battery type and the type is Li-ion in this case.
--- drivers/power/supply/sbs-battery.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/power/supply/sbs-battery.c +9 -1
@@ -813,9 +813,17 @@ static int sbs_get_chemistry(struct i2c_client *client,else val->intval =POWER_SUPPLY_TECHNOLOGY_UNKNOWN ; -if (val->intval ==POWER_SUPPLY_TECHNOLOGY_UNKNOWN ) +if (val->intval ==POWER_SUPPLY_TECHNOLOGY_UNKNOWN ) { +struct sbs_info *chip =i2c_get_clientdata (client); +dev_warn_once (&client->dev,"Unknown chemistry: %s\n" , chemistry); +if (chip->flags &SBS_FLAGS_TI_BQ20ZX5 ) { +dev_warn_once (&client->dev,"Falling back to Li-ion\n" ); + val->intval =POWER_SUPPLY_TECHNOLOGY_LION ; + } + } +return 0 ; }
Signature · 1 line
2.30.2
Re: [PATCH v1 2/2] power: supply: sbs-battery: Fall back to Li-ion battery type for bq20z75
Hi,
Quoted 7 lines · Dmitry Osipenko
The older bq20z75 controller doesn't support reporting the battery type and the type is Li-ion in this case.
Signed-off-by Dmitry Osipenko <[email protected]>---
If it does not support reporting the battery type you should get an error from sbs_get_battery_string_property. Obviously a string has been returned, or you would not end up that far in the code. What string do you see?
Considering BQ20Z65 and BQ20Z75 also support Li-Po I don't think it's a good idea to fall back to Li-Ion. Kernel should never lie about this, since I know some people use userspace based charging setup and the charge limits are different for Li-Ion and Li-Po. When reaching this place we do not know 100%, that it is a Li-ion, so returning UNKNOWN is the safe option.
If you know, that your device (TF101) only supports Li-Ion batteries, we can add a device specific override. But is this worth the added maintenance burden? What is your plan for using this information?
-- Sebastian
Quoted 29 lines
drivers/power/supply/sbs-battery.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)drivers/power/supply/sbs-battery.c +9
-1@@ -813,9 +813,17 @@ static int sbs_get_chemistry(struct i2c_client *client,else val->intval =POWER_SUPPLY_TECHNOLOGY_UNKNOWN ; -if (val->intval ==POWER_SUPPLY_TECHNOLOGY_UNKNOWN ) +if (val->intval ==POWER_SUPPLY_TECHNOLOGY_UNKNOWN ) { +struct sbs_info *chip =i2c_get_clientdata (client); +dev_warn_once (&client->dev,"Unknown chemistry: %s\n" , chemistry); +if (chip->flags &SBS_FLAGS_TI_BQ20ZX5 ) { +dev_warn_once (&client->dev,"Falling back to Li-ion\n" ); + val->intval =POWER_SUPPLY_TECHNOLOGY_LION ; + } + } +return 0 ; }Signature · 1 line
2.30.2
Re: [PATCH v1 2/2] power: supply: sbs-battery: Fall back to Li-ion battery type for bq20z75
13.05.2021 18:31, Sebastian Reichel пишет:
Quoted 11 lines
Hi,
Quoted 7 lines · Dmitry Osipenko
The older bq20z75 controller doesn't support reporting the battery type and the type is Li-ion in this case.
Signed-off-by Dmitry Osipenko <[email protected]>---
Quoted 4 lines
If it does not support reporting the battery type you should get an error from
sbs_get_battery_string_property. Obviously a string has been returned, or you would not end up that far in the code. What string do you see?
There is no visible error. Where the error condition should be set?
The returned string is:
sbs-battery 5-000b: Unknown chemistry: OAI0
Quoted 11 lines
Considering BQ20Z65 and BQ20Z75 also support Li-Po I don't think it's a good idea to fall back to Li-Ion. Kernel should never lie about this, since I know some people use userspace based charging setup and the charge limits are different for Li-Ion and Li-Po. When reaching this place we do not know 100%, that it is a Li-ion, so returning UNKNOWN is the safe option.
If you know, that your device (TF101) only supports Li-Ion batteries, we can add a device specific override. But is this worth the added maintenance burden? What is your plan for using this information?
There is no plan of using that information. Previously battery type was reported properly by userspace, then it regressed. There are other older device-trees in upstream which should have seen the same regression, apparently nobody noticed or cared about it. Yours variant of solution will take more effort, in this case it should be better to leave the regression as-is for now.
Hi,
Quoted 9 lines · Dmitry Osipenko
Older variants of controller don't support reporting type of the battery. Make warning message about unknown chemistry to be printed only once in order to stop flooding kernel log with the message on each request of the property. This patch fixes the noisy messages on Asus Transformer TF101.
Signed-off-by Dmitry Osipenko <[email protected]>---
I believe the problem should be fixed as side-effect of the following patch:
https://lore.kernel.org/linux-pm/[email protected]/
With my suggested change the message is printed once for each battery plug, so probably only once per boot for most users.
-- Sebastian
Quoted 19 lines
drivers/power/supply/sbs-battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/power/supply/sbs-battery.c +1
-1@@ -814,7 +814,7 @@ static int sbs_get_chemistry(struct i2c_client *client, val->intval =POWER_SUPPLY_TECHNOLOGY_UNKNOWN ;if (val->intval ==POWER_SUPPLY_TECHNOLOGY_UNKNOWN ) -dev_warn (&client->dev,"Unknown chemistry: %s\n" , chemistry); +dev_warn_once (&client->dev,"Unknown chemistry: %s\n" , chemistry);return 0 ; }Signature · 1 line
2.30.2
13.05.2021 18:11, Sebastian Reichel пишет:
Quoted 13 lines
Hi,
Quoted 9 lines · Dmitry Osipenko
Older variants of controller don't support reporting type of the battery. Make warning message about unknown chemistry to be printed only once in order to stop flooding kernel log with the message on each request of the property. This patch fixes the noisy messages on Asus Transformer TF101.
Signed-off-by Dmitry Osipenko <[email protected]>---
Quoted 7 lines
I believe the problem should be fixed as side-effect of the following patch:
https://lore.kernel.org/linux-pm/[email protected]/
With my suggested change the message is printed once for each battery plug, so probably only once per boot for most users.
Looks like that patch indeed should work too, thank you.