| Message ID | 20210510220827.11595-2-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/2] power: supply: sbs-battery: Silence warning about unknown chemistry | expand |
Hi, On Tue, May 11, 2021 at 01:08:27AM +0300, Dmitry Osipenko wrote: > The older bq20z75 controller doesn't support reporting the battery type > and the type is Li-ion in this case. > > Tested-by: Antoni Aloy Torrens <aaloytorrens@gmail.com> # TF101 > Tested-by: Nikola Milosavljević <mnidza@outlook.com> # TF101 > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- 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 > drivers/power/supply/sbs-battery.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c > index b71fbf543428..fec6c139d4ff 100644 > --- a/drivers/power/supply/sbs-battery.c > +++ b/drivers/power/supply/sbs-battery.c > @@ -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; > } > > -- > 2.30.2 >
13.05.2021 18:31, Sebastian Reichel пишет: > Hi, > > On Tue, May 11, 2021 at 01:08:27AM +0300, Dmitry Osipenko wrote: >> The older bq20z75 controller doesn't support reporting the battery type >> and the type is Li-ion in this case. >> >> Tested-by: Antoni Aloy Torrens <aaloytorrens@gmail.com> # TF101 >> Tested-by: Nikola Milosavljević <mnidza@outlook.com> # TF101 >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- > > 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 > 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.
diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c index b71fbf543428..fec6c139d4ff 100644 --- a/drivers/power/supply/sbs-battery.c +++ b/drivers/power/supply/sbs-battery.c @@ -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; }