[PATCH v6 0/5] regulator: add the Will Semiconductor WL2868C camera PMIC

[PATCH v6 0/5] regulator: add the Will Semiconductor WL2868C camera PMIC

[Qianhui Xu]

This series was drafted with AI assistance; I reviewed and edited every
line myself, and I am responsible for its content, measurements and
decisions.  The commands behind the verification claims below are listed
in the series README that accompanies this posting.

The WL2868C is the camera PMIC on the Lenovo Legion Tab (TB322FC,
SM8750); its seven LDOs feed the rear camera, the front camera and the
VCM on i2c10.

Changes in v6:

 - Rebased onto current mainline (v7.3-20183-g05441d697f7f, tip of
   master on 2026-11-03).  v5 no longer applied: the board patch anchors
   its arch/arm64/boot/dts/qcom/Makefile hunk on the sm8650-qrd.dtb line,
   and sm8650-valve-deckard.dtb was inserted right there in between, so
   git am stopped on that file (Mark Brown: "This doesn't apply against
   current code, please check and resend").  v6 applies cleanly on the
   new base.

 - The SM8750 CCI patch is gone.  There is an identical CCI series in
   flight from Hangxiang Ma (Konrad: "There's an identical patchset in
   flight, please don't send competing series without a clear reason"),
   so this series no longer competes for those nodes.  The two
   observations sashiko made on the CCI patch (cci2 is not described; the
   sleep states change the pin function to gpio instead of keeping
   cci_i2c_sda/scl) belong to that series as well and are not addressed
   here.

 - The board compatible in arm/qcom.yaml is now a patch of its own
   instead of riding along with another binding (Conor asked for the
   split; checkpatch wants the same).

 - regulator binding: the paragraph that described which board supply
   comes from which LDO is gone from the binding description, it belongs
   to the board patch (Conor).  vsys-supply is added and at least one of
   vin7-supply and vsys-supply is now required, because both feed the
   internal control logic, which is powered from the higher of the two
   (Conor: "the higher of the two powers the internal control logic").
   The other inputs stay optional: they only feed LDOs, so an input may
   be left unconnected on a board that does not use those outputs, which
   the description now says.

 - driver: the enable register is written as a whole instead of doing a
   read-modify-write of SYS_EN plus one LDO bit.  The per-LDO bits can be
   preset by the firmware while SYS_EN is clear, so setting SYS_EN and
   the requested bit would switch the preset outputs on as well
   (sashiko, Medium).  The driver keeps the set of LDOs that consumers
   enabled and writes exactly those bits, so enabling one output cannot
   power another one.

 - The board DTS is split in two: a skeleton patch (compatible, chassis
   type, reserved GPIOs, board clocks) and the WL2868C regulators.  The
   rear sensor and its VCM are gone from this series together with the
   CCI controllers they hang off; they follow with the camera series
   that depends on the CCI nodes.  The board clocks stay: they are the
   XO, sleep and divided CXO clocks that sm8750.dtsi consumes, so every
   SM8750 board file has to provide them (Konrad asked why the board
   repeats what the binding example has; the binding example contains no
   clock, and these four are consumed by the SoC dtsi, as on
   sm8750-mtp.dts).

 - The VSYS input is described as a fixed regulator, as it is the
   always-on system rail.  VIN12, VIN34, VIN5 and VIN7 are fed by SPMI
   PMIC rails that this series does not describe yet, so they are left
   out and follow with the board series that adds those PMICs.

Verification, host-side only: every patch passes checkpatch.pl --strict
with 0 errors (the only two warnings are "MAINTAINERS need updating" for
the two new files, one of them being the driver's own entry added in
2/5); the series replays with plain git am on a pristine checkout of the
base commit, 5/5, and the result is the same tree as the branch it is
generated from; drivers/regulator/ builds with W=1 without warnings;
sm8750-lenovo-elden.dtb builds and dt-validate has nothing to report on
it; dt_binding_check passes for the new binding.

The regulators are still not exercised on hardware: the i2c10 probe, the
regulator registration and the LDO voltages come from our own downstream
board DTS, not from a measured boot of this series.

Qianhui Xu (5):
  regulator: dt-bindings: willsemi,wl2868c: add the WL2868C camera PMIC
  regulator: willsemi,wl2868c: add the WL2868C camera PMIC driver
  dt-bindings: arm: qcom: document the Lenovo Legion Y700 Gen4
  arm64: dts: qcom: sm8750-lenovo-elden: add the Lenovo Legion Y700 Gen4
  arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

 .../devicetree/bindings/arm/qcom.yaml         |   1 +
 .../bindings/regulator/willsemi,wl2868c.yaml  | 104 +++++++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/sm8750-lenovo-elden.dts     | 129 +++++++++++
 drivers/regulator/Kconfig                     |  11 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/wl2868c-regulator.c         | 212 ++++++++++++++++++
 8 files changed, 466 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml
 create mode 100644 arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
 create mode 100644 drivers/regulator/wl2868c-regulator.c

-- 
2.53.0

[PATCH v6 1/5] 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 ++++++++++++++++++
 1 file changed, 104 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";
+    };
+...
-- 
2.53.0

[PATCH v6 2/5] 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 keeps the
set of LDOs enabled by consumers and writes exactly those bits, so
enabling one output cannot power another one.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 93abbb3150..a107ad901c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -30122,6 +30122,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
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..35bcdc3971
--- /dev/null
+++ b/drivers/regulator/wl2868c-regulator.c
@@ -0,0 +1,212 @@
+// 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.
+	 */
+	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 |= bits;
+
+	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;
+
+	guard(mutex)(&chip->lock);
+
+	chip->enabled &= ~bits;
+
+	/*
+	 * 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 & WL2868C_LDO_ENABLE_MASK;
+
+	return regmap_write(chip->regmap, WL2868C_LDO_ENABLE,
+			    ldos ? WL2868C_SYS_ENABLE | ldos : 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;
+	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");
+
+	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);
+
+	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");
-- 
2.53.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] regulator: wl2868c: uninitialized chip->enabled in wl2868c_probe()
- [Medium] regulator: wl2868c: missing vsys-supply enable in wl2868c_probe()
- [Medium] regulator: wl2868c: premature state update in wl2868c_disable()
--

commit 43e6bd60d4f6c3f0ce8478df036a5927be90da6e
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 I2C
camera power management IC supporting its seven LDO outputs.

> diff --git a/drivers/regulator/wl2868c-regulator.c b/drivers/regulator/wl2868c-regulator.c
> new file mode 100644
> index 0000000000000..35bcdc3971eca
> --- /dev/null
> +++ b/drivers/regulator/wl2868c-regulator.c
> @@ -0,0 +1,212 @@

[ ... ]

> +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;
> +
> +	guard(mutex)(&chip->lock);
> +
> +	chip->enabled &= ~bits;
> +
> +	/*
> +	 * 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 & WL2868C_LDO_ENABLE_MASK;
> +
> +	return regmap_write(chip->regmap, WL2868C_LDO_ENABLE,
> +			    ldos ? WL2868C_SYS_ENABLE | ldos : 0);
> +}

[Severity: Medium]
If regmap_write() fails in wl2868c_disable(), does chip->enabled get out
of sync with both the hardware and the regulator core?

In wl2868c_enable(), chip->enabled |= bits is only updated after
regmap_write() succeeds. Here in wl2868c_disable(), chip->enabled &= ~bits
is cleared before calling regmap_write().

When ops->disable() returns a negative error code in _regulator_disable():

drivers/regulator/core.c:_regulator_disable() {
    ...
    ret = _regulator_do_disable(rdev);
    if (ret < 0) {
        rdev_err(rdev, "failed to disable: %pe\n", ERR_PTR(ret));
        _notifier_call_chain(rdev,
                REGULATOR_EVENT_ABORT_DISABLE,
                NULL);
        return ret;
    }
    ...
    rdev->use_count = 0;
}

the regulator core aborts the disable and keeps rdev->use_count at 1 while
the LDO remains enabled in hardware, but chip->enabled has already lost
that LDO's bit. A subsequent wl2868c_enable() or wl2868c_disable() on
another LDO will then overwrite WL2868C_LDO_ENABLE with that bit cleared,
shutting off the still-enabled output.

Should chip->enabled only be updated after regmap_write() succeeds,
matching wl2868c_enable()?

> +
> +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_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 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;
> +	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");
> +
> +	ret = regmap_read(config.regmap, WL2868C_DEVICE_ID, &id);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read device ID\n");

[Severity: Medium]
Does wl2868c_probe() need to request and enable vsys-supply (or the
control logic supply) before accessing the chip over I2C?

Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml defines
vsys-supply and notes that the internal control logic is powered from the
higher of VIN7 and VSYS, requiring at least one of vin7-supply or
vsys-supply.

However, vsys-supply is never requested or enabled by the driver, and
vin7-supply is only registered as the supply_name for LDO7. On systems
where the control logic is powered by a switchable vsys-supply (or
vin7-supply when LDO7 is not enabled), the chip may not be powered when
regmap_read() reads WL2868C_DEVICE_ID in wl2868c_probe(), or
regulator_late_cleanup() may disable an unclaimed vsys-supply while LDO1
through LDO6 are in use.

> +	if (id != WL2868C_DEVICE_ID_VALUE)
> +		return dev_err_probe(dev, -ENODEV, "Unexpected device ID: %#x\n", id);
> +
> +	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);
> +	}

[Severity: Medium]
What happens if WL2868C_SYS_ENABLE and one or more LDO bits are already
set in WL2868C_LDO_ENABLE when wl2868c_probe() runs (for example, when the
optional reset GPIO is not present or was already deasserted by the
bootloader)?

In wl2868c_probe(), chip->enabled starts at 0, whereas
wl2868c_is_enabled() reads WL2868C_LDO_ENABLE directly from the hardware
and returns 1 for any LDO that is already active alongside
WL2868C_SYS_ENABLE.

When a consumer calls regulator_enable() on a boot-enabled LDO,
_regulator_enable() checks _regulator_is_enabled(rdev):

drivers/regulator/core.c:_regulator_enable() {
    ...
    if (rdev->use_count == 0) {
        /* The regulator may on if it's not switchable or left on */
        ret = _regulator_is_enabled(rdev);
        if (ret == -EINVAL || ret == 0) {
            if (!regulator_ops_is_valid(rdev,
                    REGULATOR_CHANGE_STATUS)) {
                ret = -EPERM;
                goto err_consumer_disable;
            }

            ret = _regulator_do_enable(rdev);
            if (ret < 0)
                goto err_consumer_disable;

            _notifier_call_chain(rdev, REGULATOR_EVENT_ENABLE,
                         NULL);
        } else if (ret < 0) {
            ...
        }
    }

    rdev->use_count++;
    ...
}

