| Message ID | 20230309225041.477440-7-sre@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Add DT support for generic ADC battery | expand |
On Thu, Mar 9, 2023 at 11:50 PM Sebastian Reichel <sre@kernel.org> wrote: > Drop CHARGE_NOW support, which requires a platform specific > calculation method. > > Signed-off-by: Sebastian Reichel <sre@kernel.org> I agree. If we want to support this, we should use the generic methods with interpolation tables defined in DT as well, and it also ideally requires load compensated resistance calculation to figure out Ri so this can bring any kind of reasonable precision. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Hi dee Ho all, On 3/10/23 10:29, Linus Walleij wrote: > On Thu, Mar 9, 2023 at 11:50 PM Sebastian Reichel <sre@kernel.org> wrote: > >> Drop CHARGE_NOW support, which requires a platform specific >> calculation method. >> >> Signed-off-by: Sebastian Reichel <sre@kernel.org> > > I agree. If we want to support this, we should use the generic > methods with interpolation tables defined in DT as well, and it also > ideally requires load compensated resistance calculation to figure > out Ri so this can bring any kind of reasonable precision. I guess you have your reasons, besides you have far better insight to things than I do - hence I am not really objecting this - just asking a question ;) Do we have generic facilities of computing this based on the DT tables / Ri in place(?) I guess that we do need/see platform specific implementations as long as there is no generic "de-facto" way of doing this available... Well, maybe this helps kicking things to that direction :) Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com> -- Matti
On Mon, Mar 13, 2023 at 8:49 AM Matti Vaittinen <mazziesaccount@gmail.com> wrote: > On 3/10/23 10:29, Linus Walleij wrote: > > On Thu, Mar 9, 2023 at 11:50 PM Sebastian Reichel <sre@kernel.org> wrote: > > > >> Drop CHARGE_NOW support, which requires a platform specific > >> calculation method. > >> > >> Signed-off-by: Sebastian Reichel <sre@kernel.org> > > > > I agree. If we want to support this, we should use the generic > > methods with interpolation tables defined in DT as well, and it also > > ideally requires load compensated resistance calculation to figure > > out Ri so this can bring any kind of reasonable precision. > > I guess you have your reasons, besides you have far better insight to > things than I do - hence I am not really objecting this - just asking a > question ;) > > Do we have generic facilities of computing this based on the DT tables / > Ri in place(?) Not yet, for the Samsung batteries I used a static look-up table derived from the compatible string for calculating Ri from VBAT and from that calculate the capacity from estimated open circuit voltage, see drivers/power/supply/samsung-sdi-battery.c > I guess that we do need/see platform specific > implementations as long as there is no generic "de-facto" way of doing > this available... The method I used with Samsung batteries is fine as long as all you need to know to know everything about a battery is the compatible string. Pretty much any Lion battery with a clearly defined product name can be done this way. The only reason to put the interpolation tables into the device tree would be to support any random battery, such as one that you do not know the model or this can change. I am however mildly sceptic about adding that: if you know the VBAT-to-Ri and OCV-to-capacity tables, you must have a datasheet, and then you know the name of the battery product and hence you know the right compatible string... I think the right way to handle any capacity curves for any battery would be to create static data like I did for the Samsung batteries. Yours, Linus Walleij
diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c index e20894460d7f..d07eeb7d46d3 100644 --- a/drivers/power/supply/generic-adc-battery.c +++ b/drivers/power/supply/generic-adc-battery.c @@ -72,7 +72,6 @@ static const enum power_supply_property gab_props[] = { POWER_SUPPLY_PROP_STATUS, POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN, POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN, - POWER_SUPPLY_PROP_CHARGE_NOW, POWER_SUPPLY_PROP_VOLTAGE_NOW, POWER_SUPPLY_PROP_CURRENT_NOW, POWER_SUPPLY_PROP_TECHNOLOGY, @@ -166,9 +165,6 @@ static int gab_get_property(struct power_supply *psy, case POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN: val->intval = 0; break; - case POWER_SUPPLY_PROP_CHARGE_NOW: - val->intval = pdata->cal_charge(result); - break; case POWER_SUPPLY_PROP_VOLTAGE_NOW: case POWER_SUPPLY_PROP_CURRENT_NOW: case POWER_SUPPLY_PROP_POWER_NOW: diff --git a/include/linux/power/generic-adc-battery.h b/include/linux/power/generic-adc-battery.h index 50eb4bf28286..54434e4304d3 100644 --- a/include/linux/power/generic-adc-battery.h +++ b/include/linux/power/generic-adc-battery.h @@ -10,11 +10,9 @@ * struct gab_platform_data - platform_data for generic adc iio battery driver. * @battery_info: recommended structure to specify static power supply * parameters - * @cal_charge: calculate charge level. */ struct gab_platform_data { struct power_supply_info battery_info; - int (*cal_charge)(long value); }; #endif /* GENERIC_ADC_BATTERY_H */
Drop CHARGE_NOW support, which requires a platform specific calculation method. Signed-off-by: Sebastian Reichel <sre@kernel.org> --- drivers/power/supply/generic-adc-battery.c | 4 ---- include/linux/power/generic-adc-battery.h | 2 -- 2 files changed, 6 deletions(-)