Skip to contentSkip to conversation

power: supply: Add Support for RK817 Charger

PATCH Superseded Chris Morgan 3 participants 15 messages Apr 4, 2022 – Apr 6, 2022

Subsystems (4)

v5 of 10 Superseded Latest is v10 →

lore
Share & tools
lore
#0 lore raw

v5 cover power: supply: Add Support for RK817 Charger

From: Chris Morgan <[email protected]>

This series is to add support for the Rockchip rk817 battery charger which is present in all Rockchip RK817 PMICs. The driver was written as a joint effort by Maya Matuszczyk <[email protected]> and myself Chris Morgan <[email protected]>.

The driver requires some basic parameters be described about the battery in the devicetree such as the maximum capacity, the minimum and maximum voltage for the battery, the maximum charge current, the maximum charge voltage, and the value of sample resistors and sleep currents.

The hardware itself contains an ADC capable of measuring the voltage, current, and temperature of the battery (though my implementation of an Odroid Go Advance lacks a thermistor for temperature). It also contains a columb counter, registers for tracking the measured voltage and current at boot, and a few bytes of nvram for storing data.

Changes from V4:
- Massively redesigned the battery driver to improve measurement accuracy and reliability. Driver now checks values every 8 seconds (mimicking BSP driver behavior), or whenever a plug event is detected.
- Removed OCV, boot voltage, and boot current as values were found to be unreliable.
- Updated first-boot behavior to perform a "best guess" at capacity.
- Added ability to calibrate columb counter to full state of charge when charger reports full, and added ability calibrate full charge capacity when battery discharges from full charge to minimum voltage.
- Expose state of charge as capacity (in percents).
- Updated storing of values to nvram to preserve compatibility with BSP kernel.

Changes from V3:

- Corrected issues in device tree documentation.
- Added additional logic to battery to correct for columb counter drift when the device stays plugged in at a full charge state.

Changes from V2:

- Updated devicetree bindings to use common property units.
- Removed unneeded includes.
- Updated rk817_chg_cur_to_reg to make more legible.
- Simplified formula for displaying calibrated voltage.
- Updated power supply type to POWER_SUPPLY_TYPE_USB.
- Implemented get/put_unaligned macros for bulk reads and writes.
- Changed numerous dev_err() to dev_err_probe().
- Call power_supply_put_battery_info() at end of probe function.
- Removed unneeded whitespace.

Changes from V1:

- Fixed a CLANG warning regarding an uninitalized variable.
- Fixed a CLANG warning regarding a pointer as a bool value always returning as true.
- Added Maya Matuszczyk to the Signed-off-by.

Chris Morgan (4):
dt-bindings: Add Rockchip rk817 battery charger support
mfd: Add Rockchip rk817 battery charger support
power: supply: Add charger driver for Rockchip RK817
arm64: dts: rockchip: add rk817 chg to Odroid Go Advance

 .../bindings/mfd/rockchip,rk817.yaml          |   48 +
 .../boot/dts/rockchip/rk3326-odroid-go2.dts   |   26 +
 drivers/mfd/rk808.c                           |   16 +-
 drivers/power/supply/Kconfig                  |    6 +
 drivers/power/supply/Makefile                 |    1 +
 drivers/power/supply/rk817_charger.c          | 1157 +++++++++++++++++
 include/linux/mfd/rk808.h                     |   91 ++
 7 files changed, 1344 insertions(+), 1 deletion(-)
 create mode 100644 drivers/power/supply/rk817_charger.c
Signature · 7 lines
2.25.1


_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
#1 lore raw

PATCH v5 1/4 dt-bindings: Add Rockchip rk817 battery charger support

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +48 -0
index bfc1720adc43..b949d406a487 100644
@@ -117,6 +117,47 @@ properties:
         description:
           Describes if the microphone uses differential mode.
 
