[PATCH v7 0/2] regulator: add the Will Semiconductor WL2868C camera PMIC

[PATCH v7 0/2] regulator: add the Will Semiconductor WL2868C camera PMIC

[Qianhui Xu]

*** AI-assistance disclosure ***
This cover letter and the two patches below were prepared with the help of
an LLM coding assistant.  The author reviewed every line, ran every gate
quoted in this letter himself, and takes full responsibility for the
content.

This is a two-patch series: the binding and the driver for the WL2868C, a
seven-LDO I2C camera PMIC.  It supersedes the v6 posting
(<20261104032648.209747-1-xiaqinghuan@mail.ustc.edu.cn>) in full, and it is
deliberately smaller: the board devicetree patches of v6 (4/5 and 5/5) are
not part of it.  The reason is in "Board devicetree held back" below.

What changed, and why
=====================

Two reviews were waiting on this series: Krzysztof Kozlowski's
changes-requested on 5/5 and 3/5, and sashiko's three Medium findings on
the driver and two on the board devicetree.

Krzysztof, on 5/5: "I do not understand why this is a separate patch.  You
add new hardware, so the commit is 'add new hardware'.  One logical
action, one commit."  Taken -- and applied further than asked: the whole
board enablement (compatible in arm/qcom.yaml, the board DTS, and the
WL2868C regulators node) leaves this series, because it is not the same
logical action as "add the driver for this chip", and because the board
DTS cannot be sent on its own yet (see below).  The 3/5 commit message
objection ("Board cannot be shipped with such compatible ... 'Add Lenovo
Legion Y700 Gen4, an Qualcomm SM8750-based tablet'") is moot here for the
same reason; when the board series goes out, the compatible and the
message will be exactly that.

Board devicetree held back
==========================

v6 4/5 was a board skeleton: board clocks, reserved GPIOs and
chassis-type, with SPMI PMICs, console and storage explicitly deferred.
sashiko called it an unverified skeleton, and the v6 5/5 regulators node
then referenced VIN12/VIN34/VIN5/VIN7 from SPMI rails that do not exist in
the tree yet, so six LDOs ended up with dummy parents.  Krzysztof and
Konrad have both said the same thing in other words: a board is not
shipped as a skeleton.

So the board devicetree is not sent here.  It will go out with the SM8750
PMIC description and the camera consumers (CCI plus the two sensors) that
give it meaning, in the board series that is already in progress.  The
v6 4/5 + 5/5 content is kept, merged into one patch, in
held-back/ of this series directory, so nothing is lost; it needs the
PMIC rails described before it can be sent.

Response to sashiko on the driver (three Medium findings)
========================================================

 - "chip->enabled is not initialized, so a boot-enabled LDO bit is
   cleared by the next write": fixed, the tracked set is seeded from
   WL2868C_LDO_ENABLE during probe, so an output the firmware left on is
   carried over by enable() and disable() instead of being dropped.

 - "vsys-supply is never requested or enabled": fixed, probe enables it
   with devm_regulator_get_enable() before reading the device ID.  This is
   not cosmetic: the I2C interface is powered from the higher of VIN7 and
   VSYS, so without it the device does not answer on the bus at all.

 - "disable() clears the tracked bit before writing the register, so a
   failed write desynchronises the driver from the core": fixed, the new
   bit set is committed only after the register write succeeded.  A failed
   write leaves the output on and the core's use count unchanged, which is
   the only self-consistent state.

Response to sashiko on the board patches (two Medium findings)
==============================================================

Both are answered by holding the board patches back, as described above:
the skeleton devicetree is no longer part of this series, and the missing
VIN parents go away with it (the regulators node travels with the PMIC
description that provides them).

Changes relative to v6
======================

 - 1/2: unchanged content, moved MAINTAINERS into this patch.
 - 2/2: driver only; three state-consistency fixes above, and the commit
   message now describes the seeded state and the VSYS requirement.
 - dropped: v6 4/5 and 5/5 (board enablement), kept in held-back/.
 - net: 5 files, 369 insertions(+), 0 deletions(-).

Testing
=======

All re-run on this exact series:

 - checkpatch.pl --strict --patch - (mbox on stdin): 0 errors, 0 warnings
   for 1/2 (117 lines); 0 errors for 2/2 (270 lines) with one
   FILE_PATH_CHANGES note ("does MAINTAINERS need updating?"), which is
   checkpatch's per-patch heuristic -- MAINTAINERS is updated by 1/2 of
   this same series, so the note does not apply.
 - dt-doc-validate on willsemi,wl2868c.yaml: clean, exit 0.
 - make W=1 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
   drivers/regulator/ with CONFIG_REGULATOR_WL2868C=m: 0 warnings, 0
   errors.
 - git am of 1/2..2/2 on f66469d9905b (torvalds master, 2026-11-06): applies
   cleanly, resulting tree is identical to the series tip.
 - scripts/check-series-selfcontained.sh: PASS, both patches replayed in
   order.

Board behaviour: this driver has been running on the tablet (Lenovo Legion
Y700 Gen4, SM8750) for several weeks as a module, with all seven LDOs
registered and the two camera sensors powered from LDO1/LDO2/LDO4/LDO7;
the LDO voltages match the values the stock firmware programs.  The
regulator node itself is part of the held-back board patch, so that
evidence is about the driver, not about an in-tree user.

Links
=====

v6: https://lore.kernel.org/all/20261104032648.209747-1-xiaqinghuan@mail.ustc.edu.cn/
Krzysztof on 5/5: <20261104-elfish-booby-of-awe-27d2b6@quoll>
Krzysztof on 3/5: <20261104-offbeat-cerulean-degu-cfec1b@quoll>
sashiko on 2/5: <sashiko-outbox-196615@kernel.org>
sashiko on 4/5: <sashiko-outbox-196620@kernel.org>
sashiko on 5/5: <sashiko-outbox-196621@kernel.org>

Qianhui Xu (2):
  regulator: dt-bindings: willsemi,wl2868c: add the WL2868C camera PMIC
  regulator: willsemi,wl2868c: add the WL2868C camera PMIC driver

 .../bindings/regulator/willsemi,wl2868c.yaml  | 104 ++++++++
 MAINTAINERS                                   |   7 +
 drivers/regulator/Kconfig                     |  11 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/wl2868c-regulator.c         | 246 ++++++++++++++++++
 5 files changed, 369 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
 create mode 100644 drivers/regulator/wl2868c-regulator.c


base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1

[PATCH 1/2] regulator: dt-bindings: willsemi,wl2868c: add the WL2868C camera PMIC

[Qianhui Xu]

The WL2868C is an I2C-controlled camera power management IC from Will
Semiconductor. It provides seven programmable LDOs: LDO1 and LDO2 are fed
from a shared VIN12 input and cover 0.496 V to 1.512 V, LDO3 and LDO4 share
VIN34, and LDO5 to LDO7 each have their own input, covering 1.504 V to
3.544 V. All outputs are programmable in 8 mV steps and are gated by
per-output enable bits with a chip-wide SYS_EN master. The chip also
exposes an optional active-low RST_N input.

VIN7 and VSYS are both inputs of the internal control logic, which is
powered from the higher of the two voltages, so a device tree has to
describe at least one of them. The remaining inputs only feed LDOs, so
they stay optional: an input can be left unconnected when the outputs it
feeds are unused on a given board.

Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
 .../bindings/regulator/willsemi,wl2868c.yaml  | 104 ++++++++++++++++++
 MAINTAINERS                                   |   7 ++
 2 files changed, 111 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml

diff --git a/Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml b/Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
new file mode 100644
index 0000000000..df7990d0d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/willsemi,wl2868c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Will Semiconductor WL2868C camera power management IC
+
+maintainers:
+  - Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
+
+description:
+  The WL2868C is a camera power management IC with seven programmable LDOs.
+  LDO1 and LDO2 share the VIN12 input and cover 0.496 V to 1.512 V; LDO3 and
+  LDO4 share VIN34, and LDO5 to LDO7 have one input each, covering 1.504 V to
+  3.544 V. All outputs are programmable in 8 mV steps.
+
+  VIN7 and VSYS feed the internal control logic, which is powered from the
+  higher of the two, so at least one of them has to be connected. The other
+  inputs only feed LDOs, so an input may be left unconnected on a board
+  that does not use the outputs it feeds, which is why they are not
+  mandatory.
+
+properties:
+  compatible:
+    const: willsemi,wl2868c
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+    description: Active-low RST_N input.
+
+  vin12-supply:
+    description: Input supply for LDO1 and LDO2.
+
+  vin34-supply:
+    description: Input supply for LDO3 and LDO4.
+
+  vin5-supply:
+    description: Input supply for LDO5.
+
+  vin6-supply:
+    description: Input supply for LDO6.
+
+  vin7-supply:
+    description:
+      Input supply for LDO7 and one of the two supplies of the internal
+      control logic.
+
+  vsys-supply:
+    description:
+      System supply, the other supply of the internal control logic. The
+      control logic is powered from the higher of VIN7 and VSYS.
+
+  regulators:
+    type: object
+    additionalProperties: false
+
+    patternProperties:
+      "^ldo[1-7]$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+anyOf:
+  - required:
+      - vin7-supply
+  - required:
+      - vsys-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@2f {
+            compatible = "willsemi,wl2868c";
+            reg = <0x2f>;
+            vsys-supply = <&vsys_reg>;
+
+            regulators {
+                ldo1 {
+                    regulator-max-microvolt = <1128000>;
+                    regulator-min-microvolt = <1128000>;
+                };
+            };
+        };
+    };
+
+    vsys_reg: regulator-vsys {
+        compatible = "regulator-fixed";
+        regulator-name = "vsys";
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index fe23913b1e..08cfa3db0e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -30180,6 +30180,13 @@ L:	linux-input@vger.kernel.org
 S:	Maintained
 F:	drivers/hid/hid-wiimote*
 
+WILLSEMI WL2868C CAMERA PMIC
+M:	Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
+F:	drivers/regulator/wl2868c-regulator.c
+
 WILOCITY WIL6210 WIRELESS DRIVER
 L:	linux-wireless@vger.kernel.org
 S:	Orphan

[PATCH 2/2] regulator: willsemi,wl2868c: add the WL2868C camera PMIC driver

[Qianhui Xu]

The WL2868C is an I2C camera power management IC with seven LDOs.
LDO1 and LDO2 are fed from VIN12 and cover 0.496 V to 1.512 V with a
7-bit voltage selector; LDO3 and LDO4 share VIN34, and LDO5 to LDO7
have one input each, covering 1.504 V to 3.544 V with an 8-bit
selector. All outputs step by 8 mV.

Every output is gated by a per-LDO bit in the enable register, and the
whole chip is additionally gated by the SYS_EN bit in the same
register. The driver sets SYS_EN together with the LDO bit on enable
and requires both to read back before reporting the regulator as
enabled, because no output comes up while SYS_EN is clear. An
optional active-low RST_N GPIO is released during probe, followed by
a 1 ms delay for the device to come up, and is asserted again when
the driver is unbound.

The enable register is written as a whole rather than updated bit by
bit: the per-LDO bits can be preset by the firmware while SYS_EN is
clear, and setting SYS_EN together with a single LDO bit would then
switch the preset outputs on as well. The driver therefore tracks the
set of LDOs enabled by consumers and writes exactly those bits, so
enabling one output cannot power another one. That set is seeded from
the register during probe, so an output the firmware left on is not
cleared by the first enable() of a different one, and disable()
commits the tracked state only after the register write went through,
so a failed write cannot make a later enable() switch off an output
that the core still believes is on.

The I2C interface is powered from the higher of VIN7 and VSYS, so the
"vsys" supply is enabled for as long as the driver is bound, before
the device ID is read; without it the device does not answer on the
bus.

Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
 drivers/regulator/Kconfig             |  11 ++
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/wl2868c-regulator.c | 246 ++++++++++++++++++++++++++
 3 files changed, 258 insertions(+)
 create mode 100644 drivers/regulator/wl2868c-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index be6b72daa7..f0a604afe5 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1877,6 +1877,17 @@ config REGULATOR_VQMMC_IPQ4019
 	  This driver provides support for the VQMMC LDO I/0
 	  voltage regulator of the IPQ4019 SD/EMMC controller.
 
+config REGULATOR_WL2868C
+	tristate "Will Semiconductor WL2868C regulator"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Support the seven LDO outputs of the Will Semiconductor WL2868C
+	  camera power management IC.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called wl2868c-regulator.
+
 config REGULATOR_WM831X
 	tristate "Wolfson Microelectronics WM831x PMIC regulators"
 	depends on MFD_WM831X
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6aabcc5438..37ffa93a9c 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -214,6 +214,7 @@ obj-$(CONFIG_REGULATOR_RZG2L_VBCTRL) += renesas-usb-vbus-regulator.o
 obj-$(CONFIG_REGULATOR_VCTRL) += vctrl-regulator.o
 obj-$(CONFIG_REGULATOR_VEXPRESS) += vexpress-regulator.o
 obj-$(CONFIG_REGULATOR_VQMMC_IPQ4019) += vqmmc-ipq4019-regulator.o
+obj-$(CONFIG_REGULATOR_WL2868C) += wl2868c-regulator.o
 obj-$(CONFIG_REGULATOR_WM831X) += wm831x-dcdc.o
 obj-$(CONFIG_REGULATOR_WM831X) += wm831x-isink.o
 obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
diff --git a/drivers/regulator/wl2868c-regulator.c b/drivers/regulator/wl2868c-regulator.c
new file mode 100644
index 0000000000..64baeeacf3
--- /dev/null
+++ b/drivers/regulator/wl2868c-regulator.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Will Semiconductor WL2868C seven-channel camera regulator driver.
+ *
+ * Copyright (C) 2026 Qianhui Xu
+ */
+
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+#define WL2868C_DEVICE_ID		0x00
+#define WL2868C_DEVICE_ID_VALUE		0x82
+#define WL2868C_LDO_ENABLE		0x0e
+#define WL2868C_SYS_ENABLE		BIT(7)
+#define WL2868C_LDO_ENABLE_MASK		GENMASK(6, 0)
+
+struct wl2868c {
+	struct regmap *regmap;
+	/* LDO enable bits owned by consumers, in WL2868C_LDO_ENABLE layout. */
+	unsigned int enabled;
+	/* Protects enabled. */
+	struct mutex lock;
+};
+
+static int wl2868c_enable(struct regulator_dev *rdev)
+{
+	struct wl2868c *chip = rdev_get_drvdata(rdev);
+	unsigned int bits = rdev->desc->enable_mask;
+	unsigned int ldos;
+	int ret;
+
+	guard(mutex)(&chip->lock);
+
+	/*
+	 * The enable register is written as a whole on purpose. SYS_EN gates
+	 * every output of the chip, and the per-LDO bits may be preset by the
+	 * firmware while SYS_EN is still clear: setting only SYS_EN and our own
+	 * bit would then bring the preset LDOs up as well. Writing the register
+	 * with the bits of the LDOs consumers have enabled, and only those,
+	 * keeps the other outputs off. chip->enabled was seeded from the
+	 * register in probe(), so an output the firmware left on is carried
+	 * over instead of being cleared by the first enable.
+	 */
+	ldos = (chip->enabled | bits) & WL2868C_LDO_ENABLE_MASK;
+	ret = regmap_write(chip->regmap, WL2868C_LDO_ENABLE,
+			   WL2868C_SYS_ENABLE | ldos);
+	if (ret)
+		return ret;
+
+	chip->enabled = ldos;
+
+	return 0;
+}
+
+static int wl2868c_disable(struct regulator_dev *rdev)
+{
+	struct wl2868c *chip = rdev_get_drvdata(rdev);
+	unsigned int bits = rdev->desc->enable_mask;
+	unsigned int ldos;
+	int ret;
+
+	guard(mutex)(&chip->lock);
+
+	/*
+	 * SYS_EN stays set while at least one output is enabled; the per-LDO
+	 * bits of the other outputs stay clear so that setting it cannot
+	 * switch anything else on.
+	 */
+	ldos = (chip->enabled & ~bits) & WL2868C_LDO_ENABLE_MASK;
+
+	/*
+	 * The tracked state is committed only once the register write went
+	 * through: a failed write leaves the output on, and the regulator core
+	 * keeps its own use count unchanged, so dropping the bit here would
+	 * make the next enable() of another output switch this one off.
+	 */
+	ret = regmap_write(chip->regmap, WL2868C_LDO_ENABLE,
+			   ldos ? WL2868C_SYS_ENABLE | ldos : 0);
+	if (ret)
+		return ret;
+
+	chip->enabled = ldos;
+
+	return 0;
+}
+
+static int wl2868c_is_enabled(struct regulator_dev *rdev)
+{
+	unsigned int mask = WL2868C_SYS_ENABLE | rdev->desc->enable_mask;
+	unsigned int value;
+	int ret;
+
+	ret = regmap_read(rdev->regmap, WL2868C_LDO_ENABLE, &value);
+	if (ret)
+		return ret;
+
+	return (value & mask) == mask;
+}
+
+static const struct regulator_ops wl2868c_ops = {
+	.list_voltage = regulator_list_voltage_linear,
+	.map_voltage = regulator_map_voltage_linear,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.enable = wl2868c_enable,
+	.disable = wl2868c_disable,
+	.is_enabled = wl2868c_is_enabled,
+};
+
+/*
+ * LDO1 and LDO2 have a 7-bit voltage selector, the other five have an 8-bit
+ * one; all of them step by 8 mV.
+ */
+#define WL2868C_LDO(_num, _supply, _min, _n_voltages, _vsel_reg, _mask, _en) { \
+	.name = "ldo" #_num,				\
+	.of_match = "ldo" #_num,			\
+	.regulators_node = "regulators",		\
+	.id = (_num) - 1,				\
+	.type = REGULATOR_VOLTAGE,			\
+	.owner = THIS_MODULE,				\
+	.ops = &wl2868c_ops,				\
+	.supply_name = _supply,				\
+	.min_uV = _min,					\
+	.uV_step = 8000,				\
+	.n_voltages = _n_voltages,			\
+	.vsel_reg = _vsel_reg,				\
+	.vsel_mask = _mask,				\
+	.enable_reg = WL2868C_LDO_ENABLE,		\
+	.enable_mask = _en,				\
+	.enable_time = 150,				\
+}
+
+static const struct regulator_desc wl2868c_regulators[] = {
+	WL2868C_LDO(1, "vin12", 496000, 128, 0x03, 0x7f, BIT(0)),
+	WL2868C_LDO(2, "vin12", 496000, 128, 0x04, 0x7f, BIT(1)),
+	WL2868C_LDO(3, "vin34", 1504000, 256, 0x05, 0xff, BIT(2)),
+	WL2868C_LDO(4, "vin34", 1504000, 256, 0x06, 0xff, BIT(3)),
+	WL2868C_LDO(5, "vin5", 1504000, 256, 0x07, 0xff, BIT(4)),
+	WL2868C_LDO(6, "vin6", 1504000, 256, 0x08, 0xff, BIT(5)),
+	WL2868C_LDO(7, "vin7", 1504000, 256, 0x09, 0xff, BIT(6)),
+};
+
+static const struct regmap_config wl2868c_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x25,
+};
+
+static void wl2868c_reset(void *data)
+{
+	gpiod_set_value_cansleep(data, 1);
+}
+
+static int wl2868c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct regulator_config config = { .dev = dev };
+	struct regulator_dev *rdev;
+	struct gpio_desc *reset;
+	struct wl2868c *chip;
+	unsigned int id, value;
+	int ret, i;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	mutex_init(&chip->lock);
+	config.driver_data = chip;
+
+	reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(reset))
+		return dev_err_probe(dev, PTR_ERR(reset), "Failed to get reset GPIO\n");
+
+	if (reset) {
+		ret = devm_add_action_or_reset(dev, wl2868c_reset, reset);
+		if (ret)
+			return ret;
+		fsleep(1000);
+	}
+
+	chip->regmap = devm_regmap_init_i2c(client, &wl2868c_regmap_config);
+	config.regmap = chip->regmap;
+	if (IS_ERR(config.regmap))
+		return dev_err_probe(dev, PTR_ERR(config.regmap),
+				     "Failed to create regmap\n");
+
+	/*
+	 * The I2C interface is powered from the higher of VIN7 and VSYS, so the
+	 * system supply has to be up before the device can answer at all. It
+	 * stays enabled for as long as the driver is bound.
+	 */
+	ret = devm_regulator_get_enable(dev, "vsys");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable the system supply\n");
+
+	ret = regmap_read(config.regmap, WL2868C_DEVICE_ID, &id);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read device ID\n");
+	if (id != WL2868C_DEVICE_ID_VALUE)
+		return dev_err_probe(dev, -ENODEV, "Unexpected device ID: %#x\n", id);
+
+	/*
+	 * Seed the tracked set with the outputs the firmware left enabled, so
+	 * that the first enable() or disable() of any output preserves them.
+	 * The regulator core reads the state back through is_enabled(), which
+	 * does not look at this set.
+	 */
+	ret = regmap_read(config.regmap, WL2868C_LDO_ENABLE, &value);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read the enable register\n");
+
+	chip->enabled = value & WL2868C_LDO_ENABLE_MASK;
+
+	for (i = 0; i < ARRAY_SIZE(wl2868c_regulators); i++) {
+		rdev = devm_regulator_register(dev, &wl2868c_regulators[i], &config);
+		if (IS_ERR(rdev))
+			return dev_err_probe(dev, PTR_ERR(rdev),
+					     "Failed to register LDO%d\n", i + 1);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id wl2868c_of_match[] = {
+	{ .compatible = "willsemi,wl2868c" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, wl2868c_of_match);
+
+static struct i2c_driver wl2868c_driver = {
+	.driver = {
+		.name = "wl2868c",
+		.of_match_table = wl2868c_of_match,
+	},
+	.probe = wl2868c_probe,
+};
+module_i2c_driver(wl2868c_driver);
+
+MODULE_DESCRIPTION("Will Semiconductor WL2868C regulator driver");
+MODULE_LICENSE("GPL");

Re: [PATCH 2/2] regulator: willsemi,wl2868c: add the WL2868C camera PMIC driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] regulator: wl2868c: check SYS_EN before seeding enabled in
  wl2868c_probe()
- [Medium] regulator: wl2868c: enable vsys before deasserting reset in
  wl2868c_probe()
- [Medium] regulator: wl2868c: handle vin7-supply for control logic in
  wl2868c_probe()
--

commit 3a2a7e94fa21fa44fb2fb53973938985aaea5820
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

regulator: willsemi,wl2868c: add the WL2868C camera PMIC driver

This commit adds a regulator driver for the Will Semiconductor WL2868C
seven-channel I2C camera PMIC.

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

[ ... ]

> +static int wl2868c_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct regulator_config config = { .dev = dev };
> +	struct regulator_dev *rdev;
> +	struct gpio_desc *reset;
> +	struct wl2868c *chip;
> +	unsigned int id, value;
> +	int ret, i;
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	mutex_init(&chip->lock);
> +	config.driver_data = chip;
> +
> +	reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(reset))
> +		return dev_err_probe(dev, PTR_ERR(reset), "Failed to get reset GPIO\n");
> +
> +	if (reset) {
> +		ret = devm_add_action_or_reset(dev, wl2868c_reset, reset);
> +		if (ret)
> +			return ret;
> +		fsleep(1000);
> +	}