Because wl2868c_is_enabled() returns 1, _regulator_enable() skips calling
_regulator_do_enable() -> wl2868c_enable() and increments rdev->use_count
to 1 while that LDO's bit remains 0 in chip->enabled.

When any other LDO is subsequently enabled in wl2868c_enable() or disabled
in wl2868c_disable() (including regulator_late_cleanup() turning off an
unused LDO), the driver writes WL2868C_LDO_ENABLE from chip->enabled and
clears the boot-enabled LDO's bit while its consumer is actively using it.
Similarly, if multiple LDOs are already enabled at boot and marked
regulator-always-on or regulator-boot-on, registering the first one calls
wl2868c_enable() with chip->enabled == 0 and powers off the others.

Should wl2868c_probe() initialize chip->enabled from WL2868C_LDO_ENABLE
when WL2868C_SYS_ENABLE is already set?

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

[PATCH v6 3/5] dt-bindings: arm: qcom: document the Lenovo Legion Y700 Gen4

[Qianhui Xu]

The Lenovo Legion Y700 Gen4 is an SM8750 tablet that ships with the
lenovo,elden board compatible. Add it to the SM8750 section of the
Qualcomm board binding.

Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 121ca557ce..a32cfb30a1 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -1220,6 +1220,7 @@ properties:
 
       - items:
           - enum:
