[PATCH v4 0/8] power: bq27xxx: add support for NVRAM R/W access

[PATCH v4 0/8] power: bq27xxx: add support for NVRAM R/W access

[Matt Ranostay]

Changes from v1:
* add documentation for mWh and mAh property units
* change devicetree entries to match new property units

Changes from v2:
* split i2c changes into respective patches
* add documentation for battery information for fuel gauge
* rebased documentation patches on change on the list
* abstracted the battery configuration for the state machine
  to an generic struct and platform data access function

Changes from v3:
* add "fixed-battery" compatible field to be be more consistant
  with devicetree

Matt Ranostay (8):
  devicetree: property-units: add mWh and mAh units
  devicetree: power: add battery state machine documentation
  power: power_supply: add battery information struct
  power: power_supply: add battery info platform data retrieval
  power: bq27xxx_battery: add BQ27425 chip id
  power: bq27xxx_battery: add i2c bulk read/write functions
  devicetree: power: bq27xxx: add monitored battery documentation
  power: bq27xxx_battery: add initial state machine support

 .../devicetree/bindings/power/supply/battery.txt   |  24 ++
 .../devicetree/bindings/power/supply/bq27xxx.txt   |   8 +
 .../devicetree/bindings/property-units.txt         |   2 +
 drivers/power/supply/bq27xxx_battery.c             | 253 ++++++++++++++++++++-
 drivers/power/supply/bq27xxx_battery_i2c.c         |  64 +++++-
 drivers/power/supply/power_supply_core.c           |  41 ++++
 include/linux/power/bq27xxx_battery.h              |   7 +-
 include/linux/power_supply.h                       |  16 ++
 8 files changed, 411 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

-- 
2.10.2

[PATCH v4 1/8] devicetree: property-units: add mWh and mAh units

[Matt Ranostay]

Add entries for microwatt-hours and microamp-hours to property
units.

Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
 Documentation/devicetree/bindings/property-units.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
index 12278d79f6c0..5e8d220cc2b6 100644
--- a/Documentation/devicetree/bindings/property-units.txt
+++ b/Documentation/devicetree/bindings/property-units.txt
@@ -25,8 +25,10 @@ Distance
 Electricity
 ----------------------------------------
 -microamp	: micro amps
+-microamp-hours : micro amp hours
 -ohms		: Ohms
 -micro-ohms	: micro Ohms
+-microwatt-hours: micro Watt hours
 -microvolt	: micro volts
 
 Temperature
-- 
2.10.2

Re: [PATCH v4 1/8] devicetree: property-units: add mWh and mAh units

[Rob Herring]

On Sat, Jan 21, 2017 at 11:13:57PM -0800, Matt Ranostay wrote:
> Add entries for microwatt-hours and microamp-hours to property
> units.
> 
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>

Please add acks when posting new versions. You're missing Sebastian's 
and mine.

> ---
>  Documentation/devicetree/bindings/property-units.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
> index 12278d79f6c0..5e8d220cc2b6 100644
> --- a/Documentation/devicetree/bindings/property-units.txt
> +++ b/Documentation/devicetree/bindings/property-units.txt
> @@ -25,8 +25,10 @@ Distance
>  Electricity
>  ----------------------------------------
>  -microamp	: micro amps
> +-microamp-hours : micro amp hours
>  -ohms		: Ohms
>  -micro-ohms	: micro Ohms
> +-microwatt-hours: micro Watt hours
>  -microvolt	: micro volts
>  
>  Temperature
> -- 
> 2.10.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Matt Ranostay]

Documentation on battery properties that can be defined for
fine tuning fuel gauge state machines.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
 .../devicetree/bindings/power/supply/battery.txt   | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
new file mode 100644
index 000000000000..398b4d622883
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -0,0 +1,24 @@
+Battery State Machine Support
+
+Required Properties:
+ - compatible: Must be "fixed-battery"
+
+Optional Properties:
+ - nominal-microvolt: dead battery voltage in microvolts
+ - design-microwatt-hours: battery design mWh in microwatts
+ - design-microamp-hours: battery design mAh in microamps
+
+Example:
+
+	bat: battery@0 {
+		compatible = "fixed-battery";
+		nominal-microvolt = <3700000>;
+		design-microwatt-hours = <5290000>;
+		design-microamp-hours = <1430000>;
+	};
+
+	fuel_gauge: fuel_gauge@0 {
+		....
+		monitored-battery = <&bat>;
+		...
+	};
-- 
2.10.2

[PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

I think Matt meant the following :-)

Note: nominal-microvolt is not the correct term for termination voltage.
Changed to termination-microvolt
---

Documentation on battery properties that can be defined for
fine tuning fuel gauge state machines.

From: Matt Ranostay <matt@ranostay.consulting>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
 .../devicetree/bindings/power/supply/battery.txt   | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt

diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
new file mode 100644
index 000000000000..398b4d622883
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -0,0 +1,34 @@
+Battery Characteristics
+
+Required Properties:
+ - compatible: Must be "fixed-battery"
+
+Optional Properties:
+ - termination-microvolt: dead battery voltage
+ - design-microwatt-hours: battery design energy
+ - design-microamp-hours: battery design capacity
+
+Batteries must be referenced by chargers and/or fuel-gauges
+using a phandle. The phandle's property should be named
+"monitored-battery".
+
+Example:
+
+	bat: battery {
+		compatible = "fixed-battery";
+		terminate-microvolt = <3700000>;
+		design-microwatt-hours = <5290000>;
+		design-microamp-hours = <1430000>;
+	};
+
+	charger: charger@0 {
+		....
+		monitored-battery = <&bat>;
+		...
+	};
+
+	fuel_gauge: fuel_gauge@0 {
+		....
+		monitored-battery = <&bat>;
+		...
+	};

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote:
> I think Matt meant the following :-)
>
> Note: nominal-microvolt is not the correct term for termination voltage.
> Changed to termination-microvolt
> ---
>
> Documentation on battery properties that can be defined for
> fine tuning fuel gauge state machines.
>
> From: Matt Ranostay <matt@ranostay.consulting>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
>  .../devicetree/bindings/power/supply/battery.txt   | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
>
> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> new file mode 100644
> index 000000000000..398b4d622883
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -0,0 +1,34 @@
> +Battery Characteristics
> +
> +Required Properties:
> + - compatible: Must be "fixed-battery"
> +
> +Optional Properties:
> + - termination-microvolt: dead battery voltage
> + - design-microwatt-hours: battery design energy
> + - design-microamp-hours: battery design capacity


Two more battery characteristics should be included. Battery chargers
need them (for instance, BQ24190).

- precharge-microamps: maximum charge current during precharge
  phase (typically 20% of battery capacity)

- termination-microamps: a charge cycle terminates when the
  battery voltage is above recharge threshold, and the current is below
  this setting (typically 10% of battery capacity)

Perhaps termination-microvolt should be called terminal-microvolt or
similar to avoid confusion.

Sebastian, did you want to include the other fields from your example?

        /* Nokia BL-5J */
        nominal-microvolt = <3700000>;
        chemistry-type = <POWER_SUPPLY_LI_ION>;

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2617 bytes --]

Hi,

On Tue, Jan 24, 2017 at 11:56:33AM -0800, Liam Breck wrote:
> On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
> > I think Matt meant the following :-)
> >
> > Note: nominal-microvolt is not the correct term for termination voltage.
> > Changed to termination-microvolt
> > ---
> >
> > Documentation on battery properties that can be defined for
> > fine tuning fuel gauge state machines.
> >
> > From: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> > ---
> >  .../devicetree/bindings/power/supply/battery.txt   | 34 ++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> > new file mode 100644
> > index 000000000000..398b4d622883
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> > @@ -0,0 +1,34 @@
> > +Battery Characteristics
> > +
> > +Required Properties:
> > + - compatible: Must be "fixed-battery"
> > +
> > +Optional Properties:
> > + - termination-microvolt: dead battery voltage
> > + - design-microwatt-hours: battery design energy
> > + - design-microamp-hours: battery design capacity
> 
> 
> Two more battery characteristics should be included. Battery chargers
> need them (for instance, BQ24190).
> 
> - precharge-microamps: maximum charge current during precharge
>   phase (typically 20% of battery capacity)
> 
> - termination-microamps: a charge cycle terminates when the
>   battery voltage is above recharge threshold, and the current is below
>   this setting (typically 10% of battery capacity)

I guess quite a few battery characteristics could be added. Let's
do this once we have a user for them.

> Perhaps termination-microvolt should be called terminal-microvolt or
> similar to avoid confusion.
> 
> Sebastian, did you want to include the other fields from your example?

Let's add properties once we have users for them. I just
used the stuff printed on my Nokia N900's battery as an
example :) I guess chemistry-type may actually be useful
already. I think at least some bq27xxx chips would also
work with LiPo based batteries.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote:

> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> new file mode 100644
> index 000000000000..398b4d622883
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -0,0 +1,34 @@
> +Battery Characteristics
> +
> +Required Properties:
> + - compatible: Must be "fixed-battery"
> +
> +Optional Properties:
> + - termination-microvolt: dead battery voltage
> + - design-microwatt-hours: battery design energy
> + - design-microamp-hours: battery design capacity

Also I suspect the members of struct power_supply_battery_info should
use the same names as appear in dts:

+struct power_supply_battery_info {
+  int design_microwatt_hours;
+  int design_microamp_hours;
+  int termination_microvolt;
etc

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Matt Ranostay]

On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote:
> On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote:
>
>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>> new file mode 100644
>> index 000000000000..398b4d622883
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>> @@ -0,0 +1,34 @@
>> +Battery Characteristics
>> +
>> +Required Properties:
>> + - compatible: Must be "fixed-battery"
>> +
>> +Optional Properties:
>> + - termination-microvolt: dead battery voltage
>> + - design-microwatt-hours: battery design energy
>> + - design-microamp-hours: battery design capacity
>
> Also I suspect the members of struct power_supply_battery_info should
> use the same names as appear in dts:

These are internal and we can't be thinking just device tree.. ACPI
and platform data is also an option.

>
> +struct power_supply_battery_info {
> +  int design_microwatt_hours;
> +  int design_microamp_hours;
> +  int termination_microvolt;
> etc

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]

Hi,

On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote:
> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote:
> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote:
> >
> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> >> new file mode 100644
> >> index 000000000000..398b4d622883
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> >> @@ -0,0 +1,34 @@
> >> +Battery Characteristics
> >> +
> >> +Required Properties:
> >> + - compatible: Must be "fixed-battery"
> >> +
> >> +Optional Properties:
> >> + - termination-microvolt: dead battery voltage
> >> + - design-microwatt-hours: battery design energy
> >> + - design-microamp-hours: battery design capacity
> >
> > Also I suspect the members of struct power_supply_battery_info should
> > use the same names as appear in dts:
> 
> These are internal and we can't be thinking just device tree.. ACPI
> and platform data is also an option.

well platform data just uses the struct. ACPI is probably not
relevant, since in ACPI world one usually has smart batteries.
But yes, the names can be different.

The important part is, that the API is used correctly, so it
should be clear what each property is used for. For example
termination_microvolt is not clear: Is this charge termination
voltage or system shutdown voltage? Also we do not need to add
"micro", since the power-supply subsystem always uses micro
based units.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Hi,
>
> On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote:
>> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
>> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
>> >
>> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>> >> new file mode 100644
>> >> index 000000000000..398b4d622883
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>> >> @@ -0,0 +1,34 @@
>> >> +Battery Characteristics
>> >> +
>> >> +Required Properties:
>> >> + - compatible: Must be "fixed-battery"
>> >> +
>> >> +Optional Properties:
>> >> + - termination-microvolt: dead battery voltage
>> >> + - design-microwatt-hours: battery design energy
>> >> + - design-microamp-hours: battery design capacity
>> >
>> > Also I suspect the members of struct power_supply_battery_info should
>> > use the same names as appear in dts:
>>
>> These are internal and we can't be thinking just device tree.. ACPI
>> and platform data is also an option.
>
> well platform data just uses the struct. ACPI is probably not
> relevant, since in ACPI world one usually has smart batteries.
> But yes, the names can be different.
>
> The important part is, that the API is used correctly, so it
> should be clear what each property is used for. For example
> termination_microvolt is not clear: Is this charge termination
> voltage or system shutdown voltage? Also we do not need to add
> "micro", since the power-supply subsystem always uses micro
> based units.

It's the min battery voltage. So...
drained_voltage, depleted_voltage, sapped_voltage, spent_voltage...

Help me out here :-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

Hi,

On Sun, Jan 29, 2017 at 03:22:31PM -0800, Liam Breck wrote:
> On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote:
> >> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote:
> >> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote:
> >> >
> >> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> >> >> new file mode 100644
> >> >> index 000000000000..398b4d622883
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> >> >> @@ -0,0 +1,34 @@
> >> >> +Battery Characteristics
> >> >> +
> >> >> +Required Properties:
> >> >> + - compatible: Must be "fixed-battery"
> >> >> +
> >> >> +Optional Properties:
> >> >> + - termination-microvolt: dead battery voltage
> >> >> + - design-microwatt-hours: battery design energy
> >> >> + - design-microamp-hours: battery design capacity
> >> >
> >> > Also I suspect the members of struct power_supply_battery_info should
> >> > use the same names as appear in dts:
> >>
> >> These are internal and we can't be thinking just device tree.. ACPI
> >> and platform data is also an option.
> >
> > well platform data just uses the struct. ACPI is probably not
> > relevant, since in ACPI world one usually has smart batteries.
> > But yes, the names can be different.
> >
> > The important part is, that the API is used correctly, so it
> > should be clear what each property is used for. For example
> > termination_microvolt is not clear: Is this charge termination
> > voltage or system shutdown voltage? Also we do not need to add
> > "micro", since the power-supply subsystem always uses micro
> > based units.
> 
> It's the min battery voltage. So...
> drained_voltage, depleted_voltage, sapped_voltage, spent_voltage...

This is often called EOD (end of discharge) voltage, so let's use
end_of_discharge_voltage.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Sun, Jan 29, 2017 at 6:30 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Sun, Jan 29, 2017 at 03:22:31PM -0800, Liam Breck wrote:
>> On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre@kernel.org> wrote:
>> > On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote:
>> >> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam@networkimprov.net> wrote:
>> >> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam@networkimprov.net> wrote:
>> >> >
>> >> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>> >> >> new file mode 100644
>> >> >> index 000000000000..398b4d622883
>> >> >> --- /dev/null
>> >> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>> >> >> @@ -0,0 +1,34 @@
>> >> >> +Battery Characteristics
>> >> >> +
>> >> >> +Required Properties:
>> >> >> + - compatible: Must be "fixed-battery"
>> >> >> +
>> >> >> +Optional Properties:
>> >> >> + - termination-microvolt: dead battery voltage
>> >> >> + - design-microwatt-hours: battery design energy
>> >> >> + - design-microamp-hours: battery design capacity
>> >> >
>> >> > Also I suspect the members of struct power_supply_battery_info should
>> >> > use the same names as appear in dts:
>> >>
>> >> These are internal and we can't be thinking just device tree.. ACPI
>> >> and platform data is also an option.
>> >
>> > well platform data just uses the struct. ACPI is probably not
>> > relevant, since in ACPI world one usually has smart batteries.
>> > But yes, the names can be different.
>> >
>> > The important part is, that the API is used correctly, so it
>> > should be clear what each property is used for. For example
>> > termination_microvolt is not clear: Is this charge termination
>> > voltage or system shutdown voltage? Also we do not need to add
>> > "micro", since the power-supply subsystem always uses micro
>> > based units.
>>
>> It's the min battery voltage. So...
>> drained_voltage, depleted_voltage, sapped_voltage, spent_voltage...
>
> This is often called EOD (end of discharge) voltage, so let's use
> end_of_discharge_voltage.

How's this?

Optional Properties:
  - end-of-discharge-microvolt: dead battery voltage
  - design-microwatt-hours: battery design energy
  - design-microamp-hours: battery design capacity

struct power_supply_battery_info {
   int design_energy_uwh;  /* microWatt-hours */
   int design_current_uah;  /* microAmp-hours */
   int end_of_discharge_uv; /* microVolts */
};

Default value for unset DT properties will be -EINVAL, since 0 is
valid for design_* on the chips.

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Mon, Jan 30, 2017 at 2:54 AM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
> On Sun, Jan 29, 2017 at 6:30 PM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> Hi,
>>
>> On Sun, Jan 29, 2017 at 03:22:31PM -0800, Liam Breck wrote:
>>> On Sun, Jan 29, 2017 at 9:20 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> > On Wed, Jan 25, 2017 at 11:02:03PM -0800, Matt Ranostay wrote:
>>> >> On Wed, Jan 25, 2017 at 10:19 PM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
>>> >> > On Sun, Jan 22, 2017 at 2:22 PM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
>>> >> >
>>> >> >> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>>> >> >> new file mode 100644
>>> >> >> index 000000000000..398b4d622883
>>> >> >> --- /dev/null
>>> >> >> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>>> >> >> @@ -0,0 +1,34 @@
>>> >> >> +Battery Characteristics
>>> >> >> +
>>> >> >> +Required Properties:
>>> >> >> + - compatible: Must be "fixed-battery"
>>> >> >> +
>>> >> >> +Optional Properties:
>>> >> >> + - termination-microvolt: dead battery voltage
>>> >> >> + - design-microwatt-hours: battery design energy
>>> >> >> + - design-microamp-hours: battery design capacity
>>> >> >
>>> >> > Also I suspect the members of struct power_supply_battery_info should
>>> >> > use the same names as appear in dts:
>>> >>
>>> >> These are internal and we can't be thinking just device tree.. ACPI
>>> >> and platform data is also an option.
>>> >
>>> > well platform data just uses the struct. ACPI is probably not
>>> > relevant, since in ACPI world one usually has smart batteries.
>>> > But yes, the names can be different.
>>> >
>>> > The important part is, that the API is used correctly, so it
>>> > should be clear what each property is used for. For example
>>> > termination_microvolt is not clear: Is this charge termination
>>> > voltage or system shutdown voltage? Also we do not need to add
>>> > "micro", since the power-supply subsystem always uses micro
>>> > based units.
>>>
>>> It's the min battery voltage. So...
>>> drained_voltage, depleted_voltage, sapped_voltage, spent_voltage...
>>
>> This is often called EOD (end of discharge) voltage, so let's use
>> end_of_discharge_voltage.
>
> ...

