2023-10-30 19:37:39

by Artur Weber

[permalink] [raw]
Subject: [PATCH 0/6] mfd: bcm590xx: Add support for BCM59054

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 <[email protected]>
---
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 <[email protected]>


2023-10-30 19:37:42

by Artur Weber

[permalink] [raw]
Subject: [PATCH 3/6] ARM: dts: Drop DTS for BCM59056 PMIC

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 <[email protected]>
---
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 <[email protected]>
-*/
-
-&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

2023-10-30 19:37:43

by Artur Weber

[permalink] [raw]
Subject: [PATCH 6/6] regulator: bcm590xx: Add proper handling for PMMODE registers

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 <[email protected]>
---
drivers/regulator/bcm590xx-regulator.c | 100 ++++++++++++++++++++++++++++-----
1 file changed, 86 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
index 8b90eae06ca6..af1ab8735ea7 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 */
@@ -435,16 +444,16 @@ static int bcm590xx_get_vsel_register(struct bcm590xx_reg *pmu, int id)
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);
+ return BCM59056_RFLDOCTRL + (id - BCM59054_REG_RFLDO);
else if (bcm590xx_reg_is_gpldo(pmu, id))
- return BCM59056_GPLDO1CTRL + (id - BCM59056_REG_GPLDO1);
+ return BCM59056_GPLDO1CTRL + (id - BCM59054_REG_GPLDO1);
else
return BCM59056_CSRVOUT1 + (id - BCM59056_REG_CSR) * 3;
}
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