+  battery:
+    description: |
+      The child node for the charger to hold additional properties. If a
+      battery is not in use, this node can be omitted.
+    type: object
+    properties:
+      monitored-battery:
+        description: |
+          A phandle to a monitored battery node that contains a valid
+          value for:
+          charge-full-design-microamp-hours,
+          charge-term-current-microamp,
+          constant-charge-current-max-microamp,
+          constant-charge-voltage-max-microvolt,
+          voltage-max-design-microvolt,
+          voltage-min-design-microvolt,
+          and a valid ocv-capacity table.
+
+      rockchip,resistor-sense-micro-ohms:
+        description: |
+          Value in microohms of the battery sense resistor. This value is
+          used by the driver to set the correct divisor value to translate
+          ADC readings into the proper units of measure.
+        enum: [10000, 20000]
+
+      rockchip,sleep-enter-current-microamp:
+        description: |
+          Value in microamps of the sleep enter current for the charger.
+          Value is used by the driver to calibrate the relax threshold.
+
+      rockchip,sleep-filter-current-microamp:
+        description:
+          Value in microamps of the sleep filter current for the charger.
+          Value is used by the driver to derive the sleep sample current.
+
+    required:
+      - monitored-battery
+      - rockchip,resistor-sense-micro-ohms
+      - rockchip,sleep-enter-current-microamp
+      - rockchip,sleep-filter-current-microamp
+
 allOf:
   - if:
       properties:
@@ -323,6 +364,13 @@ examples:
                 };
             };
 
+            rk817_battery: battery {
+                monitored-battery = <&battery_cell>;
+                rockchip,resistor-sense-micro-ohms = <10000>;
+                rockchip,sleep-enter-current-microamp = <300000>;
+                rockchip,sleep-filter-current-microamp = <100000>;
+            };
+
             rk817_codec: codec {
                 rockchip,mic-in-differential;
             };
Signature · 7 lines
2.25.1


_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
#2 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 23 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +0 -0
index bfc1720adc43..b949d406a487 100644

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml properties: · line 120

Quoted 4 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
@@ -117,6 +117,47 @@ properties:
         description:
           Describes if the microphone uses differential mode.
 
+  battery:

I wonder why do you call it a batter while it is a charger, isn't it?

Quoted 44 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +40 -0
+    description: |
+      The child node for the charger to hold additional properties. If a
+      battery is not in use, this node can be omitted.
+    type: object
+    properties:
+      monitored-battery:
+        description: |
+          A phandle to a monitored battery node that contains a valid
+          value for:
+          charge-full-design-microamp-hours,
+          charge-term-current-microamp,
+          constant-charge-current-max-microamp,
+          constant-charge-voltage-max-microvolt,
+          voltage-max-design-microvolt,
+          voltage-min-design-microvolt,
+          and a valid ocv-capacity table.
+
+      rockchip,resistor-sense-micro-ohms:
+        description: |
+          Value in microohms of the battery sense resistor. This value is
+          used by the driver to set the correct divisor value to translate
+          ADC readings into the proper units of measure.
+        enum: [10000, 20000]
+
+      rockchip,sleep-enter-current-microamp:
+        description: |
+          Value in microamps of the sleep enter current for the charger.
+          Value is used by the driver to calibrate the relax threshold.
+
+      rockchip,sleep-filter-current-microamp:
+        description:
+          Value in microamps of the sleep filter current for the charger.
+          Value is used by the driver to derive the sleep sample current.
+
+    required:
+      - monitored-battery
+      - rockchip,resistor-sense-micro-ohms
+      - rockchip,sleep-enter-current-microamp
+      - rockchip,sleep-filter-current-microamp
+
 allOf:
   - if:
       properties:

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml examples: · line 367

Quoted 4 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
@@ -323,6 +364,13 @@ examples:
                 };
             };
 
+            rk817_battery: battery {

The same.

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml examples: · line 376

Quoted 9 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +6 -0
+                monitored-battery = <&battery_cell>;
+                rockchip,resistor-sense-micro-ohms = <10000>;
+                rockchip,sleep-enter-current-microamp = <300000>;
+                rockchip,sleep-filter-current-microamp = <100000>;
+            };
+
             rk817_codec: codec {
                 rockchip,mic-in-differential;
             };

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#3 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 27 lines · Krzysztof Kozlowski
Quoted 26 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +0 -0
index bfc1720adc43..b949d406a487 100644
@@ -117,6 +117,47 @@ properties:
         description:
           Describes if the microphone uses differential mode.
 