Actually, let's use names from POWER_SUPPLY_PROP_*, as DT inputs may
be visible in sysfs, e.g. *_CHARGE_FULL_DESIGN. This would require new
enum elements in future for the BQ24190 properties I mentioned
up-thread.

That looks like:

Optional Properties:
   - voltage-min-design-microvolt: dead battery voltage
   - energy-full-design-microwatt-hours: battery design energy
   - charge-full-design-microamp-hours: battery design capacity

 struct power_supply_battery_info {
    int voltage_min_design_uv;   /* microVolts */
    int energy_full_design_uwh;  /* microWatt-hours */
    int charge_full_design_uah;  /* microAmp-hours */
 };

Default value for unset DT properties will be -EINVAL, since 0 is
valid for *_full_design_* on the chips.

~.~
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2946 bytes --]

On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote:
> I think Matt meant the following :-)
> 
> Note: nominal-microvolt is not the correct term for termination voltage.
> Changed to termination-microvolt

That's right.

> Documentation on battery properties that can be defined for
> fine tuning fuel gauge state machines.

Maybe:

Documentation for batteries, that cannot identify themself. The
information is required by fuel-gauge and charger chips for
proper handling of the battery.

> From: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> ---
>  .../devicetree/bindings/power/supply/battery.txt   | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> new file mode 100644
> index 000000000000..398b4d622883
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -0,0 +1,34 @@
> +Battery Characteristics
> +
> +Required Properties:
> + - compatible: Must be "fixed-battery"
> +
> +Optional Properties:
> + - termination-microvolt: dead battery voltage

I think this is not named optimally, since it's not
clear if its related to charging.

> + - design-microwatt-hours: battery design energy
> + - design-microamp-hours: battery design capacity
> +
> +Batteries must be referenced by chargers and/or fuel-gauges
> +using a phandle. The phandle's property should be named
> +"monitored-battery".
> +
> +Example:
> +
> +	bat: battery {
> +		compatible = "fixed-battery";
> +		terminate-microvolt = <3700000>;
> +		design-microwatt-hours = <5290000>;
> +		design-microamp-hours = <1430000>;
> +	};
> +
> +	charger: charger@0 {
> +		....
> +		monitored-battery = <&bat>;
> +		...
> +	};
> +
> +	fuel_gauge: fuel_gauge@0 {
> +		....
> +		monitored-battery = <&bat>;
> +		...
> +	};

The charger stuff does does not integrate well with
pre-existing support for power-supplies property
described in
Documentation/devicetree/bindings/power/supply/power_supply.txt

I think the proper chain would be:

bat: battery {
	compatible = "fixed-battery";
	terminate-microvolt = <3700000>;
	design-microwatt-hours = <5290000>;
	design-microamp-hours = <1430000>;
};

fuel_gauge: fuel_gauge@0 {
	...
	monitored-battery = <&bat>;
    power-supplies = <&charger>;
	...
};

charger: charger@0 {
	...
};

I added the power-supplies node to the fuel-gauge instead of the battery,
since fuel-gauge + fixed-battery is basically a smart battery.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre@kernel.org> wrote:
> On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote:
...
>> +Batteries must be referenced by chargers and/or fuel-gauges
>> +using a phandle. The phandle's property should be named
>> +"monitored-battery".
>> +
>> +Example:
>> +
>> +     bat: battery {
>> +             compatible = "fixed-battery";
>> +             terminate-microvolt = <3700000>;
>> +             design-microwatt-hours = <5290000>;
>> +             design-microamp-hours = <1430000>;
>> +     };
>> +
>> +     charger: charger@0 {
>> +             ....
>> +             monitored-battery = <&bat>;
>> +             ...
>> +     };
>> +
>> +     fuel_gauge: fuel_gauge@0 {
>> +             ....
>> +             monitored-battery = <&bat>;
>> +             ...
>> +     };
>
> The charger stuff does does not integrate well with
> pre-existing support for power-supplies property
> described in
> Documentation/devicetree/bindings/power/supply/power_supply.txt
>
> I think the proper chain would be:
>
> bat: battery {
>         compatible = "fixed-battery";
>         terminate-microvolt = <3700000>;
>         design-microwatt-hours = <5290000>;
>         design-microamp-hours = <1430000>;
> };
>
> fuel_gauge: fuel_gauge@0 {
>         ...
>         monitored-battery = <&bat>;
>         power-supplies = <&charger>;
>         ...
> };
>
> charger: charger@0 {
>         ...
> };
>
> I added the power-supplies node to the fuel-gauge instead of the battery,
> since fuel-gauge + fixed-battery is basically a smart battery.

BQ24190 charger will need other params from a battery node. How does
it get them in this scenario?

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2121 bytes --]

Hi,

On Sun, Jan 29, 2017 at 03:32:10PM -0800, Liam Breck wrote:
> On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote:
> ...
> >> +Batteries must be referenced by chargers and/or fuel-gauges
> >> +using a phandle. The phandle's property should be named
> >> +"monitored-battery".
> >> +
> >> +Example:
> >> +
> >> +     bat: battery {
> >> +             compatible = "fixed-battery";
> >> +             terminate-microvolt = <3700000>;
> >> +             design-microwatt-hours = <5290000>;
> >> +             design-microamp-hours = <1430000>;
> >> +     };
> >> +
> >> +     charger: charger@0 {
> >> +             ....
> >> +             monitored-battery = <&bat>;
> >> +             ...
> >> +     };
> >> +
> >> +     fuel_gauge: fuel_gauge@0 {
> >> +             ....
> >> +             monitored-battery = <&bat>;
> >> +             ...
> >> +     };
> >
> > The charger stuff does does not integrate well with
> > pre-existing support for power-supplies property
> > described in
> > Documentation/devicetree/bindings/power/supply/power_supply.txt
> >
> > I think the proper chain would be:
> >
> > bat: battery {
> >         compatible = "fixed-battery";
> >         terminate-microvolt = <3700000>;
> >         design-microwatt-hours = <5290000>;
> >         design-microamp-hours = <1430000>;
> > };
> >
> > fuel_gauge: fuel_gauge@0 {
> >         ...
> >         monitored-battery = <&bat>;
> >         power-supplies = <&charger>;
> >         ...
> > };
> >
> > charger: charger@0 {
> >         ...
> > };
> >
> > I added the power-supplies node to the fuel-gauge instead of the battery,
> > since fuel-gauge + fixed-battery is basically a smart battery.
> 
> BQ24190 charger will need other params from a battery node. How does
> it get them in this scenario?

