This patches contains supporting generic interface to get battery specification
and of-based battery driver which use the interface.
Up to now, power supply subsystem assumes that battery's charartric data is
static and also often left as fuelgauge's role. However, fuelgauge driver or
any power_supply driver can be worked with different battery with different
implementation and battery also can be changed even in runtime.
If so, it needs to notify its change to all related power_supply drivers not
let them notice all with private way. Thus, it tries to introduce generic
interface for management of the battery specification.
In addition to, for the smart battery, this'll help to abstract battery
interface which can be varied with different batteries. (SDQ, MIPI BIF..)
Jonghwa Lee (3):
power: core: Add generic interface to get battery specification.
power: core: Add variables related temperature to power_supply_info.
power: of_battery: Initial support for of-based battery specification
driver.
.../bindings/power_supply/of_battery.txt | 34 +++++++
drivers/power/Kconfig | 7 ++
drivers/power/Makefile | 1 +
drivers/power/of_battery.c | 100 ++++++++++++++++++++
drivers/power/power_supply_core.c | 91 ++++++++++++++++++
include/linux/power_supply.h | 9 ++
6 files changed, 242 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power_supply/of_battery.txt
create mode 100644 drivers/power/of_battery.c
--
1.7.9.5
To represent thermal limitation of power_supply, it adds 'temperature_max,
temperature_min' to power_supply_info structure.
Signed-off-by: Jonghwa Lee <[email protected]>
---
include/linux/power_supply.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 99306aa..0c8588c 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -241,6 +241,8 @@ struct power_supply_info {
int charge_empty_design;
int energy_full_design;
int energy_empty_design;
+ int temperature_max;
+ int temperature_min;
int use_for_apm;
};
--
1.7.9.5
This driver supports battery specification through the generic interface of
power_supply subsystem. All data are parsed from device tree and it exactly
matches with power_supply_info's content.
Signed-off-by: Jonghwa Lee <[email protected]>
---
.../bindings/power_supply/of_battery.txt | 34 +++++++
drivers/power/Kconfig | 7 ++
drivers/power/Makefile | 1 +
drivers/power/of_battery.c | 100 ++++++++++++++++++++
4 files changed, 142 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power_supply/of_battery.txt
create mode 100644 drivers/power/of_battery.c
diff --git a/Documentation/devicetree/bindings/power_supply/of_battery.txt b/Documentation/devicetree/bindings/power_supply/of_battery.txt
new file mode 100644
index 0000000..5ccd2a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/of_battery.txt
@@ -0,0 +1,34 @@
+of_battery bindings
+~~~~~~~~~~~~~~~~~~~
+
+This documentation describes how to define data in device tree
+to use of_battery driver. All properties are just matched with
+power_supply_info's.
+
+Required properties :
+ - battery-name : battery's name for identifying.
+
+Optional properties :
+ - voltage_max : Maximum voltage of the battery (uV).
+ - voltage_min : Minimum voltage of the battery (uV).
+ - charge_full : Full capacity of the battery (uAh).
+ - charge_empty : Empty capacity of the battery (uAh).
+ - energy_full : Full capacity of the battery (uWh).
+ - energy_empty : Empty capacity of the battery (uWh).
+ - temperature_max : Maximum temperature for the battery (deci 'C)
+ - temperature_min : Minium temperature for the battery (deci 'C)
+
+Example:
+ battery {
+ main-battery {
+ battery-name = "battery";
+ voltage-max = <4350000>;
+ charge-full = <2500000>;
+ temperature_max = <600>;
+ temperature_min = <(-100)>;
+ };
+ backup-battery {
+ battery-name = "coin-battery";
+ voltage-max = <3000000>;
+ };
+ };
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 73cfcdf..c324ac6 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -36,6 +36,13 @@ config GENERIC_ADC_BATTERY
Say Y here to enable support for the generic battery driver
which uses IIO framework to read adc.
+config BATTERY_OF
+ tristate "Generic of-based battery specification driver"
+ depends on OF
+ help
+ Say Y here to enable support for the of-based generic battery
+ specification driver which gives static battery data.
+
config MAX8925_POWER
tristate "MAX8925 battery charger support"
depends on MFD_MAX8925
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index dfa8942..38a83aa 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -6,6 +6,7 @@ power_supply-$(CONFIG_LEDS_TRIGGERS) += power_supply_leds.o
obj-$(CONFIG_POWER_SUPPLY) += power_supply.o
obj-$(CONFIG_GENERIC_ADC_BATTERY) += generic-adc-battery.o
+obj-$(CONFIG_BATTERY_OF) += of_battery.o
obj-$(CONFIG_PDA_POWER) += pda_power.o
obj-$(CONFIG_APM_POWER) += apm_power.o
diff --git a/drivers/power/of_battery.c b/drivers/power/of_battery.c
new file mode 100644
index 0000000..3c66765
--- /dev/null
+++ b/drivers/power/of_battery.c
@@ -0,0 +1,100 @@
+/*
+ * OF-based battery specification driver
+ *
+ * This driver supports generic interface to get static battery data.
+ *
+ * Copyright © 2014 Jonghwa Lee <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/power_supply.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+
+static struct power_supply_info *of_get_battery_info(struct device_node *np)
+{
+ struct power_supply_info *info;
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return ERR_PTR(-ENOMEM);
+
+ if (of_property_read_string(np, "battery-name", &info->name)) {
+ kfree(info);
+ return ERR_PTR(-EINVAL);
+ }
+
+ of_property_read_u32(np, "voltage_max", &info->voltage_max_design);
+ of_property_read_u32(np, "voltage_min", &info->voltage_min_design);
+ of_property_read_u32(np, "charge_full", &info->charge_full_design);
+ of_property_read_u32(np, "charge_empty", &info->charge_empty_design);
+ of_property_read_u32(np, "energy_full", &info->energy_full_design);
+ of_property_read_u32(np, "energy_empty", &info->energy_empty_design);
+ of_property_read_u32(np, "temperature_max", &info->temperature_max);
+ of_property_read_u32(np, "temperature_min", &info->temperature_min);
+
+ return info;
+}
+
+static int __init psy_of_battery_init(void)
+{
+ struct power_supply_info *info;
+ struct device_node *np, *child;
+ int ret;
+
+ np = of_find_node_by_name(NULL, "battery");
+ if (!np)
+ return -ENODEV;
+
+ for_each_child_of_node(np, child) {
+ info = of_get_battery_info(child);
+ if (IS_ERR(info)) {
+ pr_err("Failed to get battery information (%ld)\n",
+ PTR_ERR(info));
+ continue;
+ }
+ ret = psy_register_battery_info(info);
+ if (ret) {
+ pr_err("Failed to register battery information (%d)\n",
+ ret);
+ kfree(info);
+ continue;
+ }
+ }
+
+ return 0;
+}
+module_init(psy_of_battery_init);
+
+static void __exit psy_of_battery_exit(void)
+{
+ struct power_supply_info *info;
+ struct device_node *np, *child;
+ const char *name;
+
+ np = of_find_node_by_name(NULL, "battery");
+ if (!np)
+ return;
+
+ for_each_child_of_node(np, child) {
+ if (!of_property_read_string(np, "battery-name", &name))
+ continue;
+
+ info = psy_get_battery_info(name);
+ if (IS_ERR(info))
+ continue;
+
+ psy_unregister_battery_info(info);
+ }
+}
+module_exit(psy_of_battery_exit);
+
+MODULE_AUTHOR("Jonghwa Lee <[email protected]>");
+MODULE_DESCRIPTION("of battery specification driver");
+MODULE_LICENSE("GPL");
--
1.7.9.5
In power supply system, battery specification's dealt as static
information regardless of battery chainging. And it often assumed
fuelgauge's role to hold battery specification even same driver is
used with different batteries. To make subsystem handles above cases
properly, this patch adds helper functions to manager the battery
specification.
Signed-off-by: Jonghwa Lee <[email protected]>
---
drivers/power/power_supply_core.c | 91 +++++++++++++++++++++++++++++++++++++
include/linux/power_supply.h | 7 +++
2 files changed, 98 insertions(+)
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 078afd6..59207d9 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -653,6 +653,97 @@ static void __exit power_supply_class_exit(void)
subsys_initcall(power_supply_class_init);
module_exit(power_supply_class_exit);
+/****************************************************************
+ * Battery information interface *
+ ****************************************************************/
+
+ATOMIC_NOTIFIER_HEAD(psy_battery_info_notifier);
+static LIST_HEAD(psy_battery_info_list);
+
+struct psy_battery_info {
+ struct list_head entry;
+ struct power_supply_info *info;
+};
+
+enum battery_info_notifier_events {
+ PSY_BATT_INFO_REGISTERED,
+ PSY_BATT_INFO_UNREGISTERED,
+};
+
+int psy_battery_info_notifier_register(struct notifier_block *nb)
+{
+ return atomic_notifier_chain_register(&psy_battery_info_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(psy_battery_info_notifier_register);
+
+void psy_battery_info_notifier_unregister(struct notifier_block *nb)
+{
+ atomic_notifier_chain_unregister(&psy_battery_info_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(psy_battery_info_notifier_unregister);
+
+struct power_supply_info *psy_get_battery_info(const char *name)
+{
+ struct psy_battery_info *battery;
+
+ /* Sanity check */
+ if (!name)
+ goto err_out;
+
+ list_for_each_entry(battery, &psy_battery_info_list, entry) {
+ if (!strcmp(battery->info->name, name))
+ return battery->info;
+ }
+
+err_out:
+ return NULL;
+}
+EXPORT_SYMBOL(psy_get_battery_info);
+
+int psy_register_battery_info(struct power_supply_info *info)
+{
+ struct psy_battery_info *battery;
+
+ /* Sanity check */
+ if (!info->name)
+ return -EINVAL;
+
+ /* Check if same data is existed */
+ list_for_each_entry(battery, &psy_battery_info_list, entry)
+ if (!strcmp(battery->info->name, info->name))
+ return -EEXIST;
+
+ battery = kzalloc(sizeof(*battery), GFP_KERNEL);
+ if (!battery)
+ return -ENOMEM;
+
+ battery->info = info;
+ list_add_tail(&battery->entry, &psy_battery_info_list);
+
+ atomic_notifier_call_chain(&psy_battery_info_notifier,
+ PSY_BATT_INFO_REGISTERED, info);
+
+ return 0;
+}
+EXPORT_SYMBOL(psy_register_battery_info);
+
+void psy_unregister_battery_info(struct power_supply_info *info)
+{
+ struct psy_battery_info *battery, *tmp;
+
+ list_for_each_entry_safe(battery, tmp, &psy_battery_info_list, entry) {
+ if (battery->info == info) {
+ list_del(&battery->entry);
+ kfree(battery);
+ return;
+ }
+ }
+
+ atomic_notifier_call_chain(&psy_battery_info_notifier,
+ PSY_BATT_INFO_UNREGISTERED, info);
+}
+EXPORT_SYMBOL(psy_unregister_battery_info);
+
MODULE_DESCRIPTION("Universal power supply monitor class");
MODULE_AUTHOR("Ian Molton <[email protected]>, "
"Szabolcs Gyurko, "
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index f3dea41..99306aa 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -276,6 +276,13 @@ extern int power_supply_powers(struct power_supply *psy, struct device *dev);
/* For APM emulation, think legacy userspace. */
extern struct class *power_supply_class;
+/* Battery information helper */
+extern int psy_battery_info_notifier_register(struct notifier_block *);
+extern void psy_battery_info_notifier_unregister(struct notifier_block *);
+extern struct power_supply_info *psy_get_battery_info(const char *);
+extern int psy_register_battery_info(struct power_supply_info *);
+extern void psy_unregister_battery_info(struct power_supply_info *);
+
static inline bool power_supply_is_amp_property(enum power_supply_property psp)
{
switch (psp) {
--
1.7.9.5
On Tue 2014-10-07 19:58:38, Jonghwa Lee wrote:
> This driver supports battery specification through the generic interface of
> power_supply subsystem. All data are parsed from device tree and it exactly
> matches with power_supply_info's content.
>
> Signed-off-by: Jonghwa Lee <[email protected]>
Acked-by: Pavel Machek <[email protected]>
> +++ b/drivers/power/of_battery.c
> @@ -0,0 +1,100 @@
> +/*
> + * OF-based battery specification driver
> + *
> + * This driver supports generic interface to get static battery data.
> + *
> + * Copyright ? 2014 Jonghwa Lee <[email protected]>
I'd replace it with (c).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> +ATOMIC_NOTIFIER_HEAD(psy_battery_info_notifier);
Isn't it good to reuse the existing power_supply_notifier for this?
> +enum battery_info_notifier_events {
> + PSY_BATT_INFO_REGISTERED,
> + PSY_BATT_INFO_UNREGISTERED,
> +};
If we use the power_supply_notifier, then this can be moved to
enum power_supply_notifier_events
-Jenny
On 2014년 11월 08일 19:13, Tc, Jenny wrote:
>> +ATOMIC_NOTIFIER_HEAD(psy_battery_info_notifier);
>
> Isn't it good to reuse the existing power_supply_notifier for this?
>
>> +enum battery_info_notifier_events {
>> + PSY_BATT_INFO_REGISTERED,
>> + PSY_BATT_INFO_UNREGISTERED,
>> +};
>
> If we use the power_supply_notifier, then this can be moved to
> enum power_supply_notifier_events
>
It doesn't use power_supply_notifier, rather than it uses newly introduced
notifier for battery information. Intention of making of new notifier block here
is to extinguish event from power_supply_changed which might be noisy for
battery information consumer. However, If it looks wasteful code, it's not a big
deal to use existed power_supply_notifier.
Thanks,
Jonghwa
>
> >> +ATOMIC_NOTIFIER_HEAD(psy_battery_info_notifier);
> >
> > Isn't it good to reuse the existing power_supply_notifier for this?
> >
> >> +enum battery_info_notifier_events {
> >> + PSY_BATT_INFO_REGISTERED,
> >> + PSY_BATT_INFO_UNREGISTERED,
> >> +};
> >
> > If we use the power_supply_notifier, then this can be moved to
> > enum power_supply_notifier_events
> >
>
>
> It doesn't use power_supply_notifier, rather than it uses newly introduced
> notifier for battery information. Intention of making of new notifier block here
> is to extinguish event from power_supply_changed which might be noisy for
> battery information consumer. However, If it looks wasteful code, it's not a big
> deal to use existed power_supply_notifier.
Using the power_supply_notifier helps to get all power supply notifications
(power_supply_chnaged, battery info register/unregister, ..) using a single notifier. The
consumers can ignore the unwanted events.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
> @@ -241,6 +241,8 @@ struct power_supply_info {
> int charge_empty_design;
> int energy_full_design;
> int energy_empty_design;
> + int temperature_max;
> + int temperature_min;
> int use_for_apm;
> };
The CC,CV and restart threshold would vary based on the battery temperature
So I would suggest to have temperature zone table as part of battery info
along with other attributes.
int iterm; //charge termination current (used to stop charging)
int temp_zone_count; // number of temperature zone tables present
struct batt_temp_mon_table temp_mon_tbl[MAX_TEMP_MON_TABLE]; //temperature zone table array
struct batt_temp_mon_table {
short int temp_max;
short int cc;
short int cv;
short int vbat_vchk_drop_uv;
short int temp_min;
};
-Jenny
On 2014년 11월 10일 20:16, Tc, Jenny wrote:
>> @@ -241,6 +241,8 @@ struct power_supply_info {
>> int charge_empty_design;
>> int energy_full_design;
>> int energy_empty_design;
>> + int temperature_max;
>> + int temperature_min;
>> int use_for_apm;
>> };
>
>
> The CC,CV and restart threshold would vary based on the battery temperature
> So I would suggest to have temperature zone table as part of battery info
> along with other attributes.
>
> int iterm; //charge termination current (used to stop charging)
> int temp_zone_count; // number of temperature zone tables present
> struct batt_temp_mon_table temp_mon_tbl[MAX_TEMP_MON_TABLE]; //temperature zone table array
>
> struct batt_temp_mon_table {
> short int temp_max;
> short int cc;
> short int cv;
> short int vbat_vchk_drop_uv;
> short int temp_min;
> };
>
IMO, throttling cc/cv according the temperature can be done via thermal fw
interface. However voltage drop and charging termination current can be added here.
Jonghwa
> -Jenny
>
>
> > The CC,CV and restart threshold would vary based on the battery temperature
> > So I would suggest to have temperature zone table as part of battery info
> > along with other attributes.
> >
> > int iterm; //charge termination current (used to stop charging)
> > int temp_zone_count; // number of temperature zone tables present
> > struct batt_temp_mon_table temp_mon_tbl[MAX_TEMP_MON_TABLE];
> //temperature zone table array
> >
> > struct batt_temp_mon_table {
> > short int temp_max;
> > short int cc;
> > short int cv;
> > short int vbat_vchk_drop_uv;
> > short int temp_min;
> > };
> >
>
>
> IMO, throttling cc/cv according the temperature can be done via thermal fw
> interface. However voltage drop and charging termination current can be added here.
The CC/CV for each battery temperature zone is defined as part of battery spec. This is
as per the JEITA/PSE standards. So IMO, this is a battery charging information
(charging object) rather than a thermal throttling information.
Also the battery information may not fit into a standard format. Different standards have
different format for charging object. So I would suggest to make it flexible enough to
support different charging object format. For example MIPI BIF charging object format
(https://members.mipi.org/wg/BIF/document/11518) and MIPI BIF Rule based charging algorithm
(http://mipi.org/sites/default/files/mipi_BIF_rule-based-charging_white-paper_1.pdf)
has different charging object format. This is why the patch https://lkml.org/lkml/2014/8/13/355
has option to support different charging objects and different charging algorithms.
-Jenny
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Tue 2014-11-11 04:56:31, Tc, Jenny wrote:
> > > The CC,CV and restart threshold would vary based on the battery temperature
> > > So I would suggest to have temperature zone table as part of battery info
> > > along with other attributes.
> > >
> > > int iterm; //charge termination current (used to stop charging)
> > > int temp_zone_count; // number of temperature zone tables present
> > > struct batt_temp_mon_table temp_mon_tbl[MAX_TEMP_MON_TABLE];
> > //temperature zone table array
> > >
> > > struct batt_temp_mon_table {
> > > short int temp_max;
> > > short int cc;
> > > short int cv;
> > > short int vbat_vchk_drop_uv;
> > > short int temp_min;
> > > };
> > >
> >
> >
> > IMO, throttling cc/cv according the temperature can be done via thermal fw
> > interface. However voltage drop and charging termination current can be added here.
>
> The CC/CV for each battery temperature zone is defined as part of battery spec. This is
> as per the JEITA/PSE standards. So IMO, this is a battery charging information
> (charging object) rather than a thermal throttling information.
>
> Also the battery information may not fit into a standard format. Different standards have
> different format for charging object. So I would suggest to make it flexible enough to
> support different charging object format. For example MIPI BIF charging object format
> (https://members.mipi.org/wg/BIF/document/11518) and MIPI BIF Rule based charging algorithm
> (http://mipi.org/sites/default/files/mipi_BIF_rule-based-charging_white-paper_1.pdf)
> has different charging object format. This is why the patch https://lkml.org/lkml/2014/8/13/355
> has option to support different charging objects and different charging algorithms.
Yes, and this is also why your patches are not being
merged. Overengineered, too complex. Citing standards will not improve
the patches.
And yes, adding cc/cv to the thermal interface seems like a good idea
to me.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> > The CC/CV for each battery temperature zone is defined as part of battery spec.
> This is
> > as per the JEITA/PSE standards. So IMO, this is a battery charging information
> > (charging object) rather than a thermal throttling information.
> >
> > Also the battery information may not fit into a standard format. Different standards
> have
> > different format for charging object. So I would suggest to make it flexible enough to
> > support different charging object format. For example MIPI BIF charging object
> format
> > (https://members.mipi.org/wg/BIF/document/11518) and MIPI BIF Rule based
> charging algorithm
> > (http://mipi.org/sites/default/files/mipi_BIF_rule-based-charging_white-paper_1.pdf)
> > has different charging object format. This is why the patch
> https://lkml.org/lkml/2014/8/13/355
> > has option to support different charging objects and different charging algorithms.
>
> Yes, and this is also why your patches are not being
> merged. Overengineered, too complex. Citing standards will not improve
> the patches.
>
> And yes, adding cc/cv to the thermal interface seems like a good idea
> to me.
Sorry to disagree with you. IMHO it's a charging profile and not a thermal profile. The
cc/cv information is defined as part of battery spec. If the intention here is to provide a
place for battery info, then cc/cv should be part of battery info. The latest charger chips
allows to configure CC/CV for different temperature zone. IMHO adding these information
to thermal profile doesn't seems to be the right approach since the thermal subsystem
need to be aware of the charging subsystem constraints.
The standards were cited to point where the industry is moving. Anyway let the maintainer
take a final call - should we align with industry standards or stick to legacy charging
methodologies?
-Jenny