[PATCH v5 0/2] regulator: add support for SGM3804 Dual Output driver

[PATCH v5 0/2] regulator: add support for SGM3804 Dual Output driver

[Neil Armstrong]

Add support for the SG Micro SGM3804 Single Inductor Dual Output
Buck/Boost Converter used to power LCD panels a provide positive
and negative power rails with configurable voltage and active
discharge function for each output.

The SGM3804 is powered by the enable GPIO pins inputs and only
supports I2C write messages. 
In order to add flexibility and simplify the driver, the
regmap cache is enabled and populated with default values
since we can't write registers when the 2 GPIOs are down.

This regulator is used to provide vsn and vsn power to the
Ayaneo Pocket S2 dual-DSI LCD panel.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v5:
- Add dependencies on GPIOLIB
- Mark as cache only if sync fails
- Use proper selector number for n_voltages
- Link to v4: https://patch.msgid.link/20260506-topic-sm8650-ayaneo-pocket-s2-sgm3804-v4-0-7495e7905693@linaro.org

Changes in v4:
- Moved the enable-gpios to subnodes due to enable-gpio bindings constraint
- Added code to get enable gpios from subnodes using fwnode to avoid the of->fwnode dance
- Switch last gpio call to cansleep
- Moved the Kconfig/Makefile entry in the right place
- Link to v3: https://patch.msgid.link/20260504-topic-sm8650-ayaneo-pocket-s2-sgm3804-v3-0-c4783443890b@linaro.org

Changes in v3:
- Change header in c++ comment block
- Switch to regcache dirty/only instead of custom handling
- Use GPIO_ASIS to avoid turning off regulator on boot
- Use gpio cansleep
- Add my MODULE_AUTHOR and reorder the signoff
- Link to v2: https://patch.msgid.link/20260430-topic-sm8650-ayaneo-pocket-s2-sgm3804-v2-0-76108c65a560@linaro.org

Changes in v2:
- Fixed bindings subject and gpios minItems
- Fixed regulator signed-off order
- Added second MODULE_AUTHOR entry
- Added more comments on how regmap cache is configured to work
- Link to v1: https://patch.msgid.link/20260428-topic-sm8650-ayaneo-pocket-s2-sgm3804-v1-0-1d8dc7620256@linaro.org

---
KancyJoe (1):
      regulator: add SGM3804 Dual Output driver

Neil Armstrong (1):
      regulator: dt-bindings: document the SGM3804 Dual Output regulator

 .../bindings/regulator/sgmicro,sgm3804.yaml        |  77 +++++
 drivers/regulator/Kconfig                          |   8 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/sgm3804-regulator.c              | 314 +++++++++++++++++++++
 4 files changed, 400 insertions(+)
---
base-commit: 39704f00f747aba3144289870b5fd8ac230a9aaf
change-id: 20260428-topic-sm8650-ayaneo-pocket-s2-sgm3804-8764fbb72eb7

Best regards,
--  
Neil Armstrong <neil.armstrong@linaro.org>

[PATCH v5 1/2] regulator: dt-bindings: document the SGM3804 Dual Output regulator

[Neil Armstrong]

Document the SG Micro SGM3804 Single Inductor Dual Output Buck/Boost
Converter used to power LCD panels a provide positive and negative
power rails with configurable voltage and active discharge function
for each output.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../bindings/regulator/sgmicro,sgm3804.yaml        | 77 ++++++++++++++++++++++
 1 file changed, 77 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..3716eaf81aa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 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: SG Micro SGM3804 Single Inductor Dual Output Buck/Boost Converter
+
+maintainers:
+  - Neil Armstrong <neil.armstrong@linaro.org>
+
+description:
+  The SGM3804 is a dual voltage regulator, designed to support positive/negative
+  supply for driving LCD panels. It support software-configurable output
+  switching. The output voltages can be programmed via an I2C compatible interface.
+
+properties:
+  compatible:
+    const: sgmicro,sgm3804
+
+  reg:
+    maxItems: 1
+
+  vin-supply: true
+
+patternProperties:
+  "^(pos|neg)$":
+    type: object
+    $ref: regulator.yaml#
+
+    properties:
+      enable-gpios: true
+
+    unevaluatedProperties: false
+
+    required:
+      - enable-gpios
+
+required:
+  - compatible
+  - reg
+  - pos
+  - neg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        regulator@3e {
+            compatible = "sgmicro,sgm3804";
+            reg = <0x3e>;
+
+            vin-supply = <&vin_reg>;
+
+            pos {
+                regulator-name = "outpos";
+                regulator-min-microvolt = <5000000>;
+                regulator-max-microvolt = <5000000>;
+                enable-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+            };
+
+            neg {
+                regulator-name = "outneg";
+                regulator-min-microvolt = <5000000>;
+                regulator-max-microvolt = <5000000>;
+                enable-gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
+            };
+        };
+    };
+...
+

