[PATCH v2 0/6] mfd: bcm590xx: Add support for BCM59054

[PATCH v2 0/6] mfd: bcm590xx: Add support for BCM59054

[Artur Weber]

Add support for the BCM59054 MFD to the bcm590xx driver and fix a
couple of small bugs in it that also affected the already supported
BCM59056.

While we're at it - convert the devicetree bindings to YAML format
and drop the bcm59056 DTS in favor of describing the PMU in users'
DTS files, as is done for most other MFDs.

The BCM59054 is fairly similar to the BCM59056, with the primary
difference being the different number and layout of regulators.
It is primarily used in devices using the BCM21664 and BCM23550
chipsets.

I'd appreciate testing on BCM59056-equipped boards to make sure
they aren't affected negatively by the changes. So far, I've
tested this patch series on a Samsung Galaxy Grand Neo (BAFFINLITE
REV02) with a BCM23550 chipset (BCM59054 MFD); this device is not
yet supported in the mainline kernel, but I'm working on adding
support for it, and other commercially-available devices using
Broadcom Kona chips. Hopefully some of that work will make it
into mainline in the near future ;)

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
Changes in v2:
- Fixed BCM59054 ID being passed to BCM59056 function in the
  regulator driver
- Dropped linux-rpi-kernel from the CC list
- Link to v1: https://lore.kernel.org/r/20231030-bcm59054-v1-0-3517f980c1e3@gmail.com

---
Artur Weber (6):
      dt-bindings: mfd: brcm,bcm59056: Convert to YAML
      dt-bindings: mfd: brcm,bcm59056: Add compatible for BCM59054
      ARM: dts: Drop DTS for BCM59056 PMIC
      mfd: bcm590xx: Add compatible for BCM59054
      regulator: bcm590xx: Add support for BCM59054
      regulator: bcm590xx: Add proper handling for PMMODE registers

 .../devicetree/bindings/mfd/brcm,bcm59056.txt      |  39 --
 .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 142 +++++
 arch/arm/boot/dts/broadcom/bcm28155-ap.dts         |  68 +-
 arch/arm/boot/dts/broadcom/bcm59056.dtsi           |  91 ---
 drivers/mfd/bcm590xx.c                             |   5 +-
 drivers/regulator/bcm590xx-regulator.c             | 708 ++++++++++++++++-----
 include/linux/mfd/bcm590xx.h                       |   7 +
 7 files changed, 728 insertions(+), 332 deletions(-)
---
base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1
change-id: 20231029-bcm59054-3ed65e649435

Best regards,
-- 
Artur Weber <aweber.kernel@gmail.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/6] dt-bindings: mfd: brcm,bcm59056: Convert to YAML

[Artur Weber]

Convert devicetree bindings for the Broadcom BCM59056 PMU MFD from
TXT to YAML format. This patch does not change any functionality;
the bindings remain the same.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 .../devicetree/bindings/mfd/brcm,bcm59056.txt      | 39 ----------
 .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 91 ++++++++++++++++++++++
 2 files changed, 91 insertions(+), 39 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt
deleted file mode 100644
index be51a15e05f9..000000000000
--- a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt
+++ /dev/null
@@ -1,39 +0,0 @@
--------------------------------
-BCM590xx Power Management Units
--------------------------------
-
-Required properties:
-- compatible: "brcm,bcm59056"
-- reg: I2C slave address
-- interrupts: interrupt for the PMU. Generic interrupt client node bindings
-  are described in interrupt-controller/interrupts.txt
-
-------------------
-Voltage Regulators
-------------------
-
-Optional child nodes:
-- regulators: container node for regulators following the generic
-  regulator binding in regulator/regulator.txt
-
-  The valid regulator node names for BCM59056 are:
-  	rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
-	mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
-	csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
-	gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
-	vbus
-
-Example:
-	pmu: bcm59056@8 {
-		compatible = "brcm,bcm59056";
-		reg = <0x08>;
-		interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
-		regulators {
-			rfldo_reg: rfldo {
-				regulator-min-microvolt = <1200000>;
-				regulator-max-microvolt = <3300000>;
-			};
-
-			...
-		};
-	};
diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
new file mode 100644
index 000000000000..6d6acc9fd312
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/brcm,bcm59056.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: BCM590xx Power Management Unit MFD
+
+maintainers:
+  - Artur Weber <aweber.kernel@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - "brcm,bcm59056"
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  regulators:
+    type: object
+    description: Container node for regulators.
+
+    # The valid regulator node names for BCM59056 are:
+    #   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
+    #   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
+    #   csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
+    #   gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
+    #   vbus
+    patternProperties:
+      "^(cam|sim|mmc)ldo[1-2]$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+      "^(rf|sd|sdx|aud|mic|usb|vib)ldo$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+      "^(c|m|v)sr$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+      "^(io|sd)sr[1-2]$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+      "^gpldo[1-6]$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+    properties:
+      "vbus":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmu: bcm59056@8 {
+            compatible = "brcm,bcm59056";
+            reg = <0x08>;
+            interrupts = <0 215 IRQ_TYPE_LEVEL_HIGH>;
+            regulators {
+                rfldo_reg: rfldo {
+                    regulator-min-microvolt = <1200000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+            };
+        };
+    };

-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/6] dt-bindings: mfd: brcm,bcm59056: Add compatible for BCM59054

[Artur Weber]

The BCM59054 MFD is fairly similar to the BCM59056, and will use
the same driver. Add compatible and specify the allowed regulator
nodes.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 125 +++++++++++++++------
 1 file changed, 88 insertions(+), 37 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
index 6d6acc9fd312..5b5044867eca 100644
--- a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
+++ b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
@@ -12,6 +12,7 @@ maintainers:
 properties:
   compatible:
     enum:
+      - "brcm,bcm59054"
       - "brcm,bcm59056"
 
   reg:
@@ -24,43 +25,93 @@ properties:
     type: object
     description: Container node for regulators.
 
-    # The valid regulator node names for BCM59056 are:
-    #   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
-    #   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
-    #   csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
-    #   gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
-    #   vbus
-    patternProperties:
-      "^(cam|sim|mmc)ldo[1-2]$":
-        type: object
-        $ref: /schemas/regulator/regulator.yaml#
-        unevaluatedProperties: false
-
-      "^(rf|sd|sdx|aud|mic|usb|vib)ldo$":
-        type: object
-        $ref: /schemas/regulator/regulator.yaml#
-        unevaluatedProperties: false
-
-      "^(c|m|v)sr$":
-        type: object
-        $ref: /schemas/regulator/regulator.yaml#
-        unevaluatedProperties: false
-
-      "^(io|sd)sr[1-2]$":
-        type: object
-        $ref: /schemas/regulator/regulator.yaml#
-        unevaluatedProperties: false
-
-      "^gpldo[1-6]$":
-        type: object
-        $ref: /schemas/regulator/regulator.yaml#
-        unevaluatedProperties: false
-
-    properties:
-      "vbus":
-        type: object
-        $ref: /schemas/regulator/regulator.yaml#
-        unevaluatedProperties: false
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: "brcm,bcm59056"
+    then:
+      properties:
+        regulators:
+          # The valid regulator node names for BCM59056 are:
+          #   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
+          #   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
+          #   csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
+          #   gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
+          #   vbus
+          patternProperties:
+            "^(cam|sim|mmc)ldo[1-2]$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^(rf|sd|sdx|aud|mic|usb|vib)ldo$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^(c|m|v)sr$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^(io|sd)sr[1-2]$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^gpldo[1-6]$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+          properties:
+            "vbus":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+          additionalProperties: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: "brcm,bcm59054"
+    then:
+      properties:
+        regulators:
+          # The valid regulator node names for BCM59054 are:
+          #   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
+          #   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
+          #   csr, iosr1, iosr2, mmsr, sdsr1, sdsr2, vsr,
+          #   gpldo1, gpldo2, gpldo3, tcxldo, lvldo1, lvldo2
+          patternProperties:
+            "^(cam|sim|mmc)ldo[1-2]$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^(rf|sd|sdx|aud|mic|usb|vib|tcx)ldo$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^(c|mm|v)sr$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^(io|sd)sr[1-2]$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^gpldo[1-3]$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+            "^lvldo[1-2]$":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+          properties:
+            "vbus":
+              type: object
+              $ref: /schemas/regulator/regulator.yaml#
+
+          additionalProperties: false
 
 required:
   - compatible

-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/6] ARM: dts: Drop DTS for BCM59056 PMIC

[Artur Weber]

The BCM59056 PMIC has its own separate DTSI, meant to be included
in a DTS file after defining the pmu node on some I2C bus.

I'm not aware of many other devices that do this, and it seems very
unintuitive. Drop the DTS in favor of adding the BCM59056 PMIC node
directly into the device DTS files.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 arch/arm/boot/dts/broadcom/bcm28155-ap.dts | 68 +++++++++++-----------
 arch/arm/boot/dts/broadcom/bcm59056.dtsi   | 91 ------------------------------
 2 files changed, 32 insertions(+), 127 deletions(-)

diff --git a/arch/arm/boot/dts/broadcom/bcm28155-ap.dts b/arch/arm/boot/dts/broadcom/bcm28155-ap.dts
index 2f3634545e64..cefaa9a3c45c 100644
--- a/arch/arm/boot/dts/broadcom/bcm28155-ap.dts
+++ b/arch/arm/boot/dts/broadcom/bcm28155-ap.dts
@@ -37,7 +37,39 @@ &pmu_bsc {
 	status = "okay";
 
 	pmu: pmu@8 {
+		compatible = "brcm,bcm59056";
+		interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0x08>;
+
+		regulators {
+			camldo1_reg: camldo1 {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-always-on;
+			};
+
+			sdldo_reg: sdldo {
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3000000>;
+			};
+
+			sdxldo_reg: sdxldo {
+				regulator-min-microvolt = <2700000>;
+				regulator-max-microvolt = <3300000>;
+			};
+
+			usbldo_reg: usbldo {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-always-on;
+			};
+
+			iosr1_reg: iosr1 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+			};
+		};
 	};
 };
 
@@ -74,39 +106,3 @@ &usbotg {
 &usbphy {
 	status = "okay";
 };
-
-#include "bcm59056.dtsi"
-
-&pmu {
-	compatible = "brcm,bcm59056";
-	interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
-	regulators {
-		camldo1_reg: camldo1 {
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-always-on;
-		};
-
-		sdldo_reg: sdldo {
-			regulator-min-microvolt = <3000000>;
-			regulator-max-microvolt = <3000000>;
-		};
-
-		sdxldo_reg: sdxldo {
-			regulator-min-microvolt = <2700000>;
-			regulator-max-microvolt = <3300000>;
-		};
-
-		usbldo_reg: usbldo {
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-always-on;
-		};
-
-		iosr1_reg: iosr1 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-always-on;
-		};
-	};
-};
diff --git a/arch/arm/boot/dts/broadcom/bcm59056.dtsi b/arch/arm/boot/dts/broadcom/bcm59056.dtsi
deleted file mode 100644
index a9bb7ad81378..000000000000
--- a/arch/arm/boot/dts/broadcom/bcm59056.dtsi
+++ /dev/null
@@ -1,91 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
-* Copyright 2014 Linaro Limited
-* Author: Matt Porter <mporter@linaro.org>
-*/
-
-&pmu {
-	compatible = "brcm,bcm59056";
-	regulators {
-		rfldo_reg: rfldo {
-		};
-
-		camldo1_reg: camldo1 {
-		};
-
-		camldo2_reg: camldo2 {
-		};
-
-		simldo1_reg: simldo1 {
-		};
-
-		simldo2_reg: simldo2 {
-		};
-
-		sdldo_reg: sdldo {
-		};
-
-		sdxldo_reg: sdxldo {
-		};
-
-		mmcldo1_reg: mmcldo1 {
-		};
-
-		mmcldo2_reg: mmcldo2 {
-		};
-
-		audldo_reg: audldo {
-		};
-
-		micldo_reg: micldo {
-		};
-
-		usbldo_reg: usbldo {
-		};
-
-		vibldo_reg: vibldo {
-		};
-
-		csr_reg: csr {
-		};
-
-		iosr1_reg: iosr1 {
-		};
-
-		iosr2_reg: iosr2 {
-		};
-
-		msr_reg: msr {
-		};
-
-		sdsr1_reg: sdsr1 {
-		};
-
-		sdsr2_reg: sdsr2 {
-		};
-
-		vsr_reg: vsr {
-		};
-
-		gpldo1_reg: gpldo1 {
-		};
-
-		gpldo2_reg: gpldo2 {
-		};
-
-		gpldo3_reg: gpldo3 {
-		};
-
-		gpldo4_reg: gpldo4 {
-		};
-
-		gpldo5_reg: gpldo5 {
-		};
-
-		gpldo6_reg: gpldo6 {
-		};
-
-		vbus_reg: vbus {
-		};
-	};
-};

-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/6] mfd: bcm590xx: Add compatible for BCM59054

[Artur Weber]

The BCM59056 supported by the bcm590xx driver is similar to the
BCM59054 MFD. Add a compatible for it in the driver, in preparation
for adding support for this chip in the bcm590xx regulator driver.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 drivers/mfd/bcm590xx.c       | 5 ++++-
 include/linux/mfd/bcm590xx.h | 7 +++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
index 92eede9a5e61..f5c5fa97ca9a 100644
--- a/drivers/mfd/bcm590xx.c
+++ b/drivers/mfd/bcm590xx.c
@@ -50,6 +50,8 @@ static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri)
 	bcm590xx->dev = &i2c_pri->dev;
 	bcm590xx->i2c_pri = i2c_pri;
 
+	bcm590xx->device_type = (unsigned int)of_device_get_match_data(bcm590xx->dev);
+
 	bcm590xx->regmap_pri = devm_regmap_init_i2c(i2c_pri,
 						 &bcm590xx_regmap_config_pri);
 	if (IS_ERR(bcm590xx->regmap_pri)) {
@@ -91,7 +93,8 @@ static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri)
 }
 
 static const struct of_device_id bcm590xx_of_match[] = {
-	{ .compatible = "brcm,bcm59056" },
+	{ .compatible = "brcm,bcm59054", .data = (void *)BCM59054_TYPE },
+	{ .compatible = "brcm,bcm59056", .data = (void *)BCM59056_TYPE },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, bcm590xx_of_match);
diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
index 6b8791da6119..ed0d8be6387c 100644
--- a/include/linux/mfd/bcm590xx.h
+++ b/include/linux/mfd/bcm590xx.h
@@ -13,12 +13,19 @@
 #include <linux/i2c.h>
 #include <linux/regmap.h>
 
+/* device types */
+enum {
+	BCM59054_TYPE,
+	BCM59056_TYPE,
+};
+
 /* max register address */
 #define BCM590XX_MAX_REGISTER_PRI	0xe7
 #define BCM590XX_MAX_REGISTER_SEC	0xf0
 
 struct bcm590xx {
 	struct device *dev;
+	unsigned int device_type;
 	struct i2c_client *i2c_pri;
 	struct i2c_client *i2c_sec;
 	struct regmap *regmap_pri;

-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/6] regulator: bcm590xx: Add support for BCM59054

[Artur Weber]

The BCM59054 is fairly similar in terms of regulators to the already
supported BCM59056, as included in the BCM590XX driver.

Add support for the BCM59054's regulators to the BCM590XX driver.
Switch from using defines for common checks to using functions which
return different values depending on the identified MFD model.

While we're at it, fix a bug where the enable/vsel register
offsets for GPLDO and LDO regulators were calculated incorrectly.

Also, change the regulator enable bitmask to cover the entire PMMODE
register.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 drivers/regulator/bcm590xx-regulator.c | 624 ++++++++++++++++++++++++---------
 1 file changed, 465 insertions(+), 159 deletions(-)

diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
index 9f0cda46b015..8b90eae06ca6 100644
--- a/drivers/regulator/bcm590xx-regulator.c
+++ b/drivers/regulator/bcm590xx-regulator.c
@@ -18,37 +18,54 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/slab.h>
 
+struct bcm590xx_info {
+	const char *name;
+	const char *vin_name;
+	u8 n_voltages;
+	const unsigned int *volt_table;
+	u8 n_linear_ranges;
+	const struct linear_range *linear_ranges;
+};
+
+struct bcm590xx_reg {
+	struct regulator_desc *desc;
+	struct bcm590xx *mfd;
+};
+
+#define BCM590XX_REG_ENABLE		GENMASK(7, 0)
+#define BCM590XX_REG_3BIT_ENABLE	(GENMASK(4, 3) | GENMASK(1, 0))
+#define BCM590XX_VBUS_ENABLE		BIT(2)
+#define BCM590XX_LDO_VSEL_MASK		GENMASK(5, 3)
+#define BCM590XX_SR_VSEL_MASK		GENMASK(5, 0)
+
+/* BCM59056 registers */
+
 /* I2C slave 0 registers */
-#define BCM590XX_RFLDOPMCTRL1	0x60
-#define BCM590XX_IOSR1PMCTRL1	0x7a
-#define BCM590XX_IOSR2PMCTRL1	0x7c
-#define BCM590XX_CSRPMCTRL1	0x7e
-#define BCM590XX_SDSR1PMCTRL1	0x82
-#define BCM590XX_SDSR2PMCTRL1	0x86
-#define BCM590XX_MSRPMCTRL1	0x8a
-#define BCM590XX_VSRPMCTRL1	0x8e
-#define BCM590XX_RFLDOCTRL	0x96
-#define BCM590XX_CSRVOUT1	0xc0
+#define BCM59056_RFLDOPMCTRL1	0x60
+#define BCM59056_IOSR1PMCTRL1	0x7a
+#define BCM59056_IOSR2PMCTRL1	0x7c
+#define BCM59056_CSRPMCTRL1	0x7e
+#define BCM59056_SDSR1PMCTRL1	0x82
+#define BCM59056_SDSR2PMCTRL1	0x86
+#define BCM59056_MSRPMCTRL1	0x8a
+#define BCM59056_VSRPMCTRL1	0x8e
+#define BCM59056_RFLDOCTRL	0x96
+#define BCM59056_CSRVOUT1	0xc0
 
 /* I2C slave 1 registers */
-#define BCM590XX_GPLDO5PMCTRL1	0x16
-#define BCM590XX_GPLDO6PMCTRL1	0x18
-#define BCM590XX_GPLDO1CTRL	0x1a
-#define BCM590XX_GPLDO2CTRL	0x1b
-#define BCM590XX_GPLDO3CTRL	0x1c
-#define BCM590XX_GPLDO4CTRL	0x1d
-#define BCM590XX_GPLDO5CTRL	0x1e
-#define BCM590XX_GPLDO6CTRL	0x1f
-#define BCM590XX_OTG_CTRL	0x40
-#define BCM590XX_GPLDO1PMCTRL1	0x57
-#define BCM590XX_GPLDO2PMCTRL1	0x59
-#define BCM590XX_GPLDO3PMCTRL1	0x5b
-#define BCM590XX_GPLDO4PMCTRL1	0x5d
-
-#define BCM590XX_REG_ENABLE	BIT(7)
-#define BCM590XX_VBUS_ENABLE	BIT(2)
-#define BCM590XX_LDO_VSEL_MASK	GENMASK(5, 3)
-#define BCM590XX_SR_VSEL_MASK	GENMASK(5, 0)
+#define BCM59056_GPLDO5PMCTRL1	0x16
+#define BCM59056_GPLDO6PMCTRL1	0x18
+#define BCM59056_GPLDO1CTRL	0x1a
+#define BCM59056_GPLDO2CTRL	0x1b
+#define BCM59056_GPLDO3CTRL	0x1c
+#define BCM59056_GPLDO4CTRL	0x1d
+#define BCM59056_GPLDO5CTRL	0x1e
+#define BCM59056_GPLDO6CTRL	0x1f
+#define BCM59056_OTG_CTRL	0x40
+#define BCM59056_GPLDO1PMCTRL1	0x57
+#define BCM59056_GPLDO2PMCTRL1	0x59
+#define BCM59056_GPLDO3PMCTRL1	0x5b
+#define BCM59056_GPLDO4PMCTRL1	0x5d
 
 /*
  * RFLDO to VSR regulators are
@@ -56,74 +73,69 @@
  */
 
 /* LDO regulator IDs */
-#define BCM590XX_REG_RFLDO	0
-#define BCM590XX_REG_CAMLDO1	1
-#define BCM590XX_REG_CAMLDO2	2
-#define BCM590XX_REG_SIMLDO1	3
-#define BCM590XX_REG_SIMLDO2	4
-#define BCM590XX_REG_SDLDO	5
-#define BCM590XX_REG_SDXLDO	6
-#define BCM590XX_REG_MMCLDO1	7
-#define BCM590XX_REG_MMCLDO2	8
-#define BCM590XX_REG_AUDLDO	9
-#define BCM590XX_REG_MICLDO	10
-#define BCM590XX_REG_USBLDO	11
-#define BCM590XX_REG_VIBLDO	12
+#define BCM59056_REG_RFLDO	0
+#define BCM59056_REG_CAMLDO1	1
+#define BCM59056_REG_CAMLDO2	2
+#define BCM59056_REG_SIMLDO1	3
+#define BCM59056_REG_SIMLDO2	4
+#define BCM59056_REG_SDLDO	5
+#define BCM59056_REG_SDXLDO	6
+#define BCM59056_REG_MMCLDO1	7
+#define BCM59056_REG_MMCLDO2	8
+#define BCM59056_REG_AUDLDO	9
+#define BCM59056_REG_MICLDO	10
+#define BCM59056_REG_USBLDO	11
+#define BCM59056_REG_VIBLDO	12
 
 /* DCDC regulator IDs */
-#define BCM590XX_REG_CSR	13
-#define BCM590XX_REG_IOSR1	14
-#define BCM590XX_REG_IOSR2	15
-#define BCM590XX_REG_MSR	16
-#define BCM590XX_REG_SDSR1	17
-#define BCM590XX_REG_SDSR2	18
-#define BCM590XX_REG_VSR	19
+#define BCM59056_REG_CSR	13
+#define BCM59056_REG_IOSR1	14
+#define BCM59056_REG_IOSR2	15
+#define BCM59056_REG_MSR	16
+#define BCM59056_REG_SDSR1	17
+#define BCM59056_REG_SDSR2	18
+#define BCM59056_REG_VSR	19
 
 /*
  * GPLDO1 to VBUS regulators are
  * accessed via I2C slave 1
  */
 
-#define BCM590XX_REG_GPLDO1	20
-#define BCM590XX_REG_GPLDO2	21
-#define BCM590XX_REG_GPLDO3	22
-#define BCM590XX_REG_GPLDO4	23
-#define BCM590XX_REG_GPLDO5	24
-#define BCM590XX_REG_GPLDO6	25
-#define BCM590XX_REG_VBUS	26
-
-#define BCM590XX_NUM_REGS	27
+#define BCM59056_REG_GPLDO1	20
+#define BCM59056_REG_GPLDO2	21
+#define BCM59056_REG_GPLDO3	22
+#define BCM59056_REG_GPLDO4	23
+#define BCM59056_REG_GPLDO5	24
+#define BCM59056_REG_GPLDO6	25
+#define BCM59056_REG_VBUS	26
 
-#define BCM590XX_REG_IS_LDO(n)	(n < BCM590XX_REG_CSR)
-#define BCM590XX_REG_IS_GPLDO(n) \
-	((n > BCM590XX_REG_VSR) && (n < BCM590XX_REG_VBUS))
-#define BCM590XX_REG_IS_VBUS(n)	(n == BCM590XX_REG_VBUS)
+#define BCM59056_NUM_REGS	27
 
 /* LDO group A: supported voltages in microvolts */
-static const unsigned int ldo_a_table[] = {
+static const unsigned int bcm59056_ldo_a_table[] = {
 	1200000, 1800000, 2500000, 2700000, 2800000,
 	2900000, 3000000, 3300000,
 };
 
 /* LDO group C: supported voltages in microvolts */
-static const unsigned int ldo_c_table[] = {
+static const unsigned int bcm59056_ldo_c_table[] = {
 	3100000, 1800000, 2500000, 2700000, 2800000,
 	2900000, 3000000, 3300000,
 };
 
-static const unsigned int ldo_vbus[] = {
+static const unsigned int bcm59056_ldo_vbus[] = {
 	5000000,
 };
 
 /* DCDC group CSR: supported voltages in microvolts */
-static const struct linear_range dcdc_csr_ranges[] = {
+static const struct linear_range bcm59056_dcdc_csr_ranges[] = {
 	REGULATOR_LINEAR_RANGE(860000, 2, 50, 10000),
 	REGULATOR_LINEAR_RANGE(1360000, 51, 55, 20000),
 	REGULATOR_LINEAR_RANGE(900000, 56, 63, 0),
 };
 
 /* DCDC group IOSR1: supported voltages in microvolts */
-static const struct linear_range dcdc_iosr1_ranges[] = {
+static const struct linear_range bcm59056_dcdc_iosr1_ranges[] = {
 	REGULATOR_LINEAR_RANGE(860000, 2, 51, 10000),
 	REGULATOR_LINEAR_RANGE(1500000, 52, 52, 0),
 	REGULATOR_LINEAR_RANGE(1800000, 53, 53, 0),
@@ -131,121 +143,404 @@ static const struct linear_range dcdc_iosr1_ranges[] = {
 };
 
 /* DCDC group SDSR1: supported voltages in microvolts */
-static const struct linear_range dcdc_sdsr1_ranges[] = {
+static const struct linear_range bcm59056_dcdc_sdsr1_ranges[] = {
 	REGULATOR_LINEAR_RANGE(860000, 2, 50, 10000),
 	REGULATOR_LINEAR_RANGE(1340000, 51, 51, 0),
 	REGULATOR_LINEAR_RANGE(900000, 52, 63, 0),
 };
 
-struct bcm590xx_info {
-	const char *name;
-	const char *vin_name;
-	u8 n_voltages;
-	const unsigned int *volt_table;
-	u8 n_linear_ranges;
-	const struct linear_range *linear_ranges;
+#define BCM59056_REG_TABLE(_name, _table) \
+	{ \
+		.name = #_name, \
+		.n_voltages = ARRAY_SIZE(bcm59056_##_table), \
+		.volt_table = bcm59056_##_table, \
+	}
+
+#define BCM59056_REG_RANGES(_name, _ranges) \
+	{ \
+		.name = #_name, \
+		.n_voltages = 64, \
+		.n_linear_ranges = ARRAY_SIZE(bcm59056_##_ranges), \
+		.linear_ranges = bcm59056_##_ranges, \
+	}
+
+static struct bcm590xx_info bcm59056_regs[] = {
+	BCM59056_REG_TABLE(rfldo, ldo_a_table),
+	BCM59056_REG_TABLE(camldo1, ldo_c_table),
+	BCM59056_REG_TABLE(camldo2, ldo_c_table),
+	BCM59056_REG_TABLE(simldo1, ldo_a_table),
+	BCM59056_REG_TABLE(simldo2, ldo_a_table),
+	BCM59056_REG_TABLE(sdldo, ldo_c_table),
+	BCM59056_REG_TABLE(sdxldo, ldo_a_table),
+	BCM59056_REG_TABLE(mmcldo1, ldo_a_table),
+	BCM59056_REG_TABLE(mmcldo2, ldo_a_table),
+	BCM59056_REG_TABLE(audldo, ldo_a_table),
+	BCM59056_REG_TABLE(micldo, ldo_a_table),
+	BCM59056_REG_TABLE(usbldo, ldo_a_table),
+	BCM59056_REG_TABLE(vibldo, ldo_c_table),
+	BCM59056_REG_RANGES(csr, dcdc_csr_ranges),
+	BCM59056_REG_RANGES(iosr1, dcdc_iosr1_ranges),
+	BCM59056_REG_RANGES(iosr2, dcdc_iosr1_ranges),
+	BCM59056_REG_RANGES(msr, dcdc_iosr1_ranges),
+	BCM59056_REG_RANGES(sdsr1, dcdc_sdsr1_ranges),
+	BCM59056_REG_RANGES(sdsr2, dcdc_iosr1_ranges),
+	BCM59056_REG_RANGES(vsr, dcdc_iosr1_ranges),
+	BCM59056_REG_TABLE(gpldo1, ldo_a_table),
+	BCM59056_REG_TABLE(gpldo2, ldo_a_table),
+	BCM59056_REG_TABLE(gpldo3, ldo_a_table),
+	BCM59056_REG_TABLE(gpldo4, ldo_a_table),
+	BCM59056_REG_TABLE(gpldo5, ldo_a_table),
+	BCM59056_REG_TABLE(gpldo6, ldo_a_table),
+	BCM59056_REG_TABLE(vbus, ldo_vbus),
+};
+
+/* BCM59054 registers */
+
+/* I2C slave 0 registers */
+#define BCM59054_RFLDOPMCTRL1	0x60
+#define BCM59054_IOSR1PMCTRL1	0x7a
+#define BCM59054_IOSR2PMCTRL1	0x7c
+#define BCM59054_CSRPMCTRL1	0x7e
+#define BCM59054_SDSR1PMCTRL1	0x82
+#define BCM59054_SDSR2PMCTRL1	0x86
+#define BCM59054_MMSRPMCTRL1	0x8a
+#define BCM59054_VSRPMCTRL1	0x8e
+#define BCM59054_RFLDOCTRL	0x96
+#define BCM59054_CSRVOUT1	0xc0
+
+/* I2C slave 1 registers */
+#define BCM59054_LVLDO1PMCTRL1	0x16
+#define BCM59054_LVLDO2PMCTRL1	0x18
+#define BCM59054_GPLDO1CTRL	0x1a
+#define BCM59054_GPLDO2CTRL	0x1b
+#define BCM59054_GPLDO3CTRL	0x1c
+#define BCM59054_TCXLDOCTRL	0x1d
+#define BCM59054_LVLDO1CTRL	0x1e
+#define BCM59054_LVLDO2CTRL	0x1f
+#define BCM59054_OTG_CTRL	0x40
+#define BCM59054_GPLDO1PMCTRL1	0x57
+#define BCM59054_GPLDO2PMCTRL1	0x59
+#define BCM59054_GPLDO3PMCTRL1	0x5b
+#define BCM59054_TCXLDOPMCTRL1	0x5d
+
+/*
+ * RFLDO to VSR regulators are
+ * accessed via I2C slave 0
+ */
+
+/* LDO regulator IDs */
+#define BCM59054_REG_RFLDO	0
+#define BCM59054_REG_CAMLDO1	1
+#define BCM59054_REG_CAMLDO2	2
+#define BCM59054_REG_SIMLDO1	3
+#define BCM59054_REG_SIMLDO2	4
+#define BCM59054_REG_SDLDO	5
+#define BCM59054_REG_SDXLDO	6
+#define BCM59054_REG_MMCLDO1	7
+#define BCM59054_REG_MMCLDO2	8
+#define BCM59054_REG_AUDLDO	9
+#define BCM59054_REG_MICLDO	10
+#define BCM59054_REG_USBLDO	11
+#define BCM59054_REG_VIBLDO	12
+
+/* DCDC regulator IDs */
+#define BCM59054_REG_CSR	13
+#define BCM59054_REG_IOSR1	14
+#define BCM59054_REG_IOSR2	15
+#define BCM59054_REG_MMSR	16
+#define BCM59054_REG_SDSR1	17
+#define BCM59054_REG_SDSR2	18
+#define BCM59054_REG_VSR	19
+
+/*
+ * GPLDO1 to VBUS regulators are
+ * accessed via I2C slave 1
+ */
+
+#define BCM59054_REG_GPLDO1	20
+#define BCM59054_REG_GPLDO2	21
+#define BCM59054_REG_GPLDO3	22
+#define BCM59054_REG_TCXLDO	23
+#define BCM59054_REG_LVLDO1	24
+#define BCM59054_REG_LVLDO2	25
+#define BCM59054_REG_VBUS	26
+
+#define BCM59054_NUM_REGS	27
+
+/* LDO group 1: supported voltages in microvolts */
+static const unsigned int bcm59054_ldo_1_table[] = {
+	1200000, 1800000, 2500000, 2700000, 2800000,
+	2900000, 3000000, 3300000,
+};
+
+/* LDO group 2: supported voltages in microvolts */
+static const unsigned int bcm59054_ldo_2_table[] = {
+	3100000, 1800000, 2500000, 2700000, 2800000,
+	2900000, 3000000, 3300000,
+};
+
+/* LDO group 3: supported voltages in microvolts */
+static const unsigned int bcm59054_ldo_3_table[] = {
+	1000000, 1107000, 1143000, 1214000, 1250000,
+	1464000, 1500000, 1786000,
+};
+
+static const unsigned int bcm59054_ldo_vbus[] = {
+	5000000,
+};
+
+/* DCDC group SR: supported voltages in microvolts */
+static const struct linear_range bcm59054_dcdc_sr_ranges[] = {
+	REGULATOR_LINEAR_RANGE(0, 0, 1, 0),
+	REGULATOR_LINEAR_RANGE(860000, 2, 60, 10000),
+	REGULATOR_LINEAR_RANGE(1500000, 61, 61, 0),
+	REGULATOR_LINEAR_RANGE(1800000, 62, 62, 0),
+	REGULATOR_LINEAR_RANGE(900000, 63, 63, 0),
+};
+
+/* DCDC group VSR: supported voltages in microvolts */
+static const struct linear_range bcm59054_dcdc_vsr_ranges[] = {
+	REGULATOR_LINEAR_RANGE(0, 0, 1, 0),
+	REGULATOR_LINEAR_RANGE(860000, 2, 59, 10000),
+	REGULATOR_LINEAR_RANGE(1700000, 60, 60, 0),
+	REGULATOR_LINEAR_RANGE(1500000, 61, 61, 0),
+	REGULATOR_LINEAR_RANGE(1800000, 62, 62, 0),
+	REGULATOR_LINEAR_RANGE(1600000, 63, 63, 0),
 };
 
-#define BCM590XX_REG_TABLE(_name, _table) \
+/* DCDC group CSR: supported voltages in microvolts */
+static const struct linear_range bcm59054_dcdc_csr_ranges[] = {
+	REGULATOR_LINEAR_RANGE(700000, 0, 1, 100000),
+	REGULATOR_LINEAR_RANGE(860000, 2, 60, 10000),
+	REGULATOR_LINEAR_RANGE(900000, 61, 63, 0),
+};
+
+#define BCM59054_REG_TABLE(_name, _table) \
 	{ \
 		.name = #_name, \
-		.n_voltages = ARRAY_SIZE(_table), \
-		.volt_table = _table, \
+		.n_voltages = ARRAY_SIZE(bcm59054_##_table), \
+		.volt_table = bcm59054_##_table, \
 	}
 
-#define BCM590XX_REG_RANGES(_name, _ranges) \
+#define BCM59054_REG_RANGES(_name, _ranges) \
 	{ \
 		.name = #_name, \
 		.n_voltages = 64, \
-		.n_linear_ranges = ARRAY_SIZE(_ranges), \
-		.linear_ranges = _ranges, \
+		.n_linear_ranges = ARRAY_SIZE(bcm59054_##_ranges), \
+		.linear_ranges = bcm59054_##_ranges, \
 	}
 
-static struct bcm590xx_info bcm590xx_regs[] = {
-	BCM590XX_REG_TABLE(rfldo, ldo_a_table),
-	BCM590XX_REG_TABLE(camldo1, ldo_c_table),
-	BCM590XX_REG_TABLE(camldo2, ldo_c_table),
-	BCM590XX_REG_TABLE(simldo1, ldo_a_table),
-	BCM590XX_REG_TABLE(simldo2, ldo_a_table),
-	BCM590XX_REG_TABLE(sdldo, ldo_c_table),
-	BCM590XX_REG_TABLE(sdxldo, ldo_a_table),
-	BCM590XX_REG_TABLE(mmcldo1, ldo_a_table),
-	BCM590XX_REG_TABLE(mmcldo2, ldo_a_table),
-	BCM590XX_REG_TABLE(audldo, ldo_a_table),
-	BCM590XX_REG_TABLE(micldo, ldo_a_table),
-	BCM590XX_REG_TABLE(usbldo, ldo_a_table),
-	BCM590XX_REG_TABLE(vibldo, ldo_c_table),
-	BCM590XX_REG_RANGES(csr, dcdc_csr_ranges),
-	BCM590XX_REG_RANGES(iosr1, dcdc_iosr1_ranges),
-	BCM590XX_REG_RANGES(iosr2, dcdc_iosr1_ranges),
-	BCM590XX_REG_RANGES(msr, dcdc_iosr1_ranges),
-	BCM590XX_REG_RANGES(sdsr1, dcdc_sdsr1_ranges),
-	BCM590XX_REG_RANGES(sdsr2, dcdc_iosr1_ranges),
-	BCM590XX_REG_RANGES(vsr, dcdc_iosr1_ranges),
-	BCM590XX_REG_TABLE(gpldo1, ldo_a_table),
-	BCM590XX_REG_TABLE(gpldo2, ldo_a_table),
-	BCM590XX_REG_TABLE(gpldo3, ldo_a_table),
-	BCM590XX_REG_TABLE(gpldo4, ldo_a_table),
-	BCM590XX_REG_TABLE(gpldo5, ldo_a_table),
-	BCM590XX_REG_TABLE(gpldo6, ldo_a_table),
-	BCM590XX_REG_TABLE(vbus, ldo_vbus),
+static struct bcm590xx_info bcm59054_regs[] = {
+	BCM59054_REG_TABLE(rfldo, ldo_1_table),
+	BCM59054_REG_TABLE(camldo1, ldo_2_table),
+	BCM59054_REG_TABLE(camldo2, ldo_2_table),
+	BCM59054_REG_TABLE(simldo1, ldo_1_table),
+	BCM59054_REG_TABLE(simldo2, ldo_1_table),
+	BCM59054_REG_TABLE(sdldo, ldo_2_table),
+	BCM59054_REG_TABLE(sdxldo, ldo_1_table),
+	BCM59054_REG_TABLE(mmcldo1, ldo_1_table),
+	BCM59054_REG_TABLE(mmcldo2, ldo_1_table),
+	BCM59054_REG_TABLE(audldo, ldo_1_table),
+	BCM59054_REG_TABLE(micldo, ldo_1_table),
+	BCM59054_REG_TABLE(usbldo, ldo_1_table),
+	BCM59054_REG_TABLE(vibldo, ldo_2_table),
+	BCM59054_REG_RANGES(csr, dcdc_csr_ranges),
+	BCM59054_REG_RANGES(iosr1, dcdc_sr_ranges),
+	BCM59054_REG_RANGES(iosr2, dcdc_sr_ranges),
+	BCM59054_REG_RANGES(mmsr, dcdc_sr_ranges),
+	BCM59054_REG_RANGES(sdsr1, dcdc_sr_ranges),
+	BCM59054_REG_RANGES(sdsr2, dcdc_sr_ranges),
+	BCM59054_REG_RANGES(vsr, dcdc_vsr_ranges),
+	BCM59054_REG_TABLE(gpldo1, ldo_1_table),
+	BCM59054_REG_TABLE(gpldo2, ldo_1_table),
+	BCM59054_REG_TABLE(gpldo3, ldo_1_table),
+	BCM59054_REG_TABLE(tcxldo, ldo_1_table),
+	BCM59054_REG_TABLE(lvldo1, ldo_3_table),
+	BCM59054_REG_TABLE(lvldo2, ldo_3_table),
+	BCM59054_REG_TABLE(vbus, ldo_vbus),
 };
 
-struct bcm590xx_reg {
-	struct regulator_desc *desc;
-	struct bcm590xx *mfd;
-};
+static int bcm590xx_reg_is_ldo(struct bcm590xx_reg *pmu, int id)
+{
+	switch (pmu->mfd->device_type) {
+	case BCM59054_TYPE:
+		return (id < BCM59054_REG_CSR);
+	case BCM59056_TYPE:
+		return (id < BCM59056_REG_CSR);
+	}
+	return -EINVAL;
+}
 
-static int bcm590xx_get_vsel_register(int id)
+static int bcm590xx_reg_is_gpldo(struct bcm590xx_reg *pmu, int id)
 {
-	if (BCM590XX_REG_IS_LDO(id))
-		return BCM590XX_RFLDOCTRL + id;
-	else if (BCM590XX_REG_IS_GPLDO(id))
-		return BCM590XX_GPLDO1CTRL + id;
-	else
-		return BCM590XX_CSRVOUT1 + (id - BCM590XX_REG_CSR) * 3;
+	switch (pmu->mfd->device_type) {
+	case BCM59054_TYPE:
+		return ((id > BCM59054_REG_VSR) && (id < BCM59054_REG_VBUS));
+	case BCM59056_TYPE:
+		return ((id > BCM59056_REG_VSR) && (id < BCM59056_REG_VBUS));
+	}
+	return -EINVAL;
+}
+
+static int bcm590xx_reg_is_vbus(struct bcm590xx_reg *pmu, int id)
+{
+	switch (pmu->mfd->device_type) {
+	case BCM59054_TYPE:
+		return (id == BCM59054_REG_VBUS);
+	case BCM59056_TYPE:
+		return (id == BCM59056_REG_VBUS);
+	}
+	return -EINVAL;
+}
+
+static int bcm590xx_reg_is_static(struct bcm590xx_reg *pmu, int id)
+{
+	return bcm590xx_reg_is_vbus(pmu, id);
+}
+
+static int bcm590xx_reg_is_secondary(struct bcm590xx_reg *pmu, int id)
+{
+	switch (pmu->mfd->device_type) {
+	case BCM59054_TYPE:
+		return (bcm590xx_reg_is_gpldo(pmu, id) || \
+			id == BCM59054_REG_VBUS);
+	case BCM59056_TYPE:
+		return (bcm590xx_reg_is_gpldo(pmu, id) || \
+			id == BCM59056_REG_VBUS);
+	}
+	return -EINVAL;
+}
+
+static int bcm590xx_reg_mode_is_3bit(struct bcm590xx_reg *pmu, int id)
+{
+	switch (pmu->mfd->device_type) {
+	case BCM59054_TYPE:
+		return (id == BCM59054_REG_CSR || \
+			(id > BCM59054_REG_IOSR2 && id < BCM59054_REG_GPLDO1));
+	case BCM59056_TYPE:
+		return false;
+	}
+	return -EINVAL;
+}
+
+static int bcm590xx_get_vsel_register(struct bcm590xx_reg *pmu, int id)
+{
+	switch (pmu->mfd->device_type) {
+	case BCM59054_TYPE:
+		if (bcm590xx_reg_is_ldo(pmu, id))
+			return BCM59054_RFLDOCTRL + (id - BCM59054_REG_RFLDO);
+		else if (bcm590xx_reg_is_gpldo(pmu, id))
+			return BCM59054_GPLDO1CTRL + (id - BCM59054_REG_GPLDO1);
+		else
+			return BCM59054_CSRVOUT1 + (id - BCM59054_REG_CSR) * 3;
+	case BCM59056_TYPE:
+		if (bcm590xx_reg_is_ldo(pmu, id))
+			return BCM59056_RFLDOCTRL + (id - BCM59056_REG_RFLDO);
+		else if (bcm590xx_reg_is_gpldo(pmu, id))
+			return BCM59056_GPLDO1CTRL + (id - BCM59056_REG_GPLDO1);
+		else
+			return BCM59056_CSRVOUT1 + (id - BCM59056_REG_CSR) * 3;
+	}
+	return -EINVAL;
 }
 
-static int bcm590xx_get_enable_register(int id)
+static int bcm59054_get_enable_register(struct bcm590xx_reg *pmu, int id)
 {
 	int reg = 0;
 
-	if (BCM590XX_REG_IS_LDO(id))
-		reg = BCM590XX_RFLDOPMCTRL1 + id * 2;
-	else if (BCM590XX_REG_IS_GPLDO(id))
-		reg = BCM590XX_GPLDO1PMCTRL1 + id * 2;
+	if (bcm590xx_reg_is_ldo(pmu, id))
+		reg = BCM59054_RFLDOPMCTRL1 + (id - BCM59054_REG_RFLDO) * 2;
+	else if (bcm590xx_reg_is_gpldo(pmu, id))
+		reg = BCM59054_GPLDO1PMCTRL1 + (id - BCM59054_REG_GPLDO1) * 2;
 	else
 		switch (id) {
-		case BCM590XX_REG_CSR:
-			reg = BCM590XX_CSRPMCTRL1;
+		case BCM59054_REG_VSR:
+			reg = BCM59054_VSRPMCTRL1;
 			break;
-		case BCM590XX_REG_IOSR1:
-			reg = BCM590XX_IOSR1PMCTRL1;
+		case BCM59054_REG_CSR:
+			reg = BCM59054_CSRPMCTRL1;
 			break;
-		case BCM590XX_REG_IOSR2:
-			reg = BCM590XX_IOSR2PMCTRL1;
+		case BCM59054_REG_MMSR:
+			reg = BCM59054_MMSRPMCTRL1;
 			break;
-		case BCM590XX_REG_MSR:
-			reg = BCM590XX_MSRPMCTRL1;
+		case BCM59054_REG_SDSR1:
+			reg = BCM59054_SDSR1PMCTRL1;
 			break;
-		case BCM590XX_REG_SDSR1:
-			reg = BCM590XX_SDSR1PMCTRL1;
+		case BCM59054_REG_SDSR2:
+			reg = BCM59054_SDSR2PMCTRL1;
 			break;
-		case BCM590XX_REG_SDSR2:
-			reg = BCM590XX_SDSR2PMCTRL1;
+		case BCM59054_REG_IOSR1:
+			reg = BCM59054_IOSR1PMCTRL1;
 			break;
-		case BCM590XX_REG_VSR:
-			reg = BCM590XX_VSRPMCTRL1;
+		case BCM59054_REG_IOSR2:
+			reg = BCM59054_IOSR2PMCTRL1;
 			break;
-		case BCM590XX_REG_VBUS:
-			reg = BCM590XX_OTG_CTRL;
+		case BCM59054_REG_VBUS:
+			reg = BCM59054_OTG_CTRL;
 			break;
 		}
 
+	return reg;
+}
+
+static int bcm59056_get_enable_register(struct bcm590xx_reg *pmu, int id)
+{
+	int reg = 0;
+
+	if (bcm590xx_reg_is_ldo(pmu, id))
+		reg = BCM59056_RFLDOPMCTRL1 + (id - BCM59056_REG_RFLDO) * 2;
+	else if (bcm590xx_reg_is_gpldo(pmu, id))
+		reg = BCM59056_GPLDO1PMCTRL1 + (id - BCM59056_REG_GPLDO1) * 2;
+	else
+		switch (id) {
+		case BCM59056_REG_CSR:
+			reg = BCM59056_CSRPMCTRL1;
+			break;
+		case BCM59056_REG_IOSR1:
+			reg = BCM59056_IOSR1PMCTRL1;
+			break;
+		case BCM59056_REG_IOSR2:
+			reg = BCM59056_IOSR2PMCTRL1;
+			break;
+		case BCM59056_REG_MSR:
+			reg = BCM59056_MSRPMCTRL1;
+			break;
+		case BCM59056_REG_SDSR1:
+			reg = BCM59056_SDSR1PMCTRL1;
+			break;
+		case BCM59056_REG_SDSR2:
+			reg = BCM59056_SDSR2PMCTRL1;
+			break;
+		case BCM59056_REG_VSR:
+			reg = BCM59056_VSRPMCTRL1;
+			break;
+		case BCM59056_REG_VBUS:
+			reg = BCM59056_OTG_CTRL;
+			break;
+		}
 
 	return reg;
 }
 
+static int bcm590xx_get_enable_register(struct bcm590xx_reg *pmu, int id)
+{
+	switch (pmu->mfd->device_type) {
+	case BCM59054_TYPE:
+		return bcm59054_get_enable_register(pmu, id);
+	case BCM59056_TYPE:
+		return bcm59056_get_enable_register(pmu, id);
+	}
+	return -EINVAL;
+}
+
+static int bcm590xx_get_enable_mask(struct bcm590xx_reg *pmu, int id)
+{
+	if (bcm590xx_reg_mode_is_3bit(pmu, id))
+		return BCM590XX_REG_3BIT_ENABLE;
+
+	return BCM590XX_REG_ENABLE;
+}
+
 static const struct regulator_ops bcm590xx_ops_ldo = {
 	.is_enabled		= regulator_is_enabled_regmap,
 	.enable			= regulator_enable_regmap,
@@ -266,7 +561,7 @@ static const struct regulator_ops bcm590xx_ops_dcdc = {
 	.map_voltage		= regulator_map_voltage_linear_range,
 };
 
-static const struct regulator_ops bcm590xx_ops_vbus = {
+static const struct regulator_ops bcm590xx_ops_static = {
 	.is_enabled		= regulator_is_enabled_regmap,
 	.enable			= regulator_enable_regmap,
 	.disable		= regulator_disable_regmap,
@@ -279,6 +574,7 @@ static int bcm590xx_probe(struct platform_device *pdev)
 	struct regulator_config config = { };
 	struct bcm590xx_info *info;
 	struct regulator_dev *rdev;
+	int n_regulators;
 	int i;
 
 	pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
@@ -289,17 +585,23 @@ static int bcm590xx_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pmu);
 
+	if (pmu->mfd->device_type == BCM59054_TYPE) {
+		info = bcm59054_regs;
+		n_regulators = BCM59054_NUM_REGS;
+	} else if (pmu->mfd->device_type == BCM59056_TYPE) {
+		info = bcm59056_regs;
+		n_regulators = BCM59056_NUM_REGS;
+	}
+
 	pmu->desc = devm_kcalloc(&pdev->dev,
-				 BCM590XX_NUM_REGS,
+				 n_regulators,
 				 sizeof(struct regulator_desc),
 				 GFP_KERNEL);
 	if (!pmu->desc)
 		return -ENOMEM;
 
-	info = bcm590xx_regs;
-
-	for (i = 0; i < BCM590XX_NUM_REGS; i++, info++) {
-		/* Register the regulators */
+	/* Register the regulators */
+	for (i = 0; i < n_regulators; i++, info++) {
 		pmu->desc[i].name = info->name;
 		pmu->desc[i].of_match = of_match_ptr(info->name);
 		pmu->desc[i].regulators_node = of_match_ptr("regulators");
@@ -310,33 +612,37 @@ static int bcm590xx_probe(struct platform_device *pdev)
 		pmu->desc[i].linear_ranges = info->linear_ranges;
 		pmu->desc[i].n_linear_ranges = info->n_linear_ranges;
 
-		if ((BCM590XX_REG_IS_LDO(i)) || (BCM590XX_REG_IS_GPLDO(i))) {
+		if (bcm590xx_reg_is_ldo(pmu, i) || \
+				bcm590xx_reg_is_gpldo(pmu, i)) {
 			pmu->desc[i].ops = &bcm590xx_ops_ldo;
 			pmu->desc[i].vsel_mask = BCM590XX_LDO_VSEL_MASK;
-		} else if (BCM590XX_REG_IS_VBUS(i))
-			pmu->desc[i].ops = &bcm590xx_ops_vbus;
-		else {
+		} else if (bcm590xx_reg_is_static(pmu, i)) {
+			pmu->desc[i].ops = &bcm590xx_ops_static;
+		} else {
 			pmu->desc[i].ops = &bcm590xx_ops_dcdc;
 			pmu->desc[i].vsel_mask = BCM590XX_SR_VSEL_MASK;
 		}
 
-		if (BCM590XX_REG_IS_VBUS(i))
+		if (bcm590xx_reg_is_vbus(pmu, i)) {
 			pmu->desc[i].enable_mask = BCM590XX_VBUS_ENABLE;
-		else {
-			pmu->desc[i].vsel_reg = bcm590xx_get_vsel_register(i);
+		} else {
+			pmu->desc[i].vsel_reg = \
+				bcm590xx_get_vsel_register(pmu, i);
+			pmu->desc[i].enable_mask = \
+				bcm590xx_get_enable_mask(pmu, i);
 			pmu->desc[i].enable_is_inverted = true;
-			pmu->desc[i].enable_mask = BCM590XX_REG_ENABLE;
 		}
-		pmu->desc[i].enable_reg = bcm590xx_get_enable_register(i);
+		pmu->desc[i].enable_reg = \
+			bcm590xx_get_enable_register(pmu, i);
 		pmu->desc[i].type = REGULATOR_VOLTAGE;
 		pmu->desc[i].owner = THIS_MODULE;
 
-		config.dev = bcm590xx->dev;
+		config.dev = pmu->mfd->dev;
 		config.driver_data = pmu;
-		if (BCM590XX_REG_IS_GPLDO(i) || BCM590XX_REG_IS_VBUS(i))
-			config.regmap = bcm590xx->regmap_sec;
+		if (bcm590xx_reg_is_secondary(pmu, i))
+			config.regmap = pmu->mfd->regmap_sec;
 		else
-			config.regmap = bcm590xx->regmap_pri;
+			config.regmap = pmu->mfd->regmap_pri;
 
 		rdev = devm_regulator_register(&pdev->dev, &pmu->desc[i],
 					       &config);

-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 6/6] regulator: bcm590xx: Add proper handling for PMMODE registers

[Artur Weber]

The state of BCM590XX regulators is controlled by writing to the
PMCTRL registers; there are 7 selectable mode entries in those
registers, each storing a specific mode value - OFF, LPM or ON.
Which entry is selected depends on the combination of enabled
PC pins (PC1, PC2 and the optional PC3).

Add a new function to write a specific mode value to all entries,
and make a custom enable/disable function to make use of it.
Keep the is_enabled function using the naive regmap method
(a potential improvement here would be to add support for getting
the state of the PC pins to figure out the selected mode).

It should also be possible to extend this to support regulator
modes, though some work may be needed to make sure it doesn't
interfere with the enabled/disabled state.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 drivers/regulator/bcm590xx-regulator.c | 96 +++++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
index 8b90eae06ca6..1905dc2a4111 100644
--- a/drivers/regulator/bcm590xx-regulator.c
+++ b/drivers/regulator/bcm590xx-regulator.c
@@ -38,6 +38,15 @@ struct bcm590xx_reg {
 #define BCM590XX_LDO_VSEL_MASK		GENMASK(5, 3)
 #define BCM590XX_SR_VSEL_MASK		GENMASK(5, 0)
 
+#define BCM590XX_PMMODE_ON		0x0
+#define BCM590XX_PMMODE_LPM		0x1
+#define BCM590XX_PMMODE_OFF		0x2
+
+#define PMMODE_3BIT_MASK(mode)						\
+	((mode << 3) | mode)
+#define PMMODE_2BIT_MASK(mode)						\
+	((mode << 6) | (mode << 4) | (mode << 2) | mode)
+
 /* BCM59056 registers */
 
 /* I2C slave 0 registers */
@@ -444,7 +453,7 @@ static int bcm590xx_get_vsel_register(struct bcm590xx_reg *pmu, int id)
 	return -EINVAL;
 }
 
-static int bcm59054_get_enable_register(struct bcm590xx_reg *pmu, int id)
+static int bcm59054_get_pmctrl_register(struct bcm590xx_reg *pmu, int id)
 {
 	int reg = 0;
 
@@ -483,7 +492,7 @@ static int bcm59054_get_enable_register(struct bcm590xx_reg *pmu, int id)
 	return reg;
 }
 
-static int bcm59056_get_enable_register(struct bcm590xx_reg *pmu, int id)
+static int bcm59056_get_pmctrl_register(struct bcm590xx_reg *pmu, int id)
 {
 	int reg = 0;
 
@@ -522,13 +531,13 @@ static int bcm59056_get_enable_register(struct bcm590xx_reg *pmu, int id)
 	return reg;
 }
 
-static int bcm590xx_get_enable_register(struct bcm590xx_reg *pmu, int id)
+static int bcm590xx_get_pmctrl_register(struct bcm590xx_reg *pmu, int id)
 {
 	switch (pmu->mfd->device_type) {
 	case BCM59054_TYPE:
-		return bcm59054_get_enable_register(pmu, id);
+		return bcm59054_get_pmctrl_register(pmu, id);
 	case BCM59056_TYPE:
-		return bcm59056_get_enable_register(pmu, id);
+		return bcm59056_get_pmctrl_register(pmu, id);
 	}
 	return -EINVAL;
 }
@@ -541,10 +550,73 @@ static int bcm590xx_get_enable_mask(struct bcm590xx_reg *pmu, int id)
 	return BCM590XX_REG_ENABLE;
 }
 
+/*
+ * The state of BCM590XX regulators is controlled by the PM mode; most
+ * regulators have 3 such modes (off, low-power and on).
+ *
+ * These modes are then stored in the PMCTRL registers - there are 7
+ * PMMODE entries within these registers for any given regulator.
+ * Which one is selected is decided by the PC1 and PC2 pins (and the
+ * optional PC3 pin, if configured).
+ *
+ * For simplicity, to set a PM mode, we write it to all available
+ * PMMODE registers.
+ */
+static int
+_bcm590xx_set_pmmode(struct bcm590xx_reg *pmu, int reg_id, unsigned int mode)
+{
+	struct regmap *regmap;
+	u8 pmctrl_addr = bcm590xx_get_pmctrl_register(pmu, reg_id);
+	unsigned int i;
+	int pmctrl_count;
+	int mode_mask;
+	int ret;
+
+	/*
+	 * Regulators using 2-bit mode controls have 2 PMCTRL registers;
+	 * regulators using 3-bit mode controls have 4 PMCTRL registers.
+	 * This is to accommodate all 7 selectable modes.
+	 */
+	if (bcm590xx_reg_mode_is_3bit(pmu, reg_id)) {
+		pmctrl_count = 4;
+		mode_mask = PMMODE_3BIT_MASK(mode);
+	} else {
+		pmctrl_count = 2;
+		mode_mask = PMMODE_2BIT_MASK(mode);
+	}
+
+	if (bcm590xx_reg_is_secondary(pmu, reg_id))
+		regmap = pmu->mfd->regmap_sec;
+	else
+		regmap = pmu->mfd->regmap_pri;
+
+	for (i = 0; i < pmctrl_count; i++) {
+		ret = regmap_write(regmap, pmctrl_addr + i, mode_mask);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int bcm590xx_regulator_enable(struct regulator_dev *rdev)
+{
+	struct bcm590xx_reg *pmu = rdev->reg_data;
+
+	return _bcm590xx_set_pmmode(pmu, rdev->desc->id, BCM590XX_PMMODE_ON);
+}
+
+static int bcm590xx_regulator_disable(struct regulator_dev *rdev)
+{
+	struct bcm590xx_reg *pmu = rdev->reg_data;
+
+	return _bcm590xx_set_pmmode(pmu, rdev->desc->id, BCM590XX_PMMODE_OFF);
+}
+
 static const struct regulator_ops bcm590xx_ops_ldo = {
 	.is_enabled		= regulator_is_enabled_regmap,
-	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
+	.enable			= bcm590xx_regulator_enable,
+	.disable		= bcm590xx_regulator_disable,
 	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
 	.list_voltage		= regulator_list_voltage_table,
@@ -553,8 +625,8 @@ static const struct regulator_ops bcm590xx_ops_ldo = {
 
 static const struct regulator_ops bcm590xx_ops_dcdc = {
 	.is_enabled		= regulator_is_enabled_regmap,
-	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
+	.enable			= bcm590xx_regulator_enable,
+	.disable		= bcm590xx_regulator_disable,
 	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
 	.list_voltage		= regulator_list_voltage_linear_range,
@@ -563,8 +635,8 @@ static const struct regulator_ops bcm590xx_ops_dcdc = {
 
 static const struct regulator_ops bcm590xx_ops_static = {
 	.is_enabled		= regulator_is_enabled_regmap,
-	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
+	.enable			= bcm590xx_regulator_enable,
+	.disable		= bcm590xx_regulator_disable,
 };
 
 static int bcm590xx_probe(struct platform_device *pdev)
@@ -633,7 +705,7 @@ static int bcm590xx_probe(struct platform_device *pdev)
 			pmu->desc[i].enable_is_inverted = true;
 		}
 		pmu->desc[i].enable_reg = \
-			bcm590xx_get_enable_register(pmu, i);
+			bcm590xx_get_pmctrl_register(pmu, i);
 		pmu->desc[i].type = REGULATOR_VOLTAGE;
 		pmu->desc[i].owner = THIS_MODULE;
 

-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 5/6] regulator: bcm590xx: Add support for BCM59054

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1978 bytes --]

On Mon, Oct 30, 2023 at 08:41:47PM +0100, Artur Weber wrote:
> The BCM59054 is fairly similar in terms of regulators to the already
> supported BCM59056, as included in the BCM590XX driver.

> Add support for the BCM59054's regulators to the BCM590XX driver.
> Switch from using defines for common checks to using functions which
> return different values depending on the identified MFD model.

> While we're at it, fix a bug where the enable/vsel register
> offsets for GPLDO and LDO regulators were calculated incorrectly.

> Also, change the regulator enable bitmask to cover the entire PMMODE
> register.

As is very clear from the commit log this is a whole bunch of changes
which really should be split out into multiple patches.  There's the
bug fix, there's the multiple refactorings to support the new device and
the new device itself.  As covered in submitting-patches.rst you should
do one change per patch, this makes things much easier to follow.

> +	if (pmu->mfd->device_type == BCM59054_TYPE) {
> +		info = bcm59054_regs;
> +		n_regulators = BCM59054_NUM_REGS;
> +	} else if (pmu->mfd->device_type == BCM59056_TYPE) {
> +		info = bcm59056_regs;
> +		n_regulators = BCM59056_NUM_REGS;
> +	}

There's no error handling here if there's an unknown type.

> -		if ((BCM590XX_REG_IS_LDO(i)) || (BCM590XX_REG_IS_GPLDO(i))) {
> +		if (bcm590xx_reg_is_ldo(pmu, i) || \
> +				bcm590xx_reg_is_gpldo(pmu, i)) {
>  			pmu->desc[i].ops = &bcm590xx_ops_ldo;
>  			pmu->desc[i].vsel_mask = BCM590XX_LDO_VSEL_MASK;
> -		} else if (BCM590XX_REG_IS_VBUS(i))
> -			pmu->desc[i].ops = &bcm590xx_ops_vbus;
> -		else {
> +		} else if (bcm590xx_reg_is_static(pmu, i)) {
> +			pmu->desc[i].ops = &bcm590xx_ops_static;
> +		} else {
>  			pmu->desc[i].ops = &bcm590xx_ops_dcdc;
>  			pmu->desc[i].vsel_mask = BCM590XX_SR_VSEL_MASK;
>  		}

It seems like everything would be a lot simpler and clearer to just have
tables of regulators per device rather than have all these conditionals.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 6/6] regulator: bcm590xx: Add proper handling for PMMODE registers

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 329 bytes --]

On Mon, Oct 30, 2023 at 08:41:48PM +0100, Artur Weber wrote:

> +	for (i = 0; i < pmctrl_count; i++) {
> +		ret = regmap_write(regmap, pmctrl_addr + i, mode_mask);
> +		if (ret)
> +			return ret;
> +	}

Why not a bulk write?  What happens when the new values are partially
written, both if there's an error and just transiently?

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/6] dt-bindings: mfd: brcm,bcm59056: Convert to YAML

[Krzysztof Kozlowski]

On 30/10/2023 20:41, Artur Weber wrote:
> Convert devicetree bindings for the Broadcom BCM59056 PMU MFD from
> TXT to YAML format. This patch does not change any functionality;
> the bindings remain the same.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
>  .../devicetree/bindings/mfd/brcm,bcm59056.txt      | 39 ----------
>  .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 91 ++++++++++++++++++++++
>  2 files changed, 91 insertions(+), 39 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt
> deleted file mode 100644
> index be51a15e05f9..000000000000
> --- a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt
> +++ /dev/null
> @@ -1,39 +0,0 @@
> --------------------------------
> -BCM590xx Power Management Units
> --------------------------------
> -
> -Required properties:
> -- compatible: "brcm,bcm59056"
> -- reg: I2C slave address
> -- interrupts: interrupt for the PMU. Generic interrupt client node bindings
> -  are described in interrupt-controller/interrupts.txt
> -
> -------------------
> -Voltage Regulators
> -------------------
> -
> -Optional child nodes:
> -- regulators: container node for regulators following the generic
> -  regulator binding in regulator/regulator.txt
> -
> -  The valid regulator node names for BCM59056 are:
> -  	rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
> -	mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
> -	csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
> -	gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
> -	vbus
> -
> -Example:
> -	pmu: bcm59056@8 {
> -		compatible = "brcm,bcm59056";
> -		reg = <0x08>;
> -		interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
> -		regulators {
> -			rfldo_reg: rfldo {
> -				regulator-min-microvolt = <1200000>;
> -				regulator-max-microvolt = <3300000>;
> -			};
> -
> -			...
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
> new file mode 100644
> index 000000000000..6d6acc9fd312
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/brcm,bcm59056.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: BCM590xx Power Management Unit MFD
> +
> +maintainers:
> +  - Artur Weber <aweber.kernel@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - "brcm,bcm59056"

This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/6] dt-bindings: mfd: brcm,bcm59056: Add compatible for BCM59054

[Krzysztof Kozlowski]

On 30/10/2023 20:41, Artur Weber wrote:
> The BCM59054 MFD is fairly similar to the BCM59056, and will use
> the same driver. Add compatible and specify the allowed regulator
> nodes.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
>  .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 125 +++++++++++++++------
>  1 file changed, 88 insertions(+), 37 deletions(-)

I guess I was reviewing your v1. Please do not send more than one
version per day. Sending two versions within 5 minutes makes reviewers
confused and eventually they might waste effort on reviewing wrong patchset.

All my other comments apply.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel