[PATCH 0/2] regulator: add support for SGMicro SGM3804

[PATCH 0/2] regulator: add support for SGMicro SGM3804

[Alexandre Hamamdjian via B4 Relay]

This series adds support for the SGMicro SGM3804, an I2C-controlled
positive/negative output charge-pump regulator. The chip is used to
generate the AVDD/AVEE rails for display panels and is present on the
Ayaneo Pocket DS handheld, where it powers the panel and is required
before any panel driver can light up the display.

The Ayaneo Pocket DS device tree, posted as a separate series, depends
on the binding introduced here to describe its panel power supply, so
this series is a prerequisite for that work and for any subsequent
panel-related patches targeting the same board.

Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
---
Alexandre Hamamdjian (2):
      dt-bindings: regulator: add SGMicro SGM3804
      regulator: sgm3804: add SGMicro SGM3804 charge-pump regulator driver

 .../bindings/regulator/sgmicro,sgm3804.yaml        |  60 ++++++++
 MAINTAINERS                                        |   7 +
 drivers/regulator/Kconfig                          |  11 ++
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/sgm3804-regulator.c              | 164 +++++++++++++++++++++
 5 files changed, 243 insertions(+)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260510-sgm3804-c37a0ae6d7f3

Best regards,
--  
Alexandre Hamamdjian <azkali.limited@gmail.com>

[PATCH 1/2] dt-bindings: regulator: add SGMicro SGM3804

[Alexandre Hamamdjian via B4 Relay]

From: Alexandre Hamamdjian <azkali.limited@gmail.com>

The SGMicro SGM3804 is an I2C-controlled positive and negative output
charge-pump regulator. It is commonly used to provide the AVDD and AVEE
supplies for display panels, and exposes two reset GPIOs to sequence the
positive and negative output rails independently.

Document the binding for the new compatible "sgmicro,sgm3804" so it can
be referenced by board device trees and the matching driver, and add a
MAINTAINERS entry covering both the binding and the upcoming driver.

Co-developed-by: Philippe Simons <simons.philippe@gmail.com>
Signed-off-by: Philippe Simons <simons.philippe@gmail.com>
Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
---
 .../bindings/regulator/sgmicro,sgm3804.yaml        | 60 ++++++++++++++++++++++
 MAINTAINERS                                        |  7 +++
 2 files changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml b/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml
new file mode 100644
index 000000000000..f1de17bd5395
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/sgmicro,sgm3804.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SGMicro SGM3804 I2C charge-pump regulator
+
+maintainers:
+  - Alexandre Hamamdjian <azkali.limited@gmail.com>
+  - Philippe Simons <simons.philippe@gmail.com>
+
+description:
+  The SGM3804 is an I2C-controlled positive and negative output charge-pump
+  regulator typically used to provide the AVDD and AVEE supplies for display
+  panels. Two reset GPIOs are used to sequence the positive and negative
+  outputs independently.
+
+allOf:
+  - $ref: regulator.yaml#
+
+properties:
+  compatible:
+    const: sgmicro,sgm3804
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    minItems: 1
+    maxItems: 2
+    description:
+      Reset/enable GPIOs for the positive (index 0) and, optionally, the
+      negative (index 1) output rails.
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        regulator@3e {
+            compatible = "sgmicro,sgm3804";
+            reg = <0x3e>;
+            reset-gpios = <&tlmm 10 GPIO_ACTIVE_HIGH>,
+                          <&tlmm 11 GPIO_ACTIVE_HIGH>;
+            regulator-name = "panel-avdd";
+            regulator-min-microvolt = <5000000>;
+            regulator-max-microvolt = <5000000>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index f877e5aaf2c7..394216175d2e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24391,6 +24391,13 @@ M:	Steve Wahl <steve.wahl@hpe.com>
 S:	Maintained
 F:	drivers/misc/sgi-xp/
 
+SGMICRO SGM3804 REGULATOR DRIVER
+M:	Alexandre Hamamdjian <azkali.limited@gmail.com>
+M:	Philippe Simons <simons.philippe@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml
+F:	drivers/regulator/sgm3804-regulator.c
+
 SHARED MEMORY COMMUNICATIONS (SMC) SOCKETS
 M:	D. Wythe <alibuda@linux.alibaba.com>
 M:	Dust Li <dust.li@linux.alibaba.com>

-- 
2.54.0

[PATCH 2/2] regulator: sgm3804: add SGMicro SGM3804 charge-pump regulator driver

[Alexandre Hamamdjian via B4 Relay]

From: Alexandre Hamamdjian <azkali.limited@gmail.com>

Add a driver for the SGMicro SGM3804, an I2C-controlled positive and
negative output charge-pump regulator. The chip is typically used to
generate the AVDD/AVEE rails of display panels, and is for example
present on the Ayaneo Pocket DS handheld where it powers the panel.

The driver exposes a single 5V regulator and uses two reset GPIOs to
sequence the positive and negative outputs. Initialisation values for
the charge-pump configuration registers are written on enable, and the
GPIOs are dropped on disable.

Co-developed-by: Philippe Simons <simons.philippe@gmail.com>
Signed-off-by: Philippe Simons <simons.philippe@gmail.com>
Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
---
 drivers/regulator/Kconfig             |  11 +++
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/sgm3804-regulator.c | 164 ++++++++++++++++++++++++++++++++++
 3 files changed, 176 insertions(+)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index d71dac9436e3..e1adb0bda75d 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1490,6 +1490,17 @@ config REGULATOR_SC2731
 	  This driver provides support for the voltage regulators on the
 	  SC2731 PMIC.
 
+config REGULATOR_SGM3804
+	tristate "SGMicro sgm3804 voltage regulator"
+	depends on I2C && OF
+	help
+	  This driver supports the SGMicro SGM3804 I2C-controlled positive
+	  and negative output charge-pump regulator, commonly used to supply
+	  AVDD/AVEE rails to display panels. Two reset GPIOs are used to
+	  sequence the positive and negative outputs.
+
+	  Say M here to build the driver as a module called sgm3804-regulator.
+
 config REGULATOR_SKY81452
 	tristate "Skyworks Solutions SKY81452 voltage regulator"
 	depends on MFD_SKY81452
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 35639f3115fd..98ecbbc3c6b7 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -172,6 +172,7 @@ obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
 obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
 obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
 obj-$(CONFIG_REGULATOR_SC2731) += sc2731-regulator.o
+obj-$(CONFIG_REGULATOR_SGM3804) += sgm3804-regulator.o
 obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
 obj-$(CONFIG_REGULATOR_SLG51000) += slg51000-regulator.o
 obj-$(CONFIG_REGULATOR_SPACEMIT_P1) += spacemit-p1.o
diff --git a/drivers/regulator/sgm3804-regulator.c b/drivers/regulator/sgm3804-regulator.c
new file mode 100644
index 000000000000..c52f0596acf3
--- /dev/null
+++ b/drivers/regulator/sgm3804-regulator.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/gpio/consumer.h>
+
+struct sgm3804_data {
+	struct regmap *regmap;
+	struct gpio_desc *reset_gpio[2];
+	bool enabled;
+};
+
+static const struct regmap_config sgm3804_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x03,
+};
+
+static int sgm3804_enable(struct regulator_dev *rdev)
+{
+	struct sgm3804_data *data = rdev_get_drvdata(rdev);
+	struct regmap *regmap = data->regmap;
+	int ret = 0;
+
+	/* Set reset GPIO high to enable the device if available */
+	if (data->reset_gpio[0])
+		gpiod_set_value_cansleep(data->reset_gpio[0], 1);
+
+	if (data->reset_gpio[1])
+		gpiod_set_value_cansleep(data->reset_gpio[1], 1);
+
+	ret |= regmap_write(regmap, 0x00, 0x0c);
+	ret |= regmap_write(regmap, 0x01, 0x0c);
+	ret |= regmap_write(regmap, 0x03, 0x03);
+	if (ret) {
+		dev_err(rdev->dev.parent,
+			"Failed to enable SGM3804 regulator\n");
+		return ret;
+	}
+
+	data->enabled = true;
+	return 0;
+}
+
+static int sgm3804_disable(struct regulator_dev *rdev)
+{
+	struct sgm3804_data *data = rdev_get_drvdata(rdev);
+
+	if (data->reset_gpio[0])
+		gpiod_set_value_cansleep(data->reset_gpio[0], 0);
+
+	if (data->reset_gpio[1])
+		gpiod_set_value_cansleep(data->reset_gpio[1], 0);
+
+	data->enabled = false;
+	return 0;
+}
+
+static int sgm3804_is_enabled(struct regulator_dev *rdev)
+{
+	struct sgm3804_data *data = rdev_get_drvdata(rdev);
+
+	return data->enabled;
+}
+
+static int sgm3804_get_voltage(struct regulator_dev *rdev)
+{
+	return 5000000;
+}
+
+static const struct regulator_ops sgm3804_ops = {
+	.enable = sgm3804_enable,
+	.disable = sgm3804_disable,
+	.is_enabled = sgm3804_is_enabled,
+	.get_voltage = sgm3804_get_voltage,
+};
+
+static const struct regulator_desc sgm3804_reg = {
+	.name = "SGM3804",
+	.id = 0,
+	.ops = &sgm3804_ops,
+	.type = REGULATOR_VOLTAGE,
+	.n_voltages = 1,
+	.min_uV = 5000000,
+	.owner = THIS_MODULE,
+};
+
+static int sgm3804_i2c_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev;
+	struct regulator_config config = {};
+	struct regulator_dev *rdev;
+	struct sgm3804_data *data;
+	int error;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->regmap = devm_regmap_init_i2c(i2c, &sgm3804_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap),
+				     "failed to init regmap\n");
+
+	/* Get reset-gpio from device tree */
+	data->reset_gpio[0] =
+		devm_gpiod_get_index(dev, "reset", 0, GPIOD_OUT_HIGH);
+	if (IS_ERR(data->reset_gpio[0]))
+		return dev_err_probe(dev, PTR_ERR(data->reset_gpio[0]),
+				     "failed to get first reset GPIO\n");
+
+	data->reset_gpio[1] =
+		devm_gpiod_get_index(dev, "reset", 1, GPIOD_OUT_HIGH);
+	if (IS_ERR(data->reset_gpio[1]))
+		dev_warn(dev, "failed to get second reset GPIO\n");
+
+	config.dev = dev;
+	config.regmap = data->regmap;
+	config.driver_data = data;
+	config.of_node = dev->of_node;
+	config.init_data =
+		of_get_regulator_init_data(dev, dev->of_node, &sgm3804_reg);
+	if (!config.init_data)
+		return -ENOMEM;
+	data->enabled = false;
+	rdev = devm_regulator_register(dev, &sgm3804_reg, &config);
+	if (IS_ERR(rdev)) {
+		error = PTR_ERR(rdev);
+		dev_err(dev, "Failed to register SGM3804 regulator: %d\n",
+			error);
+		return error;
+	}
+
+	return 0;
+}
+
+static const struct i2c_device_id sgm3804_i2c_id[] = { { "sgm3804" }, {} };
+MODULE_DEVICE_TABLE(i2c, sgm3804_i2c_id);
+
+static const struct of_device_id sgm3804_i2c_of_match[] = {
+	{ .compatible = "sgmicro,sgm3804" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sgm3804_i2c_of_match);
+
+static struct i2c_driver sgm3804_regulator_driver = {
+	.driver = {
+		.name = "sgm3804",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = sgm3804_i2c_of_match,
+	},
+	.probe = sgm3804_i2c_probe,
+	.id_table = sgm3804_i2c_id,
+};
+
+module_i2c_driver(sgm3804_regulator_driver);
+
+MODULE_DESCRIPTION("SGMicro sgm3804 regulator Driver");
+MODULE_AUTHOR("Kancy Joe <kancy2333@outlook.com>");
+MODULE_LICENSE("GPL");

-- 
2.54.0

Re: [PATCH 1/2] dt-bindings: regulator: add SGMicro SGM3804

[Rob Herring (Arm)]

On Sun, 10 May 2026 23:45:26 +0700, Alexandre Hamamdjian wrote:
> The SGMicro SGM3804 is an I2C-controlled positive and negative output
> charge-pump regulator. It is commonly used to provide the AVDD and AVEE
> supplies for display panels, and exposes two reset GPIOs to sequence the
> positive and negative output rails independently.
> 
> Document the binding for the new compatible "sgmicro,sgm3804" so it can
> be referenced by board device trees and the matching driver, and add a
> MAINTAINERS entry covering both the binding and the upcoming driver.
> 
> Co-developed-by: Philippe Simons <simons.philippe@gmail.com>
> Signed-off-by: Philippe Simons <simons.philippe@gmail.com>
> Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
> ---
>  .../bindings/regulator/sgmicro,sgm3804.yaml        | 60 ++++++++++++++++++++++
>  MAINTAINERS                                        |  7 +++
>  2 files changed, 67 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/sgmicro,sgm3804.example.dtb: regulator@3e (sgmicro,sgm3804): reset-gpios: [[4294967295, 10, 0], [4294967295, 11, 0]] is too long
	from schema $id: http://devicetree.org/schemas/gpio/gpio-consumer-common.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260510-sgm3804-v1-1-e5e8799e0aa0@gmail.com

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

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

pip3 install dtschema --upgrade

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

Re: [PATCH 0/2] regulator: add support for SGMicro SGM3804

[Mark Brown]

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

On Sun, May 10, 2026 at 11:45:25PM +0700, Alexandre Hamamdjian via B4 Relay wrote:
> This series adds support for the SGMicro SGM3804, an I2C-controlled
> positive/negative output charge-pump regulator. The chip is used to
> generate the AVDD/AVEE rails for display panels and is present on the
> Ayaneo Pocket DS handheld, where it powers the panel and is required
> before any panel driver can light up the display.

There is a separate series from Neil Armstrong (Cced) already in review
for the same part.  Please sort out what to do about this between
yourselves.

> 
> The Ayaneo Pocket DS device tree, posted as a separate series, depends
> on the binding introduced here to describe its panel power supply, so
> this series is a prerequisite for that work and for any subsequent
> panel-related patches targeting the same board.
> 
> Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
> ---
> Alexandre Hamamdjian (2):
>       dt-bindings: regulator: add SGMicro SGM3804
>       regulator: sgm3804: add SGMicro SGM3804 charge-pump regulator driver
> 
>  .../bindings/regulator/sgmicro,sgm3804.yaml        |  60 ++++++++
>  MAINTAINERS                                        |   7 +
>  drivers/regulator/Kconfig                          |  11 ++
>  drivers/regulator/Makefile                         |   1 +
>  drivers/regulator/sgm3804-regulator.c              | 164 +++++++++++++++++++++
>  5 files changed, 243 insertions(+)
> ---
> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
> change-id: 20260510-sgm3804-c37a0ae6d7f3
> 
> Best regards,
> --  
> Alexandre Hamamdjian <azkali.limited@gmail.com>
> 
> 

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

Re: [PATCH 1/2] dt-bindings: regulator: add SGMicro SGM3804

[Mark Brown]

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

On Sun, May 10, 2026 at 11:45:26PM +0700, Alexandre Hamamdjian via B4 Relay wrote:
> From: Alexandre Hamamdjian <azkali.limited@gmail.com>
> 
> The SGMicro SGM3804 is an I2C-controlled positive and negative output
> charge-pump regulator. It is commonly used to provide the AVDD and AVEE
> supplies for display panels, and exposes two reset GPIOs to sequence the
> positive and negative output rails independently.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

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

Re: [PATCH 0/2] regulator: add support for SGMicro SGM3804

[Neil Armstrong]

Hi Alwexandre,

On 5/11/26 02:51, Mark Brown wrote:
> On Sun, May 10, 2026 at 11:45:25PM +0700, Alexandre Hamamdjian via B4 Relay wrote:
>> This series adds support for the SGMicro SGM3804, an I2C-controlled
>> positive/negative output charge-pump regulator. The chip is used to
>> generate the AVDD/AVEE rails for display panels and is present on the
>> Ayaneo Pocket DS handheld, where it powers the panel and is required
>> before any panel driver can light up the display.
> 
> There is a separate series from Neil Armstrong (Cced) already in review
> for the same part.  Please sort out what to do about this between
> yourselves.

Please find the patchset at https://lore.kernel.org/all/20260506-topic-sm8650-ayaneo-pocket-s2-sgm3804-v4-0-7495e7905693@linaro.org/
it implements full support for the SGM3804 with voltage control on both
rails.

I guess most Ayaneo boards uses common components, I also sent patches for:
- https://lore.kernel.org/all/20260430-topic-sm8650-ayaneo-pocket-s2-sy7758-v2-0-308140640de9@linaro.org/
- https://lore.kernel.org/all/20260504-topic-sm8650-ayaneo-pocket-s2-r63419-v3-0-9f61cf24aebf@linaro.org/
- Display DT https://lore.kernel.org/all/20260428-topic-sm8650-ayaneo-pocket-s2-display-dt-v1-1-ff132c00d076@linaro.org/

Neil

> 
>>
>> The Ayaneo Pocket DS device tree, posted as a separate series, depends
>> on the binding introduced here to describe its panel power supply, so
>> this series is a prerequisite for that work and for any subsequent
>> panel-related patches targeting the same board.
>>
>> Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
>> ---
>> Alexandre Hamamdjian (2):
>>        dt-bindings: regulator: add SGMicro SGM3804
>>        regulator: sgm3804: add SGMicro SGM3804 charge-pump regulator driver
>>
>>   .../bindings/regulator/sgmicro,sgm3804.yaml        |  60 ++++++++
>>   MAINTAINERS                                        |   7 +
>>   drivers/regulator/Kconfig                          |  11 ++
>>   drivers/regulator/Makefile                         |   1 +
>>   drivers/regulator/sgm3804-regulator.c              | 164 +++++++++++++++++++++
>>   5 files changed, 243 insertions(+)
>> ---
>> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
>> change-id: 20260510-sgm3804-c37a0ae6d7f3
>>
>> Best regards,
>> --
>> Alexandre Hamamdjian <azkali.limited@gmail.com>
>>
>>

Re: [PATCH 1/2] dt-bindings: regulator: add SGMicro SGM3804

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The DT binding incorrectly models a dual-output regulator (positive and negative) as a single top-level regulator node, preventing independent configuration and referencing of the two outputs.
--

commit db24a4c2f2c96be5ccf7d7d19de93b27ab20e291
Author: Alexandre Hamamdjian <azkali.limited@gmail.com>

dt-bindings: regulator: add SGMicro SGM3804

This commit introduces the device tree binding for the SGMicro SGM3804,
an I2C-controlled charge-pump regulator providing positive and negative
outputs. It also adds the corresponding MAINTAINERS entry.

> diff --git a/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml b/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml
> new file mode 100644
> index 0000000000000..f1de17bd5395a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml
> @@ -0,0 +1,60 @@

[ ... ]

> +description:
> +  The SGM3804 is an I2C-controlled positive and negative output charge-pump
> +  regulator typically used to provide the AVDD and AVEE supplies for display
> +  panels. Two reset GPIOs are used to sequence the positive and negative
> +  outputs independently.
> +
> +allOf:
> +  - $ref: regulator.yaml#

Does this binding correctly model the hardware's dual-output nature?

By applying regulator.yaml# directly to the parent I2C device node without
defining subnodes for the individual outputs, the entire device is treated
as a single regulator.

Since standard properties like regulator-name and regulator-min-microvolt
can only be specified once per node, how can consumers independently configure
or reference the negative rail (AVEE) via phandles?

Should this device be modeled using child subnodes for the outputs (for
example, using patternProperties for outp and outn)?

[ ... ]

> +  reset-gpios:
> +    minItems: 1
> +    maxItems: 2
> +    description:
> +      Reset/enable GPIOs for the positive (index 0) and, optionally, the
> +      negative (index 1) output rails.

If child subnodes are introduced for the positive and negative regulators,
would it be more standard to place an enable-gpios property within each
individual regulator subnode instead of using a shared reset-gpios array on
the parent node?

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        regulator@3e {
> +            compatible = "sgmicro,sgm3804";
> +            reg = <0x3e>;
> +            reset-gpios = <&tlmm 10 GPIO_ACTIVE_HIGH>,
> +                          <&tlmm 11 GPIO_ACTIVE_HIGH>;
> +            regulator-name = "panel-avdd";
> +            regulator-min-microvolt = <5000000>;
> +            regulator-max-microvolt = <5000000>;
> +        };
> +    };

The example demonstrates this limitation, as it only configures constraints
for panel-avdd.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-sgm3804-v1-0-e5e8799e0aa0@gmail.com?part=1

Re: [PATCH 2/2] regulator: sgm3804: add SGMicro SGM3804 charge-pump regulator driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Unhandled ERR_PTR for the optional second reset GPIO leads to invalid pointer dereference and breaks probe deferral.
- [High] Hardware state mismatch at probe leads to permanent power leak.
- [High] Missing REGMAP_I2C dependency in Kconfig causes build failures.
- [Medium] Bitwise ORing of negative error codes corrupts regmap_write() return values.
- [Medium] Incomplete error path cleanup leaves reset GPIOs asserted if I2C writes fail.
- [Medium] Incorrectly aborting probe with -ENOMEM when of_get_regulator_init_data() returns NULL.
--

commit 382f3ebf01ae63955e0f9f2e654949e5dbe548b4
Author: Alexandre Hamamdjian <azkali.limited@gmail.com>

regulator: sgm3804: add SGMicro SGM3804 charge-pump regulator driver
    
Add an I2C-controlled charge-pump regulator driver for the SGMicro SGM3804,
typically used to power display panel AVDD/AVEE rails. The driver exposes
a single 5V regulator and uses two reset GPIOs for sequencing.

> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index d71dac9436e3e..e1adb0bda75d7 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
[ ... ]
> @@ -1490,6 +1490,17 @@ config REGULATOR_SC2731
>  	  This driver provides support for the voltage regulators on the
>  	  SC2731 PMIC.
>  
> +config REGULATOR_SGM3804
> +	tristate "SGMicro sgm3804 voltage regulator"
> +	depends on I2C && OF

Is a select REGMAP_I2C needed here? 

Since the driver relies on devm_regmap_init_i2c(), if this driver is
enabled without CONFIG_REGMAP_I2C selected elsewhere, it might cause a
link-time undefined reference error.

> +	help
> +	  This driver supports the SGMicro SGM3804 I2C-controlled positive
[ ... ]
> diff --git a/drivers/regulator/sgm3804-regulator.c b/drivers/regulator/sgm3804-regulator.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/regulator/sgm3804-regulator.c
[ ... ]
> +static int sgm3804_enable(struct regulator_dev *rdev)
> +{
> +	struct sgm3804_data *data = rdev_get_drvdata(rdev);
> +	struct regmap *regmap = data->regmap;
> +	int ret = 0;
> +
> +	/* Set reset GPIO high to enable the device if available */
> +	if (data->reset_gpio[0])
> +		gpiod_set_value_cansleep(data->reset_gpio[0], 1);
> +
> +	if (data->reset_gpio[1])
> +		gpiod_set_value_cansleep(data->reset_gpio[1], 1);
> +
> +	ret |= regmap_write(regmap, 0x00, 0x0c);
> +	ret |= regmap_write(regmap, 0x01, 0x0c);
> +	ret |= regmap_write(regmap, 0x03, 0x03);

Will bitwise ORing the return values here corrupt the error codes? 

The regmap_write() function returns standard negative error codes on failure.
Applying a bitwise OR to negative integers fundamentally alters their binary
representation, generating unrelated negative values (e.g., -5 | -12 evaluates
to -1, which is -EPERM) and it also issues subsequent I2C writes to a
potentially unresponsive device instead of returning early.

> +	if (ret) {
> +		dev_err(rdev->dev.parent,
> +			"Failed to enable SGM3804 regulator\n");
> +		return ret;
> +	}

If one of these I2C writes fails, should the reset GPIOs be driven back to 0
before returning? 

Currently, it looks like an error here leaves the hardware physically
enabled with the GPIOs still driven high, but returns an error which tells
the regulator core it is disabled.

[ ... ]
> +static int sgm3804_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct device *dev = &i2c->dev;
> +	struct regulator_config config = {};
> +	struct regulator_dev *rdev;
> +	struct sgm3804_data *data;
> +	int error;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->regmap = devm_regmap_init_i2c(i2c, &sgm3804_regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return dev_err_probe(dev, PTR_ERR(data->regmap),
> +				     "failed to init regmap\n");
> +
> +	/* Get reset-gpio from device tree */
> +	data->reset_gpio[0] =
> +		devm_gpiod_get_index(dev, "reset", 0, GPIOD_OUT_HIGH);

Could requesting the GPIOs with GPIOD_OUT_HIGH cause a state mismatch? 

This physically turns on the regulator hardware immediately, but data->enabled
is set to false below.

If the regulator framework queries sgm3804_is_enabled(), it receives false
and assumes the device is disabled. If no consumers claim it, will the core
ever invoke sgm3804_disable() to turn it off, or does this leave the device
permanently powered on? Would GPIOD_OUT_LOW be more appropriate?

> +	if (IS_ERR(data->reset_gpio[0]))
> +		return dev_err_probe(dev, PTR_ERR(data->reset_gpio[0]),
> +				     "failed to get first reset GPIO\n");
> +
> +	data->reset_gpio[1] =
> +		devm_gpiod_get_index(dev, "reset", 1, GPIOD_OUT_HIGH);
> +	if (IS_ERR(data->reset_gpio[1]))
> +		dev_warn(dev, "failed to get second reset GPIO\n");

Does this correctly handle the case where the second GPIO is absent or
deferring? 

If devm_gpiod_get_index() returns ERR_PTR(-ENOENT) or ERR_PTR(-EPROBE_DEFER),
that pointer remains in data->reset_gpio[1].

Later in sgm3804_enable(), if (data->reset_gpio[1]) evaluates as true for
an error pointer, passing it to gpiod_set_value_cansleep() which can cause
a warning spam. Should this use devm_gpiod_get_index_optional() instead?

> +
> +	config.dev = dev;
> +	config.regmap = data->regmap;
> +	config.driver_data = data;
> +	config.of_node = dev->of_node;
> +	config.init_data =
> +		of_get_regulator_init_data(dev, dev->of_node, &sgm3804_reg);
> +	if (!config.init_data)
> +		return -ENOMEM;

Is it safe to fail the probe if of_get_regulator_init_data() returns NULL?

This function can legitimately return NULL if the device tree node lacks
specific regulator properties or constraints, which is completely valid for
regulators acting as simple switches with default parameters.

Since the regulator core safely handles config.init_data == NULL, and
automatically parses the init data anyway because config.of_node is set,
does returning -ENOMEM here incorrectly prevent the device from binding on
valid device tree configurations?

> +	data->enabled = false;
> +	rdev = devm_regulator_register(dev, &sgm3804_reg, &config);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-sgm3804-v1-0-e5e8799e0aa0@gmail.com?part=2