Skip to contentSkip to conversation

Support ROHM BD72720 PMIC

PATCH Superseded Matti Vaittinen 7 participants 31 messages Nov 13, 2025 – Nov 18, 2025

Subsystems (10)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v4 cover Support ROHM BD72720 PMIC

The ROHM BD72720 is a new power management IC for portable, battery powered devices. It integrates 10 BUCKs and 11 LDOs, RTC, charger, LEDs, GPIOs and a clock gate. To me the BD72720 seems like a successor to the BD71828 and BD71815 PMICs.

This series depends on
5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver") which is in power-supply tree, for-next. Thus, the series is based on it.

The testing of v4 suffered some hardware-issues after I accidentally enabled charging while the PMIC's battery pin was connected to the I/O domain. Some heat was generated, not terribly lot smoke though...

After the incident I've had occasional I2C failures. I, however, suspect the root cause is HW damage in I/O lines since changes in this revision have been made to dt-bindings. It's still fair to note that though, as my testing was impacted.

Revision history:
v3 => v4:
- dt-binding fixes to the BD72720 MFD example and regulator bindings More accurate changelog in individual patches

v2 => v3:
- rebased to power-supply/for-next as dependencies are merged to there
- plenty of dt-binding changes as suggested by reviewers
- add new patch to better document existing 'trickle-charging' property More accurate changelog in individual patches

RFCv1 => v2:
- Drop RFC status
- Use stacked regmaps to hide secondary map from the sub-drivers
- Quite a few styling fixes and improvements as suggested by reviewers. More accurate changelog in individual patches.
- Link to v1:

    https://lore.kernel.org/all/[email protected]/

---

Matti Vaittinen (16):
dt-bindings: regulator: ROHM BD72720
dt-bindings: battery: Clarify trickle-charge
dt-bindings: battery: Add trickle-charge upper limit
dt-bindings: power: supply: BD72720 managed battery
dt-bindings: mfd: ROHM BD72720
dt-bindings: leds: bd72720: Add BD72720
mfd: rohm-bd71828: Use regmap_reg_range()
mfd: bd71828: Support ROHM BD72720
regulator: bd71828: rename IC specific entities
regulator: bd71828: Support ROHM BD72720
gpio: Support ROHM BD72720 gpios
clk: clk-bd718x7: Support BD72720 clk gate
rtc: bd70528: Support BD72720 rtc
power: supply: bd71828: Support wider register addresses
power: supply: bd71828-power: Support ROHM BD72720
MAINTAINERS: Add ROHM BD72720 PMIC

 .../bindings/leds/rohm,bd71828-leds.yaml      |    7 +-
 .../bindings/mfd/rohm,bd72720-pmic.yaml       |  338 ++++++
 .../bindings/power/supply/battery.yaml        |   11 +-
 .../power/supply/rohm,vdr-battery.yaml        |   80 ++
 .../regulator/rohm,bd72720-regulator.yaml     |  148 +++
 MAINTAINERS                                   |    2 +
 drivers/clk/Kconfig                           |    4 +-
 drivers/clk/clk-bd718x7.c                     |   10 +-
 drivers/gpio/Kconfig                          |    9 +
 drivers/gpio/Makefile                         |    1 +
 drivers/gpio/gpio-bd72720.c                   |  281 +++++
 drivers/mfd/Kconfig                           |   18 +-
 drivers/mfd/rohm-bd71828.c                    |  546 ++++++++-
 drivers/power/supply/bd71828-power.c          |  160 ++-
 drivers/regulator/Kconfig                     |    8 +-
 drivers/regulator/bd71828-regulator.c         | 1025 ++++++++++++++++-
 drivers/rtc/Kconfig                           |    3 +-
 drivers/rtc/rtc-bd70528.c                     |   21 +-
 include/linux/mfd/rohm-bd72720.h              |  634 ++++++++++
 include/linux/mfd/rohm-generic.h              |    1 +
 20 files changed, 3177 insertions(+), 130 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml
 create mode 100644 Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
 create mode 100644 drivers/gpio/gpio-bd72720.c
 create mode 100644 include/linux/mfd/rohm-bd72720.h

base-commit: 8e8856396b54bea5c00a7ae88d87c6254aef2d94

Signature · 1 line
2.51.1
#1 lore raw

PATCH v4 1/16 dt-bindings: regulator: ROHM BD72720

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 is a new PMIC with 10 BUCk and 11 LDO regulators.

The BD72720 is designed to support using the BUCK10 as a supply for the LDOs 1 to 4. When the BUCK10 is used for this, it can be set to a LDON_HEAD mode. In this mode, the BUCK10 voltage can't be controlled by software, but the voltage is adjusted by PMIC to match the LDO1 .. LDO4 voltages with a given offset. Offset can be 50mV .. 300mV and is changeable at 50mV steps.

Add 'ldon-head-microvolt' property to denote a board which is designed to utilize the LDON_HEAD mode.

All other properties are already existing.

Add dt-binding doc for ROHM BD72720 regulators to make it usable.

---
Revision history:
v3 => v4:
- Drop type from ldon-head
- Fix the name patterns for regulator nodes and names

v2 => v3:
- drop unnecessary descriptions
- use microvolts for the 'ldon-head' dt-property

RFCv1 => v2:
- No changes

---
 .../regulator/rohm,bd72720-regulator.yaml     | 148 ++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml new file +148 -0
new file mode 100644
index 000000000000..5518082129bd
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd72720-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD72720 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description: |
+  This module is part of the ROHM BD72720 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml.
+
+  The regulator controller is represented as a sub-node of the PMIC node
+  on the device tree.
+
+  Regulator nodes should be named to BUCK_<number> and LDO_<number>.
+  The valid names for BD72720 regulator nodes are
+  buck1, buck2, buck3, buck4, buck5, buck6, buck7, buck8, buck9, buck10
+  ldo1, ldo2, ldo3, ldo4, ldo5, ldo6, ldo7, ldo8, ldo9, ldo10, ldo11
+
+patternProperties:
+  "^ldo([1-9]|1[0-1])$":
+    type: object
+    description:
+      Properties for single LDO regulator.
+    $ref: regulator.yaml#
+
+    properties:
+      regulator-name:
+        pattern: "^ldo([1-9]|1[0-1])$"
+
+      rohm,dvs-run-voltage:
+        description:
+          PMIC default "RUN" state voltage in uV. See below table for
+          LDOs which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+      rohm,dvs-idle-voltage:
+        description:
+          PMIC default "IDLE" state voltage in uV. See below table for
+          LDOs which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+      rohm,dvs-suspend-voltage:
+        description:
+          PMIC default "SUSPEND" state voltage in uV. See below table for
+          LDOs which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+      rohm,dvs-lpsr-voltage:
+        description:
+          PMIC default "deep-idle" state voltage in uV. See below table for
+          LDOs which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+        # Supported default DVS states:
+        #     ldo        |    run     |   idle    | suspend   | lpsr
+        # --------------------------------------------------------------
+        # 1, 2, 3, and 4 | supported  | supported | supported | supported
+        # --------------------------------------------------------------
+        # 5 - 11         |                    supported (*)
+        # --------------------------------------------------------------
+        #
+        # (*) All states use same voltage but have own enable / disable
+        #      settings. Voltage 0 can be specified for a state to make
+        #      regulator disabled on that state.
+
+    unevaluatedProperties: false
+
+  "^buck([1-9]|10)$":
+    type: object
+    description:
+      Properties for single BUCK regulator.
+    $ref: regulator.yaml#
+
+    properties:
+      regulator-name:
+        pattern: "^buck([1-9]|10)$"
+
+      rohm,ldon-head-microvolt:
+        description:
+          Set this on boards where BUCK10 is used to supply LDOs 1-4. The bucki
+          voltage will be changed by the PMIC to follow the LDO output voltages
+          with the offset voltage given here. This will improve the LDO efficiency.
+        minimum: 50000
+        maximum: 300000
+
+      rohm,dvs-run-voltage:
+        description:
+          PMIC default "RUN" state voltage in uV. See below table for
+          bucks which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+      rohm,dvs-idle-voltage:
+        description:
+          PMIC default "IDLE" state voltage in uV. See below table for
+          bucks which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+      rohm,dvs-suspend-voltage:
+        description:
+          PMIC default "SUSPEND" state voltage in uV. See below table for
+          bucks which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+      rohm,dvs-lpsr-voltage:
+        description:
+          PMIC default "deep-idle" state voltage in uV. See below table for
+          bucks which support this. 0 means disabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3300000
+
+        # Supported default DVS states:
+        #     buck       |    run     |   idle    | suspend   | lpsr
+        # --------------------------------------------------------------
+        # 1, 2, 3, and 4 | supported  | supported | supported | supported
+        # --------------------------------------------------------------
+        # 5 - 10    |                    supported (*)
+        # --------------------------------------------------------------
+        #
+        # (*) All states use same voltage but have own enable / disable
+        #      settings. Voltage 0 can be specified for a state to make
+        #      regulator disabled on that state.
+
+    required:
+      - regulator-name
+
+    unevaluatedProperties: false
+
+additionalProperties: false
Signature · 1 line
2.51.1
#2 lore raw

Re: [PATCH v4 01/16] dt-bindings: regulator: ROHM BD72720

Quoted 30 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 is a new PMIC with 10 BUCk and 11 LDO regulators.

The BD72720 is designed to support using the BUCK10 as a supply for the LDOs 1 to 4. When the BUCK10 is used for this, it can be set to a LDON_HEAD mode. In this mode, the BUCK10 voltage can't be controlled by software, but the voltage is adjusted by PMIC to match the LDO1 .. LDO4 voltages with a given offset. Offset can be 50mV .. 300mV and is changeable at 50mV steps.

Add 'ldon-head-microvolt' property to denote a board which is designed to utilize the LDON_HEAD mode.

All other properties are already existing.

Add dt-binding doc for ROHM BD72720 regulators to make it usable.

---
Revision history:
v3 => v4:
- Drop type from ldon-head
- Fix the name patterns for regulator nodes and names

v2 => v3:
- drop unnecessary descriptions
- use microvolts for the 'ldon-head' dt-property

Quoted 7 lines

RFCv1 => v2:
- No changes

---
 .../regulator/rohm,bd72720-regulator.yaml     | 148 ++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):
Warning: Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml: Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/21e83fccf2d2422f4bea1c482dcd3cb1aeda4085.1763022807.git.mazziesaccount@gmail.com

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

maintainer Reviewed-by on v4 1/16
#3 lore raw

Re: [PATCH v4 01/16] dt-bindings: regulator: ROHM BD72720

In reply to Matti Vaittinen
Quoted 30 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 is a new PMIC with 10 BUCk and 11 LDO regulators.

The BD72720 is designed to support using the BUCK10 as a supply for the LDOs 1 to 4. When the BUCK10 is used for this, it can be set to a LDON_HEAD mode. In this mode, the BUCK10 voltage can't be controlled by software, but the voltage is adjusted by PMIC to match the LDO1 .. LDO4 voltages with a given offset. Offset can be 50mV .. 300mV and is changeable at 50mV steps.

Add 'ldon-head-microvolt' property to denote a board which is designed to utilize the LDON_HEAD mode.

All other properties are already existing.

Add dt-binding doc for ROHM BD72720 regulators to make it usable.

---
Revision history:
v3 => v4:
- Drop type from ldon-head
- Fix the name patterns for regulator nodes and names

v2 => v3:
- drop unnecessary descriptions
- use microvolts for the 'ldon-head' dt-property

Quoted 7 lines

RFCv1 => v2:
- No changes

---
 .../regulator/rohm,bd72720-regulator.yaml     | 148 ++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
#4 lore raw

PATCH v4 2/16 dt-bindings: battery: Clarify trickle-charge

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The term 'trickle-charging' is used to describe a very slow charging phase, where electrons "trickle-in" the battery.

There are two different use-cases for this type of charging. At least some Li-Ion batteries can benefit from very slow, constant current, pre-pre phase 'trickle-charging', if a battery is very empty.

Some other batteries use top-off phase 'trickle-charging', which is different from the above case.

The battery bindings use the term 'trickle-charge' without specifying which of the use-cases properties are addressing. This has already caused some confusion.

Clarify that the 'trickle-charge-current-microamp' refers to the first one, the "pre-pre" -charging use-case.

---
Revision history:
v3 => :
- No changes

v2 => v3:
- New patch

---
 .../devicetree/bindings/power/supply/battery.yaml          | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/power/supply/battery.yaml +6 -1
index 491488e7b970..bfb7b716ae13 100644
@@ -64,7 +64,12 @@ properties:
     description: battery design capacity
 
   trickle-charge-current-microamp:
-    description: current for trickle-charge phase
+    description: current for trickle-charge phase.
+      Please note that the trickle-charging here, refers "wake-up" or
+      "pre-pre" -charging, for very empty batteries. Similar term is also
+      used for "maintenance" or "top-off" -charging of batteries (like
+      NiMh bq24400) - that is different and not controlled by this
+      property.
 
   precharge-current-microamp:
     description: current for pre-charge phase
Signature · 1 line
2.51.1
#5 lore raw

PATCH v4 3/16 dt-bindings: battery: Add trickle-charge upper limit

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