+              - lenovo,elden
               - qcom,sm8750-mtp
               - qcom,sm8750-qrd
           - const: qcom,sm8750
-- 
2.53.0

Re: [PATCH v6 3/5] dt-bindings: arm: qcom: document the Lenovo Legion Y700 Gen4

[Krzysztof Kozlowski]

On Wed, Nov 04, 2026 at 11:26:46AM +0800, Qianhui Xu wrote:
> The Lenovo Legion Y700 Gen4 is an SM8750 tablet that ships with the
> lenovo,elden board compatible. Add it to the SM8750 section of the

Board cannot be shipped with such compatible - it's first time usage
here. "Add Lenovo Legion Y700 Gen4, an Qualcomm SM8750-based tablet".

Best regards,
Krzysztof

[PATCH v6 4/5] arm64: dts: qcom: sm8750-lenovo-elden: add the Lenovo Legion Y700 Gen4

[Qianhui Xu]

The Lenovo Legion Y700 Gen4 (TB322FC) is an SM8750 tablet. Add a board
devicetree with the board compatible, the chassis type, the GPIOs the
boot firmware reserves and the board clocks, which the SM8750 dtsi
consumes: the XO and sleep clocks and the two divided CXO derivatives,
as on the other SM8750 boards.

The rest of the board enablement (SPMI PMICs, console and storage)
cannot be validated on hardware yet and is left to a separate board
series. The camera PMIC and the regulators it provides follow in the
next patch.

Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
 arch/arm64/boot/dts/qcom/Makefile             |  1 +
 .../boot/dts/qcom/sm8750-lenovo-elden.dts     | 50 +++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 88230c4e00..e0b65a36c5 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -453,6 +453,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-hdk.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-qrd.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-valve-deckard.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sm8750-lenovo-elden.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8750-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8750-qrd.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= talos-evk.dtb
