mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add support for ADP5055 triple buck regulator.
@ 2025-03-20  6:53 Alexis Czezar Torreno
  2025-03-20  6:53 ` [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator Alexis Czezar Torreno
  2025-03-20  6:53 ` [PATCH v2 2/2] regulator: adp5055: Add driver for adp5055 Alexis Czezar Torreno
  0 siblings, 2 replies; 9+ messages in thread
From: Alexis Czezar Torreno @ 2025-03-20  6:53 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-kernel, devicetree, 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>
---
Changes in v2:
- Kconfig - fixed indention problem, occured due to using spaces rather tabs
- adp5055-regulator.c
  - Turned initial comment block into c++ style, using // per line
  - Merged the similar read/write ranges variable called access_ranges_table
  - Set max_register value correctly to 0xE0 instead of 0xFF
  - Fixed issue where ndescs was an error code. replaced with -EINVAL
  - Enable/disable ops functions, now use helpers like regulator_enable_regmap
  - Added config.ena_gpiod for the regulator config  
  - Used of_parse_cb to do per regulator properties, new function added
  - Migrated some yaml properties to the core regulator function
-adi,adp5055-regulator.yaml
  - Moved property power_saving_mode to set/get mode under 'IDLE' mode
  - Moved property output-discharge-func to core func set_active_discharge
  - Moved property enable-delay-us to core function set_ramp_delay.
  - Removed property disable-delay-us, as suggested to not worry about
  - adjusted sample device tree to match changes due to use of of_parse_cb
- Link to v1: https://lore.kernel.org/r/20250225-upstream-adp5055-v1-0-a5a7f8e46986@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  | 161 +++++++
 MAINTAINERS                                        |   7 +
 drivers/regulator/Kconfig                          |  11 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/adp5055-regulator.c              | 480 +++++++++++++++++++++
 5 files changed, 660 insertions(+)
---
base-commit: 7fef39f0e82ff02282797d9ae2589b39b16ab614
change-id: 20250225-upstream-adp5055-adbe6262f984

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
  2025-03-20  6:53 [PATCH v2 0/2] Add support for ADP5055 triple buck regulator Alexis Czezar Torreno
@ 2025-03-20  6:53 ` Alexis Czezar Torreno
  2025-03-20  8:29   ` Rob Herring (Arm)
  2025-03-20  9:06   ` Krzysztof Kozlowski
  2025-03-20  6:53 ` [PATCH v2 2/2] regulator: adp5055: Add driver for adp5055 Alexis Czezar Torreno
  1 sibling, 2 replies; 9+ messages in thread
From: Alexis Czezar Torreno @ 2025-03-20  6:53 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-kernel, devicetree, 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  | 161 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 2 files changed, 167 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..fc8f1e61ba321f8b4c6f8c1e3d0e91d570fb4953
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
@@ -0,0 +1,161 @@
+# 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,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:
+      - regulator-name
+
+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;
+
+            DCDC0 {
+                regulator-name = "DCDC0";
+                adi,dvs-limit-upper-microvolt = <192000>;
+                adi,dvs-limit-lower-microvolt = <(-190500)>;
+                adi,fast-transient = "5G_2.5%";
+                adi,mask-power-good;
+            };
+
+            DCDC1 {
+                regulator-name = "DCDC1";
+                adi,dvs-limit-upper-microvolt = <192000>;
+                adi,dvs-limit-lower-microvolt = <(-190500)>;
+                adi,fast-transient = "5G_2.5%";
+                adi,mask-power-good;
+            };
+
+            DCDC2 {
+                regulator-name = "DCDC2";
+                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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] regulator: adp5055: Add driver for adp5055
  2025-03-20  6:53 [PATCH v2 0/2] Add support for ADP5055 triple buck regulator Alexis Czezar Torreno
  2025-03-20  6:53 ` [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator Alexis Czezar Torreno
@ 2025-03-20  6:53 ` Alexis Czezar Torreno
  2025-03-20  9:09   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 9+ messages in thread
From: Alexis Czezar Torreno @ 2025-03-20  6:53 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-kernel, devicetree, 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 | 480 ++++++++++++++++++++++++++++++++++
 4 files changed, 493 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..4c08cf755d88e5f5bc6431192fad5f27786caea8 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..08c03a31e8e12afad57e87454bd18be3717e8739
