power: supply: Add support for the Qualcomm Battery Monitoring System

Quoted 7 lines · Craig Tatlor

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then ammends the coulomb counter to that to increase the accuracy of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

Review

Quoted 8 lines

---
 drivers/power/supply/Kconfig    |   9 +
 drivers/power/supply/Makefile   |   1 +
 drivers/power/supply/qcom_bms.c | 500 ++++++++++++++++++++++++++++++++
 3 files changed, 510 insertions(+)
 create mode 100644 drivers/power/supply/qcom_bms.c

(quoted diff) +0 -0

b/drivers/power/supply/qcom_bms.c

drivers/power/supply/qcom_bms.c

Quoted 5 lines

new file mode 100644
index 000000000000..5aa6e906d1b9

drivers/power/supply/qcom_bms.c new file +0 -0

@@ -0,0 +1,500 @@

Hi,

drivers/power/supply/qcom_bms.c line 90

Quoted 3 lines

drivers/power/supply/qcom_bms.c new file +3 -0

+static unsigned int between(int left, int right, int val)
+{
+     if (left <= val && val >= right)

maybe double-check those if() conditions?
[or I need my morning coffee]

drivers/power/supply/qcom_bms.c line 94

Quoted 4 lines

drivers/power/supply/qcom_bms.c new file +4 -0

+             return 1;
+
+     return 0;
+}

thanks,
--
~Randy

Quoted 13 lines · Craig Tatlor

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then ammends the coulomb counter to that to increase the accuracy of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

---
 drivers/power/supply/Kconfig    |   9 +
 drivers/power/supply/Makefile   |   1 +
 drivers/power/supply/qcom_bms.c | 500 ++++++++++++++++++++++++++++++++
 3 files changed, 510 insertions(+)
 create mode 100644 drivers/power/supply/qcom_bms.c

drivers/power/supply/qcom_bms.c

Quoted 6 lines

drivers/power/supply/qcom_bms.c new file +0 -0

new file mode 100644
index 000000000000..5aa6e906d1b9
@@ -0,0 +1,500 @@

Hi,

drivers/power/supply/qcom_bms.c line 90

Quoted 4 lines

drivers/power/supply/qcom_bms.c new file +3 -0

+static unsigned int between(int left, int right, int val)
+{
+	if (left <= val && val >= right)

maybe double-check those if() conditions?
[or I need my morning coffee]

drivers/power/supply/qcom_bms.c line 94

Quoted 5 lines

drivers/power/supply/qcom_bms.c new file +4 -0

+		return 1;
+
+	return 0;
+}

thanks,

Signature · 1 line

~Randy

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---
 .../bindings/power/supply/qcom_bms.txt        | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +0 -0

new file mode 100644
index 000000000000..6296399edc09

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +65 -0

+			compatible = "qcom,spmi-vadc";
+			reg = <0x3100>;
+			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#io-channel-cells = <1>;
+
+			bat_temp {
+				reg = <VADC_LR_MUX1_BAT_THERM>;
+			};
+		};
+
+		bms@4000 {
+			compatible = "qcom,pm8941-bms";
+			reg = <0x4000>;
+			interrupts = <0x0 0x40 0x4 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "ocv_thr";
+
+			io-channels = <&pm8941_vadc VADC_LR_MUX1_BAT_THERM>;
+			io-channel-names = "temp";
+
+			qcom,fcc-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,fcc-lut = /bits/ 16 <6010 6070 6680 6780 6670>;
+
+			qcom,ocv-capacity-legend = /bits/ 8 <100 95 90 85
+							     80 75 70 65
+							     60 55 50 45
+							     40 35 30 25
+							     20 15 10 9
+							     8 7 6 5 4
+							     3 2 1 0>;
+
+			qcom,ocv-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,ocv-lut = /bits/ 16 <4288 4288 4306 4315 4315
+						  4261 4241 4259 4266 4246
+						  4201 4181 4201 4207 4187
+						  4153 4133 4150 4155 4135
+						  4105 4085 4100 4104 4084
+						  4058 4038 4052 4058 4038
+						  4012 3992 4004 4014 3994
+						  3970 3950 3959 3971 3951
+						  3931 3911 3915 3927 3907
+						  3899 3879 3880 3884 3864
+						  3873 3853 3851 3853 3833
+						  3848 3828 3827 3829 3809
+						  3829 3809 3808 3809 3789
+						  3815 3795 3791 3791 3771
+						  3801 3781 3775 3772 3752
+						  3785 3765 3751 3746 3726
+						  3767 3747 3727 3719 3699
+						  3750 3730 3702 3692 3672
+						  3728 3708 3680 3672 3652
+						  3720 3700 3676 3665 3645
+						  3712 3692 3670 3660 3645
+						  3695 3675 3658 3648 3633
+						  3662 3647 3629 3620 3610
+						  3620 3605 3589 3580 3570
+						  3562 3552 3538 3529 3519
+						  3490 3480 3474 3470 3465
+						  3403 3398 3388 3380 3375
+						  3320 3300 3255 3221 3206
+						  3000 3000 3000 3000 3000>;
+		};
+	};
+};

Signature · 1 line

2.17.0

Quoted 5 lines · Craig Tatlor

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---
 .../bindings/power/supply/qcom_bms.txt        | 93

+++++++++++++++++++

Quoted 2 lines

 1 file changed, 93 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 2 lines

diff --git

a/Documentation/devicetree/bindings/power/supply/qcom_bms.txt b/Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 3

Quoted 8 lines

new file mode 100644
index 000000000000..6296399edc09

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

@@ -0,0 +1,93 @@
+Qualcomm Battery Measurement System
+
+The Qualcomm Battery Measurement System is found inside of Qualcomm

PM8941

Is it Monitoring or Measurment?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 4

Quoted 1 line

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+PMICs. It provides OCV and coulomb counter registers that allow the

kernel

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 5

Quoted 1 line

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+to infer a capacity level.

s/kernel/OS/

OCV?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 5 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +4 -0

+
+Required properties:
+- compatible:               Should contain "qcom,pm8941-bms".
+- reg:                      Specifies the SPMI address and length of

the

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 3 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

+			    controller's registers.
+- interrupts:               OCV threshold interrupt.
+- io-channels:              Should contain IIO channel specifier for

the

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 3 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

+			    ADC channel that reports battery temperature.
+- io-channel-names:         Should contain "temp".
+- qcom,fcc-temp-legend:     An array containing the temperature, in

degC,

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 16

Quoted 1 line

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+			    for each column of the FCC lookup table.

What's FCC?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 17

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+- qcom,fcc-lut:             An array of FCC values in mah, one entry

for each

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 19

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+			    temperature defined in in qcom,fcc-temp-legend.
+- qcom,ocv-temp-legend:     An array containing the temperature, in

degC,

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 21

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+			    for each column of the OCV lookup table.
+- qcom,ocv-capacity-legend: An array containing the capacity for

each

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 23

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+			    row of the OCV lookup table.
+- qcom,ocv-lut:             An array of OCV values in mV, one entry

for each

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 24

Quoted 1 line

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+			    capacity defined in qcom,ocv-capacity-legend.

Need to specify the sizes of these if not 32-bit.

All these seem to have units, so add unit suffixes as defined in property-units.txt.

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 28

Quoted 5 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +4 -0

+
+Example:
+
+		pm8941_vadc: vadc@3100 {

adc@...

Quoted 69 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +65 -0

+			compatible = "qcom,spmi-vadc";
+			reg = <0x3100>;
+			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#io-channel-cells = <1>;
+
+			bat_temp {
+				reg = <VADC_LR_MUX1_BAT_THERM>;
+			};
+		};
+
+		bms@4000 {
+			compatible = "qcom,pm8941-bms";
+			reg = <0x4000>;
+			interrupts = <0x0 0x40 0x4 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "ocv_thr";
+
+			io-channels = <&pm8941_vadc VADC_LR_MUX1_BAT_THERM>;
+			io-channel-names = "temp";
+
+			qcom,fcc-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,fcc-lut = /bits/ 16 <6010 6070 6680 6780 6670>;
+
+			qcom,ocv-capacity-legend = /bits/ 8 <100 95 90 85
+							     80 75 70 65
+							     60 55 50 45
+							     40 35 30 25
+							     20 15 10 9
+							     8 7 6 5 4
+							     3 2 1 0>;
+
+			qcom,ocv-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,ocv-lut = /bits/ 16 <4288 4288 4306 4315 4315
+						  4261 4241 4259 4266 4246
+						  4201 4181 4201 4207 4187
+						  4153 4133 4150 4155 4135
+						  4105 4085 4100 4104 4084
+						  4058 4038 4052 4058 4038
+						  4012 3992 4004 4014 3994
+						  3970 3950 3959 3971 3951
+						  3931 3911 3915 3927 3907
+						  3899 3879 3880 3884 3864
+						  3873 3853 3851 3853 3833
+						  3848 3828 3827 3829 3809
+						  3829 3809 3808 3809 3789
+						  3815 3795 3791 3791 3771
+						  3801 3781 3775 3772 3752
+						  3785 3765 3751 3746 3726
+						  3767 3747 3727 3719 3699
+						  3750 3730 3702 3692 3672
+						  3728 3708 3680 3672 3652
+						  3720 3700 3676 3665 3645
+						  3712 3692 3670 3660 3645
+						  3695 3675 3658 3648 3633
+						  3662 3647 3629 3620 3610
+						  3620 3605 3589 3580 3570
+						  3562 3552 3538 3529 3519
+						  3490 3480 3474 3470 3465
+						  3403 3398 3388 3380 3375
+						  3320 3300 3255 3221 3206
+						  3000 3000 3000 3000 3000>;
+		};
+	};
+};

Signature · 1 line

2.17.0

Quoted 24 lines · Rob Herring

Quoted 5 lines · Craig Tatlor

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---
 .../bindings/power/supply/qcom_bms.txt        | 93

+++++++++++++++++++

Quoted 2 lines

 1 file changed, 93 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 2 lines

diff --git

a/Documentation/devicetree/bindings/power/supply/qcom_bms.txt b/Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 3

Quoted 8 lines

new file mode 100644
index 000000000000..6296399edc09

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