Some of the chargers for lithium-ion batteries use a trickle-charging as a first charging phase for very empty batteries, to "wake-up" the battery. Trickle-charging is a low current, constant current phase. After the voltage of the very empty battery has reached an upper limit for trickle charging, the pre-charge phase is started with a higher current.

Allow defining the upper limit for trickle charging voltage, after which the charging should be changed to the pre-charging.

---
Revision history:
v3 => :
- No changes

v2 => v3:
- Clarify the 'trickle-charging' the property refers to is the "pre-pre" -phase charging.

RFCv1 => v2:
- No changes

---
 Documentation/devicetree/bindings/power/supply/battery.yaml | 4 ++++
 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/power/supply/battery.yaml +4 -0
index bfb7b716ae13..d1a2080557a0 100644
@@ -71,6 +71,10 @@ properties:
       NiMh bq24400) - that is different and not controlled by this
       property.
 
+  tricklecharge-upper-limit-microvolt:
+    description: limit when to change to precharge from trickle charge
+      Trickle-charging here refers "wake-up" or "pre-pre" -charging.
+
   precharge-current-microamp:
     description: current for pre-charge phase
 
Signature · 1 line
2.51.1
#6 lore raw

PATCH v4 4/16 dt-bindings: power: supply: BD72720 managed battery

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD72720 PMIC has a battery charger + coulomb counter block. These can be used to manage charging of a lithium-ion battery and to do fuel gauging.

ROHM has developed a so called "zero-correction" -algorithm to improve the fuel-gauging accuracy close to the point where battery is depleted. This relies on battery specific "VDR" tables, which are measured from the battery, and which describe the voltage drop rate. More thorough explanation about the "zero correction" and "VDR" parameters is here:
https://lore.kernel.org/all/[email protected]/

Document the VDR zero-correction specific battery properties used by the BD72720 and some other ROHM chargers.

---
NOTE:
Linus' rb-tag holds only if there's no further comments from Rob.

Revision history:
v3 =>:
- No changes

v2 => v3:
- Constrain VDR threshold voltage to 48V
- Use standard '-bp' -suffix for the rohm,volt-drop-soc

RFCv1 => v2:
- Add units to rohm,volt-drop-soc (tenths of %)
- Give real temperatures matching the VDR tables, instead of vague 'high', 'normal', 'low', 'very low'. (Add table of temperatures and use number matching the right temperature index in the VDR table name).
- Fix typoed 'algorithm' in commit message.

The parameters are describing the battery voltage drop rates - so they are properties of the battery, not the charger. Thus they do not belong in the charger node.

The right place for them is the battery node, which is described by the generic "battery.yaml". I was not comfortable with adding these properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR algorithm implemented. (And even though the algorithm is not charger specific, AFAICS, it is currently only used by some ROHM PMIC drivers).
- Technique of measuring the VDR tables for a battery is not widely known. AFAICS, only folks at ROHM are measuring those for some customer products. We do have those tables available for some of the products though (Kobo?).

---
 .../power/supply/rohm,vdr-battery.yaml        | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.yaml
Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.yaml new file +80 -0
new file mode 100644
index 000000000000..be2f65a892ea
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/rohm,vdr-battery.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Battery managed by the BD72720 PMIC
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description:
+  A battery which has VDR parameters measuerd for ROHM chargers.
+
+allOf:
+  - $ref: battery.yaml#
+
+properties:
+  rohm,voltage-vdr-thresh-microvolt:
+    description: Threshold for starting the VDR correction
+    maximum: 48000000
+
+  rohm,volt-drop-soc-bp:
+    description: Table of capacity values matching the values in VDR tables.
+      The value should be given as basis points, 1/100 of a percent.
+
+  rohm,volt-drop-temperatures-millicelsius:
+    description: An array containing the temperature in milli celsius, for each
+      of the VDR lookup table.
+
+patternProperties:
+  '^rohm,volt-drop-[0-9]-microvolt':
+    description: Table of the voltage drop rate (VDR) values. Each entry in the
+      table should match a capacity value in the rohm,volt-drop-soc table.
+      Furthermore, the values should be obtained for the temperature given in
+      rohm,volt-drop-temperatures-millicelsius table at index matching the
+      number in this table's name.
+
+additionalProperties: false
+
+examples:
+  - |
+    power {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      battery: battery {
+        compatible = "simple-battery";
+
+        ocv-capacity-celsius = <25>;
+        ocv-capacity-table-0 = <4200000 100 4184314 100 4140723 95 4099487 90
+          4060656 85 4024350 80 3991121 75 3954379 70 3913265 65 3877821 60
+          3855577 55 3837466 50 3822194 45 3809012 40 3795984 35 3780647 30
+          3760505 25 3741532 20 3718837 15 3696698 10 3690594 5 3581427 0>;
+
+        rohm,volt-drop-soc-bp = <10000 10000 9500 9000 8500 8000 7500 7000 6500 6000 5500 5000
+          4500 4000 3500 3000 2500 2000 1500 1000 500 0 (-500)>;
+
+        rohm,volt-drop-temperatures-millicelsius = <45000 25000 5000 0>;
+
+        rohm,volt-drop-0-microvolt =  <100 100 102 104 106 109 114 124
+          117 107 107 109 112 116 117 108 109 109 108 109 122 126 130>;
+
+        rohm,volt-drop-1-microvolt = <100 100 102 105 98 100 105 102
+          101 99 98 100 103 105 109 117 111 109 110 114 128 141 154>;
+
+        rohm,volt-drop-2-microvolt = <100 100 98 107 112 114 118 118 112
+          108 108 110 111 113 117 123 131 144 157 181 220 283 399>;
+
+        rohm,volt-drop-3-temp-microvolt = <86 86 105 109 114 110 115 115
+          110 108 110 112 114 118 124 134 136 160 177 201 241 322 403>;
+
+        rohm,voltage-vdr-thresh-microvolt = <4150000>;
+
+        charge-full-design-microamp-hours = <1799000>;
+        voltage-max-design-microvolt = <4200000>;
+        voltage-min-design-microvolt = <3500000>;
+        degrade-cycle-microamp-hours = <131>;
+      };
+    };
Signature · 1 line
2.51.1
#7 lore raw

Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

Quoted 53 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD72720 PMIC has a battery charger + coulomb counter block. These can be used to manage charging of a lithium-ion battery and to do fuel gauging.

ROHM has developed a so called "zero-correction" -algorithm to improve the fuel-gauging accuracy close to the point where battery is depleted. This relies on battery specific "VDR" tables, which are measured from the battery, and which describe the voltage drop rate. More thorough explanation about the "zero correction" and "VDR" parameters is here:
https://lore.kernel.org/all/[email protected]/

Document the VDR zero-correction specific battery properties used by the BD72720 and some other ROHM chargers.

---
NOTE:
Linus' rb-tag holds only if there's no further comments from Rob.

Revision history:
v3 =>:
- No changes

v2 => v3:
- Constrain VDR threshold voltage to 48V
- Use standard '-bp' -suffix for the rohm,volt-drop-soc

RFCv1 => v2:
- Add units to rohm,volt-drop-soc (tenths of %)
- Give real temperatures matching the VDR tables, instead of vague 'high', 'normal', 'low', 'very low'. (Add table of temperatures and use number matching the right temperature index in the VDR table name).
- Fix typoed 'algorithm' in commit message.

The parameters are describing the battery voltage drop rates - so they are properties of the battery, not the charger. Thus they do not belong in the charger node.

The right place for them is the battery node, which is described by the generic "battery.yaml". I was not comfortable with adding these properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR algorithm implemented. (And even though the algorithm is not charger specific, AFAICS, it is currently only used by some ROHM PMIC drivers).
- Technique of measuring the VDR tables for a battery is not widely known. AFAICS, only folks at ROHM are measuring those for some customer products. We do have those tables available for some of the products though (Kobo?).

Quoted 5 lines
---
 .../power/supply/rohm,vdr-battery.yaml        | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.yaml

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/ac5a4e992e4fb9c7bffb1e641a7cd61f74af4cba.1763022807.git.mazziesaccount@gmail.com

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

maintainer
#8 lore raw

Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

Quoted 40 lines · Rob Herring (Arm)
Quoted 38 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD72720 PMIC has a battery charger + coulomb counter block. These can be used to manage charging of a lithium-ion battery and to do fuel gauging.

ROHM has developed a so called "zero-correction" -algorithm to improve the fuel-gauging accuracy close to the point where battery is depleted. This relies on battery specific "VDR" tables, which are measured from the battery, and which describe the voltage drop rate. More thorough explanation about the "zero correction" and "VDR" parameters is here:
https://lore.kernel.org/all/[email protected]/

Document the VDR zero-correction specific battery properties used by the BD72720 and some other ROHM chargers.

---
NOTE:
Linus' rb-tag holds only if there's no further comments from Rob.

Revision history:
v3 =>:
- No changes

v2 => v3:
- Constrain VDR threshold voltage to 48V
- Use standard '-bp' -suffix for the rohm,volt-drop-soc

RFCv1 => v2:
- Add units to rohm,volt-drop-soc (tenths of %)
- Give real temperatures matching the VDR tables, instead of vague 'high', 'normal', 'low', 'very low'. (Add table of temperatures and use number matching the right temperature index in the VDR table name).
- Fix typoed 'algorithm' in commit message.

Quoted 4 lines
Quoted 4 lines

The parameters are describing the battery voltage drop rates - so they are properties of the battery, not the charger. Thus they do not belong in the charger node.

// snip

Quoted 8 lines

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml

Odd. I am pretty sure I didn't see this when I ran the make dt_binding_check. Not 100% sure what happened there. I get this error now though when including all the bindings to the check.

Do I get this right - these errors result from the properties used in example not being included in the battery.yaml? So, this means that the check is done based on the binding (battery.yaml) where the compatible (simple-battery) is defined - not based on the properties which are present in this file where the example resides, (and which references the battery.yaml)?

...

Oh... Now that I wrote it I feel like an idiot.

This approach couldn't work for the validation, right? Let's assume I had a VDR battery, and I added a static-battery -node for it. Running the validation would pick the battery.yaml based on the compatible (just as it does here), and be completely unaware of this vdr-battery.yaml. I have no idea why I thought this would work. Probably because I only thought this from the documentation POV.

So, as far as I understand, the only viable options are expanding the existing battery.yaml with these properties (which I hoped to avoid, see below)

Quoted 11 lines
Quoted 11 lines

The right place for them is the battery node, which is described by the generic "battery.yaml". I was not comfortable with adding these properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR algorithm implemented. (And even though the algorithm is not charger specific, AFAICS, it is currently only used by some ROHM PMIC drivers).
- Technique of measuring the VDR tables for a battery is not widely known. AFAICS, only folks at ROHM are measuring those for some customer products. We do have those tables available for some of the products though (Kobo?).

or, to add new compatible for the "vdr-battery".
AFAICS, adding new compatible would require us to wither duplicate the used properties from battery.yaml here (as battery.yaml mandates the "simple-battery" - compatible) - or to split the battery.yaml in two files, one containing the generic properties, other containing the "simple-battery" -compatible and referencing the generic one. Then the "vdr-battery" could also reference the generic one.

Any suggestions for the next path to follow?

Oh, and sorry for asking to review something which is obviously not working approach. I should've understood this from the beginning.

Yours,
-- Matti

---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

maintainer
#9 lore raw

Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

Quoted 103 lines · Matti Vaittinen
Quoted 40 lines · Rob Herring (Arm)
Quoted 38 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD72720 PMIC has a battery charger + coulomb counter block. These can be used to manage charging of a lithium-ion battery and to do fuel gauging.

ROHM has developed a so called "zero-correction" -algorithm to improve the fuel-gauging accuracy close to the point where battery is depleted. This relies on battery specific "VDR" tables, which are measured from the battery, and which describe the voltage drop rate. More thorough explanation about the "zero correction" and "VDR" parameters is here:
https://lore.kernel.org/all/[email protected]/

Document the VDR zero-correction specific battery properties used by the BD72720 and some other ROHM chargers.

---
NOTE:
Linus' rb-tag holds only if there's no further comments from Rob.

Revision history:
v3 =>:
- No changes

v2 => v3:
- Constrain VDR threshold voltage to 48V
- Use standard '-bp' -suffix for the rohm,volt-drop-soc

RFCv1 => v2:
- Add units to rohm,volt-drop-soc (tenths of %)
- Give real temperatures matching the VDR tables, instead of vague 'high', 'normal', 'low', 'very low'. (Add table of temperatures and use number matching the right temperature index in the VDR table name).
- Fix typoed 'algorithm' in commit message.

Quoted 4 lines
Quoted 4 lines

The parameters are describing the battery voltage drop rates - so they are properties of the battery, not the charger. Thus they do not belong in the charger node.

// snip

Quoted 8 lines

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml

Odd. I am pretty sure I didn't see this when I ran the make dt_binding_check. Not 100% sure what happened there. I get this error now though when including all the bindings to the check.

Do I get this right - these errors result from the properties used in example not being included in the battery.yaml? So, this means that the check is done based on the binding (battery.yaml) where the compatible (simple-battery) is defined - not based on the properties which are present in this file where the example resides, (and which references the battery.yaml)?

...

Oh... Now that I wrote it I feel like an idiot.

This approach couldn't work for the validation, right? Let's assume I had a VDR battery, and I added a static-battery -node for it. Running the validation would pick the battery.yaml based on the compatible (just as it does here), and be completely unaware of this vdr-battery.yaml. I have no idea why I thought this would work. Probably because I only thought this from the documentation POV.