[Severity: Medium]
Should the supply be enabled before releasing reset and waiting 1 ms?

devm_regulator_get_enable(dev, "vsys") is called after this block, right
before reading the device id register.  If vsys was off before probe,
reset is deasserted and the 1 ms fsleep(1000) delay elapses while the chip
is still unpowered, and the first register read runs immediately after
vsys turns on with no startup delay.

Registering wl2868c_reset() before devm_regulator_get_enable() also means
devres teardown disables vsys before asserting reset on unbind.

> +
> +	chip->regmap = devm_regmap_init_i2c(client, &wl2868c_regmap_config);
> +	config.regmap = chip->regmap;
> +	if (IS_ERR(config.regmap))
> +		return dev_err_probe(dev, PTR_ERR(config.regmap),
> +				     "Failed to create regmap\n");
> +
> +	/*
> +	 * The I2C interface is powered from the higher of VIN7 and VSYS, so the
> +	 * system supply has to be up before the device can answer at all. It
> +	 * stays enabled for as long as the driver is bound.
> +	 */
> +	ret = devm_regulator_get_enable(dev, "vsys");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable the system supply\n");

[Severity: Medium]
What happens on boards that power the control logic from vin7-supply and
omit vsys-supply?

The devicetree binding allows supplying either vin7-supply or vsys-supply:

Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml:
    anyOf:
      - required:
          - vin7-supply
      - required:
          - vsys-supply