diff --git a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
new file mode 100644
index 0000000000..821950715c
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2026, Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
+ */
+
+/dts-v1/;
+
+#include "sm8750.dtsi"
+
+/ {
+	model = "Lenovo Legion Y700 Gen4";
+	compatible = "lenovo,elden", "qcom,sm8750";
+	chassis-type = "tablet";
+
+	clocks {
+		xo_board: xo-board {
+			compatible = "fixed-clock";
+			clock-frequency = <76800000>;
+			#clock-cells = <0>;
+		};
+
+		sleep_clk: sleep-clk {
+			compatible = "fixed-clock";
+			clock-frequency = <32000>;
+			#clock-cells = <0>;
+		};
+
+		bi_tcxo_div2: bi-tcxo-div2-clk {
+			compatible = "fixed-factor-clock";
+			#clock-cells = <0>;
+
+			clocks = <&rpmhcc RPMH_CXO_CLK>;
+			clock-mult = <1>;
+			clock-div = <2>;
+		};
+
+		bi_tcxo_ao_div2: bi-tcxo-ao-div2-clk {
+			compatible = "fixed-factor-clock";
+			#clock-cells = <0>;
+
+			clocks = <&rpmhcc RPMH_CXO_CLK_A>;
+			clock-mult = <1>;
+			clock-div = <2>;
+		};
+	};
+};
+
+&tlmm {
+	gpio-reserved-ranges = <39 1>, <74 1>;
+};
-- 
2.53.0

Re: [PATCH v6 4/5] arm64: dts: qcom: sm8750-lenovo-elden: add the Lenovo Legion Y700 Gen4

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: sm8750-lenovo-elden: unvalidated skeleton board DTS
--

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

arm64: dts: qcom: sm8750-lenovo-elden: add the Lenovo Legion Y700 Gen4