So, as far as I understand, the only viable options are expanding the existing battery.yaml with these properties (which I hoped to avoid, see below)

Quoted 11 lines
Quoted 11 lines

The right place for them is the battery node, which is described by the generic "battery.yaml". I was not comfortable with adding these properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR algorithm implemented. (And even though the algorithm is not charger specific, AFAICS, it is currently only used by some ROHM PMIC drivers).
- Technique of measuring the VDR tables for a battery is not widely known. AFAICS, only folks at ROHM are measuring those for some customer products. We do have those tables available for some of the products though (Kobo?).

or, to add new compatible for the "vdr-battery".
AFAICS, adding new compatible would require us to wither duplicate the used properties from battery.yaml here (as battery.yaml mandates the "simple-battery" - compatible) - or to split the battery.yaml in two files, one containing the generic properties, other containing the "simple-battery" -compatible and referencing the generic one. Then the "vdr-battery" could also reference the generic one.

Quoted 1 line

Any suggestions for the next path to follow?

Probably the latter option. You could do the former and make the new properties conditional on the "vdr-battery" compatible. That's fine with small differences, but gets messy as there are more properties and variations.

But is "VDR" a type of battery though? Is there a certain type/chemistry of battery we should be describing where VDR is applicable? I don't think it scales well if we define battery compatibles for every variation of charger algorithm. Honestly I don't mind just adding 1 property. I care more if we allow undocumented properties than allowing documented but invalid for the platform properties. When it becomes 10, 20, 30 properties, then I might start to care. If that happens, either we are doing a poor job of generically describing battery parameters or chargers and batteries are tightly coupled and can't be described independently.

Rob

maintainer
#10 lore raw

Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

Quoted 104 lines · On Fri, 14 Nov 2025 10:39:54 -0600 Rob Herring <[email protected]> wrote:
Quoted 103 lines · Matti Vaittinen
Quoted 40 lines · Rob Herring (Arm)
Quoted 38 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD72720 PMIC has a battery charger + coulomb counter block. These can be used to manage charging of a lithium-ion battery and to do fuel gauging.

ROHM has developed a so called "zero-correction" -algorithm to improve the fuel-gauging accuracy close to the point where battery is depleted. This relies on battery specific "VDR" tables, which are measured from the battery, and which describe the voltage drop rate. More thorough explanation about the "zero correction" and "VDR" parameters is here:
https://lore.kernel.org/all/[email protected]/

Document the VDR zero-correction specific battery properties used by the BD72720 and some other ROHM chargers.

---
NOTE:
Linus' rb-tag holds only if there's no further comments from Rob.

Revision history:
v3 =>:
- No changes

v2 => v3:
- Constrain VDR threshold voltage to 48V
- Use standard '-bp' -suffix for the rohm,volt-drop-soc

RFCv1 => v2:
- Add units to rohm,volt-drop-soc (tenths of %)
- Give real temperatures matching the VDR tables, instead of vague 'high', 'normal', 'low', 'very low'. (Add table of temperatures and use number matching the right temperature index in the VDR table name).
- Fix typoed 'algorithm' in commit message.

Quoted 4 lines
Quoted 4 lines

The parameters are describing the battery voltage drop rates - so they are properties of the battery, not the charger. Thus they do not belong in the charger node.

// snip

Quoted 8 lines

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml

Odd. I am pretty sure I didn't see this when I ran the make dt_binding_check. Not 100% sure what happened there. I get this error now though when including all the bindings to the check.

Do I get this right - these errors result from the properties used in example not being included in the battery.yaml? So, this means that the check is done based on the binding (battery.yaml) where the compatible (simple-battery) is defined - not based on the properties which are present in this file where the example resides, (and which references the battery.yaml)?

...

Oh... Now that I wrote it I feel like an idiot.

This approach couldn't work for the validation, right? Let's assume I had a VDR battery, and I added a static-battery -node for it. Running the validation would pick the battery.yaml based on the compatible (just as it does here), and be completely unaware of this vdr-battery.yaml. I have no idea why I thought this would work. Probably because I only thought this from the documentation POV.

So, as far as I understand, the only viable options are expanding the existing battery.yaml with these properties (which I hoped to avoid, see below)

Quoted 11 lines
Quoted 11 lines

The right place for them is the battery node, which is described by the generic "battery.yaml". I was not comfortable with adding these properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR algorithm implemented. (And even though the algorithm is not charger specific, AFAICS, it is currently only used by some ROHM PMIC drivers).
- Technique of measuring the VDR tables for a battery is not widely known. AFAICS, only folks at ROHM are measuring those for some customer products. We do have those tables available for some of the products though (Kobo?).

or, to add new compatible for the "vdr-battery".
AFAICS, adding new compatible would require us to wither duplicate the used properties from battery.yaml here (as battery.yaml mandates the "simple-battery" - compatible) - or to split the battery.yaml in two files, one containing the generic properties, other containing the "simple-battery" -compatible and referencing the generic one. Then the "vdr-battery" could also reference the generic one.

Quoted 7 lines
Quoted 1 line

Any suggestions for the next path to follow?

Probably the latter option. You could do the former and make the new properties conditional on the "vdr-battery" compatible. That's fine with small differences, but gets messy as there are more properties and variations.

just keep in mind, that several kobo devices have one pmic in one board revision and another one in the other (e.g. Kobo Nia rev A vs rev C). But probably the same battery. So if the "vdr-battery" is a compatible just to allow a more properties,
then "simple-battery" should be allowed as fallback.

Regards,
Andreas

maintainer
#11 lore raw

Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

In reply to Rob Herring
Quoted 5 lines · Rob Herring
Quoted 4 lines · Matti Vaittinen
Quoted 3 lines · Rob Herring (Arm)
Quoted 1 line · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

//snip

Quoted 27 lines
Quoted 25 lines

So, as far as I understand, the only viable options are expanding the existing battery.yaml with these properties (which I hoped to avoid, see below)

Quoted 11 lines
Quoted 11 lines

The right place for them is the battery node, which is described by the generic "battery.yaml". I was not comfortable with adding these properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR algorithm implemented. (And even though the algorithm is not charger specific, AFAICS, it is currently only used by some ROHM PMIC drivers).
- Technique of measuring the VDR tables for a battery is not widely known. AFAICS, only folks at ROHM are measuring those for some customer products. We do have those tables available for some of the products though (Kobo?).

or, to add new compatible for the "vdr-battery".
AFAICS, adding new compatible would require us to wither duplicate the used properties from battery.yaml here (as battery.yaml mandates the "simple-battery" - compatible) - or to split the battery.yaml in two files, one containing the generic properties, other containing the "simple-battery" -compatible and referencing the generic one. Then the "vdr-battery" could also reference the generic one.

Quoted 1 line

Any suggestions for the next path to follow?

Quoted 7 lines

Probably the latter option. You could do the former and make the new properties conditional on the "vdr-battery" compatible. That's fine with small differences, but gets messy as there are more properties and variations.

But is "VDR" a type of battery though? Is there a certain type/chemistry of battery we should be describing where VDR is applicable?

No. Not that I know. My understanding is that the "VDR (voltage drop rate)" refers to measured voltage drop-rates under certain conditions - which can be used to (more accurately) estimate the remaining capacity when battery is nearly depleted. As far as I know, this is only used with Lithium-ion batteries (I am not at all sure of this) - but I _assume_ the technique could be applied to other type of batteries as well.

Quoted 5 lines

I don't
think it scales well if we define battery compatibles for every variation of charger algorithm. Honestly I don't mind just adding 1 property. I care more if we allow undocumented properties than allowing documented but invalid for the platform properties.

I see. The "VDR" stuff is really tightly bound to the fuel-gauging algorithm. It is measured characteristics of the battery - but those values are only usable by the "VDR" algorithm. I don't really have a good insight in the amount of fuel-gauging algorithm related properties suggested to be added during the years - but don't think there have been that many of them. So, I am not that worried about adding the compatible. On the other hand, there is no technical reason (other than adding properties which are unused on many platforms) why not to add the vdr tables in the static-battey node without adding own compatible. And, reading reply from Andreas (I'll copy it here to answer it in same mail)

/// Below text is form Andreas:

Quoted 5 lines

just keep in mind, that several kobo devices have one pmic in one board revision and another one in the other (e.g. Kobo Nia rev A vs rev C). But probably the same battery. So if the "vdr-battery" is a compatible just to allow a more properties,
then "simple-battery" should be allowed as fallback.

I didn't know Kobos use multiple chargers. Thanks Andreas! So, in that sense, adding the "vdr" tables in static-battery node, without new compatible, would maybe be simplest solution. Then the charger(s) (fuel-gauge(s)) which implement VDR algorithm, can pick the tables while those chargers which don't implement the VDR will just ignore these tables.

Quoted 2 lines

When it
becomes 10, 20, 30 properties, then I might start to care.

For VDR there are only:

rohm,voltage-vdr-thresh-microvolt,
rohm,volt-drop-soc-bp,
rohm,volt-drop-temperatures-millicelsius

and

patternProperties:
'^rohm,volt-drop-[0-9]-microvolt':

So, from the binding point of view (.yaml), it's not _that_ lot. In the .dts there will be quite some noise as the tables have several values.

Quoted 4 lines

If that
happens, either we are doing a poor job of generically describing battery parameters or chargers and batteries are tightly coupled and can't be described independently.

I am under impression that chargers tend to be pretty flexible, and they can be configured to work with many different batteries by altering the charging profiles. Most of the battery properties (like and charging phases [like pre, CC, CV], their limits, currents and voltages etc) are very generally usable. So, large subset of charging functionality can be handled with standard properties. I believe it is only the fuel-gauging where things get more hairy.

I did prepare a series which does the split and adds new compatible for the 'rohm,vdr-battery'. (The power-supply class is not yet modified in the series, but we would probably want to modify the battery-info getters to also accept the 'rohm,vdr-battery' -compatible.)

I wonder if I should actually prepare also a series where these properties are just placed in the existing static battery node without adding new compatible. That way it would be easier to see which way is better.

If I do that, should I only spin these bindings as RFC to avoid the unnecessary noise?

Oh, and a big thanks to both of you Rob and Andreas! I feel this gained more clarity after your feedback :)

Yours,
-- Matti

---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

maintainer
#12 lore raw

Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

Quoted 81 lines · Matti Vaittinen
Quoted 5 lines · Rob Herring
Quoted 4 lines · Matti Vaittinen
Quoted 3 lines · Rob Herring (Arm)
Quoted 1 line · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

//snip

Quoted 27 lines
Quoted 25 lines

So, as far as I understand, the only viable options are expanding the existing battery.yaml with these properties (which I hoped to avoid, see below)

Quoted 11 lines
Quoted 11 lines

The right place for them is the battery node, which is described by the generic "battery.yaml". I was not comfortable with adding these properties to the generic battery.yaml because they are:
- Meaningful only for those charger drivers which have the VDR algorithm implemented. (And even though the algorithm is not charger specific, AFAICS, it is currently only used by some ROHM PMIC drivers).
- Technique of measuring the VDR tables for a battery is not widely known. AFAICS, only folks at ROHM are measuring those for some customer products. We do have those tables available for some of the products though (Kobo?).

or, to add new compatible for the "vdr-battery".
AFAICS, adding new compatible would require us to wither duplicate the used properties from battery.yaml here (as battery.yaml mandates the "simple-battery" - compatible) - or to split the battery.yaml in two files, one containing the generic properties, other containing the "simple-battery" -compatible and referencing the generic one. Then the "vdr-battery" could also reference the generic one.

Quoted 1 line

Any suggestions for the next path to follow?

Quoted 7 lines

Probably the latter option. You could do the former and make the new properties conditional on the "vdr-battery" compatible. That's fine with small differences, but gets messy as there are more properties and variations.

But is "VDR" a type of battery though? Is there a certain type/chemistry of battery we should be describing where VDR is applicable?

No. Not that I know. My understanding is that the "VDR (voltage drop rate)" refers to measured voltage drop-rates under certain conditions - which can be used to (more accurately) estimate the remaining capacity when battery is nearly depleted. As far as I know, this is only used with Lithium-ion batteries (I am not at all sure of this) - but I _assume_ the technique could be applied to other type of batteries as well.

Quoted 5 lines

I don't
think it scales well if we define battery compatibles for every variation of charger algorithm. Honestly I don't mind just adding 1 property. I care more if we allow undocumented properties than allowing documented but invalid for the platform properties.

I see. The "VDR" stuff is really tightly bound to the fuel-gauging algorithm. It is measured characteristics of the battery - but those values are only usable by the "VDR" algorithm. I don't really have a good insight in the amount of fuel-gauging algorithm related properties suggested to be added during the years - but don't think there have been that many of them. So, I am not that worried about adding the compatible. On the other hand, there is no technical reason (other than adding properties which are unused on many platforms) why not to add the vdr tables in the static-battey node without adding own compatible. And, reading reply from Andreas (I'll copy it here to answer it in same mail)

/// Below text is form Andreas:

Quoted 5 lines

just keep in mind, that several kobo devices have one pmic in one board revision and another one in the other (e.g. Kobo Nia rev A vs rev C). But probably the same battery. So if the "vdr-battery" is a compatible just to allow a more properties,
then "simple-battery" should be allowed as fallback.

I didn't know Kobos use multiple chargers. Thanks Andreas! So, in that sense, adding the "vdr" tables in static-battery node, without new compatible, would maybe be simplest solution. Then the charger(s) (fuel-gauge(s)) which implement VDR algorithm, can pick the tables while those chargers which don't implement the VDR will just ignore these tables.

Quoted 6 lines
Quoted 2 lines

When it
becomes 10, 20, 30 properties, then I might start to care.

For VDR there are only:

rohm,voltage-vdr-thresh-microvolt,

So "voltage voltage drop rate"? And '-microvolt' says this is voltage too. :)