It should be able to get all required properties from the bq27xxx.
Otherwise supporting smart batteries is impossible. What properties
do you need?

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Sun, Jan 29, 2017 at 6:39 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Sun, Jan 29, 2017 at 03:32:10PM -0800, Liam Breck wrote:
>> On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre@kernel.org> wrote:
>> > On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote:
>> ...
>> >> +Batteries must be referenced by chargers and/or fuel-gauges
>> >> +using a phandle. The phandle's property should be named
>> >> +"monitored-battery".
>> >> +
>> >> +Example:
>> >> +
>> >> +     bat: battery {
>> >> +             compatible = "fixed-battery";
>> >> +             terminate-microvolt = <3700000>;
>> >> +             design-microwatt-hours = <5290000>;
>> >> +             design-microamp-hours = <1430000>;
>> >> +     };
>> >> +
>> >> +     charger: charger@0 {
>> >> +             ....
>> >> +             monitored-battery = <&bat>;
>> >> +             ...
>> >> +     };
>> >> +
>> >> +     fuel_gauge: fuel_gauge@0 {
>> >> +             ....
>> >> +             monitored-battery = <&bat>;
>> >> +             ...
>> >> +     };
>> >
>> > The charger stuff does does not integrate well with
>> > pre-existing support for power-supplies property
>> > described in
>> > Documentation/devicetree/bindings/power/supply/power_supply.txt
>> >
>> > I think the proper chain would be:
>> >
>> > bat: battery {
>> >         compatible = "fixed-battery";
>> >         terminate-microvolt = <3700000>;
>> >         design-microwatt-hours = <5290000>;
>> >         design-microamp-hours = <1430000>;
>> > };
>> >
>> > fuel_gauge: fuel_gauge@0 {
>> >         ...
>> >         monitored-battery = <&bat>;
>> >         power-supplies = <&charger>;
>> >         ...
>> > };
>> >
>> > charger: charger@0 {
>> >         ...
>> > };
>> >
>> > I added the power-supplies node to the fuel-gauge instead of the battery,
>> > since fuel-gauge + fixed-battery is basically a smart battery.
>>
>> BQ24190 charger will need other params from a battery node. How does
>> it get them in this scenario?
>
> It should be able to get all required properties from the bq27xxx.
> Otherwise supporting smart batteries is impossible. What properties
> do you need?

These two, at a minimum:

- precharge-microamps: maximum charge current during precharge
  phase (typically 20% of battery capacity)

- termination-microamps: a charge cycle terminates when the
  battery voltage is above recharge threshold, and the current is below
  this setting (typically 10% of battery capacity)

Sorry, I'm not seeing how the charger will obtain these from the fuel
gauge. Could you clarify with a code snippet?

Re: [PATCH v4 2/8] devicetree: power: add battery state machine documentation

[Liam Breck]

On Sun, Jan 29, 2017 at 6:46 PM, Liam Breck <liam@networkimprov.net> wrote:
> On Sun, Jan 29, 2017 at 6:39 PM, Sebastian Reichel <sre@kernel.org> wrote:
>> Hi,
>>
>> On Sun, Jan 29, 2017 at 03:32:10PM -0800, Liam Breck wrote:
>>> On Sun, Jan 29, 2017 at 10:06 AM, Sebastian Reichel <sre@kernel.org> wrote:
>>> > On Sun, Jan 22, 2017 at 02:22:12PM -0800, Liam Breck wrote:
>>> ...
>>> >> +Batteries must be referenced by chargers and/or fuel-gauges
>>> >> +using a phandle. The phandle's property should be named
>>> >> +"monitored-battery".
>>> >> +
>>> >> +Example:
>>> >> +
>>> >> +     bat: battery {
>>> >> +             compatible = "fixed-battery";
>>> >> +             terminate-microvolt = <3700000>;
>>> >> +             design-microwatt-hours = <5290000>;
>>> >> +             design-microamp-hours = <1430000>;
>>> >> +     };
>>> >> +
>>> >> +     charger: charger@0 {
>>> >> +             ....
>>> >> +             monitored-battery = <&bat>;
>>> >> +             ...
>>> >> +     };
>>> >> +
>>> >> +     fuel_gauge: fuel_gauge@0 {
>>> >> +             ....
>>> >> +             monitored-battery = <&bat>;
>>> >> +             ...
>>> >> +     };
>>> >
>>> > The charger stuff does does not integrate well with
>>> > pre-existing support for power-supplies property
>>> > described in
>>> > Documentation/devicetree/bindings/power/supply/power_supply.txt
>>> >
>>> > I think the proper chain would be:
>>> >
>>> > bat: battery {
>>> >         compatible = "fixed-battery";
>>> >         terminate-microvolt = <3700000>;
>>> >         design-microwatt-hours = <5290000>;
>>> >         design-microamp-hours = <1430000>;
>>> > };
>>> >
>>> > fuel_gauge: fuel_gauge@0 {
>>> >         ...
>>> >         monitored-battery = <&bat>;
>>> >         power-supplies = <&charger>;
>>> >         ...
>>> > };
>>> >
>>> > charger: charger@0 {
>>> >         ...
>>> > };
>>> >
>>> > I added the power-supplies node to the fuel-gauge instead of the battery,
>>> > since fuel-gauge + fixed-battery is basically a smart battery.
>>>
>>> BQ24190 charger will need other params from a battery node. How does
>>> it get them in this scenario?
>>
>> It should be able to get all required properties from the bq27xxx.
>> Otherwise supporting smart batteries is impossible. What properties
>> do you need?
>
> These two, at a minimum:
>
> - precharge-microamps: maximum charge current during precharge
>   phase (typically 20% of battery capacity)
>
> - termination-microamps: a charge cycle terminates when the
>   battery voltage is above recharge threshold, and the current is below
>   this setting (typically 10% of battery capacity)

The relevant docs, see section 9.5.1.4
http://www.ti.com/lit/ds/symlink/bq24190.pdf#31

BQ27xxx doesn't know about these parameters, but they are
characteristics of a fixed-battery, since the values are based on its
capacity. Smart batteries may have a circuit to limit current during
these phases. The BQ24190 charger is intended for dumb batteries :-)

So I believe letting charger and fuel-gauge both reference
monitored-battery is the right scheme.

~.~

[PATCH v4 3/8] power: power_supply: add battery information struct

[Matt Ranostay]

Add power_supply_battery_info structure that is used to
enumerate battery state machine settings.

Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
---
 include/linux/power_supply.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 3965503315ef..59674a257d5e 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -288,6 +288,18 @@ struct power_supply_info {
 	int use_for_apm;
 };
 
+/*
+ * This is the recommended struct to specify static battery parameters.
+ * Power supply class itself doesn't use this, but it implements access
+ * that most platform drivers should use for consistency.
+ */
+
+struct power_supply_battery_info {
+	int energy;		/* uWh */
+	int power;		/* uAh */
+	int nominal_voltage;	/* uV */
+};
+
 extern struct atomic_notifier_head power_supply_notifier;
 extern int power_supply_reg_notifier(struct notifier_block *nb);
 extern void power_supply_unreg_notifier(struct notifier_block *nb);
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/8] power: power_supply: add battery information struct

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

Hi,

On Sat, Jan 21, 2017 at 11:13:59PM -0800, Matt Ranostay wrote:
> Add power_supply_battery_info structure that is used to
> enumerate battery state machine settings.
> 
> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> ---
>  include/linux/power_supply.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 3965503315ef..59674a257d5e 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -288,6 +288,18 @@ struct power_supply_info {
>  	int use_for_apm;
>  };
>  
> +/*
> + * This is the recommended struct to specify static battery parameters.
> + * Power supply class itself doesn't use this, but it implements access
> + * that most platform drivers should use for consistency.
> + */
> +
> +struct power_supply_battery_info {
> +	int energy;		/* uWh */
> +	int power;		/* uAh */
> +	int nominal_voltage;	/* uV */
> +};
> +
>  extern struct atomic_notifier_head power_supply_notifier;
>  extern int power_supply_reg_notifier(struct notifier_block *nb);
>  extern void power_supply_unreg_notifier(struct notifier_block *nb);

You can merge this into the next patch, which adds the API
for using this struct.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 4/8] power: power_supply: add battery info platform data retrieval

[Matt Ranostay]

Add power_supply_get_battery_info() to get battery platform data

Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
---
 drivers/power/supply/power_supply_core.c | 41 ++++++++++++++++++++++++++++++++
 include/linux/power_supply.h             |  4 ++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 1e0960b646e8..21490b5e233f 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -17,6 +17,7 @@
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/err.h>
+#include <linux/of.h>
 #include <linux/power_supply.h>
 #include <linux/thermal.h>
 #include "power_supply.h"
@@ -487,6 +488,46 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
 EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
 #endif /* CONFIG_OF */
 
+int power_supply_get_battery_info(struct power_supply *psy,
+				  struct power_supply_battery_info *info,
+				  const char *property)
+{
+	struct device_node *np = psy->of_node;
+	struct device_node *power_supply_battery_info_np;
+	struct property *p;
+	char *prefix = "fixed-battery";
+	int ret;
+
+	if (!np)
+		return -ENXIO;
+
+	power_supply_battery_info_np = of_parse_phandle(np, property, 0);
+	if (!power_supply_battery_info_np)
+		return -ENODEV;
+
+	p = of_find_property(power_supply_battery_info_np, "compatible", NULL);
+
+	if (!p || !p->value)
+		return -ENODEV;
+
+	if (strncmp(prefix, p->value, strlen(prefix)))
+		return -ENODEV;
+
+	ret = of_property_read_u32(power_supply_battery_info_np,
+				   "nominal-microvolt", &info->nominal_voltage);
+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u32(power_supply_battery_info_np,
+				   "design-microwatt-hours", &info->energy);
+	if (ret < 0)
+		return ret;
+
+	return of_property_read_u32(power_supply_battery_info_np,
+				   "design-microamp-hours", &info->power);
+}
+EXPORT_SYMBOL_GPL(power_supply_get_battery_info);
+
 int power_supply_get_property(struct power_supply *psy,
 			    enum power_supply_property psp,
 			    union power_supply_propval *val)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 59674a257d5e..859d0e7f9b6a 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -318,6 +318,10 @@ static inline struct power_supply *
 devm_power_supply_get_by_phandle(struct device *dev, const char *property)
 { return NULL; }
 #endif /* CONFIG_OF */