This commit adds an initial board devicetree for the SM8750-based Lenovo
Legion Y700 Gen4 tablet with the board compatible, chassis type, board
clocks, and firmware-reserved GPIO ranges.

> diff --git a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> new file mode 100644
> index 0000000000000..821950715c8e4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2026, Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
> + */
> +
> +/dts-v1/;
> +
> +#include "sm8750.dtsi"
> +
> +/ {
> +	model = "Lenovo Legion Y700 Gen4";
> +	compatible = "lenovo,elden", "qcom,sm8750";
> +	chassis-type = "tablet";
> +
> +	clocks {
> +		xo_board: xo-board {
> +			compatible = "fixed-clock";
> +			clock-frequency = <76800000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		sleep_clk: sleep-clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <32000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		bi_tcxo_div2: bi-tcxo-div2-clk {
> +			compatible = "fixed-factor-clock";
> +			#clock-cells = <0>;
> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> +			clock-mult = <1>;
> +			clock-div = <2>;
> +		};
> +
> +		bi_tcxo_ao_div2: bi-tcxo-ao-div2-clk {
> +			compatible = "fixed-factor-clock";
> +			#clock-cells = <0>;
> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK_A>;
> +			clock-mult = <1>;
> +			clock-div = <2>;
> +		};
> +	};
> +};
> +
> +&tlmm {
> +	gpio-reserved-ranges = <39 1>, <74 1>;
> +};

[Severity: Medium]
As reported by Konrad Dybcio on v5 [1], is there a reason to include
arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts in this series before the
base board enablement can be validated on hardware?

In this patch, the root node and tlmm node in sm8750-lenovo-elden.dts only
define the board clocks (xo_board, sleep_clk, bi_tcxo_div2, and
bi_tcxo_ao_div2) needed to satisfy phandles in sm8750.dtsi and the reserved
GPIO ranges, without SPMI PMICs, RPMh regulators, console, or storage.

Even with the follow-up commit ("arm64: dts: qcom: sm8750-lenovo-elden:
describe the WL2868C regulators"), the board devicetree only enables
gpi_dma2, qupv3_2, and i2c10 with pmic@2f (willsemi,wl2868c) and a
wl2868c_vsys fixed regulator, while leaving out the SPMI PMIC regulators
that supply the WL2868C LDO inputs (in-d12-supply, in-d34-supply,
in-d5-supply, and in-d7-supply), any regulator consumers, and base board
peripherals.

Since the WL2868C devicetree usage is already covered in the example in
Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml, should
sm8750-lenovo-elden.dts be deferred to the separate board enablement series
once it can be validated on hardware?

[1] https://lore.kernel.org/r/dddc808a-6407-4db1-aa72-0ebe5652f97c@oss.qualcomm.com

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

[PATCH v6 5/5] arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

[Qianhui Xu]

The camera devices of the Lenovo Legion Y700 Gen4 are powered by a
WL2868C camera PMIC on i2c10: LDO1 feeds the rear sensor DVDD, LDO2 the
front sensor DVDD, LDO3 the front sensor AVDD, LDO4 the rear sensor
AVDD, LDO5 the VCM VDD, LDO7 the DOVDD of both sensors and the VCM
VDDIO, all at fixed voltages including the offsets programmed by the
stock firmware. LDO6 is not wired out on this board.

i2c10 is a GENI I2C controller inside the qupv3_2 wrapper, so both
qupv3_2 and its gpi_dma2 parent are enabled alongside, otherwise i2c10
never populates and the regulators would never register.

The VSYS input of the PMIC is the always-on system rail of the tablet
and is described as a fixed regulator. The VIN12, VIN34, VIN5 and VIN7
inputs are fed by SPMI PMIC rails that are not described yet, so they
are left out and follow with the board series that adds the PMICs.

The consumers of these regulators (the rear sensor and its VCM on
cci0) are not described here: the CCI controllers are being added by
Hangxiang Ma's SM8750 series, and the sensor nodes follow with the
camera series that depends on it.

Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
 .../boot/dts/qcom/sm8750-lenovo-elden.dts     | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
index 821950715c..73f4452f46 100644
--- a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
+++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 
+#include <dt-bindings/gpio/gpio.h>
 #include "sm8750.dtsi"
 
 / {
@@ -43,8 +44,86 @@ bi_tcxo_ao_div2: bi-tcxo-ao-div2-clk {
 			clock-div = <2>;
 		};
 	};
+
+	/*
+	 * VSYS of the camera PMIC is the always-on system rail of the tablet.
+	 * The SPMI PMICs that generate it are not described yet, so it is
+	 * modelled as a fixed regulator without a voltage constraint.
+	 */
+	wl2868c_vsys: regulator-vsys {
+		compatible = "regulator-fixed";
+		regulator-name = "wl2868c_vsys";
+		regulator-always-on;
+	};
+};
+
+&gpi_dma2 {
+	status = "okay";
+};
+
+&qupv3_2 {
+	status = "okay";
+};
+
+&i2c10 {
+	status = "okay";
+
+	pmic@2f {
+		compatible = "willsemi,wl2868c";
+		reg = <0x2f>;
+		reset-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&camera_pmic_default>;
+		vsys-supply = <&wl2868c_vsys>;
+
+		regulators {
+			/* Voltages include the offsets programmed by stock firmware. */
+			camera_ldo1: ldo1 {
+				regulator-name = "camera_ldo1";
+				regulator-min-microvolt = <1128000>;
+				regulator-max-microvolt = <1128000>;
+			};
+
+			camera_ldo2: ldo2 {
+				regulator-name = "camera_ldo2";
+				regulator-min-microvolt = <1288000>;
+				regulator-max-microvolt = <1288000>;
+			};
+
+			camera_ldo3: ldo3 {
+				regulator-name = "camera_ldo3";
+				regulator-min-microvolt = <2816000>;
+				regulator-max-microvolt = <2816000>;
+			};
+
+			camera_ldo4: ldo4 {
+				regulator-name = "camera_ldo4";
+				regulator-min-microvolt = <2824000>;
+				regulator-max-microvolt = <2824000>;
+			};
+
+			camera_ldo5: ldo5 {
+				regulator-name = "camera_ldo5";
+				regulator-min-microvolt = <2808000>;
+				regulator-max-microvolt = <2808000>;
+			};
+
+			/* LDO6 is not wired out on this board. */
+			camera_ldo7: ldo7 {
+				regulator-name = "camera_ldo7";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+			};
+		};
+	};
 };
 
 &tlmm {
 	gpio-reserved-ranges = <39 1>, <74 1>;
+
+	camera_pmic_default: camera-pmic-default-state {
+		pins = "gpio126";
+		function = "gpio";
+		bias-disable;
+	};
 };
-- 
2.53.0

Re: [PATCH v6 5/5] arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: sm8750-lenovo-elden: incomplete WL2868C supplies on stub DT
--

commit 0758c044c3db5c70cf435a167f553582f960cdb2
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

This commit enables gpi_dma2, qupv3_2, and i2c10 on the Lenovo Legion Y700
Gen4 devicetree and adds the WL2868C camera PMIC node along with its LDO
regulators and a fixed VSYS supply.

> diff --git a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> index 821950715c8e4..73f4452f46120 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> @@ -5,6 +5,7 @@
>  
>  /dts-v1/;
>  
> +#include <dt-bindings/gpio/gpio.h>
>  #include "sm8750.dtsi"
>  
>  / {
> @@ -43,8 +44,86 @@ bi_tcxo_ao_div2: bi-tcxo-ao-div2-clk {
>  			clock-div = <2>;
>  		};
>  	};
> +
> +	/*
> +	 * VSYS of the camera PMIC is the always-on system rail of the tablet.
> +	 * The SPMI PMICs that generate it are not described yet, so it is
> +	 * modelled as a fixed regulator without a voltage constraint.
> +	 */
> +	wl2868c_vsys: regulator-vsys {
> +		compatible = "regulator-fixed";
> +		regulator-name = "wl2868c_vsys";
> +		regulator-always-on;
> +	};
> +};
> +
> +&gpi_dma2 {
> +	status = "okay";
> +};
> +
> +&qupv3_2 {
> +	status = "okay";
> +};
> +
> +&i2c10 {
> +	status = "okay";
> +
> +	pmic@2f {
> +		compatible = "willsemi,wl2868c";
> +		reg = <0x2f>;
> +		reset-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&camera_pmic_default>;
> +		vsys-supply = <&wl2868c_vsys>;
> +
> +		regulators {
> +			/* Voltages include the offsets programmed by stock firmware. */
> +			camera_ldo1: ldo1 {
> +				regulator-name = "camera_ldo1";
> +				regulator-min-microvolt = <1128000>;
> +				regulator-max-microvolt = <1128000>;
> +			};

[Severity: Medium]
As reported by Konrad Dybcio on v5 [1], does adding the pmic@2f node to
sm8750-lenovo-elden.dts before the board SPMI PMICs, RPMh regulators, and
camera consumers are present provide a complete or usable hardware
description beyond what the DT binding example in
Documentation/devicetree/bindings/regulator/willsemi,wl2868c.yaml already
covers?

In drivers/regulator/wl2868c-regulator.c, wl2868c_regulators[] defines
supply_name as "vin12" for ldo1 and ldo2, "vin34" for ldo3 and ldo4, "vin5"
for ldo5, and "vin7" for ldo7. When wl2868c_probe() registers each regulator
via devm_regulator_register(), the regulator core resolves those parent input
supplies on pmic@2f.

Because sm8750-lenovo-elden.dts does not yet describe the SPMI PMICs or RPMh
regulators (nor the basic board infrastructure to boot), pmic@2f omits
vin12-supply, vin34-supply, vin5-supply, and vin7-supply and only provides a
placeholder regulator-fixed wl2868c_vsys node without voltage constraints,
causing all six registered LDOs to fall back to dummy parent supplies with no
consumers attached.

Would it be better to introduce this node together with the board PMICs and
camera consumers once the parent supplies can be properly wired up?

[1] https://lore.kernel.org/r/dddc808a-6407-4db1-aa72-0ebe5652f97c@oss.qualcomm.com

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

Re: [PATCH v6 5/5] arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

[Krzysztof Kozlowski]

On Wed, Nov 04, 2026 at 11:26:48AM +0800, Qianhui Xu wrote:
> The camera devices of the Lenovo Legion Y700 Gen4 are powered by a
> WL2868C camera PMIC on i2c10: LDO1 feeds the rear sensor DVDD, LDO2 the
> front sensor DVDD, LDO3 the front sensor AVDD, LDO4 the rear sensor
> AVDD, LDO5 the VCM VDD, LDO7 the DOVDD of both sensors and the VCM
> VDDIO, all at fixed voltages including the offsets programmed by the
> stock firmware. LDO6 is not wired out on this board.
> 
> i2c10 is a GENI I2C controller inside the qupv3_2 wrapper, so both
> qupv3_2 and its gpi_dma2 parent are enabled alongside, otherwise i2c10
> never populates and the regulators would never register.
> 
> The VSYS input of the PMIC is the always-on system rail of the tablet
> and is described as a fixed regulator. The VIN12, VIN34, VIN5 and VIN7
> inputs are fed by SPMI PMIC rails that are not described yet, so they
> are left out and follow with the board series that adds the PMICs.
> 
> The consumers of these regulators (the rear sensor and its VCM on
> cci0) are not described here: the CCI controllers are being added by
> Hangxiang Ma's SM8750 series, and the sensor nodes follow with the
> camera series that depends on it.
> 
> Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
> ---
>  .../boot/dts/qcom/sm8750-lenovo-elden.dts     | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)

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.

Best regards,
Krzysztof