Quoted 25 lines

rohm,volt-drop-soc-bp,
rohm,volt-drop-temperatures-millicelsius

and

patternProperties:
'^rohm,volt-drop-[0-9]-microvolt':

So, from the binding point of view (.yaml), it's not _that_ lot. In the .dts there will be quite some noise as the tables have several values.

Quoted 4 lines

If that
happens, either we are doing a poor job of generically describing battery parameters or chargers and batteries are tightly coupled and can't be described independently.

I am under impression that chargers tend to be pretty flexible, and they can be configured to work with many different batteries by altering the charging profiles. Most of the battery properties (like and charging phases [like pre, CC, CV], their limits, currents and voltages etc) are very generally usable. So, large subset of charging functionality can be handled with standard properties. I believe it is only the fuel-gauging where things get more hairy.

Quoted 4 lines

I did prepare a series which does the split and adds new compatible for the 'rohm,vdr-battery'. (The power-supply class is not yet modified in the series, but we would probably want to modify the battery-info getters to also accept the 'rohm,vdr-battery' -compatible.)

I don't think that's the right direction. It's not a Rohm battery.

Quoted 3 lines

I wonder if I should actually prepare also a series where these properties are just placed in the existing static battery node without adding new compatible. That way it would be easier to see which way is better.

That seems like the right thing to do here.

The main question for me is whether these should even be Rohm specific? That would probably require a 2nd user to answer for sure.

Quoted 2 lines

If I do that, should I only spin these bindings as RFC to avoid the unnecessary noise?

Only if you think something is not complete and/or the patches should not be applied.

Rob

maintainer
#13 lore raw

Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery

Quoted 12 lines · Rob Herring
Quoted 10 lines · Matti Vaittinen
Quoted 5 lines · Rob Herring
Quoted 4 lines · Matti Vaittinen
Quoted 3 lines · Rob Herring (Arm)
Quoted 1 line · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

//snip

Quoted 1 line
Quoted 1 line
Quoted 6 lines
Quoted 3 lines

For VDR there are only:

rohm,voltage-vdr-thresh-microvolt,

So "voltage voltage drop rate"? And '-microvolt' says this is voltage too. :)

Hm. Yes. This is a threshold voltage for applying the "zero-correction" algorithm, which uses these "VDR" (a.k.a voltage drop rate) tables. Eg, the algorithm should only used for the correction when battery voltage drops below this threshold. AFAICS, this is usually designed to be slightly higher than the voltage where the system stays still operable. I suppose this could also be "zero-correction-threshold", but this would introduce another "buzzword".

Quoted 32 lines
Quoted 25 lines

rohm,volt-drop-soc-bp,
rohm,volt-drop-temperatures-millicelsius

and

patternProperties:

   '^rohm,volt-drop-[0-9]-microvolt':

So, from the binding point of view (.yaml), it's not _that_ lot. In the .dts there will be quite some noise as the tables have several values.

Quoted 4 lines

If that
happens, either we are doing a poor job of generically describing battery parameters or chargers and batteries are tightly coupled and can't be described independently.

I am under impression that chargers tend to be pretty flexible, and they can be configured to work with many different batteries by altering the charging profiles. Most of the battery properties (like and charging phases [like pre, CC, CV], their limits, currents and voltages etc) are very generally usable. So, large subset of charging functionality can be handled with standard properties. I believe it is only the fuel-gauging where things get more hairy.

Quoted 4 lines

I did prepare a series which does the split and adds new compatible for the 'rohm,vdr-battery'. (The power-supply class is not yet modified in the series, but we would probably want to modify the battery-info getters to also accept the 'rohm,vdr-battery' -compatible.)

I don't think that's the right direction. It's not a Rohm battery.

Quoted 9 lines
Quoted 3 lines

I wonder if I should actually prepare also a series where these properties are just placed in the existing static battery node without adding new compatible. That way it would be easier to see which way is better.

That seems like the right thing to do here.

The main question for me is whether these should even be Rohm specific? That would probably require a 2nd user to answer for sure.

This is a question Linus W asked as well :)
I believe this technique could be applied to other batteries. I, however, am not aware of any other than ROHM charger drivers which implement the algorithm. Furthermore, I was told that the mechanism to measure these "VDR-tables" for batteries is one of those things which should be "kept under your hat". I think ROHM has also patented some stuff related to that. Hence I prefixed these tables by "rohm,".

I have no strong objections to dropping the "rohm," though - but I doubt these tables will be heavily used by any other but ROHM chargers.

Quoted 5 lines
Quoted 2 lines

If I do that, should I only spin these bindings as RFC to avoid the unnecessary noise?

Only if you think something is not complete and/or the patches should not be applied.

Oh, Ok. Then I will send only one of the approaches - probably the one where properties are added to the simple-battery.

Thanks for all the support!

Yours,
-- Matti

---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

#14 lore raw

PATCH v4 5/16 dt-bindings: mfd: ROHM BD72720

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 is a power management IC integrating regulators, GPIOs, charger, LEDs, RTC and a clock gate.

Add dt-binding doc for ROHM BD72720.

---
Revision history:
v3 => v4:
- Fix typo from the reference to regulator binding
- Fix Rsense limits to micro Ohms
- Fix compatible string in the example
- Fix regulator node names (to lower-case) in the example
- Add the missing regulator nodes to the example

v2 => v3:
- Styling
- Document all pin functions
- use pattern-properties
- re-use existing Rsense binding
- correct the example

RFCv1 => v2:
- Typofixes

---
 .../bindings/mfd/rohm,bd72720-pmic.yaml       | 338 ++++++++++++++++++
 1 file changed, 338 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml
Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml new file +338 -0
new file mode 100644
index 000000000000..0e06e780dba2
@@ -0,0 +1,338 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/rohm,bd72720-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD72720 Power Management Integrated Circuit
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description:
+  BD72720 is a single-chip power management IC for battery-powered portable
+  devices. The BD72720 integrates 10 bucks and 11 LDOs, and a 3000 mA
+  switching charger. The IC also includes a Coulomb counter, a real-time
+  clock (RTC), GPIOs and a 32.768 kHz clock gate.
+
+# In addition to the properties found from the charger node, the ROHM BD72720
+# uses properties from a static battery node. Please see the:
+# Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.yaml
+#
+# Following properties are used
+# when present:
+#
+# charge-full-design-microamp-hours: Battry capacity in mAh
+# voltage-max-design-microvolt:      Maximum voltage
+# voltage-min-design-microvolt:      Minimum voltage system is still operating.
+# degrade-cycle-microamp-hours:      Capacity lost due to aging at each full
+#                                    charge cycle.
+# ocv-capacity-celsius:              Array of OCV table temperatures. 1/table.
+# ocv-capacity-table-<N>:            Table of OCV voltage/SOC pairs. Corresponds
+#                                    N.th temperature in ocv-capacity-celsius
+#
+# ROHM specific properties:
+# rohm,voltage-vdr-thresh-microvolt: Threshold for starting the VDR correction
+# rohm,volt-drop-soc:                Table of capacity values matching the
+#                                    values in VDR tables.
+# rohm,volt-drop-high-temp-microvolt: VDR table for high temperature
+# rohm,volt-drop-normal-temp-microvolt: VDR table for normal temperature
+# rohm,volt-drop-low-temp-microvolt:  VDR table for low temperature
+# rohm,volt-drop-very-low-temp-microvolt: VDR table for very low temperature
+#
+# VDR tables are (usually) determined for a specific battery by ROHM.
+# The battery node would then be referred from the charger node:
+#
+# monitored-battery = <&battery>;
+
+properties:
+  compatible:
+    const: rohm,bd72720
+
+  reg:
+    description:
+      I2C slave address.
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+    description:
+      The first cell is the pin number and the second cell is used to specify
+      flags. See the gpio binding document for more information.
+
+  clocks:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 0
+
+  clock-output-names:
+    const: bd71828-32k-out
+
+  rohm,clkout-open-drain:
+    description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos".
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maximum: 1
+
+  rohm,charger-sense-resistor-micro-ohms:
+    minimum: 10000
+    maximum: 50000
+    description:
+      BD72720 has a SAR ADC for measuring charging currents. External sense
+      resistor (RSENSE in data sheet) should be used. If some other but
+      30 mOhm resistor is used the resistance value should be given here in
+      micro Ohms.
+
+  regulators:
+    $ref: /schemas/regulator/rohm,bd72720-regulator.yaml
+    description:
+      List of child nodes that specify the regulators.
+
+  leds:
+    $ref: /schemas/leds/rohm,bd71828-leds.yaml
+
+  rohm,pin-fault_b:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      BD72720 has an OTP option to use fault_b-pin for different
+      purposes. Set this property accordingly. OTP options are
+      OTP0 - bi-directional FAULT_B or READY indicator depending on a
+      'sub option'
+      OTP1 - GPO
+      OTP2 - Power sequencer output.
+    enum:
+      - faultb
+      - readyind
+      - gpo
+      - pwrseq
+
+patternProperties:
+  "^rohm,pin-dvs[0-1]$":
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      BD72720 has 4 different OTP options to determine the use of dvs<X>-pins.
+      OTP0 - regulator RUN state control.
+      OTP1 - GPI.
+      OTP2 - GPO.
+      OTP3 - Power sequencer output.
+      This property specifies the use of the pin.
+    enum:
+      - dvs-input
+      - gpi
+      - gpo
+      - pwrseq
+
+  "^rohm,pin-exten[0-1]$":
+    $ref: /schemas/types.yaml#/definitions/string
+    description: BD72720 has an OTP option to use exten0-pin for different
+      purposes. Set this property accordingly.
+      OTP0 - GPO
+      OTP1 - Power sequencer output.
+    enum:
+      - gpo
+      - pwrseq
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - "#clock-cells"
+  - regulators
+  - gpio-controller
+  - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/leds/common.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pmic: pmic@4b {
+            compatible = "rohm,bd72720";
+            reg = <0x4b>;
+
+            interrupt-parent = <&gpio1>;
+            interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
+
+            clocks = <&osc 0>;
+            #clock-cells = <0>;
+            clock-output-names = "bd71828-32k-out";
+
+            gpio-controller;
+            #gpio-cells = <2>;
+
+            rohm,pin-dvs0 = "gpi";
+            rohm,pin-dvs1 = "gpi";
+            rohm,pin-exten0 = "gpo";
+            rohm,pin-exten1 = "gpo";
+            rohm,pin-fault_b = "faultb";
+
+            rohm,charger-sense-resistor-micro-ohms = <10000>;
+
+            regulators {
+                buck1 {
+                    regulator-name = "buck1";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <2000000>;
+                    regulator-ramp-delay = <2500>;
+                };
+                buck2 {
+                    regulator-name = "buck2";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <2000000>;
+                    regulator-ramp-delay = <2500>;
+                };
+                buck3 {
+                    regulator-name = "buck3";
+                    regulator-min-microvolt = <1200000>;
+                    regulator-max-microvolt = <2000000>;
+                };
+                buck4 {
+                    regulator-name = "buck4";
+                    regulator-min-microvolt = <1000000>;
+                    regulator-max-microvolt = <1800000>;
+                };
+                buck5 {
+                    regulator-name = "buck5";
+                    regulator-min-microvolt = <2500000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+                buck6 {
+                    regulator-name = "buck6";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <2000000>;
+                    regulator-ramp-delay = <2500>;
+                };
+                buck7 {
+                    regulator-name = "buck7";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <2000000>;
+                    regulator-ramp-delay = <2500>;
+                };
+                buck8 {
+                    regulator-name = "buck8";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <1700000>;
+                    regulator-ramp-delay = <2500>;
+                    rohm,dvs-run-voltage = <1700000>;
+                    rohm,dvs-idle-voltage = <1>;
+                    rohm,dvs-suspend-voltage = <1>;
+                    rohm,dvs-lpsr-voltage = <0>;
+                    regulator-boot-on;
+                };
+                buck9 {
+                    regulator-name = "buck9";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <1700000>;
+                    regulator-ramp-delay = <2500>;
+                    rohm,dvs-run-voltage = <1700000>;
+                    rohm,dvs-idle-voltage = <1>;
+                    rohm,dvs-suspend-voltage = <1>;
+                    rohm,dvs-lpsr-voltage = <0>;
+                    regulator-boot-on;
+                };
+                buck10 {
+                    regulator-name = "buck10";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <1700000>;
+                    regulator-ramp-delay = <2500>;
+                    rohm,dvs-run-voltage = <1700000>;
+                    rohm,dvs-idle-voltage = <1>;
+                    rohm,dvs-suspend-voltage = <1>;
+                    rohm,dvs-lpsr-voltage = <0>;
+                    regulator-boot-on;
+                };
+                ldo1 {
+                    regulator-name = "ldo1";
+                    regulator-min-microvolt = <800000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+                ldo2 {
+                    regulator-name = "ldo2";
+                    regulator-min-microvolt = <800000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+                ldo3 {
+                    regulator-name = "ldo3";
+                    regulator-min-microvolt = <800000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+                ldo4 {
+                    regulator-name = "ldo4";
+                    regulator-min-microvolt = <800000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+                ldo5 {
+                    regulator-name = "ldo5";
+                    regulator-min-microvolt = <800000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+                ldo6 {
+                    regulator-name = "ldo6";
+                    regulator-min-microvolt = <1800000>;
+                    regulator-max-microvolt = <1800000>;
+                };
+                ldo7 {
+                    regulator-name = "ldo7";
+                    regulator-min-microvolt = <800000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+                ldo8 {
+                    regulator-name = "ldo8";
+                    regulator-min-microvolt = <750000>;
+                    regulator-max-microvolt = <3300000>;
+                    rohm,dvs-suspend-voltage = <0>;
+                    rohm,dvs-lpsr-voltage = <1>;
+                    rohm,dvs-run-voltage = <750000>;
+                };
+                ldo9 {
+                    regulator-name = "ldo9";
+                    regulator-min-microvolt = <750000>;
+                    regulator-max-microvolt = <3300000>;
+                    rohm,dvs-suspend-voltage = <0>;
+                    rohm,dvs-lpsr-voltage = <1>;
+                    rohm,dvs-run-voltage = <750000>;
+                };
+                ldo10 {
+                    regulator-name = "ldo10";
+                    regulator-min-microvolt = <750000>;
+                    regulator-max-microvolt = <3300000>;
+                    rohm,dvs-suspend-voltage = <0>;
+                    rohm,dvs-lpsr-voltage = <1>;
+                    rohm,dvs-run-voltage = <750000>;
+                };
+                ldo11 {
+                    regulator-name = "ldo11";
+                    regulator-min-microvolt = <750000>;
+                    regulator-max-microvolt = <3300000>;
+                    rohm,dvs-suspend-voltage = <0>;
+                    rohm,dvs-lpsr-voltage = <1>;
+                    rohm,dvs-run-voltage = <750000>;
+                };
+            };
+
+            leds {
+                compatible = "rohm,bd71828-leds";
+
+                led-1 {
+                    rohm,led-compatible = "bd71828-grnled";
+                    function = LED_FUNCTION_INDICATOR;
+                    color = <LED_COLOR_ID_GREEN>;
+                };
+                led-2 {
+                    rohm,led-compatible = "bd71828-ambled";
+                    function = LED_FUNCTION_CHARGING;
+                    color = <LED_COLOR_ID_AMBER>;
+                };
+            };
+        };
+    };
Signature · 1 line
2.51.1
Reviewed-by on v4 5/16
#16 lore raw

Re: [PATCH v4 05/16] dt-bindings: mfd: ROHM BD72720

In reply to Matti Vaittinen
Quoted 8 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 is a power management IC integrating regulators, GPIOs, charger, LEDs, RTC and a clock gate.

Add dt-binding doc for ROHM BD72720.

This is looking good!

Yours,
Linus Walleij

#17 lore raw

PATCH v4 6/16 dt-bindings: leds: bd72720: Add BD72720

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

Add the ROHM BD72720 documentation to the binding documents.

---
Revision history:
RFCv1 =>:
- No changes

NOTE: The Linux LED driver does currently have: values bd72720-grnled and bd72720-ambled for the rohm,led-compatible. These are handled identically to the existing bd71828-grnled and bd71828-ambled and should be removed from the driver. Thus they are not documented in the binding document.

Furthermore, the BD72720 Linux driver does not use the compatible property from the LED node. The Linux driver is load and probed based on the PMIC compatible in the MFD node. Thus no compatible string for the BD72720 LED node is added.

---
 .../devicetree/bindings/leds/rohm,bd71828-leds.yaml        | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/leds/rohm,bd71828-leds.yaml +4 -3
index b7a3ef76cbf4..64cc40523e3d 100644
@@ -10,11 +10,12 @@ maintainers:
   - Matti Vaittinen <[email protected]>
 
 description: |
-  This module is part of the ROHM BD71828 MFD device. For more details
-  see Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml.
+  This module is part of the ROHM BD71828 and BD72720 MFD device. For more
+  details see Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
+  and Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml
 
   The LED controller is represented as a sub-node of the PMIC node on the device
-  tree.
+  tree. This should be located under "leds" - node in PMIC node.
 
   The device has two LED outputs referred as GRNLED and AMBLED in data-sheet.
 
Signature · 1 line
2.51.1
#18 lore raw

PATCH v4 7/16 mfd: rohm-bd71828: Use regmap_reg_range()

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The regmap range tables tend to be somewhat verbose. Using the regmap_reg_range() can make the definitions slightly mode compact.

Tidy the regmap range tables by using the regmap_reg_range().

---
Revision history:
v2 => :
- no changes
RFCv1 => v2:
- New patch

---
 drivers/mfd/rohm-bd71828.c | 64 +++++++++++---------------------------
 1 file changed, 18 insertions(+), 46 deletions(-)
drivers/mfd/rohm-bd71828.c +18 -46
index 84a64c3b9c9f..2a43005b67ee 100644
@@ -157,55 +157,27 @@ static struct mfd_cell bd71828_mfd_cells[] = {
 };
 
 static const struct regmap_range bd71815_volatile_ranges[] = {
-	{
-		.range_min = BD71815_REG_SEC,
-		.range_max = BD71815_REG_YEAR,
-	}, {
-		.range_min = BD71815_REG_CONF,
-		.range_max = BD71815_REG_BAT_TEMP,
-	}, {
-		.range_min = BD71815_REG_VM_IBAT_U,
-		.range_max = BD71815_REG_CC_CTRL,
-	}, {
-		.range_min = BD71815_REG_CC_STAT,
-		.range_max = BD71815_REG_CC_CURCD_L,
-	}, {
-		.range_min = BD71815_REG_VM_BTMP_MON,
-		.range_max = BD71815_REG_VM_BTMP_MON,
-	}, {
-		.range_min = BD71815_REG_INT_STAT,
-		.range_max = BD71815_REG_INT_UPDATE,
-	}, {
-		.range_min = BD71815_REG_VM_VSYS_U,
-		.range_max = BD71815_REG_REX_CTRL_1,
-	}, {
-		.range_min = BD71815_REG_FULL_CCNTD_3,
-		.range_max = BD71815_REG_CCNTD_CHG_2,
-	},
+	regmap_reg_range(BD71815_REG_SEC, BD71815_REG_YEAR),
+	regmap_reg_range(BD71815_REG_CONF, BD71815_REG_BAT_TEMP),
+	regmap_reg_range(BD71815_REG_VM_IBAT_U, BD71815_REG_CC_CTRL),
+	regmap_reg_range(BD71815_REG_CC_STAT, BD71815_REG_CC_CURCD_L),
+	regmap_reg_range(BD71815_REG_VM_BTMP_MON, BD71815_REG_VM_BTMP_MON),
+	regmap_reg_range(BD71815_REG_INT_STAT, BD71815_REG_INT_UPDATE),
+	regmap_reg_range(BD71815_REG_VM_VSYS_U, BD71815_REG_REX_CTRL_1),
+	regmap_reg_range(BD71815_REG_FULL_CCNTD_3, BD71815_REG_CCNTD_CHG_2),
 };
 
 static const struct regmap_range bd71828_volatile_ranges[] = {
-	{
-		.range_min = BD71828_REG_PS_CTRL_1,
-		.range_max = BD71828_REG_PS_CTRL_1,
-	}, {
-		.range_min = BD71828_REG_PS_CTRL_3,
-		.range_max = BD71828_REG_PS_CTRL_3,
-	}, {
-		.range_min = BD71828_REG_RTC_SEC,
-		.range_max = BD71828_REG_RTC_YEAR,
-	}, {
-		/*
-		 * For now make all charger registers volatile because many
-		 * needs to be and because the charger block is not that
-		 * performance critical.
-		 */
-		.range_min = BD71828_REG_CHG_STATE,
-		.range_max = BD71828_REG_CHG_FULL,
-	}, {
-		.range_min = BD71828_REG_INT_MAIN,
-		.range_max = BD71828_REG_IO_STAT,
-	},
+	regmap_reg_range(BD71828_REG_PS_CTRL_1, BD71828_REG_PS_CTRL_1),
+	regmap_reg_range(BD71828_REG_PS_CTRL_3, BD71828_REG_PS_CTRL_3),
+	regmap_reg_range(BD71828_REG_RTC_SEC, BD71828_REG_RTC_YEAR),
+	/*
+	 * For now make all charger registers volatile because many
+	 * needs to be and because the charger block is not that
+	 * performance critical.
+	 */
+	regmap_reg_range(BD71828_REG_CHG_STATE, BD71828_REG_CHG_FULL),
+	regmap_reg_range(BD71828_REG_INT_MAIN, BD71828_REG_IO_STAT),
 };
 
 static const struct regmap_access_table bd71815_volatile_regs = {
Signature · 1 line
2.51.1
#19 lore raw

PATCH v4 8/16 mfd: bd71828: Support ROHM BD72720

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 is a power management IC which continues the BD71828 family of PMICs. Similarly to the BD71815 and BD71828, the BD72720 integrates regulators, charger, RTC, clock gate and GPIOs.

The main difference to the earlier PMICs is that the BD72720 has two different I2C slave addresses. In addition to the registers behind the 'main I2C address', most of the charger (and to some extent LED) control is done via registers behind a 'secondary I2C slave address', 0x4c.

---
Revision history:
v2 =>:
- no changes

RFCv1 => v2: (Mostly addressed comments from Lee and Andreas)
- Use stacked regmaps to avoid platform data and the tango with multiple regmaps in the power-supply driver
- Use regmap_reg_range()
- make it clear bd72720_irq_type_base is an array
- tab-out definitions in the bd72720 header
- minor styling

Note: This patch depends on the series: "power: supply: add charger for BD71828" by Andreas:
https://lore.kernel.org/all/[email protected]/

There are some new variants being planned. Most notably, the BD73900 should be almost identical to the BD72720 - for everything else except the charger block.

---
 drivers/mfd/Kconfig              |  18 +-
 drivers/mfd/rohm-bd71828.c       | 488 +++++++++++++++++++++++-
 include/linux/mfd/rohm-bd72720.h | 634 +++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-generic.h |   1 +
 4 files changed, 1126 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/mfd/rohm-bd72720.h
drivers/mfd/Kconfig +10 -8
index 6cec1858947b..61e238b316f4 100644
@@ -2211,20 +2211,22 @@ config MFD_ROHM_BD718XX
 	  and emergency shut down as well as 32,768KHz clock output.
 
 config MFD_ROHM_BD71828
-	tristate "ROHM BD71828 and BD71815 Power Management IC"
+	tristate "ROHM BD718[15/28/79], BD72720 and BD73900 PMICs"
 	depends on I2C=y
 	depends on OF
 	select REGMAP_I2C
 	select REGMAP_IRQ
 	select MFD_CORE
 	help
-	  Select this option to get support for the ROHM BD71828 and BD71815
-	  Power Management ICs. BD71828GW and BD71815AGW are single-chip power
-	  management ICs mainly for battery-powered portable devices.
-	  The BD71828 integrates 7 buck converters and 7 LDOs. The BD71815
-	  has 5 bucks, 7 LDOs, and a boost for driving LEDs. Both ICs provide
-	  also a single-cell linear charger, a Coulomb counter, a real-time
-	  clock (RTC), GPIOs and a 32.768 kHz clock gate.
+	  Select this option to get support for the ROHM BD71815, BD71828,
+	  BD71879, BD72720 and BD73900 Power Management ICs. These are
+	  single-chip power management ICs mainly for battery-powered portable
+	  devices.
+	  The BD71815 has 5 bucks, 7 LDOs, and a boost for driving LEDs.
+	  The BD718[28/79] have 7 buck converters and 7 LDOs.
+	  The BD72720 and the BD73900 have 10 bucks and 11 LDOs.
+	  All ICs provide a single-cell linear charger, a Coulomb counter,
+	  a real-time clock (RTC), GPIOs and a 32.768 kHz clock gate.
 
 config MFD_ROHM_BD957XMUF
 	tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs"
drivers/mfd/rohm-bd71828.c +481 -7

603 lines not on this page · raw message

include/linux/mfd/rohm-bd72720.h new file +634 -0

634 lines not on this page · raw message

include/linux/mfd/rohm-generic.h +1 -0
index 579e8dcfcca4..0a284919a6c3 100644
@@ -16,6 +16,7 @@ enum rohm_chip_type {
 	ROHM_CHIP_TYPE_BD71828,
 	ROHM_CHIP_TYPE_BD71837,
 	ROHM_CHIP_TYPE_BD71847,
+	ROHM_CHIP_TYPE_BD72720,
 	ROHM_CHIP_TYPE_BD96801,
 	ROHM_CHIP_TYPE_BD96802,
 	ROHM_CHIP_TYPE_BD96805,
Signature · 1 line
2.51.1
#20 lore raw

PATCH v4 9/16 regulator: bd71828: rename IC specific entities

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The new ROHM BD72720 PMIC has similarities with the BD71828. It makes sense to support the regulator control for both PMICs using the same driver. It is often more clear to have the IC specific functions and globals named starting with the chip-name. So, as a preparatory step, prefix the BD71828 specific functions and globals with the bd71828.

It would be tempting to try also removing the chip ID from those functions which will be common for both PMICs. I have bad experiences on this as it tends to lead to problems when yet another IC is being supported with the same driver, and we will have some functions used for all, some for two of the three, and some for just one. At this point I used to start inventing wildcards like BD718XX or BD7272X. This approach is pretty much always failing as we tend to eventually have something like BD73900 - where all the wildcard stuff will break down.

So, my approach these days is to:
- keep the original chip-id prefix for anything that had it already (and avoid the churn).
- use same prefix for all things that are used by multiple ICs - typically the chip-ID of the first chip. This typically matches also the driver and file names.
- use specific chip-ID as a prefix for anything which is specific to just one chip.

As a preparatory step to adding the BD72720, add bd71828 prefix to all commonly usable functions and globals.

---
Revision history:
RFCv1 =>:
- No changes
No functional changes intended.

---
 drivers/regulator/bd71828-regulator.c | 32 +++++++++++++--------------
 1 file changed, 16 insertions(+), 16 deletions(-)
drivers/regulator/bd71828-regulator.c +16 -16
index dd871ffe979c..3d18dbfdb84e 100644
@@ -28,7 +28,7 @@ struct bd71828_regulator_data {
 	int reg_init_amnt;
 };
 
-static const struct reg_init buck1_inits[] = {
+static const struct reg_init bd71828_buck1_inits[] = {
 	/*
 	 * DVS Buck voltages can be changed by register values or via GPIO.
 	 * Use register accesses by default.
@@ -40,7 +40,7 @@ static const struct reg_init buck1_inits[] = {
 	},
 };
 
-static const struct reg_init buck2_inits[] = {
+static const struct reg_init bd71828_buck2_inits[] = {
 	{
 		.reg = BD71828_REG_PS_CTRL_1,
 		.mask = BD71828_MASK_DVS_BUCK2_CTRL,
@@ -48,7 +48,7 @@ static const struct reg_init buck2_inits[] = {
 	},
 };
 
-static const struct reg_init buck6_inits[] = {
+static const struct reg_init bd71828_buck6_inits[] = {
 	{
 		.reg = BD71828_REG_PS_CTRL_1,
 		.mask = BD71828_MASK_DVS_BUCK6_CTRL,
@@ -56,7 +56,7 @@ static const struct reg_init buck6_inits[] = {
 	},
 };
 
-static const struct reg_init buck7_inits[] = {
+static const struct reg_init bd71828_buck7_inits[] = {
 	{
 		.reg = BD71828_REG_PS_CTRL_1,
 		.mask = BD71828_MASK_DVS_BUCK7_CTRL,
@@ -102,9 +102,9 @@ static int buck_set_hw_dvs_levels(struct device_node *np,
 	return rohm_regulator_set_dvs_levels(&data->dvs, np, desc, cfg->regmap);
 }
 
-static int ldo6_parse_dt(struct device_node *np,
-			 const struct regulator_desc *desc,
-			 struct regulator_config *cfg)
+static int bd71828_ldo6_parse_dt(struct device_node *np,
+				 const struct regulator_desc *desc,
+				 struct regulator_config *cfg)
 {
 	int ret, i;
 	uint32_t uv = 0;
@@ -212,8 +212,8 @@ static const struct bd71828_regulator_data bd71828_rdata[] = {
 			 */
 			.lpsr_on_mask = BD71828_MASK_LPSR_EN,
 		},
-		.reg_inits = buck1_inits,
-		.reg_init_amnt = ARRAY_SIZE(buck1_inits),
+		.reg_inits = bd71828_buck1_inits,
+		.reg_init_amnt = ARRAY_SIZE(bd71828_buck1_inits),
 	},
 	{
 		.desc = {
@@ -253,8 +253,8 @@ static const struct bd71828_regulator_data bd71828_rdata[] = {
 			.lpsr_reg = BD71828_REG_BUCK2_SUSP_VOLT,
 			.lpsr_mask = BD71828_MASK_BUCK1267_VOLT,
 		},
-		.reg_inits = buck2_inits,
-		.reg_init_amnt = ARRAY_SIZE(buck2_inits),
+		.reg_inits = bd71828_buck2_inits,
+		.reg_init_amnt = ARRAY_SIZE(bd71828_buck2_inits),
 	},
 	{
 		.desc = {
@@ -399,8 +399,8 @@ static const struct bd71828_regulator_data bd71828_rdata[] = {
 			.lpsr_reg = BD71828_REG_BUCK6_SUSP_VOLT,
 			.lpsr_mask = BD71828_MASK_BUCK1267_VOLT,
 		},
-		.reg_inits = buck6_inits,
-		.reg_init_amnt = ARRAY_SIZE(buck6_inits),
+		.reg_inits = bd71828_buck6_inits,
+		.reg_init_amnt = ARRAY_SIZE(bd71828_buck6_inits),
 	},
 	{
 		.desc = {
@@ -440,8 +440,8 @@ static const struct bd71828_regulator_data bd71828_rdata[] = {
 			.lpsr_reg = BD71828_REG_BUCK7_SUSP_VOLT,
 			.lpsr_mask = BD71828_MASK_BUCK1267_VOLT,
 		},
-		.reg_inits = buck7_inits,
-		.reg_init_amnt = ARRAY_SIZE(buck7_inits),
+		.reg_inits = bd71828_buck7_inits,
+		.reg_init_amnt = ARRAY_SIZE(bd71828_buck7_inits),
 	},
 	{
 		.desc = {
@@ -633,7 +633,7 @@ static const struct bd71828_regulator_data bd71828_rdata[] = {
 			 * LDO6 only supports enable/disable for all states.
 			 * Voltage for LDO6 is fixed.
 			 */
-			.of_parse_cb = ldo6_parse_dt,
+			.of_parse_cb = bd71828_ldo6_parse_dt,
 		},
 	}, {
 		.desc = {
Signature · 1 line
2.51.1
#21 lore raw

PATCH v4 10/16 regulator: bd71828: Support ROHM BD72720

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

ROHM BD72720 is a power management IC which integrates 10 buck and 11 LDO regulators. This PMIC has plenty of commonalities with the BD71828 and BD71879.

The BD72720 does also have similar 'run-level'-concept as the BD71828 had. It allows controlling the regulator's 'en masse', although only BUCK1 and LDO1 can utilize this in BD72720. Similar to BD71828, this 'en masse' -control is not supported by this driver.

Support the voltage and enable/disable state control for the BD72720.

---
Revision history:
v3 =>:
- No changes

v2 => v3:
- The ldon-head dt-property was changed to microvolts. Adapt the driver to that
RFCv1 => v2:
- No changes

There are some new variants planned. Most notably, the BD73900 should be similar to the BD72720 what comes to the regulator control logic.

If the run-level control is needed, there are some downstream extensions available at:
https://rohmsemiconductor.github.io/Linux-Kernel-PMIC-Drivers/BD72720/

---
 drivers/regulator/Kconfig             |   8 +-
 drivers/regulator/bd71828-regulator.c | 993 +++++++++++++++++++++++++-
 2 files changed, 992 insertions(+), 9 deletions(-)
drivers/regulator/Kconfig +4 -4
index d84f3d054c59..660863f096e3 100644
@@ -241,13 +241,13 @@ config REGULATOR_BD71815
 	  will be called bd71815-regulator.
 
 config REGULATOR_BD71828
-	tristate "ROHM BD71828 Power Regulator"
+	tristate "ROHM BD71828, BD72720 and BD73900 Power Regulators"
 	depends on MFD_ROHM_BD71828
 	select REGULATOR_ROHM
 	help
-	  This driver supports voltage regulators on ROHM BD71828 PMIC.
-	  This will enable support for the software controllable buck
-	  and LDO regulators.
+	  This driver supports voltage regulators on ROHM BD71828,
+	  BD71879, BD72720 and BD73900 PMICs. This will enable
+	  support for the software controllable buck and LDO regulators.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called bd71828-regulator.
drivers/regulator/bd71828-regulator.c +988 -5

1,069 lines not on this page · raw message

Signature · 1 line
2.51.1
#22 lore raw

PATCH v4 11/16 gpio: Support ROHM BD72720 gpios

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 has 6 pins which may be configured as GPIOs. The GPIO1 ... GPIO5 and EPDEN pins. The configuration is done to OTP at the manufacturing, and it can't be read at runtime. The device-tree is required to tell the software which of the pins are used as GPIOs.

Keep the pin mapping static regardless the OTP. This way the user-space can always access the BASE+N for GPIO(N+1) (N = 0 to 4), and BASE + 5 for the EPDEN pin. Do this by setting always the number of GPIOs to 6, and by using the valid-mask to invalidate the pins which aren't configured as GPIOs.

First two pins can be set to be either input or output by OTP. Direction can't be changed by software. Rest of the pins can be set as outputs only. All of the pins support generating interrupts.

Support the Input/Output state getting/setting and the output mode configuration (open-drain/push-pull).

---
Revision history:
RFCv1 => :
- No changes

---
 drivers/gpio/Kconfig        |   9 ++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-bd72720.c | 281 ++++++++++++++++++++++++++++++++++++
 3 files changed, 291 insertions(+)
 create mode 100644 drivers/gpio/gpio-bd72720.c
drivers/gpio/Kconfig +9 -0
index 7ee3afbc2b05..0c612c5163c5 100644
@@ -1319,6 +1319,15 @@ config GPIO_BD71828
 	  This driver can also be built as a module. If so, the module
 	  will be called gpio-bd71828.
 
+config GPIO_BD72720
+	tristate "ROHM BD72720 and BD73900 PMIC GPIO support"
+	depends on MFD_ROHM_BD71828
+	help
+	  Support for GPIO on ROHM BD72720 and BD73900 PMICs. There are two
+	  pins which can be configured to GPI or GPO, and three pins which can
+	  be configured to GPO on the ROHM PMIC. The pin configuration is done
+	  on OTP at manufacturing.
+
 config GPIO_BD9571MWV
 	tristate "ROHM BD9571 GPIO support"
 	depends on MFD_BD9571MWV
drivers/gpio/Makefile +1 -0
index ec296fa14bfd..7a5d03db3021 100644
@@ -45,6 +45,7 @@ obj-$(CONFIG_GPIO_BCM_KONA)		+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BCM_XGS_IPROC)	+= gpio-xgs-iproc.o
 obj-$(CONFIG_GPIO_BD71815)		+= gpio-bd71815.o
 obj-$(CONFIG_GPIO_BD71828)		+= gpio-bd71828.o
+obj-$(CONFIG_GPIO_BD72720)		+= gpio-bd72720.o
 obj-$(CONFIG_GPIO_BD9571MWV)		+= gpio-bd9571mwv.o
 obj-$(CONFIG_GPIO_BLZP1600)		+= gpio-blzp1600.o
 obj-$(CONFIG_GPIO_BRCMSTB)		+= gpio-brcmstb.o
drivers/gpio/gpio-bd72720.c new file +281 -0

281 lines not on this page · raw message

Signature · 1 line
2.51.1
#23 lore raw

PATCH v4 12/16 clk: clk-bd718x7: Support BD72720 clk gate

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD72720 has similar simple clk gate as a few other ROHM PMICs.

Add support for BD72720 clk gate.

---
Revision history:
RFCv1 =>:
- No changes

---
 drivers/clk/Kconfig       |  4 ++--
 drivers/clk/clk-bd718x7.c | 10 ++++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)
drivers/clk/Kconfig +2 -2
index 3a1611008e48..619bd63a3c77 100644
@@ -475,8 +475,8 @@ config COMMON_CLK_BD718XX
 	tristate "Clock driver for 32K clk gates on ROHM PMICs"
 	depends on MFD_ROHM_BD718XX || MFD_ROHM_BD71828
 	help
-	  This driver supports ROHM BD71837, BD71847, BD71850, BD71815
-	  and BD71828 PMICs clock gates.
+	  This driver supports ROHM BD71837, BD71847, BD71850, BD71815,
+	  BD71828, and BD72720 PMICs clock gates.
 
 config COMMON_CLK_FIXED_MMIO
 	bool "Clock driver for Memory Mapped Fixed values"
drivers/clk/clk-bd718x7.c +8 -2
index ac40b669d60b..1cae974e6d1d 100644
@@ -19,7 +19,8 @@
 #define BD71828_REG_OUT32K	0x4B
 /* BD71837 and BD71847 */
 #define BD718XX_REG_OUT32K	0x2E
-
+/* BD72720 */
+#define BD72720_REG_OUT32K	0x9a
 /*
  * BD71837, BD71847, and BD71828 all use bit [0] to clk output control
  */
@@ -118,6 +119,10 @@ static int bd71837_clk_probe(struct platform_device *pdev)
 		c->reg = BD71815_REG_OUT32K;
 		c->mask = CLK_OUT_EN_MASK;
 		break;
+	case ROHM_CHIP_TYPE_BD72720:
+		c->reg = BD72720_REG_OUT32K;
+		c->mask = CLK_OUT_EN_MASK;
+		break;
 	default:
 		dev_err(&pdev->dev, "Unknown clk chip\n");
 		return -EINVAL;
@@ -146,6 +151,7 @@ static const struct platform_device_id bd718x7_clk_id[] = {
 	{ "bd71847-clk", ROHM_CHIP_TYPE_BD71847 },
 	{ "bd71828-clk", ROHM_CHIP_TYPE_BD71828 },
 	{ "bd71815-clk", ROHM_CHIP_TYPE_BD71815 },
+	{ "bd72720-clk", ROHM_CHIP_TYPE_BD72720 },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, bd718x7_clk_id);
@@ -161,6 +167,6 @@ static struct platform_driver bd71837_clk = {
 module_platform_driver(bd71837_clk);
 
 MODULE_AUTHOR("Matti Vaittinen <[email protected]>");
-MODULE_DESCRIPTION("BD718(15/18/28/37/47/50) and chip clk driver");
+MODULE_DESCRIPTION("BD718(15/18/28/37/47/50) and BD72720 chip clk driver");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:bd718xx-clk");
Signature · 1 line
2.51.1
#25 lore raw

PATCH v4 13/16 rtc: bd70528: Support BD72720 rtc

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD72720 has similar RTC block as a few other ROHM PMICs.

Add support for BD72720 RTC.

---
Revision history:
RFCv1 =>:
- No changes

---
 drivers/rtc/Kconfig       |  3 ++-
 drivers/rtc/rtc-bd70528.c | 21 ++++++++++++++-------
 2 files changed, 16 insertions(+), 8 deletions(-)
drivers/rtc/Kconfig +2 -1
index 2933c41c77c8..418f6c28847a 100644
@@ -561,7 +561,8 @@ config RTC_DRV_BD70528
 	depends on MFD_ROHM_BD71828
 	help
 	  If you say Y here you will get support for the RTC
-	  block on ROHM BD71815 and BD71828 Power Management IC.
+	  block on ROHM BD71815, BD71828 and BD72720 Power
+	  Management ICs.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-bd70528.
drivers/rtc/rtc-bd70528.c +14 -7
index 954ac4ef53e8..4c8599761b2e 100644
@@ -7,6 +7,7 @@
 #include <linux/bcd.h>
 #include <linux/mfd/rohm-bd71815.h>
 #include <linux/mfd/rohm-bd71828.h>
+#include <linux/mfd/rohm-bd72720.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -262,13 +263,13 @@ static int bd70528_probe(struct platform_device *pdev)
 
 		/*
 		 * See also BD718XX_ALM_EN_OFFSET:
-		 * This works for BD71828 and BD71815 as they have same offset
-		 * between ALM0 start and ALM0_MASK. If new ICs are to be
-		 * added this requires proper check as ALM0_MASK is not located
-		 * at the end of ALM0 block - but after all ALM blocks so if
-		 * amount of ALMs differ the offset to enable/disable is likely
-		 * to be incorrect and enable/disable must be given as own
-		 * reg address here.
+		 * This works for BD71828, BD71815, and BD72720 as they all
+		 * have same offset between the ALM0 start and the ALM0_MASK.
+		 * If new ICs are to be added this requires proper check as
+		 * the  ALM0_MASK is not located at the end of ALM0 block -
+		 * but after all ALM blocks. If amount of ALMs differ, the
+		 * offset to enable/disable is likely to be incorrect and
+		 * enable/disable must be given as own reg address here.
 		 */
 		bd_rtc->bd718xx_alm_block_start = BD71815_REG_RTC_ALM_START;
 		hour_reg = BD71815_REG_HOUR;
@@ -278,6 +279,11 @@ static int bd70528_probe(struct platform_device *pdev)
 		bd_rtc->bd718xx_alm_block_start = BD71828_REG_RTC_ALM_START;
 		hour_reg = BD71828_REG_RTC_HOUR;
 		break;
+	case ROHM_CHIP_TYPE_BD72720:
+		bd_rtc->reg_time_start = BD72720_REG_RTC_START;
+		bd_rtc->bd718xx_alm_block_start = BD72720_REG_RTC_ALM_START;
+		hour_reg = BD72720_REG_RTC_HOUR;
+		break;
 	default:
 		dev_err(&pdev->dev, "Unknown chip\n");
 		return -ENOENT;
@@ -337,6 +343,7 @@ static int bd70528_probe(struct platform_device *pdev)
 static const struct platform_device_id bd718x7_rtc_id[] = {
 	{ "bd71828-rtc", ROHM_CHIP_TYPE_BD71828 },
 	{ "bd71815-rtc", ROHM_CHIP_TYPE_BD71815 },
+	{ "bd72720-rtc", ROHM_CHIP_TYPE_BD72720 },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, bd718x7_rtc_id);
Signature · 1 line
2.51.1
#26 lore raw

PATCH v4 14/16 power: supply: bd71828: Support wider register addresses

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD71828 power-supply driver assumes register addresses to be 8-bit. The new BD72720 will use stacked register maps to hide paging which is done using secondary I2C slave address. This requires use of 9-bit register addresses in the power-supply driver (added offset 0x100 to the 8-bit hardware register addresses).

The cost is slightly used memory consumption as the members in the struct pwr_regs will be changed from u8 to unsigned int, which means 3 byte increase / member / instance.
This is currently 14 members (expected to possibly be increased when adding new variants / new functionality which may introduce new registers, but not expected to grow much) and 2 instances (will be 3 instances when BD72720 gets added).

So, even if the number of registers grew to 50 it'd be 150 bytes / instance. Assuming we eventually supported 5 variants, it'd be 5 * 150 bytes, which stays very reasonable considering systems we are dealing with.

As a side note, we can reduce the "wasted space / member / instance" from 3 bytes to 1 byte, by using u16 instead of the unsigned int if needed. I rather use unsigned int to be initially prepared for devices with 32 bit registers if there is no need to count bytes.

---
Revision history:
v2 => :
- No changes

RFCv1 => v2:
- New patch

---
 drivers/power/supply/bd71828-power.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
drivers/power/supply/bd71828-power.c +13 -13
index f667baedeb77..ce73c0f48397 100644
@@ -44,19 +44,19 @@
 #define VBAT_LOW_TH			0x00D4
 
 struct pwr_regs {
-	u8 vbat_avg;
-	u8 ibat;
-	u8 ibat_avg;
-	u8 btemp_vth;
-	u8 chg_state;
-	u8 bat_temp;
-	u8 dcin_stat;
-	u8 dcin_collapse_limit;
-	u8 chg_set1;
-	u8 chg_en;
-	u8 vbat_alm_limit_u;
-	u8 conf;
-	u8 vdcin;
+	unsigned int vbat_avg;
+	unsigned int ibat;
+	unsigned int ibat_avg;
+	unsigned int btemp_vth;
+	unsigned int chg_state;
+	unsigned int bat_temp;
+	unsigned int dcin_stat;
+	unsigned int dcin_collapse_limit;
+	unsigned int chg_set1;
+	unsigned int chg_en;
+	unsigned int vbat_alm_limit_u;
+	unsigned int conf;
+	unsigned int vdcin;
 };
 
 static const struct pwr_regs pwr_regs_bd71828 = {
Signature · 1 line
2.51.1
maintainer
#27 lore raw

Re: [PATCH v4 14/16] power: supply: bd71828: Support wider register addresses

Quoted 4 lines · On Thu, 13 Nov 2025 10:55:39 +0200 Matti Vaittinen <[email protected]>…

As a side note, we can reduce the "wasted space / member / instance" from 3 bytes to 1 byte, by using u16 instead of the unsigned int if needed. I rather use unsigned int to be initially prepared for devices with 32 bit registers if there is no need to count bytes.

Well, this is totally internal to the module, so no ABI/API changes, so there is no advantage of using 32bit now I think. We can switch any time. But we have 32bit stuff in the regmap cache anyways, so that is not above the general level of wasting space.

Regards,
Andreas

maintainer
#28 lore raw

Re: [PATCH v4 14/16] power: supply: bd71828: Support wider register addresses

Thanks Andreas,

Quoted 10 lines · Andreas Kemnade
Quoted 4 lines · On Thu, 13 Nov 2025 10:55:39 +0200 Matti Vaittinen <[email protected]>…

As a side note, we can reduce the "wasted space / member / instance" from 3 bytes to 1 byte, by using u16 instead of the unsigned int if needed. I rather use unsigned int to be initially prepared for devices with 32 bit registers if there is no need to count bytes.

Well, this is totally internal to the module, so no ABI/API changes, so there is no advantage of using 32bit now I think. We can switch any time.

The only advantage is to avoid the churn if 32bit ICs are to be added.

Quoted 2 lines

But we have 32bit stuff in the regmap cache anyways, so that is not above the general level of wasting space.

Exactly. And, I am not sure if sparing ~hundred bytes is worth the hassle - even if it is hassle internal to the driver. But yeah, we can squeeze a few bytes if it is seen beneficial. That's why I mentioned it here :)

Yours,
-- Matti

---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

#29 lore raw

PATCH v4 15/16 power: supply: bd71828-power: Support ROHM BD72720

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD72720 is a power management IC with a charger and coulomb counter block which is closely related to the charger / coulomb counter found from the BD71815, BD71828, BD71879 which are all supported by the bd71828-power driver. Due to the similarities it makes sense to support also the BD72720 with the same driver.

Add basic support for the charger logic on ROHM BD72720.

---
Revision history:
v2 => :
- No changes

RFCv1 => v2:
- Support using 9-bit register addresses (offset of 0x100) with the BD72720
- Simplify probe and IC data as we don't need two regmaps
- Drop two BD72720 specific functions as we no longer need different regmap for it.

Note: This patch depends on the series: "power: supply: add charger for BD71828" by Andreas:
https://lore.kernel.org/all/[email protected]/

NOTE: Fuel-gauging is not supported. You can find an unmaintained downstream reference-driver with a fuel-gauge example from:
https://github.com/RohmSemiconductor/Linux-Kernel-PMIC-Drivers/releases/tag/bd72720-reference-driver-v1

---
 drivers/power/supply/bd71828-power.c | 134 +++++++++++++++++++++++----
 1 file changed, 116 insertions(+), 18 deletions(-)
drivers/power/supply/bd71828-power.c +116 -18
index ce73c0f48397..438e220a9cb7 100644
@@ -5,6 +5,7 @@
 #include <linux/kernel.h>
 #include <linux/mfd/rohm-bd71815.h>
 #include <linux/mfd/rohm-bd71828.h>
+#include <linux/mfd/rohm-bd72720.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
@@ -51,12 +52,14 @@ struct pwr_regs {
 	unsigned int chg_state;
 	unsigned int bat_temp;
 	unsigned int dcin_stat;
+	unsigned int dcin_online_mask;
 	unsigned int dcin_collapse_limit;
 	unsigned int chg_set1;
 	unsigned int chg_en;
 	unsigned int vbat_alm_limit_u;
 	unsigned int conf;
 	unsigned int vdcin;
+	unsigned int vdcin_himask;
 };
 
 static const struct pwr_regs pwr_regs_bd71828 = {
@@ -67,12 +70,14 @@ static const struct pwr_regs pwr_regs_bd71828 = {
 	.chg_state = BD71828_REG_CHG_STATE,
 	.bat_temp = BD71828_REG_BAT_TEMP,
 	.dcin_stat = BD71828_REG_DCIN_STAT,
+	.dcin_online_mask = BD7182x_MASK_DCIN_DET,
 	.dcin_collapse_limit = BD71828_REG_DCIN_CLPS,
 	.chg_set1 = BD71828_REG_CHG_SET1,
 	.chg_en   = BD71828_REG_CHG_EN,
 	.vbat_alm_limit_u = BD71828_REG_ALM_VBAT_LIMIT_U,
 	.conf = BD71828_REG_CONF,
 	.vdcin = BD71828_REG_VDCIN_U,
+	.vdcin_himask = BD7182x_MASK_VDCIN_U,
 };
 
 static const struct pwr_regs pwr_regs_bd71815 = {
@@ -85,6 +90,7 @@ static const struct pwr_regs pwr_regs_bd71815 = {
 	.chg_state = BD71815_REG_CHG_STATE,
 	.bat_temp = BD71815_REG_BAT_TEMP,
 	.dcin_stat = BD71815_REG_DCIN_STAT,
+	.dcin_online_mask = BD7182x_MASK_DCIN_DET,
 	.dcin_collapse_limit = BD71815_REG_DCIN_CLPS,
 	.chg_set1 = BD71815_REG_CHG_SET1,
 	.chg_en   = BD71815_REG_CHG_SET1,
@@ -92,6 +98,31 @@ static const struct pwr_regs pwr_regs_bd71815 = {
 	.conf = BD71815_REG_CONF,
 
 	.vdcin = BD71815_REG_VM_DCIN_U,
+	.vdcin_himask = BD7182x_MASK_VDCIN_U,
+};
+
+static struct pwr_regs pwr_regs_bd72720 = {
+	.vbat_avg = BD72720_REG_VM_SA_VBAT_U,
+	.ibat = BD72720_REG_CC_CURCD_U,
+	.ibat_avg = BD72720_REG_CC_SA_CURCD_U,
+	.btemp_vth = BD72720_REG_VM_BTMP_U,
+	/*
+	 * Note, state 0x40 IMP_CHK. not documented
+	 * on other variants but was still handled in
+	 * existing code. No memory traces as to why.
+	 */
+	.chg_state = BD72720_REG_CHG_STATE,
+	.bat_temp = BD72720_REG_CHG_BAT_TEMP_STAT,
+	.dcin_stat = BD72720_REG_INT_VBUS_SRC,
+	.dcin_online_mask = BD72720_MASK_DCIN_DET,
+	.dcin_collapse_limit = -1, /* Automatic. Setting not supported */
+	.chg_set1 = BD72720_REG_CHG_SET_1,
+	.chg_en = BD72720_REG_CHG_EN,
+	/* 15mV note in data-sheet */
+	.vbat_alm_limit_u = BD72720_REG_ALM_VBAT_TH_U,
+	.conf = BD72720_REG_CONF, /* o XSTB, only PON. Seprate slave addr */
+	.vdcin = BD72720_REG_VM_VBUS_U, /* 10 bits not 11 as with other ICs */
+	.vdcin_himask = BD72720_MASK_VDCIN_U,
 };
 
 struct bd71828_power {
@@ -298,7 +329,7 @@ static int get_chg_online(struct bd71828_power *pwr, int *chg_online)
 		dev_err(pwr->dev, "Failed to read DCIN status\n");
 		return ret;
 	}
-	*chg_online = ((r & BD7182x_MASK_DCIN_DET) != 0);
+	*chg_online = ((r & pwr->regs->dcin_online_mask) != 0);
 
 	return 0;
 }
@@ -329,8 +360,8 @@ static int bd71828_bat_inserted(struct bd71828_power *pwr)
 	ret = val & BD7182x_MASK_CONF_PON;
 
 	if (ret)
-		regmap_update_bits(pwr->regmap, pwr->regs->conf,
-				   BD7182x_MASK_CONF_PON, 0);
+		if (regmap_update_bits(pwr->regmap, pwr->regs->conf, BD7182x_MASK_CONF_PON, 0))
+			dev_err(pwr->dev, "Failed to write CONF register\n");
 
 	return ret;
 }
@@ -358,11 +389,13 @@ static int bd71828_init_hardware(struct bd71828_power *pwr)
 	int ret;
 
 	/* TODO: Collapse limit should come from device-tree ? */
-	ret = regmap_write(pwr->regmap, pwr->regs->dcin_collapse_limit,
-			   BD7182x_DCIN_COLLAPSE_DEFAULT);
-	if (ret) {
-		dev_err(pwr->dev, "Failed to write DCIN collapse limit\n");
-		return ret;
+	if (pwr->regs->dcin_collapse_limit != (unsigned int)-1) {
+		ret = regmap_write(pwr->regmap, pwr->regs->dcin_collapse_limit,
+				   BD7182x_DCIN_COLLAPSE_DEFAULT);
+		if (ret) {
+			dev_err(pwr->dev, "Failed to write DCIN collapse limit\n");
+			return ret;
+		}
 	}
 
 	ret = pwr->bat_inserted(pwr);
@@ -419,7 +452,7 @@ static int bd71828_charger_get_property(struct power_supply *psy,
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 		ret = bd7182x_read16_himask(pwr, pwr->regs->vdcin,
-					    BD7182x_MASK_VDCIN_U, &tmp);
+					    pwr->regs->vdcin_himask, &tmp);
 		if (ret)
 			return ret;
 
@@ -630,6 +663,9 @@ BD_ISR_AC(dcin_ovp_det, "DCIN OVER VOLTAGE", true)
 BD_ISR_DUMMY(dcin_mon_det, "DCIN voltage below threshold")
 BD_ISR_DUMMY(dcin_mon_res, "DCIN voltage above threshold")
 
+BD_ISR_DUMMY(vbus_curr_limit, "VBUS current limited")
+BD_ISR_DUMMY(vsys_ov_res, "VSYS over-voltage cleared")
+BD_ISR_DUMMY(vsys_ov_det, "VSYS over-voltage")
 BD_ISR_DUMMY(vsys_uv_res, "VSYS under-voltage cleared")
 BD_ISR_DUMMY(vsys_uv_det, "VSYS under-voltage")
 BD_ISR_DUMMY(vsys_low_res, "'VSYS low' cleared")
@@ -878,6 +914,51 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
 		BDIRQ("bd71828-temp-125-over", bd71828_temp_vf125_det),
 		BDIRQ("bd71828-temp-125-under", bd71828_temp_vf125_res),
 	};
+	static const struct bd7182x_irq_res bd72720_irqs[] = {
+		BDIRQ("bd72720_int_vbus_rmv", BD_ISR_NAME(dcin_removed)),
+		BDIRQ("bd72720_int_vbus_det", bd7182x_dcin_detected),
+		BDIRQ("bd72720_int_vbus_mon_res", BD_ISR_NAME(dcin_mon_res)),
+		BDIRQ("bd72720_int_vbus_mon_det", BD_ISR_NAME(dcin_mon_det)),
+		BDIRQ("bd72720_int_vsys_mon_res", BD_ISR_NAME(vsys_mon_res)),
+		BDIRQ("bd72720_int_vsys_mon_det", BD_ISR_NAME(vsys_mon_det)),
+		BDIRQ("bd72720_int_vsys_uv_res", BD_ISR_NAME(vsys_uv_res)),
+		BDIRQ("bd72720_int_vsys_uv_det", BD_ISR_NAME(vsys_uv_det)),
+		BDIRQ("bd72720_int_vsys_lo_res", BD_ISR_NAME(vsys_low_res)),
+		BDIRQ("bd72720_int_vsys_lo_det", BD_ISR_NAME(vsys_low_det)),
+		BDIRQ("bd72720_int_vsys_ov_res", BD_ISR_NAME(vsys_ov_res)),
+		BDIRQ("bd72720_int_vsys_ov_det", BD_ISR_NAME(vsys_ov_det)),
+		BDIRQ("bd72720_int_bat_ilim", BD_ISR_NAME(vbus_curr_limit)),
+		BDIRQ("bd72720_int_chg_done", bd718x7_chg_done),
+		BDIRQ("bd72720_int_extemp_tout", BD_ISR_NAME(chg_wdg_temp)),
+		BDIRQ("bd72720_int_chg_wdt_exp", BD_ISR_NAME(chg_wdg)),
+		BDIRQ("bd72720_int_bat_mnt_out", BD_ISR_NAME(rechg_res)),
+		BDIRQ("bd72720_int_bat_mnt_in", BD_ISR_NAME(rechg_det)),
+		BDIRQ("bd72720_int_chg_trns", BD_ISR_NAME(chg_state_changed)),
+
+		BDIRQ("bd72720_int_vbat_mon_res", BD_ISR_NAME(bat_mon_res)),
+		BDIRQ("bd72720_int_vbat_mon_det", BD_ISR_NAME(bat_mon)),
+		BDIRQ("bd72720_int_vbat_sht_res", BD_ISR_NAME(bat_short_res)),
+		BDIRQ("bd72720_int_vbat_sht_det", BD_ISR_NAME(bat_short)),
+		BDIRQ("bd72720_int_vbat_lo_res", BD_ISR_NAME(bat_low_res)),
+		BDIRQ("bd72720_int_vbat_lo_det", BD_ISR_NAME(bat_low)),
+		BDIRQ("bd72720_int_vbat_ov_res", BD_ISR_NAME(bat_ov_res)),
+		BDIRQ("bd72720_int_vbat_ov_det", BD_ISR_NAME(bat_ov)),
+		BDIRQ("bd72720_int_bat_rmv", BD_ISR_NAME(bat_removed)),
+		BDIRQ("bd72720_int_bat_det", BD_ISR_NAME(bat_det)),
+		BDIRQ("bd72720_int_dbat_det", BD_ISR_NAME(bat_dead)),
+		BDIRQ("bd72720_int_bat_temp_trns", BD_ISR_NAME(temp_transit)),
+		BDIRQ("bd72720_int_lobtmp_res", BD_ISR_NAME(temp_bat_low_res)),
+		BDIRQ("bd72720_int_lobtmp_det", BD_ISR_NAME(temp_bat_low)),
+		BDIRQ("bd72720_int_ovbtmp_res", BD_ISR_NAME(temp_bat_hi_res)),
+		BDIRQ("bd72720_int_ovbtmp_det", BD_ISR_NAME(temp_bat_hi)),
+		BDIRQ("bd72720_int_ocur1_res", BD_ISR_NAME(bat_oc1_res)),
+		BDIRQ("bd72720_int_ocur1_det", BD_ISR_NAME(bat_oc1)),
+		BDIRQ("bd72720_int_ocur2_res", BD_ISR_NAME(bat_oc2_res)),
+		BDIRQ("bd72720_int_ocur2_det", BD_ISR_NAME(bat_oc2)),
+		BDIRQ("bd72720_int_ocur3_res", BD_ISR_NAME(bat_oc3_res)),
+		BDIRQ("bd72720_int_ocur3_det", BD_ISR_NAME(bat_oc3)),
+		BDIRQ("bd72720_int_cc_mon2_det", BD_ISR_NAME(bat_cc_mon)),
+	};
 	int num_irqs;
 	const struct bd7182x_irq_res *irqs;
 
@@ -890,6 +971,10 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
 		irqs = &bd71815_irqs[0];
 		num_irqs = ARRAY_SIZE(bd71815_irqs);
 		break;
+	case ROHM_CHIP_TYPE_BD72720:
+		irqs = &bd72720_irqs[0];
+		num_irqs = ARRAY_SIZE(bd72720_irqs);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -958,21 +1043,27 @@ static int bd71828_power_probe(struct platform_device *pdev)
 	struct power_supply_config ac_cfg = {};
 	struct power_supply_config bat_cfg = {};
 	int ret;
-	struct regmap *regmap;
-
-	regmap = dev_get_regmap(pdev->dev.parent, NULL);
-	if (!regmap) {
-		dev_err(&pdev->dev, "No parent regmap\n");
-		return -EINVAL;
-	}
 
 	pwr = devm_kzalloc(&pdev->dev, sizeof(*pwr), GFP_KERNEL);
 	if (!pwr)
 		return -ENOMEM;
 
-	pwr->regmap = regmap;
-	pwr->dev = &pdev->dev;
+	/*
+	 * The BD72720 MFD device registers two regmaps. Power-supply driver
+	 * uses the "wrap-map", which provides access to both of the I2C slave
+	 * addresses used by the BD72720
+	 */
 	pwr->chip_type = platform_get_device_id(pdev)->driver_data;
+	if (pwr->chip_type != ROHM_CHIP_TYPE_BD72720)
+		pwr->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	else
+		pwr->regmap = dev_get_regmap(pdev->dev.parent, "wrap-map");
+	if (!pwr->regmap) {
+		dev_err(&pdev->dev, "No parent regmap\n");
+		return -EINVAL;
+	}
+
+	pwr->dev = &pdev->dev;
 
 	switch (pwr->chip_type) {
 	case ROHM_CHIP_TYPE_BD71828:
@@ -985,6 +1076,12 @@ static int bd71828_power_probe(struct platform_device *pdev)
 		pwr->get_temp = bd71815_get_temp;
 		pwr->regs = &pwr_regs_bd71815;
 		break;
+	case ROHM_CHIP_TYPE_BD72720:
+		pwr->bat_inserted = bd71828_bat_inserted;
+		pwr->regs = &pwr_regs_bd72720;
+		pwr->get_temp = bd71828_get_temp;
+		dev_dbg(pwr->dev, "Found ROHM BD72720\n");
+		break;
 	default:
 		dev_err(pwr->dev, "Unknown PMIC\n");
 		return -EINVAL;
@@ -1030,6 +1127,7 @@ static int bd71828_power_probe(struct platform_device *pdev)
 static const struct platform_device_id bd71828_charger_id[] = {
 	{ "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
 	{ "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
+	{ "bd72720-power", ROHM_CHIP_TYPE_BD72720 },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
Signature · 1 line
2.51.1
#30 lore raw

PATCH v4 16/16 MAINTAINERS: Add ROHM BD72720 PMIC

In reply to Matti Vaittinen

From: Matti Vaittinen <[email protected]>

Add the ROHM BD72720 PMIC driver files to be maintained by undersigned.

---
Revision history:
RFCv1 =>:
- No changes

---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)
MAINTAINERS +2 -0
index fe01aa31c58b..7e3c1eac7cda 100644
@@ -22353,6 +22353,7 @@ S:	Supported
 F:	drivers/clk/clk-bd718x7.c
 F:	drivers/gpio/gpio-bd71815.c
 F:	drivers/gpio/gpio-bd71828.c
+F:	drivers/gpio/gpio-bd72720.c
 F:	drivers/mfd/rohm-bd71828.c
 F:	drivers/mfd/rohm-bd718x7.c
 F:	drivers/mfd/rohm-bd9576.c
@@ -22369,6 +22370,7 @@ F:	drivers/watchdog/bd96801_wdt.c
 F:	include/linux/mfd/rohm-bd71815.h
 F:	include/linux/mfd/rohm-bd71828.h
 F:	include/linux/mfd/rohm-bd718x7.h
+F:	include/linux/mfd/rohm-bd72720.h
 F:	include/linux/mfd/rohm-bd957x.h
 F:	include/linux/mfd/rohm-bd96801.h
 F:	include/linux/mfd/rohm-bd96802.h
Signature · 1 line
2.51.1