Quoted 3 lines

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml properties: · line 120

Quoted 1 line
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
+  battery:

I wonder why do you call it a batter while it is a charger, isn't it?

It is a driver for both the battery and charger. I'd argue about 95% of it is battery functions and the other 5% is managing the IRQs for plug removal/insertion and capturing the incoming voltage and current. In the BSP kernel these were two seperate drivers, but there was so little that needed to be done for the charger (and users probably don't need plug IRQs if they aren't using a battery anyway since the system will shut off on a plug out event due to no power...).

Quoted 63 lines
Quoted 45 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +40 -0
+    description: |
+      The child node for the charger to hold additional properties. If a
+      battery is not in use, this node can be omitted.
+    type: object
+    properties:
+      monitored-battery:
+        description: |
+          A phandle to a monitored battery node that contains a valid
+          value for:
+          charge-full-design-microamp-hours,
+          charge-term-current-microamp,
+          constant-charge-current-max-microamp,
+          constant-charge-voltage-max-microvolt,
+          voltage-max-design-microvolt,
+          voltage-min-design-microvolt,
+          and a valid ocv-capacity table.
+
+      rockchip,resistor-sense-micro-ohms:
+        description: |
+          Value in microohms of the battery sense resistor. This value is
+          used by the driver to set the correct divisor value to translate
+          ADC readings into the proper units of measure.
+        enum: [10000, 20000]
+
+      rockchip,sleep-enter-current-microamp:
+        description: |
+          Value in microamps of the sleep enter current for the charger.
+          Value is used by the driver to calibrate the relax threshold.
+
+      rockchip,sleep-filter-current-microamp:
+        description:
+          Value in microamps of the sleep filter current for the charger.
+          Value is used by the driver to derive the sleep sample current.
+
+    required:
+      - monitored-battery
+      - rockchip,resistor-sense-micro-ohms
+      - rockchip,sleep-enter-current-microamp
+      - rockchip,sleep-filter-current-microamp
+
 allOf:
   - if:
       properties:

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml examples: · line 367

Quoted 4 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
@@ -323,6 +364,13 @@ examples:
                 };
             };
 
+            rk817_battery: battery {

The same.

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml examples: · line 376

Quoted 9 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +6 -0
+                monitored-battery = <&battery_cell>;
+                rockchip,resistor-sense-micro-ohms = <10000>;
+                rockchip,sleep-enter-current-microamp = <300000>;
+                rockchip,sleep-filter-current-microamp = <100000>;
+            };
+
             rk817_codec: codec {
                 rockchip,mic-in-differential;
             };
Quoted 2 lines

Best regards,
Krzysztof

Thank you.

I forgot to note in my patch notes that this relies on this series being upstreamed to convert the rockchip rk808 bindings from txt into yaml:
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]/

That series is mostly upstreamed (the patches fixing issues in various devicetrees). All that remains is the main one switching to yaml.

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#4 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 32 lines · Chris Morgan
Quoted 27 lines · Krzysztof Kozlowski
Quoted 26 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +0 -0
index bfc1720adc43..b949d406a487 100644
@@ -117,6 +117,47 @@ properties:
         description:
           Describes if the microphone uses differential mode.
 
Quoted 3 lines

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml properties: · line 120

Quoted 1 line
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
+  battery:

I wonder why do you call it a batter while it is a charger, isn't it?

Quoted 7 lines

It is a driver for both the battery and charger. I'd argue about 95% of it is battery functions and the other 5% is managing the IRQs for plug removal/insertion and capturing the incoming voltage and current. In the BSP kernel these were two seperate drivers, but there was so little that needed to be done for the charger (and users probably don't need plug IRQs if they aren't using a battery anyway since the system will shut off on a plug out event due to no power...).

What do you mean by driver for "battery"? Like some smart-battery system? with embedded battery (RK817 comes with embedded battery) Or a fuel gauge? Judging by power supply properties it looks like fuel gauge.

"Battery" should rather be used for the node referenced by "monitored-battery"...

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#5 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 41 lines · Krzysztof Kozlowski
Quoted 32 lines · Chris Morgan
Quoted 27 lines · Krzysztof Kozlowski
Quoted 26 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +0 -0
index bfc1720adc43..b949d406a487 100644
@@ -117,6 +117,47 @@ properties:
         description:
           Describes if the microphone uses differential mode.
 