-- 
2.34.1

Re: [PATCH v5 1/2] regulator: dt-bindings: document the SGM3804 Dual Output regulator

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `enable-gpios` property is incorrectly marked as strictly required and lacks a `maxItems: 1` constraint.
--

commit f50493aa554fa5cd96589605f9a49a0134ff39ab
Author: Neil Armstrong <neil.armstrong@linaro.org>

regulator: dt-bindings: document the SGM3804 Dual Output regulator

This commit documents the sgm3804 single inductor dual output buck/boost
converter. It provides bindings for the positive and negative power rails,
including configurable voltages and active discharge functions.

> diff --git a/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml b/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml
> new file mode 100644
> index 0000000000000..3716eaf81aa99
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/sgmicro,sgm3804.yaml

[ ... ]

> +patternProperties:
> +  "^(pos|neg)$":
> +    type: object
> +    $ref: regulator.yaml#
> +
> +    properties:
> +      enable-gpios: true

[Severity: Medium]
Should enable-gpios have a maxItems constraint here? The core regulator.yaml
schema does not define a maxItems limit for enable-gpios, so setting this to
true allows an arbitrary number of gpios to be specified in the device tree
instead of a single enable pin per output.

> +
> +    unevaluatedProperties: false
> +
> +    required:
> +      - enable-gpios

[Severity: Medium]
Is it strictly necessary to require enable-gpios for these child nodes? If a
board design hardwires the hardware enable pins to an always-on supply to
conserve processor pins, the regulator would be constantly powered but still
configurable via i2c. Marking enable-gpios as strictly required will cause a
schema validation failure for those valid hardware configurations, even if
the driver currently expects the pins to be present.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-topic-sm8650-ayaneo-pocket-s2-sgm3804-v5-0-bd6b1c300ecc@linaro.org?part=1

[PATCH v5 2/2] regulator: add SGM3804 Dual Output driver

[Neil Armstrong]

From: KancyJoe <kancy2333@outlook.com>

Add support for the SG Micro SGM3804 Single Inductor Dual Output
Buck/Boost Converter used to power LCD panels a provide positive
and negative power rails with configurable voltage and active
discharge function for each output.

The SGM3804 is powered by the enable GPIO pins inputs and only
supports I2C write messages.
In order to add flexibility and simplify the driver, the
regmap cache is enabled and populated with default values
since we can't write registers when the 2 GPIOs are down.

Signed-off-by: KancyJoe <kancy2333@outlook.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/regulator/Kconfig             |   8 +
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/sgm3804-regulator.c | 314 ++++++++++++++++++++++++++++++++++
 3 files changed, 323 insertions(+)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index e8002526cfb0..b79ad8852033 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1489,6 +1489,14 @@ 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
