diff mbox series

[v2,1/4] power: supply: core: Introduce one property to present the battery internal resistance

Message ID 358665e3f4f9ec105dc2f8a2dc6dd98dbe761fae.1537930252.git.baolin.wang@linaro.org (mailing list archive)
State Not Applicable, archived
Headers show
Series [v2,1/4] power: supply: core: Introduce one property to present the battery internal resistance | expand

Commit Message

(Exiting) Baolin Wang Sept. 26, 2018, 2:59 a.m. UTC
Introduce one property to present the battery internal resistance for battery
information.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
Changes from v1:
 - New patch in v2.
---
 .../devicetree/bindings/power/supply/battery.txt   |    2 ++
 drivers/power/supply/power_supply_core.c           |    3 +++
 include/linux/power_supply.h                       |    1 +
 3 files changed, 6 insertions(+)

Comments

Linus Walleij Sept. 26, 2018, 8 a.m. UTC | #1
On Wed, Sep 26, 2018 at 4:59 AM Baolin Wang <baolin.wang@linaro.org> wrote:

> Introduce one property to present the battery internal resistance for battery
> information.
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> Changes from v1:
>  - New patch in v2.

I'm a bit confused by the physics in this patch.

The internal resistance of a battery is not a constant in its life cycle,
this varies over the age of the battery, and the reason I thing is
chemical residuals accumulating on the anode and cathode inside
the battery and the energy storage medium aging. (Plus/minus my
ignorance about how batteries actually work.)

AFAIK the fact that the internal resistance varies is of high
importance for people developing algorithms of battery capacity
and longevity. Such that some (hardware) capacity monitors go
to great lengths to measure with high precision the current
internal resistance of the battery for their algorithms.

Sorry for making things more complex, but should it be named
"factory-internal-resistance-micro-ohms" or
"typical-internal-resistance-micro-ohms"?

Yours,
Linus Walleij
(Exiting) Baolin Wang Sept. 26, 2018, 8:30 a.m. UTC | #2
Hi Linus,

On 26 September 2018 at 16:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Sep 26, 2018 at 4:59 AM Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> Introduce one property to present the battery internal resistance for battery
>> information.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>> Changes from v1:
>>  - New patch in v2.
>
> I'm a bit confused by the physics in this patch.
>
> The internal resistance of a battery is not a constant in its life cycle,
> this varies over the age of the battery, and the reason I thing is
> chemical residuals accumulating on the anode and cathode inside
> the battery and the energy storage medium aging. (Plus/minus my
> ignorance about how batteries actually work.)

Yes, you are right. The internal resistance can be affected by
temperature or battery age or other factors. But our solution just
uses one constant internal resistance to calculate OCV value to look
up the capacity table when system boots on, in this case we do not
need one more accuracy OCV, since we will calculate the battery
capacity in future. So we just introduce one estimation constant
internal resistance.

>
> AFAIK the fact that the internal resistance varies is of high
> importance for people developing algorithms of battery capacity
> and longevity. Such that some (hardware) capacity monitors go
> to great lengths to measure with high precision the current
> internal resistance of the battery for their algorithms.
>
> Sorry for making things more complex, but should it be named
> "factory-internal-resistance-micro-ohms" or
> "typical-internal-resistance-micro-ohms"?

I am fine with this change. If Sebastian also agree with this change,
I will fix.
Thanks for your reviewing and comments.
Sebastian Reichel Sept. 26, 2018, 12:45 p.m. UTC | #3
Hi,

On Wed, Sep 26, 2018 at 04:30:39PM +0800, Baolin Wang wrote:
> Hi Linus,
> 
> On 26 September 2018 at 16:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Sep 26, 2018 at 4:59 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> >
> >> Introduce one property to present the battery internal resistance for battery
> >> information.
> >>
> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> >> ---
> >> Changes from v1:
> >>  - New patch in v2.
> >
> > I'm a bit confused by the physics in this patch.
> >
> > The internal resistance of a battery is not a constant in its life cycle,
> > this varies over the age of the battery, and the reason I thing is
> > chemical residuals accumulating on the anode and cathode inside
> > the battery and the energy storage medium aging. (Plus/minus my
> > ignorance about how batteries actually work.)
> 
> Yes, you are right. The internal resistance can be affected by
> temperature or battery age or other factors. But our solution just
> uses one constant internal resistance to calculate OCV value to look
> up the capacity table when system boots on, in this case we do not
> need one more accuracy OCV, since we will calculate the battery
> capacity in future. So we just introduce one estimation constant
> internal resistance.
> 
> >
> > AFAIK the fact that the internal resistance varies is of high
> > importance for people developing algorithms of battery capacity
> > and longevity. Such that some (hardware) capacity monitors go
> > to great lengths to measure with high precision the current
> > internal resistance of the battery for their algorithms.
> >
> > Sorry for making things more complex, but should it be named
> > "factory-internal-resistance-micro-ohms" or
> > "typical-internal-resistance-micro-ohms"?
> 
> I am fine with this change. If Sebastian also agree with this change,
> I will fix. Thanks for your reviewing and comments.