@@ -0,0 +1,93 @@
+Qualcomm Battery Measurement System
+
+The Qualcomm Battery Measurement System is found inside of Qualcomm

PM8941

Quoted 1 line

Is it Monitoring or Measurment?

Quoted 8 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 4

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+PMICs. It provides OCV and coulomb counter registers that allow the

kernel

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 5

Quoted 1 line

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+to infer a capacity level.

s/kernel/OS/

OCV?

Open Circuit Voltage - should I expand this?

Quoted 5 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 5 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +4 -0

+
+Required properties:
+- compatible:               Should contain "qcom,pm8941-bms".
+- reg:                      Specifies the SPMI address and length

of

Quoted 4 lines

the

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 3 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

+			    controller's registers.
+- interrupts:               OCV threshold interrupt.
+- io-channels:              Should contain IIO channel specifier

for

Quoted 8 lines

the

Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Quoted 3 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

+			    ADC channel that reports battery temperature.
+- io-channel-names:         Should contain "temp".
+- qcom,fcc-temp-legend:     An array containing the temperature, in

degC,

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 16

Quoted 1 line

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+			    for each column of the FCC lookup table.

What's FCC?

Full charge capacity - Should i also expand this? or maybe explain in header paragraph?

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 17

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+- qcom,fcc-lut:             An array of FCC values in mah, one

entry

Quoted 7 lines

for each

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 19

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+			    temperature defined in in qcom,fcc-temp-legend.
+- qcom,ocv-temp-legend:     An array containing the temperature, in

degC,

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 21

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+			    for each column of the OCV lookup table.
+- qcom,ocv-capacity-legend: An array containing the capacity for

each

Quoted 6 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 23

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+			    row of the OCV lookup table.
+- qcom,ocv-lut:             An array of OCV values in mV, one entry

for each

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 24

Quoted 1 line

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +1 -0

+			    capacity defined in qcom,ocv-capacity-legend.

Need to specify the sizes of these if not 32-bit.

Right.

Quoted 3 lines

All these seem to have units, so add unit suffixes as defined in property-units.txt.

Will do.

Quoted 7 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 28

Quoted 5 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +4 -0

+
+Example:
+
+		pm8941_vadc: vadc@3100 {

adc@...

Okay.

Quoted 69 lines

Quoted 69 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +65 -0

+			compatible = "qcom,spmi-vadc";
+			reg = <0x3100>;
+			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#io-channel-cells = <1>;
+
+			bat_temp {
+				reg = <VADC_LR_MUX1_BAT_THERM>;
+			};
+		};
+
+		bms@4000 {
+			compatible = "qcom,pm8941-bms";
+			reg = <0x4000>;
+			interrupts = <0x0 0x40 0x4 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "ocv_thr";
+
+			io-channels = <&pm8941_vadc VADC_LR_MUX1_BAT_THERM>;
+			io-channel-names = "temp";
+
+			qcom,fcc-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,fcc-lut = /bits/ 16 <6010 6070 6680 6780 6670>;
+
+			qcom,ocv-capacity-legend = /bits/ 8 <100 95 90 85
+							     80 75 70 65
+							     60 55 50 45
+							     40 35 30 25
+							     20 15 10 9
+							     8 7 6 5 4
+							     3 2 1 0>;
+
+			qcom,ocv-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,ocv-lut = /bits/ 16 <4288 4288 4306 4315 4315
+						  4261 4241 4259 4266 4246
+						  4201 4181 4201 4207 4187
+						  4153 4133 4150 4155 4135
+						  4105 4085 4100 4104 4084
+						  4058 4038 4052 4058 4038
+						  4012 3992 4004 4014 3994
+						  3970 3950 3959 3971 3951
+						  3931 3911 3915 3927 3907
+						  3899 3879 3880 3884 3864
+						  3873 3853 3851 3853 3833
+						  3848 3828 3827 3829 3809
+						  3829 3809 3808 3809 3789
+						  3815 3795 3791 3791 3771
+						  3801 3781 3775 3772 3752
+						  3785 3765 3751 3746 3726
+						  3767 3747 3727 3719 3699
+						  3750 3730 3702 3692 3672
+						  3728 3708 3680 3672 3652
+						  3720 3700 3676 3665 3645
+						  3712 3692 3670 3660 3645
+						  3695 3675 3658 3648 3633
+						  3662 3647 3629 3620 3610
+						  3620 3605 3589 3580 3570
+						  3562 3552 3538 3529 3519
+						  3490 3480 3474 3470 3465
+						  3403 3398 3388 3380 3375
+						  3320 3300 3255 3221 3206
+						  3000 3000 3000 3000 3000>;
+		};
+	};
+};

Signature · 1 line

2.17.0

Quoted 14 lines · Craig Tatlor

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---
 .../bindings/power/supply/qcom_bms.txt        | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +0 -0

new file mode 100644
index 000000000000..6296399edc09

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 3

Quoted 3 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

@@ -0,0 +1,93 @@
+Qualcomm Battery Measurement System
+
+The Qualcomm Battery Measurement System is found inside of Qualcomm PM8941

Is it Monitoring or Measurment?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 5

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+PMICs. It provides OCV and coulomb counter registers that allow the kernel
+to infer a capacity level.

s/kernel/OS/

OCV?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 16

Quoted 11 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +11 -0

+
+Required properties:
+- compatible:               Should contain "qcom,pm8941-bms".
+- reg:                      Specifies the SPMI address and length of the
+			    controller's registers.
+- interrupts:               OCV threshold interrupt.
+- io-channels:              Should contain IIO channel specifier for the
+			    ADC channel that reports battery temperature.
+- io-channel-names:         Should contain "temp".
+- qcom,fcc-temp-legend:     An array containing the temperature, in degC,
+			    for each column of the FCC lookup table.

What's FCC?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 24

Quoted 8 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +8 -0

+- qcom,fcc-lut:             An array of FCC values in mah, one entry for each
+			    temperature defined in in qcom,fcc-temp-legend.
+- qcom,ocv-temp-legend:     An array containing the temperature, in degC,
+			    for each column of the OCV lookup table.
+- qcom,ocv-capacity-legend: An array containing the capacity for each
+			    row of the OCV lookup table.
+- qcom,ocv-lut:             An array of OCV values in mV, one entry for each
+			    capacity defined in qcom,ocv-capacity-legend.

Need to specify the sizes of these if not 32-bit.

All these seem to have units, so add unit suffixes as defined in property-units.txt.

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 28

Quoted 5 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +4 -0

+
+Example:
+
+		pm8941_vadc: vadc@3100 {

adc@...

Quoted 68 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +65 -0

+			compatible = "qcom,spmi-vadc";
+			reg = <0x3100>;
+			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#io-channel-cells = <1>;
+
+			bat_temp {
+				reg = <VADC_LR_MUX1_BAT_THERM>;
+			};
+		};
+
+		bms@4000 {
+			compatible = "qcom,pm8941-bms";
+			reg = <0x4000>;
+			interrupts = <0x0 0x40 0x4 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "ocv_thr";
+
+			io-channels = <&pm8941_vadc VADC_LR_MUX1_BAT_THERM>;
+			io-channel-names = "temp";
+
+			qcom,fcc-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,fcc-lut = /bits/ 16 <6010 6070 6680 6780 6670>;
+
+			qcom,ocv-capacity-legend = /bits/ 8 <100 95 90 85
+							     80 75 70 65
+							     60 55 50 45
+							     40 35 30 25
+							     20 15 10 9
+							     8 7 6 5 4
+							     3 2 1 0>;
+
+			qcom,ocv-temp-legend = /bits/ 8 <(-10) 0 25 50 65>;
+			qcom,ocv-lut = /bits/ 16 <4288 4288 4306 4315 4315
+						  4261 4241 4259 4266 4246
+						  4201 4181 4201 4207 4187
+						  4153 4133 4150 4155 4135
+						  4105 4085 4100 4104 4084
+						  4058 4038 4052 4058 4038
+						  4012 3992 4004 4014 3994
+						  3970 3950 3959 3971 3951
+						  3931 3911 3915 3927 3907
+						  3899 3879 3880 3884 3864
+						  3873 3853 3851 3853 3833
+						  3848 3828 3827 3829 3809
+						  3829 3809 3808 3809 3789
+						  3815 3795 3791 3791 3771
+						  3801 3781 3775 3772 3752
+						  3785 3765 3751 3746 3726
+						  3767 3747 3727 3719 3699
+						  3750 3730 3702 3692 3672
+						  3728 3708 3680 3672 3652
+						  3720 3700 3676 3665 3645
+						  3712 3692 3670 3660 3645
+						  3695 3675 3658 3648 3633
+						  3662 3647 3629 3620 3610
+						  3620 3605 3589 3580 3570
+						  3562 3552 3538 3529 3519
+						  3490 3480 3474 3470 3465
+						  3403 3398 3388 3380 3375
+						  3320 3300 3255 3221 3206
+						  3000 3000 3000 3000 3000>;
+		};
+	};
+};

Signature · 1 line

2.17.0

Quoted 51 lines · Rob Herring

Quoted 14 lines · Craig Tatlor

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---
 .../bindings/power/supply/qcom_bms.txt        | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +0 -0

new file mode 100644
index 000000000000..6296399edc09

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 3

Quoted 3 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +3 -0

@@ -0,0 +1,93 @@
+Qualcomm Battery Measurement System
+
+The Qualcomm Battery Measurement System is found inside of Qualcomm PM8941

Is it Monitoring or Measurment?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 5

Quoted 2 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +2 -0

+PMICs. It provides OCV and coulomb counter registers that allow the kernel
+to infer a capacity level.

s/kernel/OS/

OCV?

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 16

Quoted 11 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +11 -0

+
+Required properties:
+- compatible:               Should contain "qcom,pm8941-bms".
+- reg:                      Specifies the SPMI address and length of the
+                       controller's registers.
+- interrupts:               OCV threshold interrupt.
+- io-channels:              Should contain IIO channel specifier for the
+                       ADC channel that reports battery temperature.
+- io-channel-names:         Should contain "temp".
+- qcom,fcc-temp-legend:     An array containing the temperature, in degC,
+                       for each column of the FCC lookup table.