When vsys-supply is omitted, devm_regulator_get_enable(dev, "vsys") falls
back to a dummy regulator with a warning and leaves vin7-supply disabled
during the register reads below.  Because vin7 is only attached as the
supply for ldo7, the chip also loses control power whenever ldo7 is off.

> +
> +	ret = regmap_read(config.regmap, WL2868C_DEVICE_ID, &id);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read device ID\n");
> +	if (id != WL2868C_DEVICE_ID_VALUE)
> +		return dev_err_probe(dev, -ENODEV, "Unexpected device ID: %#x\n", id);
> +
> +	/*
> +	 * Seed the tracked set with the outputs the firmware left enabled, so
> +	 * that the first enable() or disable() of any output preserves them.
> +	 * The regulator core reads the state back through is_enabled(), which
> +	 * does not look at this set.
> +	 */
> +	ret = regmap_read(config.regmap, WL2868C_LDO_ENABLE, &value);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read the enable register\n");
> +
> +	chip->enabled = value & WL2868C_LDO_ENABLE_MASK;

[Severity: Medium]
Does seeding chip->enabled without checking WL2868C_SYS_ENABLE turn on
preset outputs when the first regulator is enabled?

As described in wl2868c_enable(), firmware may preset per-output bits in
WL2868C_LDO_ENABLE while WL2868C_SYS_ENABLE is still clear, and
wl2868c_is_enabled() reports those outputs as disabled.  Copying those
bits into chip->enabled when WL2868C_SYS_ENABLE is clear causes the first
wl2868c_enable() call on any output to write WL2868C_SYS_ENABLE | ldos and
power on all preset outputs as well.  Seeding chip->enabled only when
value & WL2868C_SYS_ENABLE is set avoids this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106113005.970822-1-xiaqinghuan@mail.ustc.edu.cn?part=2