Ack.

FWIW for proper battery status you need to collect battery specific
statistics, that is the reason fuel gauge chip providers recommend to
combine the chip with the battery cells into a "smart battery".

-- Sebastian
(Exiting) Baolin Wang Sept. 27, 2018, 1:06 a.m. UTC | #4
On 26 September 2018 at 20:45, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Wed, Sep 26, 2018 at 04:30:39PM +0800, Baolin Wang wrote:
>> Hi Linus,
>>
>> On 26 September 2018 at 16:00, Linus Walleij <linus.walleij@linaro.org> wrote:
>> > On Wed, Sep 26, 2018 at 4:59 AM Baolin Wang <baolin.wang@linaro.org> wrote:
>> >
>> >> Introduce one property to present the battery internal resistance for battery
>> >> information.
>> >>
>> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> >> ---
>> >> Changes from v1:
>> >>  - New patch in v2.
>> >
>> > I'm a bit confused by the physics in this patch.
>> >
>> > The internal resistance of a battery is not a constant in its life cycle,
>> > this varies over the age of the battery, and the reason I thing is
>> > chemical residuals accumulating on the anode and cathode inside
>> > the battery and the energy storage medium aging. (Plus/minus my
>> > ignorance about how batteries actually work.)
>>
>> Yes, you are right. The internal resistance can be affected by
>> temperature or battery age or other factors. But our solution just
>> uses one constant internal resistance to calculate OCV value to look
>> up the capacity table when system boots on, in this case we do not
>> need one more accuracy OCV, since we will calculate the battery
>> capacity in future. So we just introduce one estimation constant
>> internal resistance.
>>
>> >
>> > AFAIK the fact that the internal resistance varies is of high
>> > importance for people developing algorithms of battery capacity
>> > and longevity. Such that some (hardware) capacity monitors go
>> > to great lengths to measure with high precision the current
>> > internal resistance of the battery for their algorithms.
>> >
>> > Sorry for making things more complex, but should it be named
>> > "factory-internal-resistance-micro-ohms" or
>> > "typical-internal-resistance-micro-ohms"?
>>
>> I am fine with this change. If Sebastian also agree with this change,
>> I will fix. Thanks for your reviewing and comments.
>
> Ack.
>
> FWIW for proper battery status you need to collect battery specific
> statistics, that is the reason fuel gauge chip providers recommend to
> combine the chip with the battery cells into a "smart battery".

OK. I will rename it as "factory-internal-resistance-micro-ohms" in
next version. Thanks.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
index f4d3b4a..25b9d2e 100644
--- a/Documentation/devicetree/bindings/power/supply/battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -22,6 +22,7 @@  Optional Properties:
  - charge-term-current-microamp: current for charge termination phase
  - constant-charge-current-max-microamp: maximum constant input current
  - constant-charge-voltage-max-microvolt: maximum constant input voltage
+ - internal-resistance-micro-ohms: battery internal resistance
 
 Battery properties are named, where possible, for the corresponding
 elements in enum power_supply_property, defined in
@@ -42,6 +43,7 @@  Example:
 		charge-term-current-microamp = <128000>;
 		constant-charge-current-max-microamp = <900000>;
 		constant-charge-voltage-max-microvolt = <4200000>;
+		internal-resistance-micro-ohms = <250000>;
 	};
 
 	charger: charger@11 {
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index e853618..9f3452f 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -579,6 +579,7 @@  int power_supply_get_battery_info(struct power_supply *psy,
 	info->charge_term_current_ua         = -EINVAL;
 	info->constant_charge_current_max_ua = -EINVAL;
 	info->constant_charge_voltage_max_uv = -EINVAL;
+	info->internal_resistance_uohm       = -EINVAL;
 
 	if (!psy->of_node) {
 		dev_warn(&psy->dev, "%s currently only supports devicetree\n",
@@ -616,6 +617,8 @@  int power_supply_get_battery_info(struct power_supply *psy,
 			     &info->constant_charge_current_max_ua);
 	of_property_read_u32(battery_np, "constant_charge_voltage_max_microvolt",
 			     &info->constant_charge_voltage_max_uv);
+	of_property_read_u32(battery_np, "internal-resistance-micro-ohms",
+			     &info->internal_resistance_uohm);
 
 	return 0;
 }
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index f807691..019452d 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -326,6 +326,7 @@  struct power_supply_battery_info {
 	int charge_term_current_ua;	    /* microAmps */
 	int constant_charge_current_max_ua; /* microAmps */
 	int constant_charge_voltage_max_uv; /* microVolts */
+	int internal_resistance_uohm;       /* microOhms */
 };
 
 extern struct atomic_notifier_head power_supply_notifier;