+	depends on GPIOLIB
+	select REGMAP_I2C
+	help
+	  This driver supports SGMicro SGM3804 dual-output voltage 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..c3406cfb73d0
--- /dev/null
+++ b/drivers/regulator/sgm3804-regulator.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// SGMicro SGM3804 regulator Driver
+//
+// Copyright (C) 2025 Kancy Joe <kancy2333@outlook.com>
+// Copyright (C) 2026 Linaro Limited
+// Author: Neil Armstrong <neil.armstrong@linaro.org>
+
+#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>
+
+#define SGM3804_POS_RAIL_VOLTAGE_REG	0x0
+#define SGM3804_NEG_RAIL_VOLTAGE_REG	0x1
+#define SGM3804_RAIL_DISCHARGE_REG	0x3
+
+#define RAIL_VOLTAGE_MASK	GENMASK(5, 0)
+
+#define POS_RAIL_DISCHARGE_EN	BIT(1)
+#define NEG_RAIL_DISCHARGE_EN	BIT(0)
+
+#define RAIL_VOLTAGE_INVALID		RAIL_VOLTAGE_MASK
+#define RAIL_DISCHARGE_REG_DEFAULT	(POS_RAIL_DISCHARGE_EN | NEG_RAIL_DISCHARGE_EN)
+
+#define SGM3804_VOLTAGES_MAX_SELECTOR	0x2f
+
+enum {
+	SGM3804_POS_RAIL = 0,
+	SGM3804_NEG_RAIL,
+	SGM3804_RAIL_COUNT,
+};
+
+/*
+ * The registers are only writable when the gpio is enabled, so
+ * we need to use the cache for read operations and set the regmap
+ * as cache_only when both GPIOs are down.
+ */
+struct sgm3804_data {
+	struct regmap *regmap;
+	/* Protects the regcache state update */
+	struct mutex lock;
+	struct gpio_desc *gpios[SGM3804_RAIL_COUNT];
+};
+
+static const struct linear_range sgm3804_voltages[] = {
+	REGULATOR_LINEAR_RANGE(2400000, 0x20, 0x2f, 100000),
+	REGULATOR_LINEAR_RANGE(4000000, 0x00, 0x17, 100000),
+};
+
+/*
+ * The cache is populated with those hardware default values
+ * so the regmap_update_bits operation will use the cached
+ * value to build a new register value and write it when GPIOs
+ * are enabled.
+ */
+static const struct reg_default sgm3804_reg_defaults[] = {
+	{ SGM3804_POS_RAIL_VOLTAGE_REG, RAIL_VOLTAGE_INVALID },
+	{ SGM3804_NEG_RAIL_VOLTAGE_REG, RAIL_VOLTAGE_INVALID },
+	{ SGM3804_RAIL_DISCHARGE_REG, RAIL_DISCHARGE_REG_DEFAULT },
+};
+
+/* Registers are only writable */
+static bool sgm3804_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SGM3804_POS_RAIL_VOLTAGE_REG:
+	case SGM3804_NEG_RAIL_VOLTAGE_REG:
+	case SGM3804_RAIL_DISCHARGE_REG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/*
+ * Since all registers are only writeable, regmap will only read from the cache data.
+ */
+static bool sgm3804_readable_reg(struct device *dev, unsigned int reg)
+{
+	return false;
+}
+
+static const struct regmap_config sgm3804_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x03,
+	.writeable_reg = sgm3804_writeable_reg,
+	.readable_reg = sgm3804_readable_reg,
+	.cache_type = REGCACHE_MAPLE,
+	.reg_defaults = sgm3804_reg_defaults,
+	.num_reg_defaults = ARRAY_SIZE(sgm3804_reg_defaults),
+};
+
+static int sgm3804_sync_regcache_state(struct sgm3804_data *ctx)
+{
+	guard(mutex)(&ctx->lock);
+
+	/* If both GPIOs are down, IC is powered down and I2C writes will fail */
+	if (!gpiod_get_value_cansleep(ctx->gpios[SGM3804_POS_RAIL]) &&
+	    !gpiod_get_value_cansleep(ctx->gpios[SGM3804_NEG_RAIL])) {
+		regcache_cache_only(ctx->regmap, true);
+		regcache_mark_dirty(ctx->regmap);
+	} else {
+		int ret;
+
+		/* At least a GPIO is up, we can write registers */
+		regcache_cache_only(ctx->regmap, false);
+		ret = regcache_sync(ctx->regmap);
+		if (ret) {
+			regcache_cache_only(ctx->regmap, true);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int sgm3804_get_voltage_sel(struct regulator_dev *rdev)
+{
+	int ret;
+
+	ret = regulator_get_voltage_sel_regmap(rdev);
+	if (ret < 0)
+		return ret;
+
+	/* Force setting a voltage on probe */
+	if (ret == RAIL_VOLTAGE_INVALID)
+		return -ENOTRECOVERABLE;
+
+	return ret;
+}
+
+static int sgm3804_enable(struct regulator_dev *rdev)
+{
+	struct sgm3804_data *ctx = rdev->reg_data;
+	int ret;
+
+	ret = gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 1);
+	if (ret)
+		return ret;
+
+	ret = sgm3804_sync_regcache_state(ctx);
+	if (ret)
+		goto err;
+
+	return 0;
+
+err:
+	gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 0);
+	return ret;
+}
+
+static int sgm3804_disable(struct regulator_dev *rdev)
+{
+	struct sgm3804_data *ctx = rdev->reg_data;
+	int ret;
+
+	ret = gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 0);
+	if (ret)
+		return ret;
+
+	return sgm3804_sync_regcache_state(ctx);
+}
+
+static int sgm3804_is_enabled(struct regulator_dev *rdev)
+{
+	struct sgm3804_data *ctx = rdev->reg_data;
+
+	return gpiod_get_value_cansleep(ctx->gpios[rdev_get_id(rdev)]);
+}
+
+static const struct regulator_ops sgm3804_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 = sgm3804_get_voltage_sel,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
+	.enable = sgm3804_enable,
+	.disable = sgm3804_disable,
+	.is_enabled = sgm3804_is_enabled,
+};
+
+static const struct regulator_desc sgm3804_regulator_desc[] = {
+	/* Positive Output */
+	{
+		.name = "pos",
+		.of_match = "pos",
+		.supply_name = "vin",
+		.id = SGM3804_POS_RAIL,
+		.ops = &sgm3804_ops,
+		.type = REGULATOR_VOLTAGE,
+		.linear_ranges = sgm3804_voltages,
+		.n_linear_ranges = ARRAY_SIZE(sgm3804_voltages),
+		.n_voltages = SGM3804_VOLTAGES_MAX_SELECTOR + 1,
+		.vsel_reg = SGM3804_POS_RAIL_VOLTAGE_REG,
+		.vsel_mask = RAIL_VOLTAGE_MASK,
+		.active_discharge_on = POS_RAIL_DISCHARGE_EN,
+		.active_discharge_mask = POS_RAIL_DISCHARGE_EN,
+		.active_discharge_reg = SGM3804_RAIL_DISCHARGE_REG,
+		.enable_time = 40000,
+		.owner = THIS_MODULE,
+	},
+	/* Negative Output */
+	{
+		.name = "neg",
+		.of_match = "neg",
+		.supply_name = "vin",
+		.id = SGM3804_NEG_RAIL,
+		.ops = &sgm3804_ops,
+		.type = REGULATOR_VOLTAGE,
+		.linear_ranges = sgm3804_voltages,
+		.n_linear_ranges = ARRAY_SIZE(sgm3804_voltages),
+		.n_voltages = SGM3804_VOLTAGES_MAX_SELECTOR + 1,
+		.vsel_reg = SGM3804_NEG_RAIL_VOLTAGE_REG,
+		.vsel_mask = RAIL_VOLTAGE_MASK,
+		.active_discharge_on = NEG_RAIL_DISCHARGE_EN,
+		.active_discharge_mask = NEG_RAIL_DISCHARGE_EN,
+		.active_discharge_reg = SGM3804_RAIL_DISCHARGE_REG,
+		.enable_time = 40000,
+		.owner = THIS_MODULE,
+	},
+};
+
+static int sgm3804_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev;
+	struct sgm3804_data *ctx;
+	int ret, i;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	mutex_init(&ctx->lock);
+
+	ctx->regmap = devm_regmap_init_i2c(i2c, &sgm3804_regmap_config);
+	if (IS_ERR(ctx->regmap))
+		return dev_err_probe(dev, PTR_ERR(ctx->regmap),
+				     "failed to init regmap\n");
+
+	/* Get enable GPIOs */
+	for (i = 0; i < ARRAY_SIZE(sgm3804_regulator_desc); i++) {
+		const struct regulator_desc *reg = &sgm3804_regulator_desc[i];
+		struct fwnode_handle *child;
+
+		child = device_get_named_child_node(dev, reg->of_match);
+		if (!child) {
+			dev_err(dev, "missing child '%s'\n", reg->of_match);
+			return -EINVAL;
+		}
+
+		ctx->gpios[i] = devm_fwnode_gpiod_get(dev, child, "enable",
+						      GPIOD_ASIS, reg->name);
+		fwnode_handle_put(child);
+		if (IS_ERR(ctx->gpios[i]))
+			return dev_err_probe(dev, PTR_ERR(ctx->gpios[i]),
+					     "failed to get '%s' enable GPIO\n",
+					     reg->name);
+	}
+
+	ret = sgm3804_sync_regcache_state(ctx);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(sgm3804_regulator_desc); i++) {
+		struct regulator_config config = { };
+		struct regulator_dev *rdev;
+
+		config.dev = dev;
+		config.regmap = ctx->regmap;
+		config.of_node = dev_of_node(dev);
+		config.driver_data = ctx;
+		rdev = devm_regulator_register(dev, &sgm3804_regulator_desc[i],
+					       &config);
+		if (IS_ERR(rdev))
+			return dev_err_probe(dev, PTR_ERR(rdev),
+					     "failed to register regulator %d\n", i);
+	}
+
+	return 0;
+}
+
+static const struct i2c_device_id sgm3804_id[] = {
+	{ "sgm3804" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sgm3804_id);
+
+static const struct of_device_id sgm3804_of_match[] = {
+	{ .compatible = "sgmicro,sgm3804" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sgm3804_of_match);
+
+static struct i2c_driver sgm3804_regulator_driver = {
+	.driver = {
+		.name = "sgm3804",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = sgm3804_of_match,
+	},
+	.probe = sgm3804_probe,
+	.id_table = sgm3804_id,
+};
+
+module_i2c_driver(sgm3804_regulator_driver);
+
+MODULE_DESCRIPTION("SGMicro SGM3804 regulator Driver");
+MODULE_AUTHOR("Kancy Joe <kancy2333@outlook.com>");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v5 2/2] regulator: add SGM3804 Dual Output driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Enable GPIOs are requested with `GPIOD_ASIS` but never explicitly configured as outputs, preventing the regulator from powering up.
- [High] The driver performs I2C communication immediately after asserting the enable GPIO, bypassing the required IC power-on delay.
- [Medium] Incorrect regmap state handling on `regcache_sync()` failure leads to silent I2C write failures for active rails.
- [Medium] Hardware is powered down before setting the regmap to cache-only mode in `sgm3804_disable()`, creating a race condition.
- [Low] The `ctx->lock` mutex is initialized without a corresponding `mutex_destroy()`, bypassing mutex debugging.
--

commit cd53576c2dc0f1b61e28aacf0d0ccbf347793fe0
Author: KancyJoe <kancy2333@outlook.com>

regulator: add SGM3804 Dual Output driver

Add support for the SG Micro SGM3804 Single Inductor Dual Output
Buck/Boost Converter. It uses a regmap cache because registers are only
writable when the enable GPIOs are active.

> diff --git a/drivers/regulator/sgm3804-regulator.c b/drivers/regulator/sgm3804-regulator.c
> new file mode 100644
> index 0000000000000..c3406cfb73d03
> --- /dev/null
> +++ b/drivers/regulator/sgm3804-regulator.c

[ ... ]

> +static int sgm3804_sync_regcache_state(struct sgm3804_data *ctx)
> +{
> +	guard(mutex)(&ctx->lock);
> +
> +	/* If both GPIOs are down, IC is powered down and I2C writes will fail */
> +	if (!gpiod_get_value_cansleep(ctx->gpios[SGM3804_POS_RAIL]) &&
> +	    !gpiod_get_value_cansleep(ctx->gpios[SGM3804_NEG_RAIL])) {
> +		regcache_cache_only(ctx->regmap, true);
> +		regcache_mark_dirty(ctx->regmap);
> +	} else {
> +		int ret;
> +
> +		/* At least a GPIO is up, we can write registers */
> +		regcache_cache_only(ctx->regmap, false);
> +		ret = regcache_sync(ctx->regmap);
> +		if (ret) {
> +			regcache_cache_only(ctx->regmap, true);

[Severity: Medium]
If one regulator is already successfully enabled, and the user attempts to
enable the second one but the I2C transfer fails, does this permanently stick
the regmap cache in cache-only mode?

Any subsequent voltage or discharge changes to the active rail will only update
the software cache and fail to write to the physical hardware.

> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}

[ ... ]

> +static int sgm3804_enable(struct regulator_dev *rdev)
> +{
> +	struct sgm3804_data *ctx = rdev->reg_data;
> +	int ret;
> +
> +	ret = gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 1);
> +	if (ret)
> +		return ret;
> +
> +	ret = sgm3804_sync_regcache_state(ctx);

[Severity: High]
Does this perform I2C communication immediately after asserting the enable
GPIO, bypassing the required IC power-on delay?

The driver specifies .enable_time = 40000, but because the driver issues
I2C commands synchronously inside the enable callback, the regulator core's
delay hasn't occurred yet.

Will sending I2C commands immediately result in I2C NACK errors?

> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +
> +err:
> +	gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 0);
> +	return ret;
> +}
> +
> +static int sgm3804_disable(struct regulator_dev *rdev)
> +{
> +	struct sgm3804_data *ctx = rdev->reg_data;
> +	int ret;
> +
> +	ret = gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 0);
> +	if (ret)
> +		return ret;
> +
> +	return sgm3804_sync_regcache_state(ctx);

[Severity: Medium]
Is there a race condition here if the hardware is powered down before
setting the regmap to cache-only mode?

If this is the last active rail, the physical IC loses power immediately.
If another thread concurrently accesses the shared regmap to set the voltage
on the other rail, it could attempt an I2C transaction on the powered-off IC.

> +}

[ ... ]

> +static int sgm3804_probe(struct i2c_client *i2c)
> +{
> +	struct device *dev = &i2c->dev;
> +	struct sgm3804_data *ctx;
> +	int ret, i;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mutex_init(&ctx->lock);

[Severity: Low]
Is this missing a corresponding mutex_destroy()?

Since ctx is allocated with devm_kzalloc(), the memory will be freed without
destroying the mutex, which bypasses the kernel's CONFIG_DEBUG_MUTEXES
infrastructure.

Would it be better to use devm_mutex_init() instead?

> +
> +	ctx->regmap = devm_regmap_init_i2c(i2c, &sgm3804_regmap_config);
> +	if (IS_ERR(ctx->regmap))
> +		return dev_err_probe(dev, PTR_ERR(ctx->regmap),
> +				     "failed to init regmap\n");
> +
> +	/* Get enable GPIOs */
> +	for (i = 0; i < ARRAY_SIZE(sgm3804_regulator_desc); i++) {
> +		const struct regulator_desc *reg = &sgm3804_regulator_desc[i];
> +		struct fwnode_handle *child;
> +
> +		child = device_get_named_child_node(dev, reg->of_match);
> +		if (!child) {
> +			dev_err(dev, "missing child '%s'\n", reg->of_match);
> +			return -EINVAL;
> +		}
> +
> +		ctx->gpios[i] = devm_fwnode_gpiod_get(dev, child, "enable",
> +						      GPIOD_ASIS, reg->name);

[Severity: High]
Does this prevent the regulator from powering up?

The GPIOD_ASIS flag leaves the pin's hardware direction unconfigured.
If the bootloader left the pin as an input, calling gpiod_set_value_cansleep()
in sgm3804_enable() will fail and return -EPERM because the pin is not
configured as an output.

Could the GPIO direction be explicitly configured as an output instead?

> +		fwnode_handle_put(child);
> +		if (IS_ERR(ctx->gpios[i]))
> +			return dev_err_probe(dev, PTR_ERR(ctx->gpios[i]),
> +					     "failed to get '%s' enable GPIO\n",
> +					     reg->name);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-topic-sm8650-ayaneo-pocket-s2-sgm3804-v5-0-bd6b1c300ecc@linaro.org?part=2

Re: [PATCH v5 0/2] regulator: add support for SGM3804 Dual Output driver

[Mark Brown]

On Fri, 22 May 2026 15:09:11 +0200, Neil Armstrong wrote:
> regulator: add support for SGM3804 Dual Output driver
> 
> Add support for the SG Micro SGM3804 Single Inductor Dual Output
> Buck/Boost Converter used to power LCD panels a provide positive
> and negative power rails with configurable voltage and active
> discharge function for each output.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-7.2

Thanks!

[1/2] regulator: dt-bindings: document the SGM3804 Dual Output regulator
      https://git.kernel.org/broonie/regulator/c/8480ea35d715
[2/2] regulator: add SGM3804 Dual Output driver
      https://git.kernel.org/broonie/regulator/c/0c47e1a8cf5d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark