[PATCHv2,08/12] power: supply: generic-adc-battery: use simple-battery API
Commit Message
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
Comments
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
@@ -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
deleted file mode 100644
@@ -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 */