| Message ID | 20230314225535.1321736-9-sre@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add DT support for generic ADC battery | expand |
On 3/15/23 00:55, Sebastian Reichel wrote: > Use standard simple-battery API for constant battery > information like min and max voltage. This simplifies > the driver a lot and brings automatic support for DT. > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Sebastian Reichel <sre@kernel.org> > --- > drivers/power/supply/generic-adc-battery.c | 64 ++-------------------- > include/linux/power/generic-adc-battery.h | 18 ------ > 2 files changed, 4 insertions(+), 78 deletions(-) > delete mode 100644 include/linux/power/generic-adc-battery.h > > diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c > index 771e5cfc49c3..d4f63d945b2c 100644 > --- a/drivers/power/supply/generic-adc-battery.c > +++ b/drivers/power/supply/generic-adc-battery.c > @@ -22,7 +22,6 @@ > #include <linux/slab.h> > #include <linux/iio/consumer.h> > #include <linux/iio/types.h> > -#include <linux/power/generic-adc-battery.h> > #include <linux/devm-helpers.h> > > #define JITTER_DEFAULT 10 /* hope 10ms is enough */ > @@ -48,9 +47,7 @@ struct gab { > struct power_supply *psy; > struct power_supply_desc psy_desc; > struct iio_channel *channel[GAB_MAX_CHAN_TYPE]; > - struct gab_platform_data *pdata; > struct delayed_work bat_work; > - int level; > int status; > bool cable_plugged; > struct gpio_desc *charge_finished; > @@ -70,14 +67,6 @@ static void gab_ext_power_changed(struct power_supply *psy) > > 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_VOLTAGE_NOW, > - POWER_SUPPLY_PROP_CURRENT_NOW, > - POWER_SUPPLY_PROP_TECHNOLOGY, > - POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN, > - POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN, > - POWER_SUPPLY_PROP_MODEL_NAME, > }; > > /* > @@ -97,17 +86,6 @@ static bool gab_charge_finished(struct gab *adc_bat) > return gpiod_get_value(adc_bat->charge_finished); > } > > -static int gab_get_status(struct gab *adc_bat) > -{ > - struct gab_platform_data *pdata = adc_bat->pdata; > - struct power_supply_info *bat_info; > - > - bat_info = &pdata->battery_info; > - if (adc_bat->level == bat_info->charge_full_design) > - return POWER_SUPPLY_STATUS_FULL; Not sure if this is intentional but I don't see the POWER_SUPPLY_STATUS_FULL being reported after applying your series. If this is intended, maybe it could be mentioned in commit log? Other than that - this really cleans up the driver in a nice way! Yours, -- Matti
Hi, On Thu, Mar 16, 2023 at 09:38:21AM +0200, Matti Vaittinen wrote: > On 3/15/23 00:55, Sebastian Reichel wrote: > > Use standard simple-battery API for constant battery > > information like min and max voltage. This simplifies > > the driver a lot and brings automatic support for DT. > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Sebastian Reichel <sre@kernel.org> > > --- > > drivers/power/supply/generic-adc-battery.c | 64 ++-------------------- > > include/linux/power/generic-adc-battery.h | 18 ------ > > 2 files changed, 4 insertions(+), 78 deletions(-) > > delete mode 100644 include/linux/power/generic-adc-battery.h > > > > diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c > > index 771e5cfc49c3..d4f63d945b2c 100644 > > --- a/drivers/power/supply/generic-adc-battery.c > > +++ b/drivers/power/supply/generic-adc-battery.c > > @@ -22,7 +22,6 @@ > > #include <linux/slab.h> > > #include <linux/iio/consumer.h> > > #include <linux/iio/types.h> > > -#include <linux/power/generic-adc-battery.h> > > #include <linux/devm-helpers.h> > > #define JITTER_DEFAULT 10 /* hope 10ms is enough */ > > @@ -48,9 +47,7 @@ struct gab { > > struct power_supply *psy; > > struct power_supply_desc psy_desc; > > struct iio_channel *channel[GAB_MAX_CHAN_TYPE]; > > - struct gab_platform_data *pdata; > > struct delayed_work bat_work; > > - int level; > > int status; > > bool cable_plugged; > > struct gpio_desc *charge_finished; > > @@ -70,14 +67,6 @@ static void gab_ext_power_changed(struct power_supply *psy) > > 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_VOLTAGE_NOW, > > - POWER_SUPPLY_PROP_CURRENT_NOW, > > - POWER_SUPPLY_PROP_TECHNOLOGY, > > - POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN, > > - POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN, > > - POWER_SUPPLY_PROP_MODEL_NAME, > > }; > > /* > > @@ -97,17 +86,6 @@ static bool gab_charge_finished(struct gab *adc_bat) > > return gpiod_get_value(adc_bat->charge_finished); > > } > > -static int gab_get_status(struct gab *adc_bat) > > -{ > > - struct gab_platform_data *pdata = adc_bat->pdata; > > - struct power_supply_info *bat_info; > > - > > - bat_info = &pdata->battery_info; > > - if (adc_bat->level == bat_info->charge_full_design) > > - return POWER_SUPPLY_STATUS_FULL; > > Not sure if this is intentional but I don't see the POWER_SUPPLY_STATUS_FULL > being reported after applying your series. If this is intended, maybe it > could be mentioned in commit log? Right. I removed it, because 'level' is never written to and thus is always 0. Apart from that the code needs to be a bit more flexible anyways (i.e. should not just report full for exactly the same value) and we currently do not have a way to calculate the current charge value. Thus removing it makes sense at the moment. Something better can be added once we have the required infrastructure. -- Sebastian
diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c index 771e5cfc49c3..d4f63d945b2c 100644 --- a/drivers/power/supply/generic-adc-battery.c +++ b/drivers/power/supply/generic-adc-battery.c @@ -22,7 +22,6 @@ #include <linux/slab.h> #include <linux/iio/consumer.h> #include <linux/iio/types.h> -#include <linux/power/generic-adc-battery.h> #include <linux/devm-helpers.h> #define JITTER_DEFAULT 10 /* hope 10ms is enough */ @@ -48,9 +47,7 @@ struct gab { struct power_supply *psy; struct power_supply_desc psy_desc; struct iio_channel *channel[GAB_MAX_CHAN_TYPE]; - struct gab_platform_data *pdata; struct delayed_work bat_work; - int level; int status; bool cable_plugged; struct gpio_desc *charge_finished; @@ -70,14 +67,6 @@ static void gab_ext_power_changed(struct power_supply *psy) 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_VOLTAGE_NOW, - POWER_SUPPLY_PROP_CURRENT_NOW, - POWER_SUPPLY_PROP_TECHNOLOGY, - POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN, - POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN, - POWER_SUPPLY_PROP_MODEL_NAME, }; /* @@ -97,17 +86,6 @@ static bool gab_charge_finished(struct gab *adc_bat) return gpiod_get_value(adc_bat->charge_finished); } -static int gab_get_status(struct gab *adc_bat) -{ - struct gab_platform_data *pdata = adc_bat->pdata; - struct power_supply_info *bat_info; - - bat_info = &pdata->battery_info; - if (adc_bat->level == bat_info->charge_full_design) - return POWER_SUPPLY_STATUS_FULL; - return adc_bat->status; -} - static enum gab_chan_type gab_prop_to_chan(enum power_supply_property psp) { switch (psp) { @@ -144,27 +122,12 @@ static int read_channel(struct gab *adc_bat, enum power_supply_property psp, static int gab_get_property(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val) { - struct gab *adc_bat; - struct gab_platform_data *pdata; - struct power_supply_info *bat_info; - int result = 0; - int ret = 0; - - adc_bat = to_generic_bat(psy); - if (!adc_bat) { - dev_err(&psy->dev, "no battery infos ?!\n"); - return -EINVAL; - } - pdata = adc_bat->pdata; - bat_info = &pdata->battery_info; + struct gab *adc_bat = to_generic_bat(psy); switch (psp) { case POWER_SUPPLY_PROP_STATUS: - val->intval = gab_get_status(adc_bat); - break; - case POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN: - val->intval = 0; - break; + val->intval = adc_bat->status; + return 0; case POWER_SUPPLY_PROP_VOLTAGE_NOW: case POWER_SUPPLY_PROP_CURRENT_NOW: case POWER_SUPPLY_PROP_POWER_NOW: @@ -173,26 +136,9 @@ static int gab_get_property(struct power_supply *psy, goto err; val->intval = result; break; - case POWER_SUPPLY_PROP_TECHNOLOGY: - val->intval = bat_info->technology; - break; - case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN: - val->intval = bat_info->voltage_min_design; - break; - case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN: - val->intval = bat_info->voltage_max_design; - break; - case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: - val->intval = bat_info->charge_full_design; - break; - case POWER_SUPPLY_PROP_MODEL_NAME: - val->strval = bat_info->name; - break; default: return -EINVAL; } -err: - return ret; } static void gab_work(struct work_struct *work) @@ -235,7 +181,6 @@ static int gab_probe(struct platform_device *pdev) struct gab *adc_bat; struct power_supply_desc *psy_desc; struct power_supply_config psy_cfg = {}; - struct gab_platform_data *pdata = pdev->dev.platform_data; enum power_supply_property *properties; int ret = 0; int chan; @@ -248,7 +193,7 @@ static int gab_probe(struct platform_device *pdev) psy_cfg.drv_data = adc_bat; psy_desc = &adc_bat->psy_desc; - psy_desc->name = pdata->battery_info.name; + psy_desc->name = dev_name(&pdev->dev); /* bootup default values for the battery */ adc_bat->cable_plugged = false; @@ -256,7 +201,6 @@ static int gab_probe(struct platform_device *pdev) psy_desc->type = POWER_SUPPLY_TYPE_BATTERY; psy_desc->get_property = gab_get_property; psy_desc->external_power_changed = gab_ext_power_changed; - adc_bat->pdata = pdata; /* * copying the static properties and allocating extra memory for holding diff --git a/include/linux/power/generic-adc-battery.h b/include/linux/power/generic-adc-battery.h deleted file mode 100644 index 54434e4304d3..000000000000 --- a/include/linux/power/generic-adc-battery.h +++ /dev/null @@ -1,18 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Copyright (C) 2012, Anish Kumar <anish198519851985@gmail.com> - */ - -#ifndef GENERIC_ADC_BATTERY_H -#define GENERIC_ADC_BATTERY_H - -/** - * struct gab_platform_data - platform_data for generic adc iio battery driver. - * @battery_info: recommended structure to specify static power supply - * parameters - */ -struct gab_platform_data { - struct power_supply_info battery_info; -}; - -#endif /* GENERIC_ADC_BATTERY_H */