What's FCC?

Quoted 8 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +8 -0

+- qcom,fcc-lut:             An array of FCC values in mah, one entry for each
+                       temperature defined in in qcom,fcc-temp-legend.
+- qcom,ocv-temp-legend:     An array containing the temperature, in degC,
+                       for each column of the OCV lookup table.
+- qcom,ocv-capacity-legend: An array containing the capacity for each
+                       row of the OCV lookup table.
+- qcom,ocv-lut:             An array of OCV values in mV, one entry for each
+                       capacity defined in qcom,ocv-capacity-legend.

Quoted 4 lines

Need to specify the sizes of these if not 32-bit.

All these seem to have units, so add unit suffixes as defined in property-units.txt.

Should i change code to use microamphours & microvolts or add a new suffix?

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then ammends the coulomb counter to that to increase the accuracy of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

drivers/power/supply/qcom_bms.c new file +7 -0

+static unsigned int interpolate(int y0, int x0, int y1, int x1, int x)
+{
+       if (y0 == y1 || x == x0)
+               return y0;
+       if (x1 == x0 || x == x1)
+               return y1;
+

drivers/power/supply/qcom_bms.c new file +30 -0

+static unsigned int interpolate_capacity(int temp, u16 ocv,
+                               struct bms_ocv_lut ocv_lut)
+{
+       unsigned int pcj_minus_one = 0, pcj = 0;
+       int i, j;
+
+       for (j = 0; j < TEMPERATURE_COLS; j++)
+               if (temp <= ocv_lut.temp_legend[j])
+                       break;
+
+       if (ocv >= ocv_lut.lut[0][j])
+               return ocv_lut.capacity_legend[0];
+
+       if (ocv <= ocv_lut.lut[ocv_lut.rows - 1][j - 1])
+               return ocv_lut.capacity_legend[ocv_lut.rows - 1];
+
+       for (i = 0; i < ocv_lut.rows - 1; i++) {
+               if (pcj == 0 && between(ocv_lut.lut[i][j],
+                                       ocv_lut.lut[i+1][j], ocv))
+                       pcj = interpolate(ocv_lut.capacity_legend[i],
+                                         ocv_lut.lut[i][j],
+                                         ocv_lut.capacity_legend[i + 1],
+                                         ocv_lut.lut[i+1][j],
+                                         ocv);
+
+               if (pcj_minus_one == 0 && between(ocv_lut.lut[i][j-1],
+                                                 ocv_lut.lut[i+1][j-1], ocv))
+                       pcj_minus_one = interpolate(ocv_lut.capacity_legend[i],
+                                                   ocv_lut.lut[i][j-1],
+                                                   ocv_lut.capacity_legend[i + 1],

drivers/power/supply/qcom_bms.c new file +6 -0

+static unsigned long interpolate_fcc(int temp, struct bms_fcc_lut fcc_lut)
+{
+       int i, fcc_mv;
+
+       for (i = 0; i < TEMPERATURE_COLS; i++)
+               if (temp <= fcc_lut.temp_legend[i])

+ temp_degc = (temp + 500) / 1000;

drivers/power/supply/qcom_bms.c new file +36 -0

+       ret = of_property_read_u8_array(di->dev->of_node,
+                                                "qcom,ocv-temp-legend",
+                                                (u8 *)di->ocv_lut.temp_legend,
+                                                TEMPERATURE_COLS);
+       if (ret < 0) {
+               dev_err(di->dev, "no ocv temperature legend found");
+               return ret;
+       }
+
+       di->ocv_lut.rows = of_property_read_variable_u8_array(di->dev->of_node,
+                                                "qcom,ocv-capacity-legend",
+                                                di->ocv_lut.capacity_legend, 0,
+                                                MAX_CAPACITY_ROWS);
+       if (di->ocv_lut.rows < 0) {
+               dev_err(di->dev, "no ocv capacity legend found");
+               return ret;
+       }
+
+       ret = of_property_read_variable_u16_array(di->dev->of_node,
+                                                 "qcom,ocv-lut",
+                                                 (u16 *)di->ocv_lut.lut,
+                                                 TEMPERATURE_COLS,
+                                                 TEMPERATURE_COLS *
+                                                 MAX_CAPACITY_ROWS);
+       if (ret < 0) {
+               dev_err(di->dev, "no ocv lut array found");
+               return ret;
+       }
+
+       ret = of_property_read_u8_array(di->dev->of_node,
+                                                "qcom,fcc-temp-legend",
+                                                (u8 *)di->fcc_lut.temp_legend,
+                                                TEMPERATURE_COLS);
+       if (ret < 0) {
+               dev_err(di->dev, "no fcc temperature legend found");
+               return ret;

On Sat, Apr 7, 2018 at 7:57 PM, Craig Tatlor <[email protected]> wrote:

Hi Craig! Thanks for your patch!

Quoted 6 lines

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then ammends the coulomb counter to that to increase the accuracy of the estimated capacity.

Quoted 1 line

Signed-off-by Craig Tatlor <[email protected]>

Just some minor remarks.

NB: I see that you are writing from a private email address so if you're working as a hobbyist on your precious sparetime I have lower expectation on how much work you will put into this, so see it more as suggestions than demands.

My overall feedback is that for algorithmic charger what we need is infrastructure. What is currently piling up in drivers/power/supply scares me a bit in it's lack of framework and code reuse.

It also scares me because this is vital technology dealing with physical devices and as such really need to have modularized reusable reviewed code with several users and deployments.

Code reuse would include:

- Mathematical helpers such as interpolation of
values from absolute values or tables
Suggestions below!
- State machines and transitions
- CC/CV alorithms (using the above)
- Many other things

Not that *I* can make the situation much better, I'm just sharing my fears,

drivers/power/supply/qcom_bms.c line 76

Quoted 7 lines

drivers/power/supply/qcom_bms.c new file +7 -0

+static s64 sign_extend_s36(uint64_t raw)
+{
+       raw = raw & CC_36_BIT_MASK;
+
+       return (raw >> 35) == 0LL ?
+               raw : (SIGN_EXTEND_36_TO_64_MASK | raw);
+}

#include <linux/bitops.h>

Use sign_extend32()

Quoted 8 lines

drivers/power/supply/qcom_bms.c new file +7 -0

+static unsigned int interpolate(int y0, int x0, int y1, int x1, int x)
+{
+       if (y0 == y1 || x == x0)
+               return y0;
+       if (x1 == x0 || x == x1)
+               return y1;
+

drivers/power/supply/qcom_bms.c line 94

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+       return y0 + ((y1 - y0) * (x - x0) / (x1 - x0));
+}
+
+static unsigned int between(int left, int right, int val)
+{
+       if (left <= val && val <= right)
+               return 1;
+
+       return 0;
+}

How are these things not library functions?

Every cell of my brain says this code should be reusable.

Can you put this in <linux/fixp-arith.h>?

I bet a million to one that the video people will sooner or later need linear interpolation and there are more users in the kernel than drivers/power/, certainly drivers/iio as well.

If noone else says anything I vote to put at least the linear interpolation into <linux/fixp-arith.h> with a bonus if you move some of the current users in drivers/power over while you're at it.

Quoted 39 lines

drivers/power/supply/qcom_bms.c new file +38 -0

+static unsigned int interpolate_capacity(int temp, u16 ocv,
+                               struct bms_ocv_lut ocv_lut)
+{
+       unsigned int pcj_minus_one = 0, pcj = 0;
+       int i, j;
+
+       for (j = 0; j < TEMPERATURE_COLS; j++)
+               if (temp <= ocv_lut.temp_legend[j])
+                       break;
+
+       if (ocv >= ocv_lut.lut[0][j])
+               return ocv_lut.capacity_legend[0];
+
+       if (ocv <= ocv_lut.lut[ocv_lut.rows - 1][j - 1])
+               return ocv_lut.capacity_legend[ocv_lut.rows - 1];
+
+       for (i = 0; i < ocv_lut.rows - 1; i++) {
+               if (pcj == 0 && between(ocv_lut.lut[i][j],
+                                       ocv_lut.lut[i+1][j], ocv))
+                       pcj = interpolate(ocv_lut.capacity_legend[i],
+                                         ocv_lut.lut[i][j],
+                                         ocv_lut.capacity_legend[i + 1],
+                                         ocv_lut.lut[i+1][j],
+                                         ocv);
+
+               if (pcj_minus_one == 0 && between(ocv_lut.lut[i][j-1],
+                                                 ocv_lut.lut[i+1][j-1], ocv))
+                       pcj_minus_one = interpolate(ocv_lut.capacity_legend[i],
+                                                   ocv_lut.lut[i][j-1],
+                                                   ocv_lut.capacity_legend[i + 1],
+                                                   ocv_lut.lut[i+1][j-1],
+                                                   ocv);
+
+               if (pcj && pcj_minus_one)
+                       return interpolate(pcj_minus_one,
+                                          ocv_lut.temp_legend[j-1],
+                                          pcj,
+                                          ocv_lut.temp_legend[j],

Review

Quoted 2 lines

(quoted diff) +2 -0

+                                          temp);
+       }

This code really needs some comments to tell what is going on here. Also I sense that you can break out a smaller function for interpolation based on table values, such as a function that would take a standard format of tables, look up where we are in that table and interpolate from the neighboring values.

People can then later go in and refine the algorithms if they e.g. want to introduce spline or RMS interpolation instead and we can get better interpolation for everybody.

Quoted 6 lines

drivers/power/supply/qcom_bms.c new file +6 -0

+static unsigned long interpolate_fcc(int temp, struct bms_fcc_lut fcc_lut)
+{
+       int i, fcc_mv;
+
+       for (i = 0; i < TEMPERATURE_COLS; i++)
+               if (temp <= fcc_lut.temp_legend[i])

drivers/power/supply/qcom_bms.c line 161

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+                       break;
+
+       fcc_mv = interpolate(fcc_lut.lut[i - 1],
+                            fcc_lut.temp_legend[i - 1],
+                            fcc_lut.lut[i],
+                            fcc_lut.temp_legend[i],
+                            temp);
+
+       return fcc_mv * 10000;
+}

So then only this would really remain: pass a table and interpolate.

drivers/power/supply/qcom_bms.c line 173

Quoted 11 lines

drivers/power/supply/qcom_bms.c new file +11 -0

+static int bms_lock_output_data(struct bms_device_info *di)
+{
+       int ret;
+
+       ret = regmap_update_bits(di->regmap, di->base_addr +
+                                REG_BMS_CC_DATA_CTL,
+                                BMS_HOLD_OREG_DATA, BMS_HOLD_OREG_DATA);
+       if (ret < 0) {
+               dev_err(di->dev, "failed to lock bms output: %d", ret);
+               return ret;
+       }

Reuse of regmap is very nice, thanks for doing it this way.

drivers/power/supply/qcom_bms.c line 274

Quoted 2 lines

drivers/power/supply/qcom_bms.c new file +2 -0

+static int bms_read_cc(struct bms_device_info *di, s64 *cc_uah)
+static void bms_reset_cc(struct bms_device_info *di)

These indeed need to be mostly hardware-specific so they are fine.

drivers/power/supply/qcom_bms.c line 313

Quoted 11 lines

drivers/power/supply/qcom_bms.c new file +11 -0

+static int bms_calculate_capacity(struct bms_device_info *di, int *capacity)
+{
+       unsigned long ocv_capacity, fcc;
+       int ret, temp, temp_degc;
+       s64 cc, capacity_nodiv;
+
+       ret = iio_read_channel_raw(di->adc, &temp);
+       if (ret < 0) {
+               dev_err(di->dev, "failed to read temperature: %d", ret);
+               return ret;
+       }

Very nice that you use IIO ADC as a back-end.

Quoted 1 line

+ temp_degc = (temp + 500) / 1000;

That deserves a comment I think. Like what the manual says about this and why the raw temperature is given like this.

Maybe you want to use DIV_ROUND_CLOSEST()
or DIV_ROUND_UP() from <linux/kernel.h> instead
of just "/"?

Maybe you want to use that in other places too like the below divisions.

drivers/power/supply/qcom_bms.c line 328

Quoted 13 lines

drivers/power/supply/qcom_bms.c new file +12 -0

+       ret = bms_read_cc(di, &cc);
+       if (ret < 0) {
+               dev_err(di->dev, "failed to read coulomb counter: %d", ret);
+               return ret;
+       }
+
+       ocv_capacity = interpolate_capacity(temp_degc, (di->ocv + 5) / 10,
+                                           di->ocv_lut);
+       fcc = interpolate_fcc(temp_degc, di->fcc_lut);
+
+       capacity_nodiv = ((fcc * ocv_capacity) / 100 - cc) * 100;
+       *capacity = div64_ul(capacity_nodiv, fcc);

So I guess you fit the capacity between 0..100, please add some comment on what's going on here.

drivers/power/supply/qcom_bms.c line 373

Quoted 11 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+static irqreturn_t bms_ocv_thr_irq_handler(int irq, void *dev_id)
+{
+       struct bms_device_info *di = dev_id;
+
+       if (bms_read_ocv(di, &di->ocv) < 0)
+               return IRQ_HANDLED;
+
+       bms_reset_cc(di);
+       return IRQ_HANDLED;
+}

So that is a coloumb counter interrupt? Please add some comment on when this gets called. Is it called whenever a coloumb is added/removed from the battery?

Quoted 44 lines

drivers/power/supply/qcom_bms.c new file +43 -0

+       ret = of_property_read_u8_array(di->dev->of_node,
+                                                "qcom,ocv-temp-legend",
+                                                (u8 *)di->ocv_lut.temp_legend,
+                                                TEMPERATURE_COLS);
+       if (ret < 0) {
+               dev_err(di->dev, "no ocv temperature legend found");
+               return ret;
+       }
+
+       di->ocv_lut.rows = of_property_read_variable_u8_array(di->dev->of_node,
+                                                "qcom,ocv-capacity-legend",
+                                                di->ocv_lut.capacity_legend, 0,
+                                                MAX_CAPACITY_ROWS);
+       if (di->ocv_lut.rows < 0) {
+               dev_err(di->dev, "no ocv capacity legend found");
+               return ret;
+       }
+
+       ret = of_property_read_variable_u16_array(di->dev->of_node,
+                                                 "qcom,ocv-lut",
+                                                 (u16 *)di->ocv_lut.lut,
+                                                 TEMPERATURE_COLS,
+                                                 TEMPERATURE_COLS *
+                                                 MAX_CAPACITY_ROWS);
+       if (ret < 0) {
+               dev_err(di->dev, "no ocv lut array found");
+               return ret;
+       }
+
+       ret = of_property_read_u8_array(di->dev->of_node,
+                                                "qcom,fcc-temp-legend",
+                                                (u8 *)di->fcc_lut.temp_legend,
+                                                TEMPERATURE_COLS);
+       if (ret < 0) {
+               dev_err(di->dev, "no fcc temperature legend found");
+               return ret;
+       }
+
+       ret = of_property_read_u16_array(di->dev->of_node,
+                                                 "qcom,fcc-lut",
+                                                 di->fcc_lut.lut,
+                                                 TEMPERATURE_COLS);
+       if (ret < 0) {

drivers/power/supply/qcom_bms.c line 448

Quoted 3 lines

drivers/power/supply/qcom_bms.c new file +3 -0

+               dev_err(di->dev, "no fcc lut array found");
+               return ret;
+       }

These tables (that I also suggest to use libraries to parse) should probably have standardized DT names so other battery drivers can use the same properties and we can use the same DT parsing code for all.

drivers/power/supply/qcom_bms.c line 454

Quoted 6 lines

drivers/power/supply/qcom_bms.c new file +5 -0

+       ret = bms_read_ocv(di, &di->ocv);
+       if (ret < 0) {
+               dev_err(di->dev, "failed to read initial ocv: %d", ret);
+               return ret;
+       }

OCV = original coloumb counter value?

Please expand the acronym somewhere because I get
a bit lost.

Overall it is a very nicely coded driver. My worries are all about code reuse, not code quality per se.

Yours,
Linus Walleij

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then ammends the coulomb counter to that to increase the accuracy

of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

---

drivers/power/supply/Kconfig +0 -0

index 428b426842f4..6c354c37bc55 100644

drivers/power/supply/Kconfig +4 -0

+	help
+	  Say Y to include support for the Battery Monitoring hardware
+	  found in some Qualcomm PM series PMICs.
+
 config BATTERY_CPCAP
 	tristate "Motorola CPCAP PMIC battery driver"
 	depends on MFD_CPCAP && IIO

Quoted 4 lines · Craig Tatlor

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then ammends the coulomb counter to that to increase the accuracy

amends

Quoted 10 lines

of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

---

drivers/power/supply/Kconfig +0 -0

index 428b426842f4..6c354c37bc55 100644

drivers/power/supply/Kconfig config BATTERY_ACT8945A · line 89

Quoted 8 lines

drivers/power/supply/Kconfig +5 -0

@@ -82,6 +82,15 @@ config BATTERY_ACT8945A
 	  Say Y here to enable support for power supply provided by
 	  Active-semi ActivePath ACT8945A charger.
 
+config BATTERY_BMS
+	tristate "Qualcomm Battery Monitoring System driver"
+	depends on MFD_SPMI_PMIC || COMPILE_TEST
+	depends on OF
+	depends on REGMAP_SPMI

select REGMAP_SPMI

I'm not a fan of "select", but it does not have a prompt string so it isn't user settable (not directly; it can be set indirectly).
And all other uses of it in Kconfig files use "select".

drivers/power/supply/Kconfig config BATTERY_ACT8945A · line 96

Quoted 7 lines

drivers/power/supply/Kconfig +4 -0

+	help
+	  Say Y to include support for the Battery Monitoring hardware
+	  found in some Qualcomm PM series PMICs.
+
 config BATTERY_CPCAP
 	tristate "Motorola CPCAP PMIC battery driver"
 	depends on MFD_CPCAP && IIO

Signature · 2 lines

-- 
~Randy

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---
* Changes from v4:
Uses proper units and expands some definitions,
along with changing vadc@ to adc@.

 .../bindings/power/supply/qcom_bms.txt        | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +16 -0

new file mode 100644
index 000000000000..a1a32d6ab460
@@ -0,0 +1,91 @@
+Qualcomm Battery Monitoring System
+
+The Qualcomm Battery Monitoring System is found inside of Qualcomm PM8941
+PMICs. It provides open circuit voltage (OCV) and coulomb counter registers
+that allow the OS to infer a capacity level.
+
+Required properties:
+- compatible:                   Should contain "qcom,pm8941-bms".
+- reg:                          Specifies the SPMI address and length of the
+				controller's registers.
+- interrupts:                   OCV threshold interrupt.
+- io-channels:                  Should contain IIO channel specifier for the
+				ADC channel that reports battery temperature.
+- io-channel-names:             Should contain "temp".
+- qcom,fcc-temp-legend-celcius: An array containing the temperature, in degC,
+				for each column of the full charge capacity

Quoted 34 lines · Craig Tatlor

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---
* Changes from v4:
Uses proper units and expands some definitions,
along with changing vadc@ to adc@.

 .../bindings/power/supply/qcom_bms.txt        | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +16 -0

new file mode 100644
index 000000000000..a1a32d6ab460
@@ -0,0 +1,91 @@
+Qualcomm Battery Monitoring System
+
+The Qualcomm Battery Monitoring System is found inside of Qualcomm PM8941
+PMICs. It provides open circuit voltage (OCV) and coulomb counter registers
+that allow the OS to infer a capacity level.
+
+Required properties:
+- compatible:                   Should contain "qcom,pm8941-bms".
+- reg:                          Specifies the SPMI address and length of the
+				controller's registers.
+- interrupts:                   OCV threshold interrupt.
+- io-channels:                  Should contain IIO channel specifier for the
+				ADC channel that reports battery temperature.
+- io-channel-names:             Should contain "temp".
+- qcom,fcc-temp-legend-celcius: An array containing the temperature, in degC,
+				for each column of the full charge capacity

Documentation/devicetree/bindings/power/supply/qcom_bms.txt line 26

Quoted 10 lines

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +10 -0

+				lookup table.
+- qcom,fcc-lut-microamp-hours:  An array of full charge capacity values in uah,
+				one entry for each temperature defined in in
+				qcom,fcc-temp-legend-celcius.
+- qcom,ocv-temp-legend-celcius: An array containing the temperature, in degC,
+				for each column of the OCV lookup table.
+- qcom,ocv-capacity-legend:     An array containing the capacity for each
+				row of the OCV lookup table.
+- qcom,ocv-lut-microvolt:       An array of OCV values in uV, one entry for each
+				capacity defined in qcom,ocv-capacity-legend.

Need to say which of these are 8-bit values.

With that,

Reviewed-by Rob Herring <[email protected]>

Adds a function to interpolate against two points, this is carried arount as a helper function by tons of drivers.

Signed-off-by Craig Tatlor <[email protected]>

Quoted 3 lines · Craig Tatlor

Adds a function to interpolate against two points, this is carried arount as a helper function by tons of drivers.

Quoted 1 line

Signed-off-by Craig Tatlor <[email protected]>

The linear formula seems to fit the most natural form of linear interpolation.

I bet some John Carmack algorithm type people will soon start to optimize this... :D

Quoted 10 lines · Linus Walleij

Quoted 4 lines · Craig Tatlor

Adds a function to interpolate against two points, this is carried arount as a helper function by tons of drivers.

Signed-off-by Craig Tatlor <[email protected]>

The linear formula seems to fit the most natural form of linear interpolation.

Quoted 2 lines

I bet some John Carmack algorithm type people will soon start to optimize this... :D

For your entertainment here is some sunday reading:
https://fgiesen.wordpress.com/2012/08/15/linear-interpolation-past-present-and-future/

Yours,
Linus Walleij

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then amends the coulomb counter to that to increase the accuracy of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

---

* Changes from v5:
Uses select for REGMAP_SPMI.

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---

* Changes from v5:
Mentions which values are 8 bit.

* Changes from v4:
Uses proper units and expands some definitions,
along with changing vadc@ to adc@.

 .../bindings/power/supply/qcom_bms.txt        | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +7 -0

new file mode 100644
index 000000000000..9a29a2eb9f04
@@ -0,0 +1,92 @@
+Qualcomm Battery Monitoring System
+
+The Qualcomm Battery Monitoring System is found inside of Qualcomm PM8941
+PMICs. It provides open circuit voltage (OCV) and coulomb counter registers
+that allow the OS to infer a capacity level.
+
+Required properties:

Add bindings for the Qualcomm Battery Monitoring system.

Signed-off-by Craig Tatlor <[email protected]>

---

* Changes from v5:
Mentions which values are 8 bit.

* Changes from v4:
Uses proper units and expands some definitions,
along with changing vadc@ to adc@.

 .../bindings/power/supply/qcom_bms.txt        | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom_bms.txt

Documentation/devicetree/bindings/power/supply/qcom_bms.txt new file +7 -0

new file mode 100644
index 000000000000..9a29a2eb9f04
@@ -0,0 +1,92 @@
+Qualcomm Battery Monitoring System
+
+The Qualcomm Battery Monitoring System is found inside of Qualcomm PM8941
+PMICs. It provides open circuit voltage (OCV) and coulomb counter registers
+that allow the OS to infer a capacity level.
+
+Required properties:

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then amends the coulomb counter to that to increase the accuracy of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

Reviewed-by Linus Walleij <[email protected]>

---

* Changes from v5:
Uses select for REGMAP_SPMI.

* Changes from v4:
Cleaned up percentage interpolation function,
uses new fixp interpolation helper,
added some more error cases,
uses devm_power_supply_register(),
uses a DIV_ROUND_CLOSEST for division and
uses micro(volts / amp hours) instead of
milli (volts / amp hours).

 drivers/power/supply/Kconfig    |   9 +
 drivers/power/supply/Makefile   |   1 +
 drivers/power/supply/qcom_bms.c | 487 ++++++++++++++++++++++++++++++++
 3 files changed, 497 insertions(+)
 create mode 100644 drivers/power/supply/qcom_bms.c

drivers/power/supply/Kconfig +9 -0

index 428b426842f4..75f2f375f992 100644
@@ -82,6 +82,15 @@ config BATTERY_ACT8945A
 	  Say Y here to enable support for power supply provided by
 	  Active-semi ActivePath ACT8945A charger.
 
+config BATTERY_BMS
+	tristate "Qualcomm Battery Monitoring System driver"
+	depends on MFD_SPMI_PMIC || COMPILE_TEST
+	depends on OF
+	select REGMAP_SPMI
+	help
+	  Say Y to include support for the Battery Monitoring hardware
+	  found in some Qualcomm PM series PMICs.
+
 config BATTERY_CPCAP
 	tristate "Motorola CPCAP PMIC battery driver"
 	depends on MFD_CPCAP && IIO

drivers/power/supply/Makefile +1 -0

index e83aa843bcc6..04204174b047 100644
@@ -21,6 +21,7 @@ obj-$(CONFIG_BATTERY_88PM860X)	+= 88pm860x_battery.o
 obj-$(CONFIG_BATTERY_ACT8945A)	+= act8945a_charger.o
 obj-$(CONFIG_BATTERY_AXP20X)	+= axp20x_battery.o
 obj-$(CONFIG_CHARGER_AXP20X)	+= axp20x_ac_power.o
+obj-$(CONFIG_BATTERY_BMS)	+= qcom_bms.o
 obj-$(CONFIG_BATTERY_CPCAP)	+= cpcap-battery.o
 obj-$(CONFIG_BATTERY_DS2760)	+= ds2760_battery.o
 obj-$(CONFIG_BATTERY_DS2780)	+= ds2780_battery.o

drivers/power/supply/qcom_bms.c new file +186 -0

new file mode 100644
index 000000000000..718fd745c0f7
@@ -0,0 +1,487 @@
+// SPDX-License-Identifier: GPL
+
+/*
+ * Qualcomm Battery Monitoring System driver
+ *
+ * Copyright (C) 2018 Craig Tatlor <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/fixp-arith.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/slab.h>
+#include <linux/bitops.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/iio/consumer.h>
+
+#define REG_BMS_OCV_FOR_SOC_DATA0	0x90
+#define REG_BMS_SHDW_CC_DATA0		0xA8
+#define REG_BMS_CC_DATA_CTL		0x42
+#define REG_BMS_CC_CLEAR_CTL		0x4
+
+#define BMS_HOLD_OREG_DATA		BIT(0)
+#define BMS_CLEAR_SHDW_CC		BIT(6)
+
+#define BMS_CC_READING_RESOLUTION_N	542535
+#define BMS_CC_READING_RESOLUTION_D	10000
+#define BMS_CC_READING_TICKS		56
+#define BMS_SLEEP_CLK_HZ		32764
+
+#define SECONDS_PER_HOUR		3600
+#define TEMPERATURE_COLS		5
+#define MAX_CAPACITY_ROWS		50
+
+/* lookup table for ocv -> capacity conversion */
+struct bms_ocv_lut {
+	int rows;
+	s8 temp_legend[TEMPERATURE_COLS];
+	u8 capacity_legend[MAX_CAPACITY_ROWS];
+	u32 lut[MAX_CAPACITY_ROWS][TEMPERATURE_COLS];
+};
+
+/* lookup table for battery temperature -> fcc conversion */
+struct bms_fcc_lut {
+	s8 temp_legend[TEMPERATURE_COLS];
+	u32 lut[TEMPERATURE_COLS];
+};
+
+struct bms_device_info {
+	struct device *dev;
+	struct regmap *regmap;
+	struct bms_ocv_lut ocv_lut;
+	struct power_supply_desc bat_desc;
+	struct bms_fcc_lut fcc_lut;
+	struct iio_channel *adc;
+	struct mutex bms_output_lock;
+	u32 base_addr;
+
+	int ocv_thr_irq;
+	u32 ocv;
+};
+
+static bool between(int left, int right, int val)
+{
+	if (left <= val && val <= right)
+		return true;
+
+	if (left >= val && val >= right)
+		return true;
+
+	return false;
+}
+
+static int interpolate_capacity(int temp, u32 ocv,
+				struct bms_ocv_lut *ocv_lut)
+{
+	int pcj_minus_one = 0, pcj = 0, i2 = 0, i3 = 0, i, j;
+
+	for (j = 0; j < TEMPERATURE_COLS; j++)
+		if (temp <= ocv_lut->temp_legend[j])
+			break;
+
+	if (ocv >= ocv_lut->lut[0][j])
+		return ocv_lut->capacity_legend[0];
+
+	if (ocv <= ocv_lut->lut[ocv_lut->rows-1][j-1])
+		return ocv_lut->capacity_legend[ocv_lut->rows-1];
+
+	for (i = 0; i < ocv_lut->rows-1; i++) {
+		if (between(ocv_lut->lut[i][j],
+			    ocv_lut->lut[i+1][j], ocv))
+			i2 = i;
+
+		if (between(ocv_lut->lut[i][j-1],
+			    ocv_lut->lut[i+1][j-1], ocv))
+			i3 = i;
+	}
+
+	/* interpolate two capacities */
+	pcj = fixp_linear_interpolate(ocv_lut->lut[i2][j],
+				      ocv_lut->capacity_legend[i2],
+				      ocv_lut->lut[i2+1][j],
+				      ocv_lut->capacity_legend[i2+1],
+				      ocv);
+
+	pcj_minus_one = fixp_linear_interpolate(ocv_lut->lut[i3][j-1],
+						ocv_lut->capacity_legend[i3],
+						ocv_lut->lut[i3+1][j-1],
+						ocv_lut->capacity_legend[i3+1],
+						ocv);
+
+	/* interpolate them with the battery temperature */
+	return fixp_linear_interpolate(ocv_lut->temp_legend[j-1],
+				       pcj_minus_one,
+				       ocv_lut->temp_legend[j],
+				       pcj,
+				       temp);
+}
+
+static int interpolate_fcc(int temp, struct bms_fcc_lut *fcc_lut)
+{
+	int i;
+
+	for (i = 0; i < TEMPERATURE_COLS; i++)
+		if (temp <= fcc_lut->temp_legend[i])
+			break;
+
+	return fixp_linear_interpolate(fcc_lut->temp_legend[i-1],
+			     fcc_lut->lut[i-1],
+			     fcc_lut->temp_legend[i],
+			     fcc_lut->lut[i],
+			     temp);
+}
+
+static int bms_lock_output_data(struct bms_device_info *di)
+{
+	int ret;
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +
+				 REG_BMS_CC_DATA_CTL,
+				 BMS_HOLD_OREG_DATA, BMS_HOLD_OREG_DATA);
+	if (ret) {
+		dev_err(di->dev, "failed to lock bms output: %d", ret);
+		return ret;
+	}
+
+	/*
+	 * Sleep for at least 100 microseconds here to make sure
+	 * there has been at least three cycles of the sleep clock
+	 * so that the registers are correctly locked.
+	 */
+	usleep_range(100, 1000);
+
+	return 0;
+}
+
+static int bms_unlock_output_data(struct bms_device_info *di)
+{
+	int ret;
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +
+				 REG_BMS_CC_DATA_CTL,
+				 BMS_HOLD_OREG_DATA, 0);
+	if (ret) {
+		dev_err(di->dev, "failed to unlock bms output: %d", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bms_read_ocv(struct bms_device_info *di, u32 *ocv)
+{
+	int ret;
+	u16 read_ocv;
+
+	mutex_lock(&di->bms_output_lock);
+
+	ret = bms_lock_output_data(di);
+	if (ret)
+		goto err_lock;
+

drivers/power/supply/qcom_bms.c new file +116 -0

+	*ocv = read_ocv * 1000;
+
+err_read:
+	bms_unlock_output_data(di);
+
+err_lock:
+	mutex_unlock(&di->bms_output_lock);
+
+	return ret;
+}
+
+static int bms_read_cc(struct bms_device_info *di, s64 *cc_uah)
+{
+	int ret;
+	s64 cc_raw_s36, cc_raw, cc_uv, cc_pvh;
+
+	mutex_lock(&di->bms_output_lock);
+
+	ret = bms_lock_output_data(di);
+	if (ret)
+		goto err_lock;
+
+	ret = regmap_bulk_read(di->regmap, di->base_addr +
+			       REG_BMS_SHDW_CC_DATA0,
+			       &cc_raw_s36, 5);
+	if (ret) {
+		dev_err(di->dev, "coulomb counter read failed: %d", ret);
+		goto err_read;
+	}
+
+	ret = bms_unlock_output_data(di);
+	if (ret)
+		goto err_lock;
+
+	mutex_unlock(&di->bms_output_lock);
+
+	cc_raw = sign_extend32(cc_raw_s36, 28);
+
+	/* convert raw to uv */
+	cc_uv = div_s64(cc_raw * BMS_CC_READING_RESOLUTION_N,
+			BMS_CC_READING_RESOLUTION_D);
+
+	/* convert uv to pvh */
+	cc_pvh = div_s64(cc_uv * BMS_CC_READING_TICKS * 100000,
+			 BMS_SLEEP_CLK_HZ * SECONDS_PER_HOUR);
+
+	/* divide by impedance */
+	*cc_uah = div_s64(cc_pvh, 10000);
+
+	dev_dbg(di->dev, "read coulomb counter value of: %lld uah", *cc_uah);
+
+	return 0;
+
+err_read:
+	bms_unlock_output_data(di);
+
+err_lock:
+	mutex_unlock(&di->bms_output_lock);
+
+	return ret;
+}
+
+static void bms_reset_cc(struct bms_device_info *di)
+{
+	int ret;
+
+	mutex_lock(&di->bms_output_lock);
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +
+				 REG_BMS_CC_CLEAR_CTL,
+				 BMS_CLEAR_SHDW_CC,
+				 BMS_CLEAR_SHDW_CC);
+	if (ret) {
+		dev_err(di->dev, "coulomb counter reset failed: %d", ret);
+		goto err_lock;
+	}
+
+	/* wait at least three sleep cycles for cc to reset */
+	usleep_range(100, 1000);
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +
+				 REG_BMS_CC_CLEAR_CTL,
+				 BMS_CLEAR_SHDW_CC, 0);
+	if (ret)
+		dev_err(di->dev, "coulomb counter re-enable failed: %d", ret);
+
+err_lock:
+	mutex_unlock(&di->bms_output_lock);
+}
+
+static int bms_calculate_capacity(struct bms_device_info *di, int *capacity)
+{
+	unsigned long fcc;
+	int ret, temp, ocv_capacity, temp_degc;
+	s64 cc = 0;
+
+	ret = iio_read_channel_raw(di->adc, &temp);
+	if (ret < 0) {
+		dev_err(di->dev, "failed to read temperature: %d", ret);
+		return ret;
+	}
+
+	temp_degc = DIV_ROUND_CLOSEST(temp, 1000);
+
+	ret = bms_read_cc(di, &cc);
+	if (ret < 0) {
+		dev_err(di->dev, "failed to read coulomb counter: %d", ret);
+		return ret;
+	}
+
+	/* interpolate capacity from open circuit voltage */
+	ocv_capacity = interpolate_capacity(temp_degc, di->ocv,
+					    &di->ocv_lut);
+
+	/* interpolate the full charge capacity from temperature */
+	fcc = interpolate_fcc(temp_degc, &di->fcc_lut);

drivers/power/supply/qcom_bms.c new file +18 -0

+/*
+ * Return power_supply property
+ */
+static int bms_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct bms_device_info *di = power_supply_get_drvdata(psy);
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = bms_calculate_capacity(di, &val->intval);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}

drivers/power/supply/qcom_bms.c new file +85 -0

+static int bms_probe(struct platform_device *pdev)
+{
+	struct power_supply_config psy_cfg = {};
+	struct bms_device_info *di;
+	struct power_supply *bat;
+	int ret;
+
+	di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL);
+	if (!di)
+		return -ENOMEM;
+
+	di->dev = &pdev->dev;
+
+	di->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!di->regmap) {
+		dev_err(di->dev, "Unable to get regmap");
+		return -EINVAL;
+	}
+
+	di->adc = devm_iio_channel_get(&pdev->dev, "temp");
+	if (IS_ERR(di->adc))
+		return PTR_ERR(di->adc);
+
+	ret = of_property_read_u32(di->dev->of_node, "reg", &di->base_addr);
+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u8_array(di->dev->of_node,
+						 "qcom,ocv-temp-legend-celsius",
+						 (u8 *)di->ocv_lut.temp_legend,
+						 TEMPERATURE_COLS);
+	if (ret < 0) {
+		dev_err(di->dev, "no open circuit voltage temperature legend found");
+		return ret;
+	}
+
+	di->ocv_lut.rows = of_property_read_variable_u8_array(di->dev->of_node,
+						 "qcom,ocv-capacity-legend",
+						 di->ocv_lut.capacity_legend, 0,
+						 MAX_CAPACITY_ROWS);
+	if (di->ocv_lut.rows < 0) {
+		dev_err(di->dev, "no open circuit voltage capacity legend found");
+		return ret;
+	}
+
+	ret = of_property_read_variable_u32_array(di->dev->of_node,
+						  "qcom,ocv-lut-microvolt",
+						  (u32 *)di->ocv_lut.lut,
+						  TEMPERATURE_COLS,
+						  TEMPERATURE_COLS *
+						  MAX_CAPACITY_ROWS);
+	if (ret < 0) {
+		dev_err(di->dev, "no open circuit voltage lut array found");
+		return ret;
+	}
+
+	ret = of_property_read_u8_array(di->dev->of_node,
+						 "qcom,fcc-temp-legend-celsius",
+						 (u8 *)di->fcc_lut.temp_legend,
+						 TEMPERATURE_COLS);
+	if (ret < 0) {
+		dev_err(di->dev, "no full charge capacity temperature legend found");
+		return ret;
+	}
+
+	ret = of_property_read_u32_array(di->dev->of_node,
+						  "qcom,fcc-lut-microamp-hours",
+						  di->fcc_lut.lut,
+						  TEMPERATURE_COLS);
+	if (ret < 0) {
+		dev_err(di->dev, "no full charge capacity lut array found");
+		return ret;
+	}
+
+	ret = bms_read_ocv(di, &di->ocv);
+	if (ret < 0) {
+		dev_err(di->dev, "failed to read initial open circuit voltage: %d",
+			ret);
+		return ret;
+	}
+
+	mutex_init(&di->bms_output_lock);
+
+	di->ocv_thr_irq = platform_get_irq_byname(pdev, "ocv_thr");
+

drivers/power/supply/qcom_bms.c new file +13 -0

+	bat = devm_power_supply_register(di->dev, &di->bat_desc, &psy_cfg);
+
+	return PTR_ERR_OR_ZERO(bat);
+}
+
+static const struct of_device_id bms_of_match[] = {
+	{.compatible = "qcom,pm8941-bms", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bms_of_match);
+
+static struct platform_driver bms_driver = {
+	.probe = bms_probe,

Hi,

First of all thanks for the patch and big sorry for the long delay in reviewing this. I did not find enough time to do it properly until now :(

Quoted 28 lines · Craig Tatlor

This patch adds a driver for the BMS (Battery Monitoring System) block of the PM8941 PMIC, it uses a lookup table defined in the device tree to generate a capacity from the BMS supplied OCV, it then amends the coulomb counter to that to increase the accuracy of the estimated capacity.

Signed-off-by Craig Tatlor <[email protected]>

Reviewed-by Linus Walleij <[email protected]>

---

* Changes from v5:

Uses select for REGMAP_SPMI.

* Changes from v4:

Cleaned up percentage interpolation function,

uses new fixp interpolation helper,

added some more error cases,

uses devm_power_supply_register(),

uses a DIV_ROUND_CLOSEST for division and

Quoted 7 lines

uses micro(volts / amp hours) instead of

milli (volts / amp hours).

 drivers/power/supply/Kconfig    |   9 +
 drivers/power/supply/Makefile   |   1 +
 drivers/power/supply/qcom_bms.c | 487

++++++++++++++++++++++++++++++++

Review

Quoted 4 lines

 3 files changed, 497 insertions(+)
 create mode 100644 drivers/power/supply/qcom_bms.c

(quoted diff) +0 -0

b/drivers/power/supply/Kconfig

Review

Quoted 20 lines

index 428b426842f4..75f2f375f992 100644

drivers/power/supply/Kconfig +9 -0

@@ -82,6 +82,15 @@ config BATTERY_ACT8945A
 	  Say Y here to enable support for power supply provided by
 	  Active-semi ActivePath ACT8945A charger.
 
+config BATTERY_BMS
+	tristate "Qualcomm Battery Monitoring System driver"
+	depends on MFD_SPMI_PMIC || COMPILE_TEST
+	depends on OF
+	select REGMAP_SPMI
+	help
+	  Say Y to include support for the Battery Monitoring hardware
+	  found in some Qualcomm PM series PMICs.
+
 config BATTERY_CPCAP
 	tristate "Motorola CPCAP PMIC battery driver"
 	depends on MFD_CPCAP && IIO

(quoted diff) +0 -0

b/drivers/power/supply/Makefile

drivers/power/supply/Makefile obj-$(CONFIG_BATTERY_88PM860X) +=

Quoted 4 lines

index e83aa843bcc6..04204174b047 100644

drivers/power/supply/Makefile +0 -0

@@ -21,6 +21,7 @@ obj-$(CONFIG_BATTERY_88PM860X)	+=

88pm860x_battery.o

Review

Quoted 8 lines

drivers/power/supply/Makefile +1 -0

 obj-$(CONFIG_BATTERY_ACT8945A)	+= act8945a_charger.o
 obj-$(CONFIG_BATTERY_AXP20X)	+= axp20x_battery.o
 obj-$(CONFIG_CHARGER_AXP20X)	+= axp20x_ac_power.o
+obj-$(CONFIG_BATTERY_BMS)	+= qcom_bms.o
 obj-$(CONFIG_BATTERY_CPCAP)	+= cpcap-battery.o
 obj-$(CONFIG_BATTERY_DS2760)	+= ds2760_battery.o
 obj-$(CONFIG_BATTERY_DS2780)	+= ds2780_battery.o

(quoted diff) +0 -0

b/drivers/power/supply/qcom_bms.c

Quoted 195 lines

new file mode 100644
index 000000000000..718fd745c0f7

drivers/power/supply/qcom_bms.c new file +190 -0

@@ -0,0 +1,487 @@
+// SPDX-License-Identifier: GPL
+
+/*
+ * Qualcomm Battery Monitoring System driver
+ *
+ * Copyright (C) 2018 Craig Tatlor <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/fixp-arith.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/slab.h>
+#include <linux/bitops.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/iio/consumer.h>
+
+#define REG_BMS_OCV_FOR_SOC_DATA0	0x90
+#define REG_BMS_SHDW_CC_DATA0		0xA8
+#define REG_BMS_CC_DATA_CTL		0x42
+#define REG_BMS_CC_CLEAR_CTL		0x4
+
+#define BMS_HOLD_OREG_DATA		BIT(0)
+#define BMS_CLEAR_SHDW_CC		BIT(6)
+
+#define BMS_CC_READING_RESOLUTION_N	542535
+#define BMS_CC_READING_RESOLUTION_D	10000
+#define BMS_CC_READING_TICKS		56
+#define BMS_SLEEP_CLK_HZ		32764
+
+#define SECONDS_PER_HOUR		3600
+#define TEMPERATURE_COLS		5
+#define MAX_CAPACITY_ROWS		50
+
+/* lookup table for ocv -> capacity conversion */
+struct bms_ocv_lut {
+	int rows;
+	s8 temp_legend[TEMPERATURE_COLS];
+	u8 capacity_legend[MAX_CAPACITY_ROWS];
+	u32 lut[MAX_CAPACITY_ROWS][TEMPERATURE_COLS];
+};
+
+/* lookup table for battery temperature -> fcc conversion */
+struct bms_fcc_lut {
+	s8 temp_legend[TEMPERATURE_COLS];
+	u32 lut[TEMPERATURE_COLS];
+};
+
+struct bms_device_info {
+	struct device *dev;
+	struct regmap *regmap;
+	struct bms_ocv_lut ocv_lut;
+	struct power_supply_desc bat_desc;
+	struct bms_fcc_lut fcc_lut;
+	struct iio_channel *adc;
+	struct mutex bms_output_lock;
+	u32 base_addr;
+
+	int ocv_thr_irq;
+	u32 ocv;
+};
+
+static bool between(int left, int right, int val)
+{
+	if (left <= val && val <= right)
+		return true;
+
+	if (left >= val && val >= right)
+		return true;
+
+	return false;
+}
+
+static int interpolate_capacity(int temp, u32 ocv,
+				struct bms_ocv_lut *ocv_lut)
+{
+	int pcj_minus_one = 0, pcj = 0, i2 = 0, i3 = 0, i, j;
+
+	for (j = 0; j < TEMPERATURE_COLS; j++)
+		if (temp <= ocv_lut->temp_legend[j])
+			break;
+
+	if (ocv >= ocv_lut->lut[0][j])
+		return ocv_lut->capacity_legend[0];
+
+	if (ocv <= ocv_lut->lut[ocv_lut->rows-1][j-1])
+		return ocv_lut->capacity_legend[ocv_lut->rows-1];
+
+	for (i = 0; i < ocv_lut->rows-1; i++) {
+		if (between(ocv_lut->lut[i][j],
+			    ocv_lut->lut[i+1][j], ocv))
+			i2 = i;
+
+		if (between(ocv_lut->lut[i][j-1],
+			    ocv_lut->lut[i+1][j-1], ocv))
+			i3 = i;
+	}
+
+	/* interpolate two capacities */
+	pcj = fixp_linear_interpolate(ocv_lut->lut[i2][j],
+				      ocv_lut->capacity_legend[i2],
+				      ocv_lut->lut[i2+1][j],
+				      ocv_lut->capacity_legend[i2+1],
+				      ocv);
+
+	pcj_minus_one = fixp_linear_interpolate(ocv_lut->lut[i3][j-1],
+						ocv_lut->capacity_legend[i3],
+						ocv_lut->lut[i3+1][j-1],
+						ocv_lut->capacity_legend[i3+1],
+						ocv);
+
+	/* interpolate them with the battery temperature */
+	return fixp_linear_interpolate(ocv_lut->temp_legend[j-1],
+				       pcj_minus_one,
+				       ocv_lut->temp_legend[j],
+				       pcj,
+				       temp);
+}
+
+static int interpolate_fcc(int temp, struct bms_fcc_lut *fcc_lut)
+{
+	int i;
+
+	for (i = 0; i < TEMPERATURE_COLS; i++)
+		if (temp <= fcc_lut->temp_legend[i])
+			break;
+
+	return fixp_linear_interpolate(fcc_lut->temp_legend[i-1],
+			     fcc_lut->lut[i-1],
+			     fcc_lut->temp_legend[i],
+			     fcc_lut->lut[i],
+			     temp);
+}
+
+static int bms_lock_output_data(struct bms_device_info *di)
+{
+	int ret;
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +
+				 REG_BMS_CC_DATA_CTL,
+				 BMS_HOLD_OREG_DATA, BMS_HOLD_OREG_DATA);
+	if (ret) {
+		dev_err(di->dev, "failed to lock bms output: %d", ret);
+		return ret;
+	}
+
+	/*
+	 * Sleep for at least 100 microseconds here to make sure
+	 * there has been at least three cycles of the sleep clock
+	 * so that the registers are correctly locked.
+	 */
+	usleep_range(100, 1000);
+
+	return 0;
+}
+
+static int bms_unlock_output_data(struct bms_device_info *di)
+{
+	int ret;
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +
+				 REG_BMS_CC_DATA_CTL,
+				 BMS_HOLD_OREG_DATA, 0);
+	if (ret) {
+		dev_err(di->dev, "failed to unlock bms output: %d", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bms_read_ocv(struct bms_device_info *di, u32 *ocv)
+{
+	int ret;
+	u16 read_ocv;
+
+	mutex_lock(&di->bms_output_lock);
+
+	ret = bms_lock_output_data(di);
+	if (ret)
+		goto err_lock;
+
+	ret = regmap_bulk_read(di->regmap, di->base_addr+
+			       REG_BMS_OCV_FOR_SOC_DATA0, &read_ocv, 2);
+	if (ret) {
+		dev_err(di->dev, "open circuit voltage read failed: %d", ret);

drivers/power/supply/qcom_bms.c line 196

Quoted 6 lines

drivers/power/supply/qcom_bms.c new file +6 -0

+		goto err_read;
+	}
+
+	dev_dbg(di->dev, "read open circuit voltage of: %d mv", read_ocv);
+
+

one empty line is enough.

Quoted 41 lines

drivers/power/supply/qcom_bms.c new file +40 -0

+	*ocv = read_ocv * 1000;
+
+err_read:
+	bms_unlock_output_data(di);
+
+err_lock:
+	mutex_unlock(&di->bms_output_lock);
+
+	return ret;
+}
+
+static int bms_read_cc(struct bms_device_info *di, s64 *cc_uah)
+{
+	int ret;
+	s64 cc_raw_s36, cc_raw, cc_uv, cc_pvh;
+
+	mutex_lock(&di->bms_output_lock);
+
+	ret = bms_lock_output_data(di);
+	if (ret)
+		goto err_lock;
+
+	ret = regmap_bulk_read(di->regmap, di->base_addr +
+			       REG_BMS_SHDW_CC_DATA0,
+			       &cc_raw_s36, 5);
+	if (ret) {
+		dev_err(di->dev, "coulomb counter read failed: %d", ret);
+		goto err_read;
+	}
+
+	ret = bms_unlock_output_data(di);
+	if (ret)
+		goto err_lock;
+
+	mutex_unlock(&di->bms_output_lock);
+
+	cc_raw = sign_extend32(cc_raw_s36, 28);
+
+	/* convert raw to uv */
+	cc_uv = div_s64(cc_raw * BMS_CC_READING_RESOLUTION_N,

drivers/power/supply/qcom_bms.c

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+			BMS_CC_READING_RESOLUTION_D);
+
+	/* convert uv to pvh */
+	cc_pvh = div_s64(cc_uv * BMS_CC_READING_TICKS * 100000,
+			 BMS_SLEEP_CLK_HZ * SECONDS_PER_HOUR);
+
+	/* divide by impedance */
+	*cc_uah = div_s64(cc_pvh, 10000);
+
+	dev_dbg(di->dev, "read coulomb counter value of: %lld uah",

*cc_uah);

Quoted 31 lines

drivers/power/supply/qcom_bms.c new file +31 -0

+
+	return 0;
+
+err_read:
+	bms_unlock_output_data(di);
+
+err_lock:
+	mutex_unlock(&di->bms_output_lock);
+
+	return ret;
+}
+
+static void bms_reset_cc(struct bms_device_info *di)
+{
+	int ret;
+
+	mutex_lock(&di->bms_output_lock);
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +
+				 REG_BMS_CC_CLEAR_CTL,
+				 BMS_CLEAR_SHDW_CC,
+				 BMS_CLEAR_SHDW_CC);
+	if (ret) {
+		dev_err(di->dev, "coulomb counter reset failed: %d", ret);
+		goto err_lock;
+	}
+
+	/* wait at least three sleep cycles for cc to reset */
+	usleep_range(100, 1000);
+
+	ret = regmap_update_bits(di->regmap, di->base_addr +

drivers/power/supply/qcom_bms.c

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+				 REG_BMS_CC_CLEAR_CTL,
+				 BMS_CLEAR_SHDW_CC, 0);
+	if (ret)
+		dev_err(di->dev, "coulomb counter re-enable failed: %d", ret);
+
+err_lock:
+	mutex_unlock(&di->bms_output_lock);
+}
+
+static int bms_calculate_capacity(struct bms_device_info *di, int

*capacity)

Quoted 29 lines

drivers/power/supply/qcom_bms.c new file +29 -0

+{
+	unsigned long fcc;
+	int ret, temp, ocv_capacity, temp_degc;
+	s64 cc = 0;
+
+	ret = iio_read_channel_raw(di->adc, &temp);
+	if (ret < 0) {
+		dev_err(di->dev, "failed to read temperature: %d", ret);
+		return ret;
+	}
+
+	temp_degc = DIV_ROUND_CLOSEST(temp, 1000);
+
+	ret = bms_read_cc(di, &cc);
+	if (ret < 0) {
+		dev_err(di->dev, "failed to read coulomb counter: %d", ret);
+		return ret;
+	}
+
+	/* interpolate capacity from open circuit voltage */
+	ocv_capacity = interpolate_capacity(temp_degc, di->ocv,
+					    &di->ocv_lut);
+
+	/* interpolate the full charge capacity from temperature */
+	fcc = interpolate_fcc(temp_degc, &di->fcc_lut);
+
+	/* append coloumb counter to capacity */
+	*capacity = DIV_ROUND_CLOSEST(fcc * ocv_capacity, 100);
+	*capacity = div_s64((*capacity - cc) * 100, fcc);

drivers/power/supply/qcom_bms.c line 172

Quoted 6 lines

drivers/power/supply/qcom_bms.c new file +6 -0

+
+	return 0;
+}
+
+
+

one empty line is enough.

Quoted 19 lines

drivers/power/supply/qcom_bms.c new file +18 -0

+/*
+ * Return power_supply property
+ */
+static int bms_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct bms_device_info *di = power_supply_get_drvdata(psy);
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = bms_calculate_capacity(di, &val->intval);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}

drivers/power/supply/qcom_bms.c line 350

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+
+	if (val->intval == INT_MAX || val->intval == INT_MIN)
+		ret = -EINVAL;
+
+	return ret;
+}
+
+static enum power_supply_property bms_props[] = {
+	POWER_SUPPLY_PROP_CAPACITY,
+};

Why does the driver not expose the following battery information?

temperature info via POWER_SUPPLY_PROP_TEMP
ocv info via POWER_SUPPLY_PROP_VOLTAGE_OCV
CC info via POWER_SUPPLY_PROP_CHARGE_COUNTER
max current via POWER_SUPPLY_PROP_CURRENT_MAX

drivers/power/supply/qcom_bms.c line 361

Quoted 11 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+static irqreturn_t bms_ocv_thr_irq_handler(int irq, void *dev_id)
+{
+	struct bms_device_info *di = dev_id;
+
+	if (bms_read_ocv(di, &di->ocv) < 0)
+		return IRQ_HANDLED;
+
+	bms_reset_cc(di);
+	return IRQ_HANDLED;
+}

You want to call power_supply_changed() here to notify userspace.

Quoted 15 lines

drivers/power/supply/qcom_bms.c new file +14 -0

+static int bms_probe(struct platform_device *pdev)
+{
+	struct power_supply_config psy_cfg = {};
+	struct bms_device_info *di;
+	struct power_supply *bat;
+	int ret;
+
+	di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL);
+	if (!di)
+		return -ENOMEM;
+
+	di->dev = &pdev->dev;
+
+	di->regmap = dev_get_regmap(pdev->dev.parent, NULL);

drivers/power/supply/qcom_bms.c

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+	if (!di->regmap) {
+		dev_err(di->dev, "Unable to get regmap");
+		return -EINVAL;
+	}
+
+	di->adc = devm_iio_channel_get(&pdev->dev, "temp");
+	if (IS_ERR(di->adc))
+		return PTR_ERR(di->adc);
+
+	ret = of_property_read_u32(di->dev->of_node, "reg",

&di->base_addr);

drivers/power/supply/qcom_bms.c

Quoted 9 lines

drivers/power/supply/qcom_bms.c new file +9 -0

+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u8_array(di->dev->of_node,
+						 "qcom,ocv-temp-legend-celsius",
+						 (u8 *)di->ocv_lut.temp_legend,
+						 TEMPERATURE_COLS);
+	if (ret < 0) {
+		dev_err(di->dev, "no open circuit voltage temperature legend

found");

drivers/power/supply/qcom_bms.c

Quoted 4 lines

drivers/power/supply/qcom_bms.c new file +4 -0

+		return ret;
+	}
+
+	di->ocv_lut.rows =

of_property_read_variable_u8_array(di->dev->of_node,

Quoted 15 lines

drivers/power/supply/qcom_bms.c new file +15 -0

+						 "qcom,ocv-capacity-legend",
+						 di->ocv_lut.capacity_legend, 0,
+						 MAX_CAPACITY_ROWS);
+	if (di->ocv_lut.rows < 0) {
+		dev_err(di->dev, "no open circuit voltage capacity legend found");
+		return ret;
+	}
+
+	ret = of_property_read_variable_u32_array(di->dev->of_node,
+						  "qcom,ocv-lut-microvolt",
+						  (u32 *)di->ocv_lut.lut,
+						  TEMPERATURE_COLS,
+						  TEMPERATURE_COLS *
+						  MAX_CAPACITY_ROWS);
+	if (ret < 0) {

drivers/power/supply/qcom_bms.c

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+		dev_err(di->dev, "no open circuit voltage lut array found");
+		return ret;
+	}
+
+	ret = of_property_read_u8_array(di->dev->of_node,
+						 "qcom,fcc-temp-legend-celsius",
+						 (u8 *)di->fcc_lut.temp_legend,
+						 TEMPERATURE_COLS);
+	if (ret < 0) {
+		dev_err(di->dev, "no full charge capacity temperature legend

found");

Quoted 5 lines

drivers/power/supply/qcom_bms.c new file +5 -0

+		return ret;
+	}
+
+	ret = of_property_read_u32_array(di->dev->of_node,
+						  "qcom,fcc-lut-microamp-hours",

drivers/power/supply/qcom_bms.c

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+						  di->fcc_lut.lut,
+						  TEMPERATURE_COLS);
+	if (ret < 0) {
+		dev_err(di->dev, "no full charge capacity lut array found");
+		return ret;
+	}
+
+	ret = bms_read_ocv(di, &di->ocv);
+	if (ret < 0) {
+		dev_err(di->dev, "failed to read initial open circuit voltage:

%d",

Quoted 4 lines

drivers/power/supply/qcom_bms.c new file +4 -0

+			ret);
+		return ret;
+	}
+

drivers/power/supply/qcom_bms.c

Quoted 10 lines

drivers/power/supply/qcom_bms.c new file +10 -0

+	mutex_init(&di->bms_output_lock);
+
+	di->ocv_thr_irq = platform_get_irq_byname(pdev, "ocv_thr");
+
+	ret = devm_request_threaded_irq(di->dev, di->ocv_thr_irq, NULL,
+					bms_ocv_thr_irq_handler,
+					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					pdev->name, di);
+	if (ret < 0) {
+		dev_err(di->dev, "failed to request handler for open circuit

voltage threshold IRQ");

drivers/power/supply/qcom_bms.c line 157

Quoted 4 lines

drivers/power/supply/qcom_bms.c new file +4 -0

+		return ret;
+	}
+
+

one empty line is enough.

drivers/power/supply/qcom_bms.c line 464

Quoted 7 lines

drivers/power/supply/qcom_bms.c new file +7 -0

+	di->bat_desc.name = "bms";
+	di->bat_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	di->bat_desc.properties = bms_props;
+	di->bat_desc.num_properties = ARRAY_SIZE(bms_props);
+	di->bat_desc.get_property = bms_get_property;
+
+	psy_cfg.drv_data = di;

psy_cfg.of_node = pdev->dev.of_node;

Quoted 19 lines

drivers/power/supply/qcom_bms.c new file +19 -0

+	bat = devm_power_supply_register(di->dev, &di->bat_desc, &psy_cfg);
+
+	return PTR_ERR_OR_ZERO(bat);
+}
+
+static const struct of_device_id bms_of_match[] = {
+	{.compatible = "qcom,pm8941-bms", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bms_of_match);
+
+static struct platform_driver bms_driver = {
+	.probe = bms_probe,
+	.driver = {
+		.name = "qcom-bms",
+		.of_match_table = of_match_ptr(bms_of_match),
+	},
+};
+module_platform_driver(bms_driver);