This patches sitting in my repo for long time and would like to get feedback first before
submitting final series. This patches adding devicetree to generic-adc-battery driver
(we reuse it for gta04 board then and drop twl4030_madc_battery). Also add formula
for computing capacity of LiIon battery. This patches was tested on gta04 and
works fine. Thanks for comments.
Marek Belisko (5):
dt-bindings: power: Add battery types
power: generic-adc-battery: Parse more properties from DT
power/generic-adc-battery: Add support for temperature and add check
for charge from iio current channel
power: Add formula for computing LiIon State of Charge from Voltage
power: generic-adc-battery: Add capacity handling
drivers/power/supply/generic-adc-battery.c | 100 ++++++++++++++++++++++++++++-
include/dt-bindings/power/power.h | 11 ++++
include/linux/power/generic-fuel-gauge.h | 38 +++++++++++
3 files changed, 146 insertions(+), 3 deletions(-)
create mode 100644 include/dt-bindings/power/power.h
create mode 100644 include/linux/power/generic-fuel-gauge.h
--
2.7.4
From: Marek Belisko <[email protected]>
Signed-off-by: Marek Belisko <[email protected]>
---
drivers/power/supply/generic-adc-battery.c | 68 ++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
index b5e9208..d4daa6a 100644
--- a/drivers/power/supply/generic-adc-battery.c
+++ b/drivers/power/supply/generic-adc-battery.c
@@ -23,6 +23,7 @@
#include <linux/iio/consumer.h>
#include <linux/iio/types.h>
#include <linux/power/generic-adc-battery.h>
+#include <linux/of_gpio.h>
#define JITTER_DEFAULT 10 /* hope 10ms is enough */
@@ -241,6 +242,69 @@ static irqreturn_t gab_charged(int irq, void *dev_id)
return IRQ_HANDLED;
}
+#ifdef CONFIG_OF
+static struct gab_platform_data *gab_dt_probe(struct platform_device *pdev)
+{
+ struct gab_platform_data *pdata;
+ struct device_node *np = pdev->dev.of_node;
+ const char *name;
+ u32 val;
+ int err;
+
+ pdata = devm_kzalloc(&pdev->dev,
+ sizeof(struct gab_platform_data),
+ GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ pdata->gpio_charge_finished = of_get_gpio(np, 0);
+
+ /* parse and fill power_supply_info struct */
+ err = of_property_read_u32(np, "technology", &val);
+ if (err) {
+ dev_info(&pdev->dev, "Battery technology unknown\n");
+ val = 0;
+ }
+ pdata->battery_info.technology = val;
+
+ err = of_property_read_string(np, "battery-name", &name);
+ if (err) {
+ dev_info(&pdev->dev, "Battery name empty, setting default\n");
+ }
+ pdata->battery_info.name = name;
+
+ val = 0;
+ err = of_property_read_u32(np, "charge_empty_design", &val);
+ pdata->battery_info.charge_empty_design = val;
+
+ val = 0;
+ err = of_property_read_u32(np, "charge_full_design", &val);
+ pdata->battery_info.charge_full_design = val;
+
+ val = 0;
+ err = of_property_read_u32(np, "voltage_min_design", &val);
+ pdata->battery_info.voltage_min_design = val;
+
+ val = 0;
+ err = of_property_read_u32(np, "voltage_max-design", &val);
+ pdata->battery_info.voltage_max_design = val;
+
+ return pdata;
+}
+
+static const struct of_device_id of_gab_match[] = {
+ { .compatible = "linux,generic-adc-battery", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_gab_match);
+
+#else
+static struct gab_platform_data gab_dt_probe(struct platform_device *pdev)
+{
+ ERR_PTR(-ENODEV);
+}
+#endif
+
static int gab_probe(struct platform_device *pdev)
{
struct gab *adc_bat;
@@ -258,6 +322,9 @@ static int gab_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ if (pdata == NULL)
+ pdata = gab_dt_probe(pdev);
+
psy_cfg.drv_data = adc_bat;
psy_desc = &adc_bat->psy_desc;
psy_desc->name = "generic-adc-batt";//pdata->battery_info.name;
@@ -418,6 +485,7 @@ static struct platform_driver gab_driver = {
.driver = {
.name = "generic-adc-battery",
.pm = &gab_pm_ops,
+ .of_match_table = of_gab_match,
},
.probe = gab_probe,
.remove = gab_remove,
--
2.7.4
From: Marek Belisko <[email protected]>
Signed-off-by: Marek Belisko <[email protected]>
---
drivers/power/supply/generic-adc-battery.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
index 0d27b59..9ce51d2 100644
--- a/drivers/power/supply/generic-adc-battery.c
+++ b/drivers/power/supply/generic-adc-battery.c
@@ -24,6 +24,7 @@
#include <linux/iio/types.h>
#include <linux/power/generic-adc-battery.h>
#include <linux/of_gpio.h>
+#include <linux/power/generic-fuel-gauge.h>
#define JITTER_DEFAULT 10 /* hope 10ms is enough */
@@ -81,6 +82,7 @@ static const enum power_supply_property gab_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_TEMP,
+ POWER_SUPPLY_PROP_CAPACITY,
};
/*
@@ -202,6 +204,18 @@ static int gab_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_MODEL_NAME:
val->strval = bat_info->name;
break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ {
+ int ret, curr, voltage;
+
+ ret = read_channel(adc_bat, POWER_SUPPLY_PROP_CURRENT_NOW, &curr);
+ ret |= read_channel(adc_bat, POWER_SUPPLY_PROP_VOLTAGE_NOW, &voltage);
+ if (ret < 0)
+ goto err;
+
+ val->intval = fuel_level_LiIon(voltage, curr, 10);
+ }
+ break;
default:
return -EINVAL;
}
--
2.7.4
From: Marek Belisko <[email protected]>
The formula appears to be known in RC model communities.
We did find the first reference on the web in a a forum post
by "SilverFox" from 04-16-2008:
http://www.candlepowerforums.com/vb/showthread.php?115871-Li-Ion-State-of-Charge-and-Voltage-Measurements#post2440539
Some other posts attribute it to Sanyo.
The linear interpplation below 19.66% was suggested by Pavel Machek.
Signed-off-by: Marek Belisko <[email protected]>
---
include/linux/power/generic-fuel-gauge.h | 38 ++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 include/linux/power/generic-fuel-gauge.h
diff --git a/include/linux/power/generic-fuel-gauge.h b/include/linux/power/generic-fuel-gauge.h
new file mode 100644
index 0000000..2da7825
--- /dev/null
+++ b/include/linux/power/generic-fuel-gauge.h
@@ -0,0 +1,38 @@
+#ifndef PWR_GENERIC_FUEL_GAUSE_H
+#define PWR_GENERIC_FUEL_GAUSE_H
+
+/* calculate remaining fuel level (in %) of a LiIon battery assuming
+ * a standard chemistry model
+ * The first reference found on the web seems to be a forum post
+ * by "SilverFox" from 04-16-2008. It appears to be attributed to Sanyo.
+ * http://www.candlepowerforums.com/vb/showthread.php?115871-Li-Ion-State-of-Charge-and-Voltage-Measurements#post2440539
+ * The linear interpplation below 19.66% was suggested by Pavel Machek.
+ *
+ * @mV: voltage measured outside the battery
+ * @mA: current flowing out of the battery
+ * @mOhm: assumed series resitance of the battery
+ *
+ * returns value between 0 and 100
+ */
+static inline int fuel_level_LiIon(int mV, int mA, int mOhm) {
+ int u;
+
+ /* internal battery voltage is higher than measured when discharging */
+ mV += (mOhm * mA) /1000;
+
+ if (mV == 0)
+ return 0;
+
+ /* apply first part of formula */
+ u = 3870000 - (14523 * (37835 - 10 * mV));
+
+ /* use linear approx. below 3.756V => 19.66% assuming 3.3V => 0% */
+ if (u < 0) {
+ return max(((mV - 3300) * ((3756 - 3300) * 1966)) / 100000000, 0);
+ }
+
+ /* apply second part of formula */
+ return min((int)(1966 + int_sqrt(u))/100, 100);
+}
+
+#endif /* PWR_GENERIC_FUEL_GAUSE_H */
--
2.7.4
From: Marek Belisko <[email protected]>
Status reporting not working yet. During boot when workqueue is called status
is updated (by reading iio current channel but value is -705 (reason unknown as later
report correct value).
Signed-off-by: Marek Belisko <[email protected]>
---
drivers/power/supply/generic-adc-battery.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
index d4daa6a..0d27b59 100644
--- a/drivers/power/supply/generic-adc-battery.c
+++ b/drivers/power/supply/generic-adc-battery.c
@@ -31,6 +31,7 @@ enum gab_chan_type {
GAB_VOLTAGE = 0,
GAB_CURRENT,
GAB_POWER,
+ GAB_TEMPERATURE,
GAB_MAX_CHAN_TYPE
};
@@ -41,7 +42,8 @@ enum gab_chan_type {
static const char *const gab_chan_name[] = {
[GAB_VOLTAGE] = "voltage",
[GAB_CURRENT] = "current",
- [GAB_POWER] = "power",
+ [GAB_POWER] = "power",
+ [GAB_TEMPERATURE] = "temperature",
};
struct gab {
@@ -78,6 +80,7 @@ static const enum power_supply_property gab_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_TEMP,
};
/*
@@ -123,6 +126,8 @@ static enum gab_chan_type gab_prop_to_chan(enum power_supply_property psp)
return GAB_VOLTAGE;
case POWER_SUPPLY_PROP_CURRENT_NOW:
return GAB_CURRENT;
+ case POWER_SUPPLY_PROP_TEMP:
+ return GAB_TEMPERATURE;
default:
WARN_ON(1);
break;
@@ -176,6 +181,7 @@ static int gab_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
case POWER_SUPPLY_PROP_CURRENT_NOW:
case POWER_SUPPLY_PROP_POWER_NOW:
+ case POWER_SUPPLY_PROP_TEMP:
ret = read_channel(adc_bat, psp, &result);
if (ret < 0)
goto err;
@@ -210,13 +216,19 @@ static void gab_work(struct work_struct *work)
struct delayed_work *delayed_work;
bool is_plugged;
int status;
+ int ret, iio_charge;
delayed_work = to_delayed_work(work);
adc_bat = container_of(delayed_work, struct gab, bat_work);
pdata = adc_bat->pdata;
status = adc_bat->status;
-
- is_plugged = power_supply_am_i_supplied(adc_bat->psy);
+ ret = read_channel(adc_bat, POWER_SUPPLY_PROP_CURRENT_NOW, &iio_charge);
+ if (ret < 0) {
+ pr_info("Cannot read current channel, ret:%d\n", ret);
+ iio_charge = 0;
+ } else
+ pr_info("iio_charge:%d\n", iio_charge);
+ is_plugged = power_supply_am_i_supplied(adc_bat->psy) || (iio_charge > 0);
adc_bat->cable_plugged = is_plugged;
if (!is_plugged)
--
2.7.4
From: Marek Belisko <[email protected]>
Signed-off-by: Marek Belisko <[email protected]>
---
include/dt-bindings/power/power.h | 11 +++++++++++
1 file changed, 11 insertions(+)
create mode 100644 include/dt-bindings/power/power.h
diff --git a/include/dt-bindings/power/power.h b/include/dt-bindings/power/power.h
new file mode 100644
index 0000000..ea36d7f
--- /dev/null
+++ b/include/dt-bindings/power/power.h
@@ -0,0 +1,11 @@
+#ifndef _DT_BINDINGS_POWER_H
+#define _DT_BINDINGS_POWER_H
+
+#define POWER_SUPPLY_TECHNOLOGY_NiMH 1
+#define POWER_SUPPLY_TECHNOLOGY_LION 2
+#define POWER_SUPPLY_TECHNOLOGY_LIPO 3
+#define POWER_SUPPLY_TECHNOLOGY_LiFe 4
+#define POWER_SUPPLY_TECHNOLOGY_NiCd 5
+#define POWER_SUPPLY_TECHNOLOGY_LiMn 6
+
+#endif /* _DT_BINDINGS_POWER_H */
--
2.7.4
On Tue 2017-08-01 22:55:22, Marek Belisko wrote:
> From: Marek Belisko <[email protected]>
>
> Signed-off-by: Marek Belisko <[email protected]>
> ---
> include/dt-bindings/power/power.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> create mode 100644 include/dt-bindings/power/power.h
>
> diff --git a/include/dt-bindings/power/power.h b/include/dt-bindings/power/power.h
> new file mode 100644
> index 0000000..ea36d7f
> --- /dev/null
> +++ b/include/dt-bindings/power/power.h
> @@ -0,0 +1,11 @@
> +#ifndef _DT_BINDINGS_POWER_H
> +#define _DT_BINDINGS_POWER_H
> +
> +#define POWER_SUPPLY_TECHNOLOGY_NiMH 1
> +#define POWER_SUPPLY_TECHNOLOGY_LION 2
Nice animal :-), but I guess this should be "LiION" or something.
> +#define POWER_SUPPLY_TECHNOLOGY_LIPO 3
Make it "LiPo" for consistency.
There's no differnce between Li-ion and Li-Po from the software side,
AFAICT, but I guess we can keep both..
> +#define POWER_SUPPLY_TECHNOLOGY_LiFe 4
> +#define POWER_SUPPLY_TECHNOLOGY_NiCd 5
> +#define POWER_SUPPLY_TECHNOLOGY_LiMn 6
> +
> +#endif /* _DT_BINDINGS_POWER_H */
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> Signed-off-by: Marek Belisko <[email protected]>
> ---
> include/dt-bindings/power/power.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> create mode 100644 include/dt-bindings/power/power.h
>
> diff --git a/include/dt-bindings/power/power.h b/include/dt-bindings/power/power.h
> new file mode 100644
> index 0000000..ea36d7f
> --- /dev/null
> +++ b/include/dt-bindings/power/power.h
> @@ -0,0 +1,11 @@
> +#ifndef _DT_BINDINGS_POWER_H
> +#define _DT_BINDINGS_POWER_H
> +
> +#define POWER_SUPPLY_TECHNOLOGY_NiMH 1
> +#define POWER_SUPPLY_TECHNOLOGY_LION 2
> +#define POWER_SUPPLY_TECHNOLOGY_LIPO 3
> +#define POWER_SUPPLY_TECHNOLOGY_LiFe 4
Actually I'd add comments here, such as "/* LiFePO4 */, because
otherwise it is a bit ambiguous.
> +#define POWER_SUPPLY_TECHNOLOGY_NiCd 5
> +#define POWER_SUPPLY_TECHNOLOGY_LiMn 6
> +
> +#endif /* _DT_BINDINGS_POWER_H */
The rest of series is ok, you can add
Acked-by: Pavel Machek <[email protected]>
, but I guess someone would want a little more verbose changelogs.
BTW what hardware are you working with?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
On Wed, Aug 2, 2017 at 1:43 PM, Pavel Machek <[email protected]> wrote:
> Hi!
>
>> Signed-off-by: Marek Belisko <[email protected]>
>> ---
>> include/dt-bindings/power/power.h | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>> create mode 100644 include/dt-bindings/power/power.h
>>
>> diff --git a/include/dt-bindings/power/power.h b/include/dt-bindings/power/power.h
>> new file mode 100644
>> index 0000000..ea36d7f
>> --- /dev/null
>> +++ b/include/dt-bindings/power/power.h
>> @@ -0,0 +1,11 @@
>> +#ifndef _DT_BINDINGS_POWER_H
>> +#define _DT_BINDINGS_POWER_H
>> +
>> +#define POWER_SUPPLY_TECHNOLOGY_NiMH 1
>> +#define POWER_SUPPLY_TECHNOLOGY_LION 2
>> +#define POWER_SUPPLY_TECHNOLOGY_LIPO 3
>> +#define POWER_SUPPLY_TECHNOLOGY_LiFe 4
>
> Actually I'd add comments here, such as "/* LiFePO4 */, because
> otherwise it is a bit ambiguous.
OK.
>
>> +#define POWER_SUPPLY_TECHNOLOGY_NiCd 5
>> +#define POWER_SUPPLY_TECHNOLOGY_LiMn 6
>> +
>> +#endif /* _DT_BINDINGS_POWER_H */
>
> The rest of series is ok, you can add
>
> Acked-by: Pavel Machek <[email protected]>
>
> , but I guess someone would want a little more verbose changelogs.
Yes I'll post final series with updated commit messages. Thanks.
>
> BTW what hardware are you working with?
We have working it on gta04 board. Also after this will be merged we
can drop custom battery driver
for gta04 and use this generic (maybe also other drivers can reuse this one).
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
Hi!
> +#ifdef CONFIG_OF
> +static struct gab_platform_data *gab_dt_probe(struct platform_device *pdev)
> +{
> + struct gab_platform_data *pdata;
> + struct device_node *np = pdev->dev.of_node;
> + const char *name;
> + u32 val;
> + int err;
> +
> + pdata = devm_kzalloc(&pdev->dev,
> + sizeof(struct gab_platform_data),
> + GFP_KERNEL);
> + if (!pdata)
> + return ERR_PTR(-ENOMEM);
> +
> + pdata->gpio_charge_finished = of_get_gpio(np, 0);
> +
> + /* parse and fill power_supply_info struct */
> + err = of_property_read_u32(np, "technology", &val);
> + if (err) {
> + dev_info(&pdev->dev, "Battery technology unknown\n");
> + val = 0;
> + }
> + pdata->battery_info.technology = val;
> +
> + err = of_property_read_string(np, "battery-name", &name);
> + if (err) {
> + dev_info(&pdev->dev, "Battery name empty, setting default\n");
> + }
> + pdata->battery_info.name = name;
Actually ... looking at this once more. These are new properties,
right?
They'll need to be documented:
pavel@duo:/data/l/linux$ grep -ri battery-name Documentation/devicetree/
shows empty.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
On Wed, Aug 2, 2017 at 1:56 PM, Pavel Machek <[email protected]> wrote:
> Hi!
>
>> +#ifdef CONFIG_OF
>> +static struct gab_platform_data *gab_dt_probe(struct platform_device *pdev)
>> +{
>> + struct gab_platform_data *pdata;
>> + struct device_node *np = pdev->dev.of_node;
>> + const char *name;
>> + u32 val;
>> + int err;
>> +
>> + pdata = devm_kzalloc(&pdev->dev,
>> + sizeof(struct gab_platform_data),
>> + GFP_KERNEL);
>> + if (!pdata)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + pdata->gpio_charge_finished = of_get_gpio(np, 0);
>> +
>> + /* parse and fill power_supply_info struct */
>> + err = of_property_read_u32(np, "technology", &val);
>> + if (err) {
>> + dev_info(&pdev->dev, "Battery technology unknown\n");
>> + val = 0;
>> + }
>> + pdata->battery_info.technology = val;
>> +
>> + err = of_property_read_string(np, "battery-name", &name);
>> + if (err) {
>> + dev_info(&pdev->dev, "Battery name empty, setting default\n");
>> + }
>> + pdata->battery_info.name = name;
>
> Actually ... looking at this once more. These are new properties,
> right?
>
> They'll need to be documented:
>
> pavel@duo:/data/l/linux$ grep -ri battery-name Documentation/devicetree/
>
> shows empty.
You're right. I'll add bindings documentation. Thanks.
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
Hi,
On Tue, Aug 01, 2017 at 10:55:26PM +0200, Marek Belisko wrote:
> From: Marek Belisko <[email protected]>
>
> Signed-off-by: Marek Belisko <[email protected]>
> ---
> drivers/power/supply/generic-adc-battery.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
> index 0d27b59..9ce51d2 100644
> --- a/drivers/power/supply/generic-adc-battery.c
> +++ b/drivers/power/supply/generic-adc-battery.c
> @@ -24,6 +24,7 @@
> #include <linux/iio/types.h>
> #include <linux/power/generic-adc-battery.h>
> #include <linux/of_gpio.h>
> +#include <linux/power/generic-fuel-gauge.h>
>
> #define JITTER_DEFAULT 10 /* hope 10ms is enough */
>
> @@ -81,6 +82,7 @@ static const enum power_supply_property gab_props[] = {
> POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> POWER_SUPPLY_PROP_MODEL_NAME,
> POWER_SUPPLY_PROP_TEMP,
> + POWER_SUPPLY_PROP_CAPACITY,
> };
>
> /*
> @@ -202,6 +204,18 @@ static int gab_get_property(struct power_supply *psy,
> case POWER_SUPPLY_PROP_MODEL_NAME:
> val->strval = bat_info->name;
> break;
> + case POWER_SUPPLY_PROP_CAPACITY:
> + {
> + int ret, curr, voltage;
> +
> + ret = read_channel(adc_bat, POWER_SUPPLY_PROP_CURRENT_NOW, &curr);
> + ret |= read_channel(adc_bat, POWER_SUPPLY_PROP_VOLTAGE_NOW, &voltage);
> + if (ret < 0)
> + goto err;
> +
> + val->intval = fuel_level_LiIon(voltage, curr, 10);
> + }
> + break;
> default:
> return -EINVAL;
> }
Without patch description: See Last Question in
Documentation/power/power_supply_class.txt
-- Sebastian
Hi,
On Tue, Aug 01, 2017 at 10:55:23PM +0200, Marek Belisko wrote:
> From: Marek Belisko <[email protected]>
>
> Signed-off-by: Marek Belisko <[email protected]>
This should use the infrastructure described here:
Documentation/devicetree/bindings/power/supply/battery.txt
-- Sebastian
> ---
> drivers/power/supply/generic-adc-battery.c | 68 ++++++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
> index b5e9208..d4daa6a 100644
> --- a/drivers/power/supply/generic-adc-battery.c
> +++ b/drivers/power/supply/generic-adc-battery.c
> @@ -23,6 +23,7 @@
> #include <linux/iio/consumer.h>
> #include <linux/iio/types.h>
> #include <linux/power/generic-adc-battery.h>
> +#include <linux/of_gpio.h>
>
> #define JITTER_DEFAULT 10 /* hope 10ms is enough */
>
> @@ -241,6 +242,69 @@ static irqreturn_t gab_charged(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +#ifdef CONFIG_OF
> +static struct gab_platform_data *gab_dt_probe(struct platform_device *pdev)
> +{
> + struct gab_platform_data *pdata;
> + struct device_node *np = pdev->dev.of_node;
> + const char *name;
> + u32 val;
> + int err;
> +
> + pdata = devm_kzalloc(&pdev->dev,
> + sizeof(struct gab_platform_data),
> + GFP_KERNEL);
> + if (!pdata)
> + return ERR_PTR(-ENOMEM);
> +
> + pdata->gpio_charge_finished = of_get_gpio(np, 0);
> +
> + /* parse and fill power_supply_info struct */
> + err = of_property_read_u32(np, "technology", &val);
> + if (err) {
> + dev_info(&pdev->dev, "Battery technology unknown\n");
> + val = 0;
> + }
> + pdata->battery_info.technology = val;
> +
> + err = of_property_read_string(np, "battery-name", &name);
> + if (err) {
> + dev_info(&pdev->dev, "Battery name empty, setting default\n");
> + }
> + pdata->battery_info.name = name;
> +
> + val = 0;
> + err = of_property_read_u32(np, "charge_empty_design", &val);
> + pdata->battery_info.charge_empty_design = val;
> +
> + val = 0;
> + err = of_property_read_u32(np, "charge_full_design", &val);
> + pdata->battery_info.charge_full_design = val;
> +
> + val = 0;
> + err = of_property_read_u32(np, "voltage_min_design", &val);
> + pdata->battery_info.voltage_min_design = val;
> +
> + val = 0;
> + err = of_property_read_u32(np, "voltage_max-design", &val);
> + pdata->battery_info.voltage_max_design = val;
> +
> + return pdata;
> +}
> +
> +static const struct of_device_id of_gab_match[] = {
> + { .compatible = "linux,generic-adc-battery", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_gab_match);
> +
> +#else
> +static struct gab_platform_data gab_dt_probe(struct platform_device *pdev)
> +{
> + ERR_PTR(-ENODEV);
> +}
> +#endif
> +
> static int gab_probe(struct platform_device *pdev)
> {
> struct gab *adc_bat;
> @@ -258,6 +322,9 @@ static int gab_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> + if (pdata == NULL)
> + pdata = gab_dt_probe(pdev);
> +
> psy_cfg.drv_data = adc_bat;
> psy_desc = &adc_bat->psy_desc;
> psy_desc->name = "generic-adc-batt";//pdata->battery_info.name;
> @@ -418,6 +485,7 @@ static struct platform_driver gab_driver = {
> .driver = {
> .name = "generic-adc-battery",
> .pm = &gab_pm_ops,
> + .of_match_table = of_gab_match,
> },
> .probe = gab_probe,
> .remove = gab_remove,
> --
> 2.7.4
>
Hi,
On Tue, Aug 01, 2017 at 10:55:24PM +0200, Marek Belisko wrote:
> From: Marek Belisko <[email protected]>
>
> Status reporting not working yet. During boot when workqueue is called status
> is updated (by reading iio current channel but value is -705 (reason unknown as later
> report correct value).
>
> Signed-off-by: Marek Belisko <[email protected]>
> ---
> drivers/power/supply/generic-adc-battery.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
> index d4daa6a..0d27b59 100644
> --- a/drivers/power/supply/generic-adc-battery.c
> +++ b/drivers/power/supply/generic-adc-battery.c
> @@ -31,6 +31,7 @@ enum gab_chan_type {
> GAB_VOLTAGE = 0,
> GAB_CURRENT,
> GAB_POWER,
> + GAB_TEMPERATURE,
> GAB_MAX_CHAN_TYPE
> };
>
> @@ -41,7 +42,8 @@ enum gab_chan_type {
> static const char *const gab_chan_name[] = {
> [GAB_VOLTAGE] = "voltage",
> [GAB_CURRENT] = "current",
> - [GAB_POWER] = "power",
> + [GAB_POWER] = "power",
> + [GAB_TEMPERATURE] = "temperature",
> };
>
> struct gab {
> @@ -78,6 +80,7 @@ static const enum power_supply_property gab_props[] = {
> POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_TEMP,
> };
>
> /*
> @@ -123,6 +126,8 @@ static enum gab_chan_type gab_prop_to_chan(enum power_supply_property psp)
> return GAB_VOLTAGE;
> case POWER_SUPPLY_PROP_CURRENT_NOW:
> return GAB_CURRENT;
> + case POWER_SUPPLY_PROP_TEMP:
> + return GAB_TEMPERATURE;
> default:
> WARN_ON(1);
> break;
> @@ -176,6 +181,7 @@ static int gab_get_property(struct power_supply *psy,
> case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> case POWER_SUPPLY_PROP_CURRENT_NOW:
> case POWER_SUPPLY_PROP_POWER_NOW:
> + case POWER_SUPPLY_PROP_TEMP:
> ret = read_channel(adc_bat, psp, &result);
> if (ret < 0)
> goto err;
> @@ -210,13 +216,19 @@ static void gab_work(struct work_struct *work)
> struct delayed_work *delayed_work;
> bool is_plugged;
> int status;
> + int ret, iio_charge;
>
> delayed_work = to_delayed_work(work);
> adc_bat = container_of(delayed_work, struct gab, bat_work);
> pdata = adc_bat->pdata;
> status = adc_bat->status;
> -
> - is_plugged = power_supply_am_i_supplied(adc_bat->psy);
> + ret = read_channel(adc_bat, POWER_SUPPLY_PROP_CURRENT_NOW, &iio_charge);
> + if (ret < 0) {
> + pr_info("Cannot read current channel, ret:%d\n", ret);
> + iio_charge = 0;
> + } else
> + pr_info("iio_charge:%d\n", iio_charge);
> + is_plugged = power_supply_am_i_supplied(adc_bat->psy) || (iio_charge > 0);
This has nothing to do with adding temperature support and
I suspect, that you are missing proper supplied_by configuration,
so that power_supply_am_i_supplied does not work. Have a look at
the following file:
Documentation/devicetree/bindings/power/supply/power_supply.txt
-- Sebastian
> adc_bat->cable_plugged = is_plugged;
>
> if (!is_plugged)
> --
> 2.7.4
>
Hi!
> > + case POWER_SUPPLY_PROP_CAPACITY:
> > + {
> > + int ret, curr, voltage;
> > +
> > + ret = read_channel(adc_bat, POWER_SUPPLY_PROP_CURRENT_NOW, &curr);
> > + ret |= read_channel(adc_bat, POWER_SUPPLY_PROP_VOLTAGE_NOW, &voltage);
> > + if (ret < 0)
> > + goto err;
> > +
> > + val->intval = fuel_level_LiIon(voltage, curr, 10);
> > + }
> > + break;
> > default:
> > return -EINVAL;
> > }
>
> Without patch description: See Last Question in
>
> Documentation/power/power_supply_class.txt
...should be done in libbattery, yet to be written. :-).
Do you have specific guidance what you'd like to see in libbattery?
I was thinking about creating lib_pocket_hardware -- that would hide
battery details, plus other phone-related stuff such as LEDs,
backlights, temperatures, ...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi,
> Am 08.09.2017 um 13:32 schrieb Pavel Machek <[email protected]>:
>
> Hi!
>
>>> + case POWER_SUPPLY_PROP_CAPACITY:
>>> + {
>>> + int ret, curr, voltage;
>>> +
>>> + ret = read_channel(adc_bat, POWER_SUPPLY_PROP_CURRENT_NOW, &curr);
>>> + ret |= read_channel(adc_bat, POWER_SUPPLY_PROP_VOLTAGE_NOW, &voltage);
>>> + if (ret < 0)
>>> + goto err;
>>> +
>>> + val->intval = fuel_level_LiIon(voltage, curr, 10);
>>> + }
>>> + break;
>>> default:
>>> return -EINVAL;
>>> }
>>
>> Without patch description: See Last Question in
>>
>> Documentation/power/power_supply_class.txt
The main argument to answer with "Most likely, no" seems to be:
" as it would require floating point calculation to deal
with things like differential equations and Kalman filters."
The code shown above uses none of such things, hence the answer
could as well be "Yes, if it is simple enough to be handled by
the kernel with some lines of code and integer arithmetic".
>
> ...should be done in libbattery, yet to be written. :-).
For 10 years... Doesn't seem like anyone is waiting for it.
>
> Do you have specific guidance what you'd like to see in libbattery?
As a poor user I'd expect it to be available and widely accepted by
major distributions and battery monitoring applications in max. 3 months.
If that is not possible, I'd recommend to toss overboard a 10 years old
"maybe" rule and just fix the single problem once and for all in the
generic-adc-battery driver instead of dreaming of user space magically
solving problems where we already have a proposal for a kernel solution.
We should not forget that all this work is just to replace the pdata
based drivers/power/supply/twl4030_madc_battery.c with a proper and
more general DT based solution.
Nothing new, no rocket science (differential equations and Kalman filters)
expected.
BR,
Nikolaus