Quoted 3 lines

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml properties: · line 120

Quoted 1 line
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
+  battery:

I wonder why do you call it a batter while it is a charger, isn't it?

Quoted 7 lines

It is a driver for both the battery and charger. I'd argue about 95% of it is battery functions and the other 5% is managing the IRQs for plug removal/insertion and capturing the incoming voltage and current. In the BSP kernel these were two seperate drivers, but there was so little that needed to be done for the charger (and users probably don't need plug IRQs if they aren't using a battery anyway since the system will shut off on a plug out event due to no power...).

Quoted 3 lines

What do you mean by driver for "battery"? Like some smart-battery system? with embedded battery (RK817 comes with embedded battery) Or a fuel gauge? Judging by power supply properties it looks like fuel gauge.

It's basically just additional registers in the rk817 PMIC. The PMIC is controlled via I2C and contains multiple regulators, an audio codec, an RTC, some GPIO, some nvram, a columb counter for the battery, and an ADC for measuring input current/voltage/temperature (for battery or charger). This driver deals with the columb counter, the ADC, and a few bits of nvram for storing battery values to retain backwards compatibility with the BSP kernel and bootloader, and handling IRQs related to inserting or removing the charger (curiously my implementation ALSO has a charger sense GPIO in addition to the PMIC being able to sense that).

The driver itself is named rk817_charger. If you think I should change this from battery to "fuel gauge" or "charger" let me know and I can resubmit. Whatever makes it clearer for everyone.

Thank you.

Quoted 6 lines

"Battery" should rather be used for the node referenced by "monitored-battery"...

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#6 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 46 lines · Chris Morgan
Quoted 41 lines · Krzysztof Kozlowski
Quoted 32 lines · Chris Morgan
Quoted 27 lines · Krzysztof Kozlowski
Quoted 26 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +0 -0
index bfc1720adc43..b949d406a487 100644
@@ -117,6 +117,47 @@ properties:
         description:
           Describes if the microphone uses differential mode.
 
Quoted 3 lines

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml properties: · line 120

Quoted 1 line
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
+  battery:

I wonder why do you call it a batter while it is a charger, isn't it?

Quoted 7 lines

It is a driver for both the battery and charger. I'd argue about 95% of it is battery functions and the other 5% is managing the IRQs for plug removal/insertion and capturing the incoming voltage and current. In the BSP kernel these were two seperate drivers, but there was so little that needed to be done for the charger (and users probably don't need plug IRQs if they aren't using a battery anyway since the system will shut off on a plug out event due to no power...).

Quoted 3 lines

What do you mean by driver for "battery"? Like some smart-battery system? with embedded battery (RK817 comes with embedded battery) Or a fuel gauge? Judging by power supply properties it looks like fuel gauge.

Quoted 5 lines

It's basically just additional registers in the rk817 PMIC. The PMIC is controlled via I2C and contains multiple regulators, an audio codec, an RTC, some GPIO, some nvram, a columb counter for the battery, and an ADC for measuring input current/voltage/temperature (for battery or charger).

Actually you mentioned it in the cover letter - it's a PMIC, not a battery. I doubt that it's embedded with a battery. :)

Quoted 10 lines

This driver deals with the columb counter, the ADC, and a few bits of nvram for storing battery values to retain backwards compatibility with the BSP kernel and bootloader, and handling IRQs related to inserting or removing the charger (curiously my implementation ALSO has a charger sense GPIO in addition to the PMIC being able to sense that).

The driver itself is named rk817_charger. If you think I should change this from battery to "fuel gauge" or "charger" let me know and I can resubmit. Whatever makes it clearer for everyone.

Yeah, the property name and bindings should describe the hardware, so in such case the hardware is rather a "charger" or "fuel-gauge". Your "battery-cell" from DTS is probably just a "battery" (unless you expect multiple cells?).

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#7 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 67 lines · Krzysztof Kozlowski
Quoted 46 lines · Chris Morgan
Quoted 41 lines · Krzysztof Kozlowski
Quoted 32 lines · Chris Morgan
Quoted 27 lines · Krzysztof Kozlowski
Quoted 26 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +0 -0
index bfc1720adc43..b949d406a487 100644
@@ -117,6 +117,47 @@ properties:
         description:
           Describes if the microphone uses differential mode.
 