+
+extern int power_supply_get_battery_info(struct power_supply *psy,
+					 struct power_supply_battery_info *info,
+					 const char *property);
 extern void power_supply_changed(struct power_supply *psy);
 extern int power_supply_am_i_supplied(struct power_supply *psy);
 extern int power_supply_set_battery_charged(struct power_supply *psy);
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/8] power: power_supply: add battery info platform data retrieval

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1837 bytes --]

Hi,

On Sat, Jan 21, 2017 at 11:14:00PM -0800, Matt Ranostay wrote:
> Add power_supply_get_battery_info() to get battery platform data
> 
> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> ---
>  drivers/power/supply/power_supply_core.c | 41 ++++++++++++++++++++++++++++++++
>  include/linux/power_supply.h             |  4 ++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 1e0960b646e8..21490b5e233f 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -17,6 +17,7 @@
>  #include <linux/device.h>
>  #include <linux/notifier.h>
>  #include <linux/err.h>
> +#include <linux/of.h>
>  #include <linux/power_supply.h>
>  #include <linux/thermal.h>
>  #include "power_supply.h"
> @@ -487,6 +488,46 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
>  EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
>  #endif /* CONFIG_OF */
>  
> +int power_supply_get_battery_info(struct power_supply *psy,
> +				  struct power_supply_battery_info *info,
> +				  const char *property)
> +{
> +	struct device_node *np = psy->of_node;
> +	struct device_node *power_supply_battery_info_np;
> +	struct property *p;
> +	char *prefix = "fixed-battery";
> +	int ret;
> +
> +	if (!np)
> +		return -ENXIO;

I guess we can print a warning here, that fixed-battery is
currently only supported for DT.

> +	power_supply_battery_info_np = of_parse_phandle(np, property, 0);
> +	if (!power_supply_battery_info_np)
> +		return -ENODEV;

Let's enforce the "monitored-battery" name and remove the property
name as parameter. Iff we need another name, it could be added later.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 5/8] power: bq27xxx_battery: add BQ27425 chip id

[Matt Ranostay]

Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
---
 drivers/power/supply/bq27xxx_battery.c     | 23 ++++++++++++++++++++++-
 drivers/power/supply/bq27xxx_battery_i2c.c |  2 +-
 include/linux/power/bq27xxx_battery.h      |  3 ++-
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 398801a21b86..12ecea308186 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -417,6 +417,25 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
 		[BQ27XXX_REG_DCAP] = 0x3c,
 		[BQ27XXX_REG_AP] = 0x18,
 	},
+	[BQ27425] = {
+		[BQ27XXX_REG_CTRL] = 0x00,
+		[BQ27XXX_REG_TEMP] = 0x02,
+		[BQ27XXX_REG_INT_TEMP] = 0x1e,
+		[BQ27XXX_REG_VOLT] = 0x04,
+		[BQ27XXX_REG_AI] = 0x10,
+		[BQ27XXX_REG_FLAGS] = 0x06,
+		[BQ27XXX_REG_TTE] = INVALID_REG_ADDR,
+		[BQ27XXX_REG_TTF] = INVALID_REG_ADDR,
+		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
+		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
+		[BQ27XXX_REG_NAC] = 0x08,
+		[BQ27XXX_REG_FCC] = 0x0e,
+		[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
+		[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
+		[BQ27XXX_REG_SOC] = 0x1c,
+		[BQ27XXX_REG_DCAP] = 0x3c,
+		[BQ27XXX_REG_AP] = 0x18,
+	},
 };
 
 static enum power_supply_property bq27000_battery_props[] = {
@@ -752,6 +771,7 @@ static struct {
 	BQ27XXX_PROP(BQ27541, bq27541_battery_props),
 	BQ27XXX_PROP(BQ27545, bq27545_battery_props),
 	BQ27XXX_PROP(BQ27421, bq27421_battery_props),
+	BQ27XXX_PROP(BQ27425, bq27421_battery_props),
 };
 
 static DEFINE_MUTEX(bq27xxx_list_lock);
@@ -1015,6 +1035,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
 		return flags & (BQ27XXX_FLAG_OTC | BQ27XXX_FLAG_OTD);
 	case BQ27530:
 	case BQ27421:
+	case BQ27425:
 		return flags & BQ27XXX_FLAG_OT;
 	default:
 		return false;
@@ -1026,7 +1047,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
  */
 static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
 {
-	if (di->chip == BQ27530 || di->chip == BQ27421)
+	if (di->chip == BQ27530 || di->chip == BQ27421 || di->chip == BQ27425)
 		return flags & BQ27XXX_FLAG_UT;
 
 	return false;
diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index c68fbc3fe50a..2ea2d0b06948 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -167,9 +167,9 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
 	{ "bq27742", BQ27541 },
 	{ "bq27545", BQ27545 },
 	{ "bq27421", BQ27421 },
-	{ "bq27425", BQ27421 },
 	{ "bq27441", BQ27421 },
 	{ "bq27621", BQ27421 },
+	{ "bq27425", BQ27425 },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index b312bcef53da..3f265dbf11af 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -17,7 +17,8 @@ enum bq27xxx_chip {
 	BQ27530, /* bq27530, bq27531 */
 	BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
 	BQ27545, /* bq27545 */
-	BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+	BQ27421, /* bq27421, bq27441, bq27621 */
+	BQ27425, /* bq27425 */
 };
 
 /**
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/8] power: bq27xxx_battery: add BQ27425 chip id

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3875 bytes --]

Hi,

I queued this into power-supply's for-next branch. I fixed up
the patch subject & description:

power: supply: bq27xxx: add BQ27425 chip id
    
This fixes over- & undertemperature detection of BQ27425.

-- Sebastian

On Sat, Jan 21, 2017 at 11:14:01PM -0800, Matt Ranostay wrote:
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
>  drivers/power/supply/bq27xxx_battery.c     | 23 ++++++++++++++++++++++-
>  drivers/power/supply/bq27xxx_battery_i2c.c |  2 +-
>  include/linux/power/bq27xxx_battery.h      |  3 ++-
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 398801a21b86..12ecea308186 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -417,6 +417,25 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>  		[BQ27XXX_REG_DCAP] = 0x3c,
>  		[BQ27XXX_REG_AP] = 0x18,
>  	},
> +	[BQ27425] = {
> +		[BQ27XXX_REG_CTRL] = 0x00,
> +		[BQ27XXX_REG_TEMP] = 0x02,
> +		[BQ27XXX_REG_INT_TEMP] = 0x1e,
> +		[BQ27XXX_REG_VOLT] = 0x04,
> +		[BQ27XXX_REG_AI] = 0x10,
> +		[BQ27XXX_REG_FLAGS] = 0x06,
> +		[BQ27XXX_REG_TTE] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_TTF] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_NAC] = 0x08,
> +		[BQ27XXX_REG_FCC] = 0x0e,
> +		[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_AE] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_SOC] = 0x1c,
> +		[BQ27XXX_REG_DCAP] = 0x3c,
> +		[BQ27XXX_REG_AP] = 0x18,
> +	},
>  };
>  
>  static enum power_supply_property bq27000_battery_props[] = {
> @@ -752,6 +771,7 @@ static struct {
>  	BQ27XXX_PROP(BQ27541, bq27541_battery_props),
>  	BQ27XXX_PROP(BQ27545, bq27545_battery_props),
>  	BQ27XXX_PROP(BQ27421, bq27421_battery_props),
> +	BQ27XXX_PROP(BQ27425, bq27421_battery_props),
>  };
>  
>  static DEFINE_MUTEX(bq27xxx_list_lock);
> @@ -1015,6 +1035,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
>  		return flags & (BQ27XXX_FLAG_OTC | BQ27XXX_FLAG_OTD);
>  	case BQ27530:
>  	case BQ27421:
> +	case BQ27425:
>  		return flags & BQ27XXX_FLAG_OT;
>  	default:
>  		return false;
> @@ -1026,7 +1047,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
>   */
>  static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
>  {
> -	if (di->chip == BQ27530 || di->chip == BQ27421)
> +	if (di->chip == BQ27530 || di->chip == BQ27421 || di->chip == BQ27425)
>  		return flags & BQ27XXX_FLAG_UT;
>  
>  	return false;
> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
> index c68fbc3fe50a..2ea2d0b06948 100644
> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
> @@ -167,9 +167,9 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>  	{ "bq27742", BQ27541 },
>  	{ "bq27545", BQ27545 },
>  	{ "bq27421", BQ27421 },
> -	{ "bq27425", BQ27421 },
>  	{ "bq27441", BQ27421 },
>  	{ "bq27621", BQ27421 },
> +	{ "bq27425", BQ27425 },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table);
> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
> index b312bcef53da..3f265dbf11af 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -17,7 +17,8 @@ enum bq27xxx_chip {
>  	BQ27530, /* bq27530, bq27531 */
>  	BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
>  	BQ27545, /* bq27545 */
> -	BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
> +	BQ27421, /* bq27421, bq27441, bq27621 */
> +	BQ27425, /* bq27425 */
>  };
>  
>  /**
> -- 
> 2.10.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 5/8] power: bq27xxx_battery: add BQ27425 chip id

[Liam Breck]

On Sun, 2017-01-29 at 15:35:12, Sebastian Reichel wrote:
>I queued this into power-supply's for-next branch. I fixed up
>the patch subject & description:
>
>power: supply: bq27xxx: add BQ27425 chip id
>    
>This fixes over- & undertemperature detection of BQ27425.

This patch is not a bugfix, it just creates a chip id for use later in
patchset. It probably should go in with the rest of the patches.

Should we roll it into the main bq27xxx_battery.c patch?

Also there is a cosmetic flaw in it.

Thanks

[PATCH v4 7/8] devicetree: power: bq27xxx: add monitored battery documentation

[Matt Ranostay]

Depends-On: http://marc.info/?l=linux-pm&m=148392292830015&w=2
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
---
 Documentation/devicetree/bindings/power/supply/bq27xxx.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
index b0c95ef63e68..0472a2db0f13 100644
--- a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
@@ -28,9 +28,17 @@ Required properties:
  * "ti,bq27621" - BQ27621
 - reg: integer, i2c address of the device.
 
+Optional properties:
+- monitored-battery: phandle of battery information devicetree node
+
+  See Documentation/devicetree/bindings/power/supply/battery.txt
+  If any of the referenced battery properties are set, then all must be.
+
 Example:
 
 bq27510g3 {
     compatible = "ti,bq27510g3";
     reg = <0x55>;
+
+    monitored-battery = <&bat>;
 };
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 6/8] power: bq27xxx_battery: add i2c bulk read/write functions

[Matt Ranostay]

Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
 drivers/power/supply/bq27xxx_battery_i2c.c | 62 ++++++++++++++++++++++++++++++
 include/linux/power/bq27xxx_battery.h      |  4 ++
 2 files changed, 66 insertions(+)

diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
index 2ea2d0b06948..feeb04ed88c8 100644
--- a/drivers/power/supply/bq27xxx_battery_i2c.c
+++ b/drivers/power/supply/bq27xxx_battery_i2c.c
@@ -68,6 +68,64 @@ static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
 	return ret;
 }
 
+static int bq27xxx_battery_i2c_write(struct bq27xxx_device_info *di, u8 reg,
+				     int value, bool single)
+{
+	struct i2c_client *client = to_i2c_client(di->dev);
+	struct i2c_msg msg;
+	unsigned char data[4];
+
+	if (!client->adapter)
+		return -ENODEV;
+
+	data[0] = reg;
+	if (single) {
+		data[1] = (unsigned char) value;
+		msg.len = 2;
+	} else {
+		put_unaligned_le16(value, &data[1]);
+		msg.len = 3;
+	}
+
+	msg.buf = data;
+	msg.addr = client->addr;
+	msg.flags = 0;
+
+	return i2c_transfer(client->adapter, &msg, 1) == 1 ? 0 : -EINVAL;
+}
+
+static int bq27xxx_battery_i2c_bulk_read(struct bq27xxx_device_info *di, u8 reg,
+					 u8 *data, int len)
+{
+	struct i2c_client *client = to_i2c_client(di->dev);
+
+	if (!client->adapter)
+		return -ENODEV;
+
+	return i2c_smbus_read_i2c_block_data(client, reg, len, data);
+}
+
+static int bq27xxx_battery_i2c_bulk_write(struct bq27xxx_device_info *di,
+					  u8 reg, u8 *data, int len)
+{
+	struct i2c_client *client = to_i2c_client(di->dev);
+	struct i2c_msg msg;
+	u8 buf[33];
+
+	if (!client->adapter)
+		return -ENODEV;
+
+	buf[0] = reg;
+	memcpy(&buf[1], data, len);
+
+	msg.buf = buf;
+	msg.addr = client->addr;
+	msg.flags = 0;
+	msg.len = len + 1;
+
+	return i2c_transfer(client->adapter, &msg, 1) == 1 ? 0 : -EINVAL;
+}
+
 static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
 				     const struct i2c_device_id *id)
 {
@@ -95,7 +153,11 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
 	di->dev = &client->dev;
 	di->chip = id->driver_data;
 	di->name = name;
+
 	di->bus.read = bq27xxx_battery_i2c_read;
+	di->bus.write = bq27xxx_battery_i2c_write;
+	di->bus.read_bulk = bq27xxx_battery_i2c_bulk_read;
+	di->bus.write_bulk = bq27xxx_battery_i2c_bulk_write;
 
 	ret = bq27xxx_battery_setup(di);
 	if (ret)
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index 3f265dbf11af..581402380d6e 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -41,6 +41,9 @@ struct bq27xxx_platform_data {
 struct bq27xxx_device_info;
 struct bq27xxx_access_methods {
 	int (*read)(struct bq27xxx_device_info *di, u8 reg, bool single);
+	int (*write)(struct bq27xxx_device_info *di, u8 reg, int value, bool single);
+	int (*read_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
+	int (*write_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
 };
 
 struct bq27xxx_reg_cache {
@@ -71,6 +74,7 @@ struct bq27xxx_device_info {
 	struct list_head list;
 	struct mutex lock;
 	u8 *regs;
+	u8 buffer[32];
 };
 
 void bq27xxx_battery_update(struct bq27xxx_device_info *di);
-- 
2.10.2

Re: [PATCH v4 6/8] power: bq27xxx_battery: add i2c bulk read/write functions

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3938 bytes --]

Hi,

On Sat, Jan 21, 2017 at 11:14:02PM -0800, Matt Ranostay wrote:
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
>  drivers/power/supply/bq27xxx_battery_i2c.c | 62 ++++++++++++++++++++++++++++++
>  include/linux/power/bq27xxx_battery.h      |  4 ++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
> index 2ea2d0b06948..feeb04ed88c8 100644
> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
> @@ -68,6 +68,64 @@ static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
>  	return ret;
>  }
>  
> +static int bq27xxx_battery_i2c_write(struct bq27xxx_device_info *di, u8 reg,
> +				     int value, bool single)
> +{
> +	struct i2c_client *client = to_i2c_client(di->dev);
> +	struct i2c_msg msg;
> +	unsigned char data[4];
> +
> +	if (!client->adapter)
> +		return -ENODEV;
> +
> +	data[0] = reg;
> +	if (single) {
> +		data[1] = (unsigned char) value;
> +		msg.len = 2;
> +	} else {
> +		put_unaligned_le16(value, &data[1]);
> +		msg.len = 3;
> +	}
> +
> +	msg.buf = data;
> +	msg.addr = client->addr;
> +	msg.flags = 0;
> +
> +	return i2c_transfer(client->adapter, &msg, 1) == 1 ? 0 : -EINVAL;
> +}
> +
> +static int bq27xxx_battery_i2c_bulk_read(struct bq27xxx_device_info *di, u8 reg,
> +					 u8 *data, int len)
> +{
> +	struct i2c_client *client = to_i2c_client(di->dev);
> +
> +	if (!client->adapter)
> +		return -ENODEV;
> +
> +	return i2c_smbus_read_i2c_block_data(client, reg, len, data);
> +}
> +
> +static int bq27xxx_battery_i2c_bulk_write(struct bq27xxx_device_info *di,
> +					  u8 reg, u8 *data, int len)
> +{
> +	struct i2c_client *client = to_i2c_client(di->dev);
> +	struct i2c_msg msg;
> +	u8 buf[33];
> +
> +	if (!client->adapter)
> +		return -ENODEV;
> +
> +	buf[0] = reg;
> +	memcpy(&buf[1], data, len);
> +
> +	msg.buf = buf;
> +	msg.addr = client->addr;
> +	msg.flags = 0;
> +	msg.len = len + 1;
> +
> +	return i2c_transfer(client->adapter, &msg, 1) == 1 ? 0 : -EINVAL;
> +}
> +
>  static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>  				     const struct i2c_device_id *id)
>  {
> @@ -95,7 +153,11 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>  	di->dev = &client->dev;
>  	di->chip = id->driver_data;
>  	di->name = name;
> +
>  	di->bus.read = bq27xxx_battery_i2c_read;
> +	di->bus.write = bq27xxx_battery_i2c_write;
> +	di->bus.read_bulk = bq27xxx_battery_i2c_bulk_read;
> +	di->bus.write_bulk = bq27xxx_battery_i2c_bulk_write;
>  
>  	ret = bq27xxx_battery_setup(di);
>  	if (ret)
> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
> index 3f265dbf11af..581402380d6e 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -41,6 +41,9 @@ struct bq27xxx_platform_data {
>  struct bq27xxx_device_info;
>  struct bq27xxx_access_methods {
>  	int (*read)(struct bq27xxx_device_info *di, u8 reg, bool single);
> +	int (*write)(struct bq27xxx_device_info *di, u8 reg, int value, bool single);
> +	int (*read_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
> +	int (*write_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);

So I had a look at patch 8 and I think we should start with finally
converting bq27xxx to regmap API and probably a second regmap for
the block data stuff. That way you get all the debugging info in
debugfs and the driver looks much cleaner.

>  };
>  
>  struct bq27xxx_reg_cache {
> @@ -71,6 +74,7 @@ struct bq27xxx_device_info {
>  	struct list_head list;
>  	struct mutex lock;
>  	u8 *regs;
> +	u8 buffer[32];

unrelated change.

>  };
>  
>  void bq27xxx_battery_update(struct bq27xxx_device_info *di);

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 8/8] power: bq27xxx_battery: add initial state machine support

[Matt Ranostay]

Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
 drivers/power/supply/bq27xxx_battery.c | 230 ++++++++++++++++++++++++++++++++-
 1 file changed, 229 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 12ecea308186..5eb608535bb1 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -43,6 +43,7 @@
  * http://www.ti.com/product/bq27621-g1
  */
 
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -777,6 +778,46 @@ static struct {
 static DEFINE_MUTEX(bq27xxx_list_lock);
 static LIST_HEAD(bq27xxx_battery_devices);
 
+#define BQ27XXX_TERM_V_MIN	2800
+#define BQ27XXX_TERM_V_MAX	3700
+
+#define BQ27XXX_BLOCK_DATA_CLASS	0x3E
+#define BQ27XXX_DATA_BLOCK		0x3F
+#define BQ27XXX_BLOCK_DATA		0x40
+#define BQ27XXX_BLOCK_DATA_CHECKSUM	0x60
+#define BQ27XXX_BLOCK_DATA_CONTROL	0x61
+#define BQ27XXX_SET_CFGUPDATE		0x13
+#define BQ27XXX_SOFT_RESET		0x42
+
+enum bq27xxx_dm_subclass_index {
+	BQ27XXX_DM_DESIGN_CAP = 0,
+	BQ27XXX_DM_DESIGN_ENERGY,
+	BQ27XXX_DM_TERMINATE_VOLTAGE,
+	BQ27XXX_NUM_IDX,
+};
+
+struct bq27xxx_dm_regs {
+	unsigned int subclass_id;
+	unsigned int offset;
+	char *name;
+};
+
+#define BQ27XXX_GAS_GAUGING_STATE_SUBCLASS	82
+
+static struct bq27xxx_dm_regs bq27425_dm_subclass_regs[] = {
+	{ BQ27XXX_GAS_GAUGING_STATE_SUBCLASS, 12, "design-capacity" },
+	{ BQ27XXX_GAS_GAUGING_STATE_SUBCLASS, 14, "design-energy" },
+	{ BQ27XXX_GAS_GAUGING_STATE_SUBCLASS, 18, "terminate-voltage" },
+};
+
+static struct bq27xxx_dm_regs *bq27xxx_dm_subclass_regs[] = {
+	[BQ27425] = bq27425_dm_subclass_regs,
+};
+
+static unsigned int bq27xxx_unseal_keys[] = {
+	[BQ27425] = 0x04143672,
+};
+
 static int poll_interval_param_set(const char *val, const struct kernel_param *kp)
 {
 	struct bq27xxx_device_info *di;
@@ -821,6 +862,165 @@ static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
 	return di->bus.read(di, di->regs[reg_index], single);
 }
 
+static int bq27xxx_battery_set_seal_state(struct bq27xxx_device_info *di,
+					      bool state)
+{
+	unsigned int key = bq27xxx_unseal_keys[di->chip];
+	int ret;
+
+	if (state)
+		return di->bus.write(di, BQ27XXX_REG_CTRL, 0x20, false);
+
+	ret = di->bus.write(di, BQ27XXX_REG_CTRL, (key >> 16) & 0xffff, false);
+	if (ret < 0)
+		return ret;
+
+	return di->bus.write(di, BQ27XXX_REG_CTRL, key & 0xffff, false);
+}
+
+static int bq27xxx_battery_read_dm_block(struct bq27xxx_device_info *di,
+					     int subclass)
+{
+	int ret = di->bus.write(di, BQ27XXX_REG_CTRL, 0, false);
+
+	if (ret < 0)
+		return ret;
+
+	ret = di->bus.write(di, BQ27XXX_BLOCK_DATA_CONTROL, 0, true);
+	if (ret < 0)
+		return ret;
+
+	ret = di->bus.write(di, BQ27XXX_BLOCK_DATA_CLASS, subclass, true);
+	if (ret < 0)
+		return ret;
+
+	ret = di->bus.write(di, BQ27XXX_DATA_BLOCK, 0, true);
+	if (ret < 0)
+		return ret;
+
+	usleep_range(1000, 1500);
+
+	return di->bus.read_bulk(di, BQ27XXX_BLOCK_DATA,
+				(u8 *) &di->buffer, sizeof(di->buffer));
+}
+
+static int bq27xxx_battery_print_config(struct bq27xxx_device_info *di)
+{
+	struct bq27xxx_dm_regs *reg = bq27xxx_dm_subclass_regs[di->chip];
+	int ret, i;
+
+	ret = bq27xxx_battery_read_dm_block(di,
+			BQ27XXX_GAS_GAUGING_STATE_SUBCLASS);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < BQ27XXX_NUM_IDX; i++) {
+		int val;
+
+		if (reg->subclass_id != BQ27XXX_GAS_GAUGING_STATE_SUBCLASS)
+			continue;
+
+		val = be16_to_cpup((u16 *) &di->buffer[reg->offset]);
+
+		dev_info(di->dev, "settings for %s set at %d\n", reg->name, val);
+
+		reg++;
+	}
+
+	return 0;
+}
+
+static bool bq27xxx_battery_update_dm_setting(struct bq27xxx_device_info *di,
+			      unsigned int reg, unsigned int val)
+{
+	struct bq27xxx_dm_regs *dm_reg = &bq27xxx_dm_subclass_regs[di->chip][reg];
+	u16 *prev = (u16 *) &di->buffer[dm_reg->offset];
+
+	if (be16_to_cpup(prev) == val)
+		return false;
+
+	*prev = cpu_to_be16(val);
+
+	return true;
+}
+
+static u8 bq27xxx_battery_checksum(struct bq27xxx_device_info *di)
+{
+	u8 *data = (u8 *) &di->buffer;
+	u16 sum = 0;
+	int i;
+
+	for (i = 0; i < sizeof(di->buffer); i++) {
+		sum += data[i];
+		sum &= 0xff;
+	}
+
+	return 0xff - sum;
+}
+
+static int bq27xxx_battery_write_nvram(struct bq27xxx_device_info *di,
+					   unsigned int subclass)
+{
+	int ret;
+
+	ret = di->bus.write(di, BQ27XXX_REG_CTRL, BQ27XXX_SET_CFGUPDATE, false);
+	if (ret)
+		return ret;
+
+	ret = di->bus.write(di, BQ27XXX_BLOCK_DATA_CONTROL, 0, true);
+	if (ret)
+		return ret;
+
+	ret = di->bus.write(di, BQ27XXX_BLOCK_DATA_CLASS, subclass, true);
+	if (ret)
+		return ret;
+
+	ret = di->bus.write(di, BQ27XXX_DATA_BLOCK, 0, true);
+	if (ret)
+		return ret;
+
+	ret = di->bus.write_bulk(di, BQ27XXX_BLOCK_DATA,
+				(u8 *) &di->buffer, sizeof(di->buffer));
+	if (ret < 0)
+		return ret;
+
+	usleep_range(1000, 1500);
+
+	di->bus.write(di, BQ27XXX_BLOCK_DATA_CHECKSUM,
+				bq27xxx_battery_checksum(di), true);
+
+	usleep_range(1000, 1500);
+
+	di->bus.write(di, BQ27XXX_REG_CTRL, BQ27XXX_SOFT_RESET, false);
+
+	return 0;
+}
+
+static int bq27xxx_battery_set_config(struct bq27xxx_device_info *di,
+				      struct power_supply_battery_info *info)
+{
+	int ret = bq27xxx_battery_read_dm_block(di,
+				BQ27XXX_GAS_GAUGING_STATE_SUBCLASS);
+
+	if (ret < 0)
+		return ret;
+
+	ret  = bq27xxx_battery_update_dm_setting(di, BQ27XXX_DM_DESIGN_CAP,
+						 info->power / 1000);
+	ret |= bq27xxx_battery_update_dm_setting(di, BQ27XXX_DM_DESIGN_ENERGY,
+						 info->energy / 1000);
+	ret |= bq27xxx_battery_update_dm_setting(di, BQ27XXX_DM_TERMINATE_VOLTAGE,
+						 info->nominal_voltage / 1000);
+
+	if (ret) {
+		dev_info(di->dev, "updating NVM settings\n");
+		return bq27xxx_battery_write_nvram(di,
+				BQ27XXX_GAS_GAUGING_STATE_SUBCLASS);
+	}
+
+	return 0;
+}
+
 /*
  * Return the battery State-of-Charge
  * Or < 0 if something fails.
@@ -1090,6 +1290,30 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
 	return POWER_SUPPLY_HEALTH_GOOD;
 }
 
+void bq27xxx_battery_settings(struct bq27xxx_device_info *di)
+{
+	struct power_supply_battery_info info = {};
+
+	/* functions don't exist for writing data so abort */
+	if (!di->bus.write || !di->bus.write_bulk)
+		return;
+
+	/* no settings to be set for this chipset so abort */
+	if (!bq27xxx_dm_subclass_regs[di->chip])
+		return;
+
+	bq27xxx_battery_set_seal_state(di, false);
+
+	if (power_supply_get_battery_info(di->bat, &info, "monitored-battery") < 0)
+		goto out;
+
+	bq27xxx_battery_set_config(di, &info);
+
+out:
+	bq27xxx_battery_print_config(di);
+	bq27xxx_battery_set_seal_state(di, true);
+}
+
 void bq27xxx_battery_update(struct bq27xxx_device_info *di)
 {
 	struct bq27xxx_reg_cache cache = {0, };
@@ -1372,7 +1596,10 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
 int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 {
 	struct power_supply_desc *psy_desc;
-	struct power_supply_config psy_cfg = { .drv_data = di, };
+	struct power_supply_config psy_cfg;
+
+	psy_cfg.of_node = di->dev->of_node;
+	psy_cfg.drv_data = di;
 
 	INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
 	mutex_init(&di->lock);
@@ -1397,6 +1624,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 
 	dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
 
+	bq27xxx_battery_settings(di);
 	bq27xxx_battery_update(di);
 
 	mutex_lock(&bq27xxx_list_lock);
-- 
2.10.2

Re: [PATCH v4 6/8] power: bq27xxx_battery: add i2c bulk read/write functions

[Liam Breck]

On Sun, 2017-01-29 at 18:38:26, Sebastian Reichel wrote:
>> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
>> index 3f265dbf11af..581402380d6e 100644
>> --- a/include/linux/power/bq27xxx_battery.h
>> +++ b/include/linux/power/bq27xxx_battery.h
>> @@ -41,6 +41,9 @@ struct bq27xxx_platform_data {
>>  struct bq27xxx_device_info;
>>  struct bq27xxx_access_methods {
>>  	int (*read)(struct bq27xxx_device_info *di, u8 reg, bool single);
>> +	int (*write)(struct bq27xxx_device_info *di, u8 reg, int value, bool single);
>> +	int (*read_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
>> +	int (*write_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
>
>So I had a look at patch 8 and I think we should start with finally
>converting bq27xxx to regmap API and probably a second regmap for
>the block data stuff. That way you get all the debugging info in
>debugfs and the driver looks much cleaner.
										
Could we complete this patchset using the I2C api? We haven't added to it
extensively, and regmap is unrelated to the purpose of this patchset.

Also maybe the primary maintainers of BQ27xxx should be on the hook for
that api change :-)

Plus we're blocked here on devicetree support for BQ24190 waiting for
power_supply_battery_info.

Thanks

Re: [PATCH v4 6/8] power: bq27xxx_battery: add i2c bulk read/write functions

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hi,

On Sun, Jan 29, 2017 at 12:08:25PM -0800, Liam Breck wrote:
> On Sun, 2017-01-29 at 18:38:26, Sebastian Reichel wrote:
> >> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
> >> index 3f265dbf11af..581402380d6e 100644
> >> --- a/include/linux/power/bq27xxx_battery.h
> >> +++ b/include/linux/power/bq27xxx_battery.h
> >> @@ -41,6 +41,9 @@ struct bq27xxx_platform_data {
> >>  struct bq27xxx_device_info;
> >>  struct bq27xxx_access_methods {
> >>  	int (*read)(struct bq27xxx_device_info *di, u8 reg, bool single);
> >> +	int (*write)(struct bq27xxx_device_info *di, u8 reg, int value, bool single);
> >> +	int (*read_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
> >> +	int (*write_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
> >
> >So I had a look at patch 8 and I think we should start with finally
> >converting bq27xxx to regmap API and probably a second regmap for
> >the block data stuff. That way you get all the debugging info in
> >debugfs and the driver looks much cleaner.
> 										
> Could we complete this patchset using the I2C api? We haven't added to it
> extensively, and regmap is unrelated to the purpose of this patchset.

Previously we had only a read operation, which could be easily
converted to regmap, since it accesses a single register. Afterwards
we have read,write,bulk_read and bulk_write. That's 400% and 2/4
functions, which do not follow the regmap style.

So what is your definition of extensively? :)

> Also maybe the primary maintainers of BQ27xxx should be on the hook for
> that api change :-)

https://lkml.org/lkml/2015/9/23/542

The w1 stuff is not that hard to implement actually by just using
the existing functions and devm_regmap_init().

> Plus we're blocked here on devicetree support for BQ24190 waiting for
> power_supply_battery_info.

Well this patch depends on the power_supply_battery_info patch, not
the other way around. I will apply the power_supply_battery_info
patch once its ok. No need to wait for this patch. So not really a
problem for your BQ24190 work?

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 6/8] power: bq27xxx_battery: add i2c bulk read/write functions

[Liam Breck]

On Sun, Jan 29, 2017 at 2:22 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Sun, Jan 29, 2017 at 12:08:25PM -0800, Liam Breck wrote:
>> On Sun, 2017-01-29 at 18:38:26, Sebastian Reichel wrote:
>> >> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
>> >> index 3f265dbf11af..581402380d6e 100644
>> >> --- a/include/linux/power/bq27xxx_battery.h
>> >> +++ b/include/linux/power/bq27xxx_battery.h
>> >> @@ -41,6 +41,9 @@ struct bq27xxx_platform_data {
>> >>  struct bq27xxx_device_info;
>> >>  struct bq27xxx_access_methods {
>> >>    int (*read)(struct bq27xxx_device_info *di, u8 reg, bool single);
>> >> +  int (*write)(struct bq27xxx_device_info *di, u8 reg, int value, bool single);
>> >> +  int (*read_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
>> >> +  int (*write_bulk)(struct bq27xxx_device_info *di, u8 reg, u8 *data, int len);
>> >
>> >So I had a look at patch 8 and I think we should start with finally
>> >converting bq27xxx to regmap API and probably a second regmap for
>> >the block data stuff. That way you get all the debugging info in
>> >debugfs and the driver looks much cleaner.
>>
>> Could we complete this patchset using the I2C api? We haven't added to it
>> extensively, and regmap is unrelated to the purpose of this patchset.
>
> Previously we had only a read operation, which could be easily
> converted to regmap, since it accesses a single register. Afterwards
> we have read,write,bulk_read and bulk_write. That's 400% and 2/4
> functions, which do not follow the regmap style.
>
> So what is your definition of extensively? :)

We added all of 51 LOC, plus whitespace. Extensive is at least greater
than UCHAR_MAX :-)

>> Also maybe the primary maintainers of BQ27xxx should be on the hook for
>> that api change :-)
>
> https://lkml.org/lkml/2015/9/23/542
>
> The w1 stuff is not that hard to implement actually by just using
> the existing functions and devm_regmap_init().

We don't really have the bandwidth or budget for general driver
maintenance. There was no indication in the I2C code that it was
deprecated. Please ask TI to fund this, not us.

>> Plus we're blocked here on devicetree support for BQ24190 waiting for
>> power_supply_battery_info.
>
> Well this patch depends on the power_supply_battery_info patch, not
> the other way around. I will apply the power_supply_battery_info
> patch once its ok. No need to wait for this patch. So not really a
> problem for your BQ24190 work?

Usually features like that aren't added without a client?