Re: [PATCH 1/2] regulator: dt-bindings: willsemi,wl2868c: add the WL2868C camera PMIC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] regulator: dt-bindings: willsemi,wl2868c: unrelated node in
  binding example
- [Low] MAINTAINERS: nonexistent wl2868c-regulator.c path in dt-bindings
  patch
--

commit 445721dc45dd58c5fcd063ef3e6a8215d244f0d7
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

regulator: dt-bindings: willsemi,wl2868c: add the WL2868C camera PMIC

This commit adds the Device Tree binding schema and maintainer entry for
the Will Semiconductor wl2868c camera power management chip.

> diff --git a/Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml b/Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
> new file mode 100644
> index 0000000000000..df7990d0d8a4e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
[ ... ]
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic@2f {
> +            compatible = "willsemi,wl2868c";
> +            reg = <0x2f>;
> +            vsys-supply = <&vsys_reg>;
> +
> +            regulators {
> +                ldo1 {
> +                    regulator-max-microvolt = <1128000>;
> +                    regulator-min-microvolt = <1128000>;
> +                };
> +            };
> +        };
> +    };
> +
> +    vsys_reg: regulator-vsys {
> +        compatible = "regulator-fixed";
> +        regulator-name = "vsys";
> +    };
> +...

[Severity: Low]
Should regulator-vsys be dropped from the example?  Per
Documentation/devicetree/bindings/writing-schema.rst, binding examples
should only describe the binding being documented and do not need provider
nodes to resolve phandles such as vsys_reg.  Also, since the example
enables ldo1 and the description states that input supplies may be omitted
only when their outputs are unused, pmic@2f should include vin12-supply.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe23913b1e432..08cfa3db0ecbe 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -30180,6 +30180,13 @@ L:	linux-input@vger.kernel.org
>  S:	Maintained
>  F:	drivers/hid/hid-wiimote*
>  
> +WILLSEMI WL2868C CAMERA PMIC
> +M:	Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
> +L:	linux-kernel@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
> +F:	drivers/regulator/wl2868c-regulator.c

[Severity: Low]
Should the drivers/regulator/wl2868c-regulator.c entry be deferred to
commit "regulator: willsemi,wl2868c: add the WL2868C camera PMIC driver"
where that file is created?  Adding the path here causes
scripts/get_maintainer.pl --self-test=patterns to warn that no file matches
drivers/regulator/wl2868c-regulator.c at this commit.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106113005.970822-1-xiaqinghuan@mail.ustc.edu.cn?part=1