[PATCH 0/2] Add support for ADP5055 triple buck regulator.

[PATCH 0/2] Add support for ADP5055 triple buck regulator.

[Alexis Czezar Torreno]

Introduce a regulator driver support for ADP5055. The device combines 3
high performance buck regulators in a 43-termminal land grid array
package. The device enables direct connection to high input voltages up
to 18V with no preregulator. Channel 1 and 2 deliver a programmable
output current of 3.5A or 7.5A or provide a single output with up to 14A
in parallel operation. Channel 3 has a programmable output current of
1.5A or 3A.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
Alexis Czezar Torreno (2):
      dt-bindings: regulator: add adi,adp5055-regulator
      regulator: adp5055: Add driver for adp5055

 .../bindings/regulator/adi,adp5055-regulator.yaml  | 214 +++++++++
 MAINTAINERS                                        |   7 +
 drivers/regulator/Kconfig                          |  11 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/adp5055-regulator.c              | 490 +++++++++++++++++++++
 5 files changed, 723 insertions(+)
---
base-commit: 7fef39f0e82ff02282797d9ae2589b39b16ab614
change-id: 20250225-upstream-adp5055-adbe6262f984

Best regards,
-- 
Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

[PATCH 1/2] dt-bindings: regulator: add adi,adp5055-regulator

[Alexis Czezar Torreno]

Add documentation for devicetree bindings for ADP5055. The device consists
of 3 buck regulators able to connect to high input voltages of up to 18V
with no preregulators.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 .../bindings/regulator/adi,adp5055-regulator.yaml  | 214 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 2 files changed, 220 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..52cebfd48b3c4cf4ecf16b660c204c38c94e0eec
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
@@ -0,0 +1,214 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/adi,adp5055-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADP5055 Triple Buck Regulator
+
+maintainers:
+  - Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
+
+description: |
+  The ADP5055 combines three high performance buck regulators.
+  The device enables direct connection to high input voltages
+  up to 18 V with no preregulators.
+  https://www.analog.com/media/en/technical-documentation/data-sheets/adp5055.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,adp5055
+
+  reg:
+    enum:
+      - 0x70
+      - 0x71
+
+  adi,tset-us:
+    description:
+      Setting time used by the device. This is changed via soldering
+      specific resistor values on the CFG2 pin.
+    enum: [2600, 20800]
+    default: 2600
+
+  adi,hw-en-array-gpios:
+    description:
+      Asserted during driver probe. Each array entry acts as the
+      hardware enable for channels 0-2. Should be marked 0 for active
+      low. Requires all three channels to be initialized. Not adding
+      the property turns the system to a software only enable mode.
+    minItems: 3
+    maxItems: 3
+
+  adi,ocp-blanking:
+    description:
+      If present, the over current protection
+      blanking (OCP) for all channels is on.
+    type: boolean
+
+  adi,delay-power-good:
+    description:
+      Configures delay timer of the power good (PWRGD) pin.
+      Delay is based on Tset which can be 2.6 ms or 20.8 ms.
+    type: boolean
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  "^channel@([0-2])$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      reg:
+        description: The channel number representing each buck converter.
+        maximum: 2
+
+      adi,power-saving-mode:
+        description:
+          If present, enables power saving mode for
+          individual channels.
+        type: boolean
+
+      adi,output-discharge-function:
+        description:
+          If present, enable output discharge functionality
+          for individual channels.
+        type: boolean
+
+      adi,disable-delay-us:
+        description:
+          Configures the disable delay for each channel. Dependent on Tset.
+        enum: [0, 5200, 10400, 15600, 20800, 26000, 31200, 36400]
+        default: 0
+
+      adi,enable-delay-us:
+        description:
+          Configures the disable delay for each channel. Dependent on Tset.
+        enum: [0, 2600, 5200, 7800, 10400, 13000, 15600, 18200]
+        default: 0
+
+      adi,dvs-limit-upper-microvolt:
+        description:
+          Configure the allowable upper side limit of the
+          voltage output of each channel in microvolt.
+          Voltages are in 12mV steps, value is autoadjusted.
+          Vout_high = Vout + DVS_upper_limit.
+        minimum: 12000
+        maximum: 192000
+        default: 192000
+
+      adi,dvs-limit-lower-microvolt:
+        description:
+          Configure the allowable lower side limit of the
+          voltage output of each channel in microvolt.
+          Voltages are in 12mV steps, value is autoadjusted.
+          Vout_low = Vout + DVS_lower_limit.
+        minimum: -190500
+        maximum: -10500
+        default: -190500
+
+      adi,fast-transient:
+        description:
+          Configures the fast transient sensitivity for each channel.
+          "none"    - No fast transient.
+          "3G_1.5%" - 1.5% window with 3*350uA/V
+          "5G_1.5%" - 1.5% window with 5*350uA/V
+          "5G_2.5%" - 2.5% window with 5*350uA/V
+        enum: [none, 3G_1.5%, 5G_1.5%, 5G_2.5%]
+        default: 5G_2.5%
+
+      adi,mask-power-good:
+        description:
+          If present, masks individual channels to the external
+          PWRGD hardware pin.
+        type: boolean
+
+    required:
+      - reg
+
+allOf:
+  - $ref: regulator.yaml#
+
+  - if:
+      properties:
+        adi,tset-us:
+          contains:
+            enum:
+              - 20800
+    then:
+      properties:
+        adi,disable-delay-us:
+          enum: [0, 41600, 83200, 124800, 166400, 208000, 249600, 291200]
+
+        adi,enable-delay-us:
+          enum: [0, 20800, 41600, 62400, 83200, 104000, 124800, 145600]
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        regulator@70 {
+            compatible = "adi,adp5055";
+            reg = <0x70>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            adi,tset-us = <2600>;
+            adi,hw-en-array-gpios = <&gpio 17 0>,
+                                    <&gpio 18 0>,
+                                    <&gpio 19 0>;
+
+            adi,ocp-blanking;
+            adi,delay-power-good;
+
+            channel@0 {
+                reg = <0>;
+                adi,power-saving-mode;
+                adi,output-discharge-function;
+                adi,disable-delay-us = <0>;
+                adi,enable-delay-us = <0>;
+                adi,dvs-limit-upper-microvolt = <192000>;
+                adi,dvs-limit-lower-microvolt = <(-190500)>;
+                adi,fast-transient = "5G_2.5%";
+                adi,mask-power-good;
+            };
+
+            channel@1 {
+                reg = <1>;
+                adi,power-saving-mode;
+                adi,output-discharge-function;
+                adi,disable-delay-us = <0>;
+                adi,enable-delay-us = <0>;
+                adi,dvs-limit-upper-microvolt = <192000>;
+                adi,dvs-limit-lower-microvolt = <(-190500)>;
+                adi,fast-transient = "5G_2.5%";
+                adi,mask-power-good;
+            };
+
+            channel@2 {
+                reg = <2>;
+                adi,power-saving-mode;
+                adi,output-discharge-function;
+                adi,disable-delay-us = <0>;
+                adi,enable-delay-us = <0>;
+                adi,dvs-limit-upper-microvolt = <192000>;
+                adi,dvs-limit-lower-microvolt = <(-190500)>;
+                adi,fast-transient = "5G_2.5%";
+                adi,mask-power-good;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 896a307fa06545e2861abe46ea7029f9b4d3628e..b2ec43f84d84765c319d8403fb5650afa273db83 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1525,6 +1525,12 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
 F:	drivers/iio/filter/admv8818.c
 
+ANALOG DEVICES INC ADP5055 DRIVER
+M:	Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
+
 ANALOG DEVICES INC ADP5061 DRIVER
 M:	Michael Hennerich <Michael.Hennerich@analog.com>
 L:	linux-pm@vger.kernel.org

-- 
2.34.1

[PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Alexis Czezar Torreno]

Add ADI ADP5055 driver support. The device consists
of 3 buck regulators able to connect to high input voltages of up to 18V
with no preregulators.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 MAINTAINERS                           |   1 +
 drivers/regulator/Kconfig             |  11 +
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/adp5055-regulator.c | 490 ++++++++++++++++++++++++++++++++++
 4 files changed, 503 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b2ec43f84d84765c319d8403fb5650afa273db83..7ac35d895b4c8297c6de70cd2bfaec516ff5b100 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1530,6 +1530,7 @@ M:	Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
+F:	drivers/regulator/adp5055-regulator.c
 
 ANALOG DEVICES INC ADP5061 DRIVER
 M:	Michael Hennerich <Michael.Hennerich@analog.com>
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 39297f7d8177193e51c99bc2b360c6d9936e62fe..2378559b3ca12e77db263418144e9b8598eb97e0 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -122,6 +122,17 @@ config REGULATOR_AD5398
 	  This driver supports AD5398 and AD5821 current regulator chips.
 	  If building into module, its name is ad5398.ko.
 
+config REGULATOR_ADP5055
+	tristate "Analog Devices ADP5055 Triple Buck Regulator"
+	depends on I2C
+        select REGMAP_I2C
+	help
+	  This driver controls an Analog Devices ADP5055 with triple buck
+          regulators using an I2C interface.
+
+	  Say M here if you want to include support for the regulator as a
+	  module.
+
 config REGULATOR_ANATOP
 	tristate "Freescale i.MX on-chip ANATOP LDO regulators"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 3d5a803dce8a0556ba9557fa069c6e37593b3c69..71f45d9317d24e7081ac919eb31efff6652edf3f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_REGULATOR_AB8500)	+= ab8500-ext.o ab8500.o
 obj-$(CONFIG_REGULATOR_ACT8865) += act8865-regulator.o
 obj-$(CONFIG_REGULATOR_ACT8945A) += act8945a-regulator.o
 obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o
+obj-$(CONFIG_REGULATOR_ADP5055) += adp5055-regulator.o
 obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
 obj-$(CONFIG_REGULATOR_ARIZONA_LDO1) += arizona-ldo1.o
 obj-$(CONFIG_REGULATOR_ARIZONA_MICSUPP) += arizona-micsupp.o
diff --git a/drivers/regulator/adp5055-regulator.c b/drivers/regulator/adp5055-regulator.c
new file mode 100644
index 0000000000000000000000000000000000000000..3b99c1b2fcbbeb1ad8c13886da242a45c90cf26f
--- /dev/null
+++ b/drivers/regulator/adp5055-regulator.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Regulator driver for Analog Devices ADP5055
+ *
+ * Copyright (C) 2025 Analog Devices, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+/*
+ * ADP5055 Register Map.
+ */
+#define ADP5055_CTRL123         0xD1
+#define ADP5055_CTRL_MODE1      0xD3
+#define ADP5055_CTRL_MODE2      0xD4
+#define ADP5055_DLY0            0xD5
+#define ADP5055_DLY1            0xD6
+#define ADP5055_DLY2            0xD7
+#define ADP5055_VID0            0xD8
+#define ADP5055_VID1            0xD9
+#define ADP5055_VID2            0xDA
+#define ADP5055_DVS_LIM0        0xDC
+#define ADP5055_DVS_LIM1        0xDD
+#define ADP5055_DVS_LIM2        0xDE
+#define ADP5055_FT_CFG          0xDF
+#define ADP5055_PG_CFG          0xE0
+
+/*
+ * ADP5055 Field Masks.
+ */
+#define	ADP5055_MASK_EN_MODE		BIT(0)
+#define	ADP5055_MASK_OCP_BLANKING	BIT(7)
+#define	ADP5055_MASK_PSM2		BIT(6)
+#define	ADP5055_MASK_PSM1		BIT(5)
+#define	ADP5055_MASK_PSM0		BIT(4)
+#define	ADP5055_MASK_DIS2		BIT(2)
+#define	ADP5055_MASK_DIS1		BIT(1)
+#define	ADP5055_MASK_DIS0		BIT(0)
+#define	ADP5055_MASK_DIS_DLY		GENMASK(6, 4)
+#define	ADP5055_MASK_EN_DLY		GENMASK(2, 0)
+#define	ADP5055_MASK_DVS_LIM_UPPER	GENMASK(7, 4)
+#define	ADP5055_MASK_DVS_LIM_LOWER	GENMASK(3, 0)
+#define	ADP5055_MASK_FAST_TRANSIENT2	GENMASK(5, 4)
+#define	ADP5055_MASK_FAST_TRANSIENT1	GENMASK(3, 2)
+#define	ADP5055_MASK_FAST_TRANSIENT0	GENMASK(1, 0)
+#define	ADP5055_MASK_DLY_PWRGD		BIT(4)
+#define	ADP5055_MASK_PWRGD2		BIT(2)
+#define	ADP5055_MASK_PWRGD1		BIT(1)
+#define	ADP5055_MASK_PWRGD0		BIT(0)
+
+#define	ADP5055_MIN_VOUT		408000
+#define ADP5055_NUM_CH			3
+
+struct adp5055 {
+	struct regmap *regmap;
+	struct gpio_descs *hw_en_array_gpios;
+};
+
+static const unsigned int adp5055_tset_vals[] = {
+	2600,
+	20800,
+};
+
+static const unsigned int adp5055_disable_delay_vals_2_6[] = {
+	0,
+	5200,
+	10400,
+	15600,
+	20800,
+	26000,
+	31200,
+	36400,
+};
+
+static const unsigned int adp5055_disable_delay_vals_20_8[] = {
+	0,
+	41600,
+	83200,
+	124800,
+	166400,
+	208000,
+	249600,
+	291200,
+};
+
+static const unsigned int adp5055_enable_delay_vals_2_6[] = {
+	0,
+	2600,
+	5200,
+	7800,
+	10400,
+	13000,
+	15600,
+	18200,
+};
+
+static const unsigned int adp5055_enable_delay_vals_20_8[] = {
+	0,
+	20800,
+	41600,
+	62400,
+	83200,
+	104000,
+	124800,
+	145600,
+};
+
+static const char * const adp5055_fast_transient_vals[] = {
+	"none",
+	"3G_1.5%",
+	"5G_1.5%",
+	"5G_2.5%",
+};
+
+static int adp5055_get_prop_index(const u32 *table, size_t table_size,
+				  u32 value)
+{
+	int i;
+
+	for (i = 0; i < table_size; i++)
+		if (table[i] == value)
+			return i;
+
+	return -EINVAL;
+}
+
+static const struct regmap_range adp5055_reg_ranges[] = {
+	regmap_reg_range(0xD1, 0xE0),
+};
+
+static const struct regmap_access_table adp5055_write_ranges_table = {
+	.yes_ranges	= adp5055_reg_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(adp5055_reg_ranges),
+};
+
+static const struct regmap_access_table adp5055_read_ranges_table = {
+	.yes_ranges	= adp5055_reg_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(adp5055_reg_ranges),
+};
+
+static const struct regmap_config adp5055_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xFF,
+	.wr_table = &adp5055_write_ranges_table,
+	.rd_table = &adp5055_read_ranges_table,
+};
+
+static const struct linear_range adp5055_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(ADP5055_MIN_VOUT, 0, 255, 1500),
+};
+
+static int adp5055_parse_fw(struct device *dev, struct  adp5055 *adp5055)
+{
+	int i, ret;
+	struct regmap *regmap = adp5055->regmap;
+	int val;
+	u32 tset;
+	bool ocp_blanking;
+	bool power_saving_mode[ADP5055_NUM_CH];
+	bool output_discharge_function[ADP5055_NUM_CH];
+	u32 disable_delay[ADP5055_NUM_CH];
+	u32 enable_delay[ADP5055_NUM_CH];
+	int dvs_limit_upper[ADP5055_NUM_CH];
+	int dvs_limit_lower[ADP5055_NUM_CH];
+	u32 fast_transient[ADP5055_NUM_CH];
+	bool delay_power_good;
+	bool mask_power_good[ADP5055_NUM_CH];
+
+	tset = 2600;
+
+	adp5055->hw_en_array_gpios = devm_gpiod_get_array_optional(dev,
+				"adi,hw-en-array", GPIOD_OUT_LOW);
+	if (IS_ERR(adp5055->hw_en_array_gpios))
+		return dev_err_probe(dev, PTR_ERR(adp5055->hw_en_array_gpios),
+				     "Failed to get hw_en_array GPIOs\n");
+
+	if (!adp5055->hw_en_array_gpios)
+		if (adp5055->hw_en_array_gpios->ndescs != ADP5055_NUM_CH)
+			return dev_err_probe(dev, adp5055->hw_en_array_gpios->ndescs,
+				     "Invalid amount of channels described\n");
+
+	ret = device_property_read_u32(dev, "adi,tset-us", &tset);
+	if (!ret) {
+		ret = adp5055_get_prop_index(adp5055_tset_vals,
+					ARRAY_SIZE(adp5055_tset_vals), tset);
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+				"Failed to initialize tset.");
+		tset = adp5055_tset_vals[ret];
+	}
+
+	ocp_blanking = device_property_read_bool(dev, "adi,ocp-blanking");
+
+	device_for_each_child_node_scoped(dev, child) {
+		ret = fwnode_property_read_u32(child, "reg", &i);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to read reg value of child node");
+		if (i >= ADP5055_NUM_CH)
+			return dev_err_probe(dev, ret, "Child node exceeds channel count");
+
+		power_saving_mode[i] = fwnode_property_read_bool(child,
+						   "adi,power-saving-mode");
+
+		output_discharge_function[i] = fwnode_property_read_bool(child,
+			"adi,output-discharge-function");
+
+		ret = fwnode_property_read_u32(child, "adi,disable-delay-us",
+					   &disable_delay[i]);
+		if (!ret) {
+			if (tset == 2600)
+				ret = adp5055_get_prop_index(adp5055_disable_delay_vals_2_6,
+							ARRAY_SIZE(adp5055_disable_delay_vals_2_6),
+							disable_delay[i]);
+			else
+				ret = adp5055_get_prop_index(adp5055_disable_delay_vals_20_8,
+							ARRAY_SIZE(adp5055_disable_delay_vals_20_8),
+							disable_delay[i]);
+			if (ret < 0)
+				return dev_err_probe(dev, ret,
+					"Failed to initialize disable-delay-us.");
+			disable_delay[i] = ret;
+		}
+
+		ret = fwnode_property_read_u32(child, "adi,enable-delay-us",
+					   &enable_delay[i]);
+		if (!ret) {
+			if (tset == 2600)
+				ret = adp5055_get_prop_index(adp5055_enable_delay_vals_2_6,
+							ARRAY_SIZE(adp5055_enable_delay_vals_2_6),
+							enable_delay[i]);
+			else
+				ret = adp5055_get_prop_index(adp5055_enable_delay_vals_20_8,
+							ARRAY_SIZE(adp5055_enable_delay_vals_20_8),
+							enable_delay[i]);
+			if (ret < 0)
+				return dev_err_probe(dev, ret,
+					"Failed to initialize enable-delay-us.");
+			enable_delay[i] = ret;
+		}
+
+		ret = fwnode_property_read_u32(child, "adi,dvs-limit-upper-microvolt",
+					&dvs_limit_upper[i]);
+		if (ret)
+			dvs_limit_upper[i] = 192000;
+		else
+			if (dvs_limit_upper[i] > 192000 || dvs_limit_upper[i] < 12000)
+				return dev_err_probe(dev, dvs_limit_upper[i],
+					"Out of range - dvs-limit-upper-microvolt value.");
+
+		ret = fwnode_property_read_u32(child, "adi,dvs-limit-lower-microvolt",
+					&dvs_limit_lower[i]);
+		if (ret)
+			dvs_limit_lower[i] = -190500;
+		else
+			if (dvs_limit_lower[i] > -10500 || dvs_limit_lower[i] < -190500)
+				return dev_err_probe(dev, dvs_limit_lower[i],
+					"Out of range - dvs-limit-lower-microvolt value.");
+
+		ret = fwnode_property_match_string(child, "adi,fast-transient",
+							*adp5055_fast_transient_vals);
+		if (ret < 0)
+			fast_transient[i] = 3;
+		else
+			fast_transient[i] = ret;
+
+		mask_power_good[i] = fwnode_property_read_bool(child,
+						 "adi,mask-power-good");
+
+		val = FIELD_PREP(ADP5055_MASK_DIS_DLY, disable_delay[i]);
+		val |= FIELD_PREP(ADP5055_MASK_EN_DLY, enable_delay[i]);
+		ret = regmap_write(regmap, ADP5055_DLY0 + i, val);
+		if (ret)
+			return ret;
+
+		val = FIELD_PREP(ADP5055_MASK_DVS_LIM_UPPER,
+				DIV_ROUND_CLOSEST_ULL(192000 - dvs_limit_upper[i], 12000));
+		val |= FIELD_PREP(ADP5055_MASK_DVS_LIM_LOWER,
+				DIV_ROUND_CLOSEST_ULL(dvs_limit_lower[i] + 190500, 12000));
+		ret = regmap_write(regmap, ADP5055_DVS_LIM0 + i, val);
+		if (ret)
+			return ret;
+
+		i++;
+	};
+
+	delay_power_good = device_property_read_bool(dev,
+				    "adi,delay-power-good");
+
+	if (!adp5055->hw_en_array_gpios)
+		val = FIELD_PREP(ADP5055_MASK_EN_MODE, 1);
+	else
+		val = FIELD_PREP(ADP5055_MASK_EN_MODE, 0);
+	ret = regmap_write(regmap, ADP5055_CTRL_MODE1, val);
+	if (ret)
+		return ret;
+
+	val = FIELD_PREP(ADP5055_MASK_OCP_BLANKING, ocp_blanking);
+	val |= FIELD_PREP(ADP5055_MASK_PSM2, power_saving_mode[2]);
+	val |= FIELD_PREP(ADP5055_MASK_PSM1, power_saving_mode[1]);
+	val |= FIELD_PREP(ADP5055_MASK_PSM0, power_saving_mode[0]);
+	val |= FIELD_PREP(ADP5055_MASK_DIS2, output_discharge_function[2]);
+	val |= FIELD_PREP(ADP5055_MASK_DIS1, output_discharge_function[1]);
+	val |= FIELD_PREP(ADP5055_MASK_DIS0, output_discharge_function[0]);
+	ret = regmap_write(regmap, ADP5055_CTRL_MODE2, val);
+	if (ret)
+		return ret;
+
+	val = FIELD_PREP(ADP5055_MASK_FAST_TRANSIENT2, fast_transient[2]);
+	val |= FIELD_PREP(ADP5055_MASK_FAST_TRANSIENT1, fast_transient[1]);
+	val |= FIELD_PREP(ADP5055_MASK_FAST_TRANSIENT0, fast_transient[0]);
+	ret = regmap_write(regmap, ADP5055_FT_CFG, val);
+	if (ret)
+		return ret;
+
+	val = FIELD_PREP(ADP5055_MASK_DLY_PWRGD, delay_power_good);
+	val |= FIELD_PREP(ADP5055_MASK_PWRGD2, mask_power_good[2]);
+	val |= FIELD_PREP(ADP5055_MASK_PWRGD1, mask_power_good[1]);
+	val |= FIELD_PREP(ADP5055_MASK_PWRGD0, mask_power_good[0]);
+	ret = regmap_write(regmap, ADP5055_PG_CFG, val);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int adp5055_is_enabled(struct regulator_dev *dev)
+{
+	struct adp5055 *adp5055 = rdev_get_drvdata(dev);
+	int id, ret;
+	int mask;
+	int val_sw, val_hw;
+
+	id = rdev_get_id(dev);
+	mask = BIT(id);
+	ret = regmap_read(adp5055->regmap, ADP5055_CTRL_MODE1, &val_sw);
+	if (ret)
+		return ret;
+
+	if (!adp5055->hw_en_array_gpios)
+		return (val_sw & mask) != 0;
+
+	val_hw = gpiod_get_value_cansleep(adp5055->hw_en_array_gpios->desc[id]);
+
+	return val_hw;
+};
+
+static int adp5055_en_func(struct regulator_dev *dev, int en_val)
+{
+	struct adp5055 *adp5055 = rdev_get_drvdata(dev);
+	int id;
+	int mask;
+
+	id = rdev_get_id(dev);
+	mask = BIT(id);
+
+	if (!adp5055->hw_en_array_gpios)
+		return regmap_update_bits(adp5055->regmap, ADP5055_CTRL_MODE1, mask, en_val);
+
+	gpiod_set_value_cansleep(adp5055->hw_en_array_gpios->desc[id], en_val);
+
+	return 0;
+}
+
+static int adp5055_enable(struct regulator_dev *dev)
+{
+	return adp5055_en_func(dev, 1);
+}
+
+static int adp5055_disable(struct regulator_dev *dev)
+{
+	return adp5055_en_func(dev, 0);
+}
+
+static const struct regulator_ops adp5055_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+	.map_voltage = regulator_map_voltage_linear_range,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.enable = adp5055_enable,
+	.disable = adp5055_disable,
+	.is_enabled = adp5055_is_enabled,
+};
+
+#define ADP5055_REG_(_name, _id, _ch, _ops)					\
+	[_id] = { \
+		.name = _name, \
+		.ops = _ops, \
+		.linear_ranges = adp5055_voltage_ranges, \
+		.n_linear_ranges = ARRAY_SIZE(adp5055_voltage_ranges), \
+		.vsel_reg = ADP5055_VID##_ch, \
+		.vsel_mask = GENMASK(7, 0), \
+		.enable_reg = ADP5055_CTRL123, \
+		.enable_mask = BIT(_ch), \
+		.owner = THIS_MODULE, \
+	}
+
+#define ADP5055_REG(_name, _id, _ch) \
+	ADP5055_REG_(_name, _id, _ch, &adp5055_ops)
+
+static const struct regulator_desc adp5055_regulators[] = {
+	ADP5055_REG("DCDC1", 0, 0),
+	ADP5055_REG("DCDC2", 1, 1),
+	ADP5055_REG("DCDC3", 2, 2),
+};
+
+static const struct of_device_id adp5055_dt_ids[] = {
+	{ .compatible = "adi,adp5055"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adp5055_dt_ids);
+
+static int adp5055_probe(struct i2c_client *client)
+{
+	struct regulator_init_data *init_data;
+	struct device *dev = &client->dev;
+	struct adp5055 *adp5055;
+	int i, ret;
+
+	init_data = of_get_regulator_init_data(dev, client->dev.of_node,
+					       &adp5055_regulators[0]);
+	if (!init_data)
+		return -EINVAL;
+
+	adp5055 = devm_kzalloc(dev, sizeof(struct adp5055), GFP_KERNEL);
+	if (!adp5055)
+		return -ENOMEM;
+
+	adp5055->regmap = devm_regmap_init_i2c(client, &adp5055_regmap_config);
+	if (IS_ERR(adp5055->regmap))
+		return dev_err_probe(dev, PTR_ERR(adp5055->regmap), "Failed to allocate reg map");
+
+	ret = adp5055_parse_fw(dev, adp5055);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < ADP5055_NUM_CH; i++) {
+		const struct regulator_desc *desc = &adp5055_regulators[i];
+		struct regulator_config config = { };
+		struct regulator_dev *rdev;
+
+		config.dev = dev;
+		config.driver_data = adp5055;
+		config.regmap = adp5055->regmap;
+		config.init_data = init_data;
+
+		rdev = devm_regulator_register(dev, desc, &config);
+		if (IS_ERR(rdev)) {
+			return dev_err_probe(dev, PTR_ERR(rdev),
+					"Failed to register %s\n", desc->name);
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id adp5055_of_match[] = {
+	{ .compatible = "adi,adp5055", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adp5055_of_match);
+
+static const struct i2c_device_id adp5055_ids[] = {
+	{ .name = "adp5055"},
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, adp5055_ids);
+
+static struct i2c_driver adp5055_driver = {
+	.driver	= {
+		.name	= "adp5055",
+		.of_match_table = adp5055_of_match,
+	},
+	.probe		= adp5055_probe,
+	.id_table	= adp5055_ids,
+};
+module_i2c_driver(adp5055_driver);
+
+MODULE_DESCRIPTION("ADP5055 Voltage Regulator Driver");
+MODULE_AUTHOR("Alexis Czezar Torreno <alexisczezar.torreno@analog.com>");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2494 bytes --]

On Tue, Feb 25, 2025 at 05:08:34PM +0800, Alexis Czezar Torreno wrote:

> Add ADI ADP5055 driver support. The device consists
> of 3 buck regulators able to connect to high input voltages of up to 18V
> with no preregulators.

There's a bunch of stuff here which should be using core features, I'll
comment some of those on the DT binding patch.

> +config REGULATOR_ADP5055
> +	tristate "Analog Devices ADP5055 Triple Buck Regulator"
> +	depends on I2C
> +        select REGMAP_I2C
> +	help
> +	  This driver controls an Analog Devices ADP5055 with triple buck
> +          regulators using an I2C interface.

The indentation for the select and the second line of the description is
weird.

> @@ -0,0 +1,490 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Regulator driver for Analog Devices ADP5055
> + *
> + * Copyright (C) 2025 Analog Devices, Inc.
> + */

Please make the entire comment block a C++ one so things look more
intentional.

> +static const struct regmap_range adp5055_reg_ranges[] = {
> +	regmap_reg_range(0xD1, 0xE0),
> +};
> +
> +static const struct regmap_access_table adp5055_write_ranges_table = {
> +	.yes_ranges	= adp5055_reg_ranges,
> +	.n_yes_ranges	= ARRAY_SIZE(adp5055_reg_ranges),
> +};
> +
> +static const struct regmap_access_table adp5055_read_ranges_table = {
> +	.yes_ranges	= adp5055_reg_ranges,
> +	.n_yes_ranges	= ARRAY_SIZE(adp5055_reg_ranges),
> +};

The read and write ranges could just be one variable.

> +static const struct regmap_config adp5055_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0xFF,

max_register is set to 0xff but the largest read or write register is
0xe0.  Might be worth considering adding cache support too, there's
read/modify/write cycles here.

> +	if (!adp5055->hw_en_array_gpios)
> +		if (adp5055->hw_en_array_gpios->ndescs != ADP5055_NUM_CH)
> +			return dev_err_probe(dev, adp5055->hw_en_array_gpios->ndescs,
> +				     "Invalid amount of channels described\n");

Are we sure that ndescs is going to be an error code?

> +static int adp5055_en_func(struct regulator_dev *dev, int en_val)
> +{
> +	struct adp5055 *adp5055 = rdev_get_drvdata(dev);
> +	int id;
> +	int mask;
> +
> +	id = rdev_get_id(dev);
> +	mask = BIT(id);
> +
> +	if (!adp5055->hw_en_array_gpios)
> +		return regmap_update_bits(adp5055->regmap, ADP5055_CTRL_MODE1, mask, en_val);
> +
> +	gpiod_set_value_cansleep(adp5055->hw_en_array_gpios->desc[id], en_val);

Just use the standard GPIO and regmap helpers for this.

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

Re: [PATCH 1/2] dt-bindings: regulator: add adi,adp5055-regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

On Tue, Feb 25, 2025 at 05:08:33PM +0800, Alexis Czezar Torreno wrote:

> +      adi,power-saving-mode:
> +        description:
> +          If present, enables power saving mode for
> +          individual channels.
> +        type: boolean

We have standard mode operations, please implement those.

> +      adi,output-discharge-function:
> +        description:
> +          If present, enable output discharge functionality
> +          for individual channels.
> +        type: boolean

set_active_discharge()

> +      adi,disable-delay-us:
> +        description:
> +          Configures the disable delay for each channel. Dependent on Tset.
> +        enum: [0, 5200, 10400, 15600, 20800, 26000, 31200, 36400]
> +        default: 0
> +
> +      adi,enable-delay-us:
> +        description:
> +          Configures the disable delay for each channel. Dependent on Tset.
> +        enum: [0, 2600, 5200, 7800, 10400, 13000, 15600, 18200]
> +        default: 0

This looks a lot like the driver should implemnt the enable_time()
and/or set_ramp_delay() operations and use the constraints to configure
this.

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

Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

On Tue, Feb 25, 2025 at 05:08:34PM +0800, Alexis Czezar Torreno wrote:

> +	device_for_each_child_node_scoped(dev, child) {
> +		ret = fwnode_property_read_u32(child, "reg", &i);
> +		if (ret)

Use of_parse_cb() to parse per-regulator properties.

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

RE: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, February 25, 2025 9:55 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055
> 
> [External]
> 
> On Tue, Feb 25, 2025 at 05:08:34PM +0800, Alexis Czezar Torreno wrote:
> 
> > Add ADI ADP5055 driver support. The device consists of 3 buck
> > regulators able to connect to high input voltages of up to 18V with no
> > preregulators.
> 
> There's a bunch of stuff here which should be using core features, I'll comment
> some of those on the DT binding patch.

(Addressed on the dt binding patch email)

> 
> > +config REGULATOR_ADP5055
> > +	tristate "Analog Devices ADP5055 Triple Buck Regulator"
> > +	depends on I2C
> > +        select REGMAP_I2C
> > +	help
> > +	  This driver controls an Analog Devices ADP5055 with triple buck
> > +          regulators using an I2C interface.
> 
> The indentation for the select and the second line of the description is weird.

Will fix, it seems my I had used space rather than tabs for those

> 
> > @@ -0,0 +1,490 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Regulator driver for Analog Devices ADP5055
> > + *
> > + * Copyright (C) 2025 Analog Devices, Inc.
> > + */
> 
> Please make the entire comment block a C++ one so things look more
> intentional.

Am not familiar with this, is this where each line use // rather than /**/?

> 
> > +static const struct regmap_range adp5055_reg_ranges[] = {
> > +	regmap_reg_range(0xD1, 0xE0),
> > +};
> > +
> > +static const struct regmap_access_table adp5055_write_ranges_table = {
> > +	.yes_ranges	= adp5055_reg_ranges,
> > +	.n_yes_ranges	= ARRAY_SIZE(adp5055_reg_ranges),
> > +};
> > +
> > +static const struct regmap_access_table adp5055_read_ranges_table = {
> > +	.yes_ranges	= adp5055_reg_ranges,
> > +	.n_yes_ranges	= ARRAY_SIZE(adp5055_reg_ranges),
> > +};
> 
> The read and write ranges could just be one variable.

Will simplify/merge

> 
> > +static const struct regmap_config adp5055_regmap_config = {
> > +	.reg_bits = 8,
> > +	.val_bits = 8,
> > +	.max_register = 0xFF,
> 
> max_register is set to 0xff but the largest read or write register is 0xe0.  Might
> be worth considering adding cache support too, there's read/modify/write
> cycles here.

Will edit to 0xe0. Will also check on cache

> 
> > +	if (!adp5055->hw_en_array_gpios)
> > +		if (adp5055->hw_en_array_gpios->ndescs !=
> ADP5055_NUM_CH)
> > +			return dev_err_probe(dev, adp5055-
> >hw_en_array_gpios->ndescs,
> > +				     "Invalid amount of channels described\n");
> 
> Are we sure that ndescs is going to be an error code?

My mistake, this should be -EINVAL instead

> 
> > +static int adp5055_en_func(struct regulator_dev *dev, int en_val) {
> > +	struct adp5055 *adp5055 = rdev_get_drvdata(dev);
> > +	int id;
> > +	int mask;
> > +
> > +	id = rdev_get_id(dev);
> > +	mask = BIT(id);
> > +
> > +	if (!adp5055->hw_en_array_gpios)
> > +		return regmap_update_bits(adp5055->regmap,
> ADP5055_CTRL_MODE1,
> > +mask, en_val);
> > +
> > +	gpiod_set_value_cansleep(adp5055->hw_en_array_gpios->desc[id],
> > +en_val);
> 
> Just use the standard GPIO and regmap helpers for this.

Confused on this, I thought these were standard 'regmap_update_bits' and 
'gpiod_set_value_cansleep'

RE: [PATCH 1/2] dt-bindings: regulator: add adi,adp5055-regulator

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, February 25, 2025 10:07 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/2] dt-bindings: regulator: add adi,adp5055-regulator
> 
> [External]
> 
> On Tue, Feb 25, 2025 at 05:08:33PM +0800, Alexis Czezar Torreno wrote:
> 
> > +      adi,power-saving-mode:
> > +        description:
> > +          If present, enables power saving mode for
> > +          individual channels.
> > +        type: boolean
> 
> We have standard mode operations, please implement those.

Will implement

> 
> > +      adi,output-discharge-function:
> > +        description:
> > +          If present, enable output discharge functionality
> > +          for individual channels.
> > +        type: boolean
> 
> set_active_discharge()

Will implement

> 
> > +      adi,disable-delay-us:
> > +        description:
> > +          Configures the disable delay for each channel. Dependent on Tset.
> > +        enum: [0, 5200, 10400, 15600, 20800, 26000, 31200, 36400]
> > +        default: 0
> > +
> > +      adi,enable-delay-us:
> > +        description:
> > +          Configures the disable delay for each channel. Dependent on Tset.
> > +        enum: [0, 2600, 5200, 7800, 10400, 13000, 15600, 18200]
> > +        default: 0
> 
> This looks a lot like the driver should implemnt the enable_time() and/or
> set_ramp_delay() operations and use the constraints to configure this.

Based on what I understand I agree implementing the enable_time() core
function for this. However, shall I keep the code for the disable-delay-us?
I don't think I saw a disable_time() equivalent

RE: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, February 25, 2025 10:07 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055
> 
> [External]
> 
> On Tue, Feb 25, 2025 at 05:08:34PM +0800, Alexis Czezar Torreno wrote:
> 
> > +	device_for_each_child_node_scoped(dev, child) {
> > +		ret = fwnode_property_read_u32(child, "reg", &i);
> > +		if (ret)
> 
> Use of_parse_cb() to parse per-regulator properties.

Will figure out how this works, thanks

Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Wed, Feb 26, 2025 at 02:24:58AM +0000, Torreno, Alexis Czezar wrote:

> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Regulator driver for Analog Devices ADP5055
> > > + *
> > > + * Copyright (C) 2025 Analog Devices, Inc.
> > > + */

> > Please make the entire comment block a C++ one so things look more
> > intentional.

> Am not familiar with this, is this where each line use // rather than /**/?

Yes.

> > > +static int adp5055_en_func(struct regulator_dev *dev, int en_val) {
> > > +	struct adp5055 *adp5055 = rdev_get_drvdata(dev);

> > Just use the standard GPIO and regmap helpers for this.

> Confused on this, I thought these were standard 'regmap_update_bits' and 
> 'gpiod_set_value_cansleep'

You've open coded the operations instead of using the framework helpers,
you shouldn't need to anything other than supply data here.

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

Re: [PATCH 1/2] dt-bindings: regulator: add adi,adp5055-regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On Wed, Feb 26, 2025 at 02:25:02AM +0000, Torreno, Alexis Czezar wrote:

> > > +      adi,disable-delay-us:
> > > +        description:
> > > +          Configures the disable delay for each channel. Dependent on Tset.
> > > +        enum: [0, 5200, 10400, 15600, 20800, 26000, 31200, 36400]
> > > +        default: 0

> > This looks a lot like the driver should implemnt the enable_time() and/or
> > set_ramp_delay() operations and use the constraints to configure this.

> Based on what I understand I agree implementing the enable_time() core
> function for this. However, shall I keep the code for the disable-delay-us?
> I don't think I saw a disable_time() equivalent

We don't generally worry about disable times, they're hugely load
dependent so the numbers tend to involve a lot of wishful thinking.

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

RE: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Torreno, Alexis Czezar]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Wednesday, February 26, 2025 7:35 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055
> 
> [External]
> 
> On Wed, Feb 26, 2025 at 02:24:58AM +0000, Torreno, Alexis Czezar wrote:
> 
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Regulator driver for Analog Devices ADP5055
> > > > + *
> > > > + * Copyright (C) 2025 Analog Devices, Inc.
> > > > + */
> 
> > > Please make the entire comment block a C++ one so things look more
> > > intentional.
> 
> > Am not familiar with this, is this where each line use // rather than /**/?
> 
> Yes.
> 
> > > > +static int adp5055_en_func(struct regulator_dev *dev, int en_val) {
> > > > +	struct adp5055 *adp5055 = rdev_get_drvdata(dev);
> 
> > > Just use the standard GPIO and regmap helpers for this.
> 
> > Confused on this, I thought these were standard 'regmap_update_bits'
> > and 'gpiod_set_value_cansleep'
> 
> You've open coded the operations instead of using the framework helpers, you
> shouldn't need to anything other than supply data here.

I did code this similar to the helper functions like regulator_enable_regmap.

I can reduce the code a bit if I use 'regulator_enable_regmap' inside my 
custom 'adp5055_en_func'.
ADP5055 can be enabled by SW or HW and I didn't see a helper function that 
can do both depending on another property/register hence I coded a custom.

if having both gpio and register enable isn't that common, I guess it's an option 
to remove the gpios and stay purely software

Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

On Fri, Feb 28, 2025 at 04:01:37AM +0000, Torreno, Alexis Czezar wrote:

> > You've open coded the operations instead of using the framework helpers, you
> > shouldn't need to anything other than supply data here.

> I did code this similar to the helper functions like regulator_enable_regmap.

Yes, that's the problem - you shouldn't be copying them at all.

> if having both gpio and register enable isn't that common, I guess it's an option 
> to remove the gpios and stay purely software

It's extremely common to have the option of choosing between a register
and a GPIO, that should just work with the framework code - we should
use the GPIO over the register if it's available.

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