--- /dev/null
+++ b/drivers/regulator/adp5055-regulator.c
@@ -0,0 +1,480 @@
+// 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_PSM		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;
+	u32 tset;
+	struct gpio_descs *hw_en_array_gpios;
+	int dvs_limit_upper[ADP5055_NUM_CH];
+	int dvs_limit_lower[ADP5055_NUM_CH];
+	u32 fast_transient[ADP5055_NUM_CH];
+	bool mask_power_good[ADP5055_NUM_CH];
+};
+
+static const unsigned int adp5055_tset_vals[] = {
+	2600,
+	20800,
+};
+
+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_access_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 = 0xE0,
+	.wr_table = &adp5055_access_ranges_table,
+	.rd_table = &adp5055_access_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;
+	bool ocp_blanking;
+	bool delay_power_good;
+
+	adp5055->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, -EINVAL,
+				     "Invalid amount of channels described\n");
+
+	ret = device_property_read_u32(dev, "adi,tset-us", &adp5055->tset);
+	if (!ret) {
+		ret = adp5055_get_prop_index(adp5055_tset_vals,
+					ARRAY_SIZE(adp5055_tset_vals), adp5055->tset);
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+				"Failed to initialize tset.");
+		adp5055->tset = adp5055_tset_vals[ret];
+	}
+
+	ocp_blanking = device_property_read_bool(dev, "adi,ocp-blanking");
+
+	delay_power_good = device_property_read_bool(dev,
+				    "adi,delay-power-good");
+
+	for (i = 0; i < ADP5055_NUM_CH; i++) {
+		val = FIELD_PREP(ADP5055_MASK_DVS_LIM_UPPER,
+				DIV_ROUND_CLOSEST_ULL(192000 - adp5055->dvs_limit_upper[i], 12000));
+		val |= FIELD_PREP(ADP5055_MASK_DVS_LIM_LOWER,
+				DIV_ROUND_CLOSEST_ULL(adp5055->dvs_limit_lower[i] + 190500, 12000));
+		ret = regmap_write(regmap, ADP5055_DVS_LIM0 + i, val);
+		if (ret)
+			return ret;
+
+		i++;
+	};
+
+	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);
+	ret = regmap_write(regmap, ADP5055_CTRL_MODE2, val);
+	if (ret)
+		return ret;
+
+	val = FIELD_PREP(ADP5055_MASK_FAST_TRANSIENT2, adp5055->fast_transient[2]);
+	val |= FIELD_PREP(ADP5055_MASK_FAST_TRANSIENT1, adp5055->fast_transient[1]);
+	val |= FIELD_PREP(ADP5055_MASK_FAST_TRANSIENT0, adp5055->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, adp5055->mask_power_good[2]);
+	val |= FIELD_PREP(ADP5055_MASK_PWRGD1, adp5055->mask_power_good[1]);
+	val |= FIELD_PREP(ADP5055_MASK_PWRGD0, adp5055->mask_power_good[0]);
+	ret = regmap_write(regmap, ADP5055_PG_CFG, val);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int adp5055_of_parse_cb(struct device_node *np,
+			      const struct regulator_desc *desc,
+			      struct regulator_config *config)
+{
+	struct adp5055 *adp5055 = config->driver_data;
+	int id, ret, pval, i;
+
+	id = desc->id;
+
+	ret = of_property_read_u32(np, "adi,dvs-limit-upper-microvolt", &pval);
+	if (ret)
+		adp5055->dvs_limit_upper[id] = 192000;
+	else
+		adp5055->dvs_limit_upper[id] = pval;
+
+	if (adp5055->dvs_limit_upper[id] > 192000 || adp5055->dvs_limit_upper[id] < 12000)
+		return dev_err_probe(config->dev, adp5055->dvs_limit_upper[id],
+			"Out of range - dvs-limit-upper-microvolt value.");
+
+	ret = of_property_read_u32(np, "adi,dvs-limit-lower-microvolt", &pval);
+	if (ret)
+		adp5055->dvs_limit_lower[id] = -190500;
+	else
+		adp5055->dvs_limit_lower[id] = pval;
+
+	if (adp5055->dvs_limit_lower[id] > -10500 || adp5055->dvs_limit_lower[id] < -190500)
+		return dev_err_probe(config->dev, adp5055->dvs_limit_lower[id],
+			"Out of range - dvs-limit-lower-microvolt value.");
+
+	for (i = 0; i < 4; i++) {
+		ret = of_property_match_string(np, "adi,fast-transient",
+					adp5055_fast_transient_vals[i]);
+		if (!ret)
+			break;
+	}
+
+	if (ret < 0)
+		adp5055->fast_transient[id] = 3;
+	else
+		adp5055->fast_transient[id] = i;
+
+	ret = of_property_read_bool(np, "adi,mask-power-good");
+	adp5055->mask_power_good[id] = 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_CTRL123, &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_enable(struct regulator_dev *dev)
+{
+	struct adp5055 *adp5055 = rdev_get_drvdata(dev);
+	int id = rdev_get_id(dev);
+
+	if (!adp5055->hw_en_array_gpios)
+		return regulator_enable_regmap(dev);
+
+	gpiod_set_value_cansleep(adp5055->hw_en_array_gpios->desc[id], 1);
+
+	return 0;
+}
+
+static int adp5055_disable(struct regulator_dev *dev)
+{
+	struct adp5055 *adp5055 = rdev_get_drvdata(dev);
+	int id = rdev_get_id(dev);
+
+	if (!adp5055->hw_en_array_gpios)
+		return regulator_disable_regmap(dev);
+
+	gpiod_set_value_cansleep(adp5055->hw_en_array_gpios->desc[id], 0);
+
+	return 0;
+}
+
+static int adp5055_set_mode(struct regulator_dev *rdev, u32 mode)
+{
+	struct adp5055 *adp5055 = rdev_get_drvdata(rdev);
+	int id, ret;
+
+	id = rdev_get_id(rdev);
+
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
+		ret = regmap_update_bits(adp5055->regmap, ADP5055_CTRL_MODE2,
+					ADP5055_MASK_PSM << id, 0);
+		break;
+	case REGULATOR_MODE_IDLE:
+		ret = regmap_update_bits(adp5055->regmap, ADP5055_CTRL_MODE2,
+					ADP5055_MASK_PSM << id, 1);
+		break;
+	default:
+		return dev_err_probe(&rdev->dev, -EINVAL,
+				"Unsupported mode: %d\n", mode);
+	}
+
+	return ret;
+}
+
+static unsigned int adp5055_get_mode(struct regulator_dev *rdev)
+{
+	struct adp5055 *adp5055 = rdev_get_drvdata(rdev);
+	int id, ret, regval;
+
+	id = rdev_get_id(rdev);
+
+	ret = regmap_read(adp5055->regmap, ADP5055_CTRL_MODE2, &regval);
+	if (ret)
+		return ret;
+
+	if (regval & (ADP5055_MASK_PSM << id))
+		return REGULATOR_MODE_IDLE;
+	else
+		return REGULATOR_MODE_NORMAL;
+}
+
+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,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
+	.enable = adp5055_enable,
+	.disable = adp5055_disable,
+	.is_enabled = adp5055_is_enabled,
+	.set_mode = adp5055_set_mode,
+	.get_mode = adp5055_get_mode,
+	.set_ramp_delay = regulator_set_ramp_delay_regmap,
+};
+
+#define ADP5055_REG_(_name, _id, _ch, _ops)					\
+	[_id] = { \
+		.name = _name, \
+		.of_match = of_match_ptr(_name), \
+		.of_parse_cb = adp5055_of_parse_cb, \
+		.id = _id, \
+		.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), \
+		.active_discharge_on = ADP5055_MASK_DIS##_id, \
+		.active_discharge_off = 0, \
+		.active_discharge_mask = ADP5055_MASK_DIS##_id, \
+		.active_discharge_reg = ADP5055_CTRL_MODE2, \
+		.ramp_reg = ADP5055_DLY##_ch, \
+		.ramp_mask = ADP5055_MASK_EN_DLY, \
+		.n_ramp_values = ARRAY_SIZE(adp5055_enable_delay_vals_2_6), \
+		.type = REGULATOR_VOLTAGE, \
+		.owner = THIS_MODULE, \
+	}
+
+#define ADP5055_REG(_name, _id, _ch) \
+	ADP5055_REG_(_name, _id, _ch, &adp5055_ops)
+
+static struct regulator_desc adp5055_regulators[] = {
+	ADP5055_REG("DCDC0", 0, 0),
+	ADP5055_REG("DCDC1", 1, 1),
+	ADP5055_REG("DCDC2", 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");
+
+	for (i = 0; i < ADP5055_NUM_CH; i++) {
+		const struct regulator_desc *desc;
+		struct regulator_config config = { };
+		struct regulator_dev *rdev;
+
+		if (adp5055->tset == 2600)
+			adp5055_regulators[i].ramp_delay_table = adp5055_enable_delay_vals_2_6;
+		else
+			adp5055_regulators[i].ramp_delay_table = adp5055_enable_delay_vals_20_8;
+
+		desc = &adp5055_regulators[i];
+
+		config.dev = dev;
+		config.driver_data = adp5055;
+		config.regmap = adp5055->regmap;
+		config.init_data = init_data;
+
+		if (adp5055->hw_en_array_gpios)
+			config.ena_gpiod = adp5055->hw_en_array_gpios->desc[i];
+
+		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);
+		}
+	}
+
+	ret = adp5055_parse_fw(dev, adp5055);
+	if (ret < 0)
+		return ret;
+
+	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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
  2025-03-20  6:53 ` [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator Alexis Czezar Torreno
@ 2025-03-20  8:29   ` Rob Herring (Arm)
  2025-03-24  8:45     ` Torreno, Alexis Czezar
  2025-03-20  9:06   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 9+ messages in thread
From: Rob Herring (Arm) @ 2025-03-20  8:29 UTC (permalink / raw)
  To: Alexis Czezar Torreno
  Cc: Conor Dooley, Liam Girdwood, Mark Brown, linux-kernel,
	devicetree, Krzysztof Kozlowski


On Thu, 20 Mar 2025 14:53:54 +0800, Alexis Czezar Torreno wrote:
> 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  | 161 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  2 files changed, 167 insertions(+)
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.example.dtb: regulator@70: Unevaluated properties are not allowed ('DCDC0', 'DCDC1', 'DCDC2' were unexpected)
	from schema $id: http://devicetree.org/schemas/regulator/adi,adp5055-regulator.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250320-upstream-adp5055-v2-1-aac2d3705802@analog.com

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

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

pip3 install dtschema --upgrade

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
  2025-03-20  6:53 ` [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator Alexis Czezar Torreno
  2025-03-20  8:29   ` Rob Herring (Arm)
@ 2025-03-20  9:06   ` Krzysztof Kozlowski
  2025-03-24  8:45     ` Torreno, Alexis Czezar
  1 sibling, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-20  9:06 UTC (permalink / raw)
  To: Alexis Czezar Torreno
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-kernel, devicetree

On Thu, Mar 20, 2025 at 02:53:54PM +0800, Alexis Czezar Torreno wrote:
> 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.
> 

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters


> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  .../bindings/regulator/adi,adp5055-regulator.yaml  | 161 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  2 files changed, 167 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..fc8f1e61ba321f8b4c6f8c1e3d0e91d570fb4953
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
> @@ -0,0 +1,161 @@
> +# 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:

Drop prefix, drop "array" and this probably will be changed anyway.

> +    description:
> +      Asserted during driver probe. Each array entry acts as the

s/Asserted during driver probe.//
If driver moves this code to other place, does it mean bindings are
wrong?

> +      hardware enable for channels 0-2. Should be marked 0 for active

What does it mean "0" for active low? No, active low has its own flag.

Use proper flags and implement it properly in the driver.

What is hardware enable and software enable? Is it enable-gpios per
regulator? Then why this isn't in the regulator node, just like we
expect for all regulator bindings?


> +      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.

Don't
wrap
at
random
places, plese.

> +    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])$":

This is a mess... never tested and makes no sense. Either this is a
regulator or a channel. Looks like regulator, but you called it a
channel. If regulator, then missing ref to regulator schema.

> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel number representing each buck converter.
> +        maximum: 2
> +
> +      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.

And how do you configure vout?

> +        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:
> +      - regulator-name
> +
> +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>;

No, use proper defines and proper flags.

> +
> +            adi,ocp-blanking;
> +            adi,delay-power-good;
> +
> +            DCDC0 {

Your schema said something else. Test your patches before sending, not
through our systems.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] regulator: adp5055: Add driver for adp5055
  2025-03-20  6:53 ` [PATCH v2 2/2] regulator: adp5055: Add driver for adp5055 Alexis Czezar Torreno
@ 2025-03-20  9:09   ` Krzysztof Kozlowski
  2025-03-24  8:45     ` Torreno, Alexis Czezar
  0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-20  9:09 UTC (permalink / raw)
  To: Alexis Czezar Torreno
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-kernel, devicetree

On Thu, Mar 20, 2025 at 02:53:55PM +0800, Alexis Czezar Torreno wrote:
> +#define ADP5055_REG(_name, _id, _ch) \
> +	ADP5055_REG_(_name, _id, _ch, &adp5055_ops)
> +
> +static struct regulator_desc adp5055_regulators[] = {
> +	ADP5055_REG("DCDC0", 0, 0),
> +	ADP5055_REG("DCDC1", 1, 1),
> +	ADP5055_REG("DCDC2", 2, 2),

Nodenames are lowercase, always.

> +};
> +
> +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");
> +
> +	for (i = 0; i < ADP5055_NUM_CH; i++) {
> +		const struct regulator_desc *desc;
> +		struct regulator_config config = { };
> +		struct regulator_dev *rdev;
> +

Where do you parse the 'reg' which you said is a channel number in the
binding?

> +		if (adp5055->tset == 2600)
> +			adp5055_regulators[i].ramp_delay_table = adp5055_enable_delay_vals_2_6;
> +		else
> +			adp5055_regulators[i].ramp_delay_table = adp5055_enable_delay_vals_20_8;
> +
> +		desc = &adp5055_regulators[i];
> +
> +		config.dev = dev;
> +		config.driver_data = adp5055;
> +		config.regmap = adp5055->regmap;
> +		config.init_data = init_data;
> +
> +		if (adp5055->hw_en_array_gpios)
> +			config.ena_gpiod = adp5055->hw_en_array_gpios->desc[i];

So a GPIO per node thus enable-gpios in each node.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
  2025-03-20  8:29   ` Rob Herring (Arm)
@ 2025-03-24  8:45     ` Torreno, Alexis Czezar
  0 siblings, 0 replies; 9+ messages in thread
From: Torreno, Alexis Czezar @ 2025-03-24  8:45 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Conor Dooley, Liam Girdwood, Mark Brown, linux-kernel,
	devicetree, Krzysztof Kozlowski



> -----Original Message-----
> From: Rob Herring (Arm) <robh@kernel.org>
> Sent: Thursday, March 20, 2025 4:29 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Conor Dooley <conor+dt@kernel.org>; Liam Girdwood
> <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; Krzysztof Kozlowski
> <krzk+dt@kernel.org>
> Subject: Re: [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
> 
> [External]
> 
> 
> On Thu, 20 Mar 2025 14:53:54 +0800, Alexis Czezar Torreno wrote:
> > 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  | 161
> +++++++++++++++++++++
> >  MAINTAINERS                                        |   6 +
> >  2 files changed, 167 insertions(+)
> >
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-
> ci/linux/Documentation/devicetree/bindings/regulator/adi,adp5055-
> regulator.example.dtb: regulator@70: Unevaluated properties are not allowed
> ('DCDC0', 'DCDC1', 'DCDC2' were unexpected)
> 	from schema $id:
> https://urldefense.com/v3/__http://devicetree.org/schemas/regulator/adi,a
> dp5055-regulator.yaml*__;Iw!!A3Ni8CS0y2Y!-eWjN-
> 2w2f0uPtkfb4hH9s1tlnXrqAvjBw2iVUyg3gsI8gIu9obk0WNxRA7iCvmTTlULk
> GGpQ6ttxWc2VwPf4A$
> 
> doc reference errors (make refcheckdocs):
> 
> See
> https://urldefense.com/v3/__https://patchwork.ozlabs.org/project/devicetr
> ee-bindings/patch/20250320-upstream-adp5055-v2-1-
> aac2d3705802@analog.com__;!!A3Ni8CS0y2Y!-eWjN-
> 2w2f0uPtkfb4hH9s1tlnXrqAvjBw2iVUyg3gsI8gIu9obk0WNxRA7iCvmTTlULk
> GGpQ6ttxWfHZ27Z4w$
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above error(s),
> then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your
> schema.

My fault, it seems that I really forgot to run 'make dt_binding_check' for v2. I only
ran b4 prep --check (for checkpatch.pl) before I submitted.

Will resubmit after I apply the reviews from the other maintainers.

Thank you for the suggestion on yamllint and dtschema, will add this in our docs.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
  2025-03-20  9:06   ` Krzysztof Kozlowski
@ 2025-03-24  8:45     ` Torreno, Alexis Czezar
  0 siblings, 0 replies; 9+ messages in thread
From: Torreno, Alexis Czezar @ 2025-03-24  8:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-kernel, devicetree



> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, March 20, 2025 5:07 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; 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 v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
> 
> [External]
> 
> On Thu, Mar 20, 2025 at 02:53:54PM +0800, Alexis Czezar Torreno wrote:
> > 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.
> >
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory your
> patch is touching. For bindings, the preferred subjects are explained here:
> https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/devic
> etree/bindings/submitting-patches.html*i-for-patch-
> submitters__;Iw!!A3Ni8CS0y2Y!-
> jfDnjTsrIleNV3xmOgakxTfgfPymC_1VWaNuF4unhOr23s35UCRin2d9qUc5Zo
> 4m92ovjLsDFzFBmdfkrzhqg$
> 

Will take note, I guess 'dt_bindings' and 'sub_system' are switched on a few 
subsystem like regulators/etc

> 
> > Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> > ---
> >  .../bindings/regulator/adi,adp5055-regulator.yaml  | 161
> +++++++++++++++++++++
> >  MAINTAINERS                                        |   6 +
> >  2 files changed, 167 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.ya
> > ml
> > b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.ya
> > ml
> > new file mode 100644
> > index
> >
> 0000000000000000000000000000000000000000..fc8f1e61ba321f8b4c6f
> 8c1e3d0e
> > 91d570fb4953
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulato
> > +++ r.yaml
> > @@ -0,0 +1,161 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://urldefense.com/v3/__http://devicetree.org/schemas/regulator/a
> > +di,adp5055-regulator.yaml*__;Iw!!A3Ni8CS0y2Y!-
> jfDnjTsrIleNV3xmOgakxTf
> >
> +gfPymC_1VWaNuF4unhOr23s35UCRin2d9qUc5Zo4m92ovjLsDFzFBmdSUR
> y26w$
> > +$schema:
> > +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
> > +aml*__;Iw!!A3Ni8CS0y2Y!-
> jfDnjTsrIleNV3xmOgakxTfgfPymC_1VWaNuF4unhOr23
> > +s35UCRin2d9qUc5Zo4m92ovjLsDFzFBmdXh_3TTw$
> > +
> > +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/a
> > +dp5055.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:
> 
> Drop prefix, drop "array" and this probably will be changed anyway.

Noted but yes, will probably move/change depending on discussion below
on the gpios/enables

> 
> > +    description:
> > +      Asserted during driver probe. Each array entry acts as the
> 
> s/Asserted during driver probe.//
> If driver moves this code to other place, does it mean bindings are wrong?

Will remove/edit. You're right, it shouldn't imply the bindings are wrong if ever.

> 
> > +      hardware enable for channels 0-2. Should be marked 0 for active
> 
> What does it mean "0" for active low? No, active low has its own flag.
> 
> Use proper flags and implement it properly in the driver.

I was using GPIO_ACTIVE_LOW before just '0' and it was generating errors.
I ended up wording the property this way due to it.

I was only made aware today of my colleagues that I was supposed to place
the flag back before sending my patch as this was more standard. 
The error is due to our setup.
Will correct it and be more careful.

> 
> What is hardware enable and software enable? Is it enable-gpios per
> regulator? Then why this isn't in the regulator node, just like we expect for all
> regulator bindings?

There's a register called CTRL_MODE1 and 2 bits of it configures how "enabling"
works. This is not per regulator but for the whole device
00 - only HW or gpios enables each regulator
01 - only SW ie register writes
10 - both gpio and register are needed to enable
11 - either sw/hw 

We decided to support 00 and 01, but the fact that all regulators are affected
there can't be 1 regulator enabled via software but the others are via gpio.

To handle this, our first idea was to create a gpio array to make sure all three gpios
are declared. If it is not exactly 3 gpios, the driver automatically uses SW enable.

Can move this to each regulator node as enable-gpios.

> 
> 
> > +      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.
> 
> Don't
> wrap
> at
> random
> places, plese.

My bad, edited the first line without considering the next.

> 
> > +    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])$":
> 
> This is a mess... never tested and makes no sense. Either this is a regulator or a
> channel. Looks like regulator, but you called it a channel. If regulator, then
> missing ref to regulator schema.

I approached this as a channel initially but understood later that it should be 
treated as a regulator instead and failed to completely update the doc after. 
Will fix, including other properties that called it channel.

> 
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        description: The channel number representing each buck converter.
> > +        maximum: 2
> > +
> > +      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.
> 
> And how do you configure vout?

Calling this vout seem inaccurate when I read the datasheet again, might change.
But this vout is tied to some feedback resistors (600mV x (1 + Rt/Rb))
I added the equation for context, but the interest is only on 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:
> > +      - regulator-name
> > +
> > +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>;
> 
> No, use proper defines and proper flags.

Yes will fix as discussed above

> 
> > +
> > +            adi,ocp-blanking;
> > +            adi,delay-power-good;
> > +
> > +            DCDC0 {
> 
> Your schema said something else. Test your patches before sending, not
> through our systems.

Apology on this again, I forgot to run the dt_binding_check for v2

> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 2/2] regulator: adp5055: Add driver for adp5055
  2025-03-20  9:09   ` Krzysztof Kozlowski
@ 2025-03-24  8:45     ` Torreno, Alexis Czezar
  0 siblings, 0 replies; 9+ messages in thread
From: Torreno, Alexis Czezar @ 2025-03-24  8:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-kernel, devicetree



> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, March 20, 2025 5:10 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@analog.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; 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 v2 2/2] regulator: adp5055: Add driver for adp5055
> 
> [External]
> 
> On Thu, Mar 20, 2025 at 02:53:55PM +0800, Alexis Czezar Torreno wrote:
> > +#define ADP5055_REG(_name, _id, _ch) \
> > +	ADP5055_REG_(_name, _id, _ch, &adp5055_ops)
> > +
> > +static struct regulator_desc adp5055_regulators[] = {
> > +	ADP5055_REG("DCDC0", 0, 0),
> > +	ADP5055_REG("DCDC1", 1, 1),
> > +	ADP5055_REG("DCDC2", 2, 2),
> 
> Nodenames are lowercase, always.

Noted on this, will apply. 
I based this on the act8865 driver, which seems to be a rare case
now that I look at the others

> 
> > +};
> > +
> > +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");
> > +
> > +	for (i = 0; i < ADP5055_NUM_CH; i++) {
> > +		const struct regulator_desc *desc;
> > +		struct regulator_config config = { };
> > +		struct regulator_dev *rdev;
> > +
> 
> Where do you parse the 'reg' which you said is a channel number in the
> binding?

As stated in my [Patch v2 1/2] reply, I changed it from channel to regulator (but I failed
to update the bindings completely)
The 'reg' is no more, I now use the name and id of regulator_desc to differentiate

> 
> > +		if (adp5055->tset == 2600)
> > +			adp5055_regulators[i].ramp_delay_table =
> adp5055_enable_delay_vals_2_6;
> > +		else
> > +			adp5055_regulators[i].ramp_delay_table =
> > +adp5055_enable_delay_vals_20_8;
> > +
> > +		desc = &adp5055_regulators[i];
> > +
> > +		config.dev = dev;
> > +		config.driver_data = adp5055;
> > +		config.regmap = adp5055->regmap;
> > +		config.init_data = init_data;
> > +
> > +		if (adp5055->hw_en_array_gpios)
> > +			config.ena_gpiod = adp5055->hw_en_array_gpios-
> >desc[i];
> 
> So a GPIO per node thus enable-gpios in each node.

Discussed in [Patch v2 1/2] reply

> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-03-24  8:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-20  6:53 [PATCH v2 0/2] Add support for ADP5055 triple buck regulator Alexis Czezar Torreno
2025-03-20  6:53 ` [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator Alexis Czezar Torreno
2025-03-20  8:29   ` Rob Herring (Arm)
2025-03-24  8:45     ` Torreno, Alexis Czezar
2025-03-20  9:06   ` Krzysztof Kozlowski
2025-03-24  8:45     ` Torreno, Alexis Czezar
2025-03-20  6:53 ` [PATCH v2 2/2] regulator: adp5055: Add driver for adp5055 Alexis Czezar Torreno
2025-03-20  9:09   ` Krzysztof Kozlowski
2025-03-24  8:45     ` Torreno, Alexis Czezar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®