Quoted 3 lines

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml properties: · line 120

Quoted 1 line
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +1 -0
+  battery:

I wonder why do you call it a batter while it is a charger, isn't it?

Quoted 7 lines

It is a driver for both the battery and charger. I'd argue about 95% of it is battery functions and the other 5% is managing the IRQs for plug removal/insertion and capturing the incoming voltage and current. In the BSP kernel these were two seperate drivers, but there was so little that needed to be done for the charger (and users probably don't need plug IRQs if they aren't using a battery anyway since the system will shut off on a plug out event due to no power...).

Quoted 3 lines

What do you mean by driver for "battery"? Like some smart-battery system? with embedded battery (RK817 comes with embedded battery) Or a fuel gauge? Judging by power supply properties it looks like fuel gauge.

Quoted 5 lines

It's basically just additional registers in the rk817 PMIC. The PMIC is controlled via I2C and contains multiple regulators, an audio codec, an RTC, some GPIO, some nvram, a columb counter for the battery, and an ADC for measuring input current/voltage/temperature (for battery or charger).

Actually you mentioned it in the cover letter - it's a PMIC, not a battery. I doubt that it's embedded with a battery. :)

Quoted 10 lines

This driver deals with the columb counter, the ADC, and a few bits of nvram for storing battery values to retain backwards compatibility with the BSP kernel and bootloader, and handling IRQs related to inserting or removing the charger (curiously my implementation ALSO has a charger sense GPIO in addition to the PMIC being able to sense that).

The driver itself is named rk817_charger. If you think I should change this from battery to "fuel gauge" or "charger" let me know and I can resubmit. Whatever makes it clearer for everyone.

Quoted 7 lines

Yeah, the property name and bindings should describe the hardware, so in such case the hardware is rather a "charger" or "fuel-gauge". Your "battery-cell" from DTS is probably just a "battery" (unless you expect multiple cells?).

Best regards,
Krzysztof

Okay, when v6 comes around I'll change it to be "charger" instead of "battery" to make it more clear. There should only be a single battery instead of multiple cells, and according to the documentation I should be okay with describing the battery in the devicetree since it's not something easy for the end-user to change.

I'd like to get someone to look at the meat and potatoes of the series before I submit a v6... I did a fairly substantial rewrite of the actual rk817_charger.c to solve for several problems and fix several bugs I found in extended testing. One of the major changes was to mirror the BSP in that I poll the PMIC every 8 seconds for updates and then store it in the driver struct rather than pull each value on demand as requested. I see other drivers doing this but I want to make sure that's acceptable upstream.

Thank you.

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#8 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 9 lines · Chris Morgan
Quoted 9 lines
Quoted 3 lines

The driver itself is named rk817_charger. If you think I should change this from battery to "fuel gauge" or "charger" let me know and I can resubmit. Whatever makes it clearer for everyone.

Yeah, the property name and bindings should describe the hardware, so in such case the hardware is rather a "charger" or "fuel-gauge". Your "battery-cell" from DTS is probably just a "battery" (unless you expect multiple cells?).

Quoted 8 lines
Quoted 2 lines

Best regards,
Krzysztof

Okay, when v6 comes around I'll change it to be "charger" instead of "battery" to make it more clear. There should only be a single battery instead of multiple cells, and according to the documentation I should be okay with describing the battery in the devicetree since it's not something easy for the end-user to change.

Yes,the description of battery fits the purpose of DT.

Quoted 9 lines

I'd like to get someone to look at the meat and potatoes of the series before I submit a v6... I did a fairly substantial rewrite of the actual rk817_charger.c to solve for several problems and fix several bugs I found in extended testing. One of the major changes was to mirror the BSP in that I poll the PMIC every 8 seconds for updates and then store it in the driver struct rather than pull each value on demand as requested. I see other drivers doing this but I want to make sure that's acceptable upstream.

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#9 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

In reply to Chris Morgan
Quoted 4 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

"Add new devicetree properties." instead of mixing tenses.

Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml properties: · line 159

Quoted 11 lines
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml +11 -0
+
+      rockchip,sleep-filter-current-microamp:
+        description:
+          Value in microamps of the sleep filter current for the charger.
+          Value is used by the driver to derive the sleep sample current.
+
+    required:
+      - monitored-battery
+      - rockchip,resistor-sense-micro-ohms
+      - rockchip,sleep-enter-current-microamp
+      - rockchip,sleep-filter-current-microamp

additionalProperties:false (on required-level of indentation)

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

maintainer
#10 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

In reply to Chris Morgan
Quoted 12 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

Quoted 5 lines
---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

Doesn't apply for me. What tree is this based on?

Rob

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#11 lore raw

Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

Quoted 13 lines · Rob Herring
Quoted 12 lines · Chris Morgan

From: Chris Morgan <[email protected]>

Create dt-binding documentation to document rk817 battery and charger usage. New device-tree properties have been added.

- rockchip,resistor-sense-micro-ohms: The value in microohms of the sample resistor.
- rockchip,sleep-enter-current-microamp: The value in microamps of the sleep enter current.
- rockchip,sleep-filter-current: The value in microamps of the sleep filter current.

Quoted 7 lines
Quoted 5 lines
---
 .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

Doesn't apply for me. What tree is this based on?

Sorry, it relies on the following being applied (all the prerequisites are now in master, but this one is still absent):

https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]/

Quoted 2 lines

Rob

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#12 lore raw

PATCH v5 2/4 mfd: Add Rockchip rk817 battery charger support

In reply to Chris Morgan

From: Chris Morgan <[email protected]>

Add rk817 charger support cell to rk808 mfd driver.

---
 drivers/mfd/rk808.c       | 16 ++++++-
 include/linux/mfd/rk808.h | 91 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 1 deletion(-)
drivers/mfd/rk808.c +15 -1
index 4142b638e5fa..283a65b64d2c 100644
@@ -67,6 +67,10 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg)
 	case RK817_SECONDS_REG ... RK817_WEEKS_REG:
 	case RK817_RTC_STATUS_REG:
 	case RK817_CODEC_DTOP_LPT_SRST:
+	case RK817_GAS_GAUGE_ADC_CONFIG0 ... RK817_GAS_GAUGE_CUR_ADC_K0:
+	case RK817_PMIC_CHRG_STS:
+	case RK817_PMIC_CHRG_OUT:
+	case RK817_PMIC_CHRG_IN:
 	case RK817_INT_STS_REG0:
 	case RK817_INT_STS_REG1:
 	case RK817_INT_STS_REG2:
@@ -74,7 +78,7 @@ static bool rk817_is_volatile_reg(struct device *dev, unsigned int reg)
 		return true;
 	}
 
-	return true;
+	return false;
 }
 
 static const struct regmap_config rk818_regmap_config = {
@@ -127,6 +131,11 @@ static const struct resource rk817_pwrkey_resources[] = {
 	DEFINE_RES_IRQ(RK817_IRQ_PWRON_FALL),
 };
 
+static const struct resource rk817_charger_resources[] = {
+	DEFINE_RES_IRQ(RK817_IRQ_PLUG_IN),
+	DEFINE_RES_IRQ(RK817_IRQ_PLUG_OUT),
+};
+
 static const struct mfd_cell rk805s[] = {
 	{ .name = "rk808-clkout", },
 	{ .name = "rk808-regulator", },
@@ -166,6 +175,11 @@ static const struct mfd_cell rk817s[] = {
 		.resources = &rk817_rtc_resources[0],
 	},
 	{ .name = "rk817-codec",},
+	{
+		.name = "rk817-charger",
+		.num_resources = ARRAY_SIZE(rk817_charger_resources),
+		.resources = &rk817_charger_resources[0],
+	},
 };
 
 static const struct mfd_cell rk818s[] = {
include/linux/mfd/rk808.h +91 -0

107 lines not on this page · raw message

Signature · 7 lines
2.25.1


_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
#13 lore raw

PATCH v5 3/4 power: supply: Add charger driver for Rockchip RK817

In reply to Chris Morgan

From: Chris Morgan <[email protected]>

Add support for the Rockchip rk817 battery charger integrated into the rk817 PMIC.

---
 drivers/power/supply/Kconfig         |    6 +
 drivers/power/supply/Makefile        |    1 +
 drivers/power/supply/rk817_charger.c | 1157 ++++++++++++++++++++++++++
 3 files changed, 1164 insertions(+)
 create mode 100644 drivers/power/supply/rk817_charger.c
drivers/power/supply/Kconfig +6 -0
index 1aa8323ad9f6..56e70a68679a 100644
@@ -708,6 +708,12 @@ config CHARGER_BQ256XX
 	  charge management and system power path management devices for single
 	  cell Li-ion and Li-polymer batteries.
 
+config CHARGER_RK817
+	tristate "Rockchip RK817 PMIC Battery Charger"
+	depends on MFD_RK808
+	help
+	  Say Y to include support for Rockchip RK817 Battery Charger.
+
 config CHARGER_SMB347
 	tristate "Summit Microelectronics SMB3XX Battery Charger"
 	depends on I2C
drivers/power/supply/Makefile +1 -0
index 7f02f36aea55..3040a1de81b8 100644
@@ -91,6 +91,7 @@ obj-$(CONFIG_CHARGER_BQ2515X)	+= bq2515x_charger.o
 obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
 obj-$(CONFIG_CHARGER_BQ25980)	+= bq25980_charger.o
 obj-$(CONFIG_CHARGER_BQ256XX)	+= bq256xx_charger.o
+obj-$(CONFIG_CHARGER_RK817)	+= rk817_charger.o
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
 obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
drivers/power/supply/rk817_charger.c new file +1157 -0

1,157 lines not on this page · raw message

Signature · 7 lines
2.25.1


_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
#14 lore raw

PATCH v5 4/4 arm64: dts: rockchip: add rk817 chg to Odroid Go Advance

In reply to Chris Morgan

From: Chris Morgan <[email protected]>

Add the new rk817 charger driver to the Odroid Go Advance. Create a monitored battery node as well for the charger to use. All values from monitored battery are gathered from the BSP kernel for the Odroid Go Advance provided by HardKernel.

---
 .../boot/dts/rockchip/rk3326-odroid-go2.dts   | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)
arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts +26 -0
index ea0695b51ecd..4ccdb6dffa88 100644
@@ -52,6 +52,25 @@ backlight: backlight {
 		pwms = <&pwm1 0 25000 0>;
 	};
 
+	battery_cell: battery-cell {
+		compatible = "simple-battery";
+		charge-full-design-microamp-hours = <3000000>;
+		charge-term-current-microamp = <300000>;
+		constant-charge-current-max-microamp = <2000000>;
+		constant-charge-voltage-max-microvolt = <4200000>;
+		factory-internal-resistance-micro-ohms = <180000>;
+		voltage-max-design-microvolt = <4100000>;
+		voltage-min-design-microvolt = <3500000>;
+
+		ocv-capacity-celsius = <20>;
+		ocv-capacity-table-0 =	<4046950 100>, <4001920 95>, <3967900 90>, <3919950 85>,
+					<3888450 80>, <3861850 75>, <3831540 70>, <3799130 65>,
+					<3768190 60>, <3745650 55>, <3726610 50>, <3711630 45>,
+					<3696720 40>, <3685660 35>, <3674950 30>, <3663050 25>,
+					<3649470 20>, <3635260 15>, <3616920 10>, <3592440 5>,
+					<3574170 0>;
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";
@@ -472,6 +491,13 @@ usb_midu: BOOST {
 			};
 		};
 
+		rk817_battery: battery {
+			monitored-battery = <&battery_cell>;
+			rockchip,resistor-sense-micro-ohms = <10000>;
+			rockchip,sleep-enter-current-microamp = <300000>;
+			rockchip,sleep-filter-current-microamp = <100000>;
+		};
+
 		rk817_codec: codec {
 			rockchip,mic-in-differential;
 		};
Signature · 7 lines
2.25.1


_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip