[PATCH 0/4] pinctrl: qcom: spmi-gpio: Add bidirectional level-shifter function support

[PATCH 0/4] pinctrl: qcom: spmi-gpio: Add bidirectional level-shifter function support

[Fenglin Wu]

The PMH0101 PMIC introduces BIDIR_LVL_SHIFTER modules that provide
bidirectional voltage translation between 1.2 V and 1.8 V power
domains, targeting open-drain signal buses such as I2C. Each level
shifter shares its two physical pins with a corresponding pair of GPIO
modules, and its enable state is centrally managed by AOP firmware as
a shared RPMh "XOB" resource.

This series adds kernel support for controlling these level shifters
through the pinctrl subsystem. Patches are ordered from infrastructure
to driver to bindings:

Patch 1 (soc: qcom: rpmh) extends the RPMh driver to accept write
commands from devices that are not children of the RPMh controller.
The existing write path assumes the caller is a child of the RSC device;
however, the SPMI GPIO driver sits under the SPMI controller bus. Two
new APIs are introduced — rpmh_get_ctrlr_dev() for resolving the
controller from a "qcom,rpmh" DT phandle, and rpmh_write_ctrlr() /
rpmh_write_async_ctrlr() which accept the controller device pointer
directly rather than deriving it through the device parent chain.

Patch 2 (dt-bindings) documents the new "level-shifter" function, the
qcom,1p2v-1p8v-ls-en property, the qcom,rpmh phandle, and the
qcom,pmic-id string required on pmh0101 nodes. The pmh0101 conditional
block is split out from pmih0108 and given its own required properties.

Patch 3 (pinctrl: qcom: spmi-gpio, rearchitect) refactors the driver's
group and function registration to use the generic pinctrl group and
function APIs (pinctrl_generic_add_group, pinmux_generic_add_function).
The previous design treated every pin as its own group with access to
all functions. The new design allows multi-pin groups with restricted
function sets, which is a prerequisite for exposing the level-shifter
function that is tied to specific GPIO pairs only.

Patch 4 (pinctrl: qcom: spmi-gpio, level-shifter) builds on the above
to introduce the "level-shifter" function. When selected, both GPIO
pads in the pair are disabled (high-impedance), and the RPMh XOB vote
for the level shifter is controlled separately via the new
qcom,1p2v-1p8v-ls-en pinconf parameter, allowing enable and disable
to be represented as distinct pinctrl states with the same function and
group.

With all these changes, the BIDIR_LVL_SHIFTER in PMH0101 could be
controlled with following settings:

    &pmh0101_gpios {
        pmh0101-ls1-en {
                groups = "gpio11, gpio12";
                function = "level-shifter";
                qcom,1p2v-1p8v-ls-en = <1>;
        };

        pmh0101-ls1-dis {
                groups = "gpio11, gpio12";
                function = "level-shifter";
                qcom,1p2v-1p8v-ls-en = <0>;
        };
    };

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
Fenglin Wu (4):
      soc: qcom: rpmh: Allow non-child devices to issue write commands
      dt-bindings: pinctrl: qcom,pmic-gpio: Add level-shifter function
      pinctrl: qcom: spmi-gpio: Rearchitect for flexible group support
      pinctrl: qcom: spmi-gpio: Add level-shifter function support

 .../bindings/pinctrl/qcom,pmic-gpio.yaml           |  69 +-
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c           | 755 ++++++++++++++++-----
 drivers/soc/qcom/rpmh.c                            | 161 ++++-
 include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |   1 +
 include/soc/qcom/rpmh.h                            |  21 +
 5 files changed, 807 insertions(+), 200 deletions(-)
---
base-commit: b8f192cec7dcb2e4f04ee57ab78d51777b0a5729
change-id: 20260527-pinctrl-level-shifter-929b286f583d

Best regards,
--  
Fenglin Wu <fenglin.wu@oss.qualcomm.com>

[PATCH 1/4] soc: qcom: rpmh: Allow non-child devices to issue write commands

[Fenglin Wu]

Currently, the RPMH driver only allows child devices of the RPMH
controller to issue commands, as it assumes dev->parent points to the
RSC device.

There is a possibility that certain devices which are not children of
the RPMH controller want to send commands for special control at the
RPMH side. For example, in PMH0101 PMICs, there are bidirectional
level shifter (LS) peripherals, and each LS works with a pair of PMIC
GPIOs. The control of the LS, which is combined with the GPIO
configuration, is handled by RPMH firmware for sharing the resource
between different subsystems. From a hardware point of view, the LS
functionality is tied to a pair of PMIC GPIOs, so its control is more
suitable to be added in the pinctrl-spmi-gpio driver by adding the
level-shifter function. However, the pinctrl-spmi-gpio device is a
child device of the SPMI controller, not the RPMH controller.

This patch extends the RPMH driver to support write commands from any
device that has a pointer to the RPMH controller device:

1. Add rpmh_get_ctrlr_dev() to lookup controller via device tree
   phandle "qcom,rpmh"
2. Add new APIs: rpmh_write_async_ctrlr() and rpmh_write_ctrlr()
   that accept controller device pointer directly

With this change, the pinctrl-spmi-gpio driver is able to issue write
commands to the RPMH controller by using the controller device pointer,
and vote for enabling the level-shifter function.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 drivers/soc/qcom/rpmh.c | 161 +++++++++++++++++++++++++++++++++++++++++++-----
 include/soc/qcom/rpmh.h |  21 +++++++
 2 files changed, 167 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index ca37da3dc2b1..9c7844434e9a 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -12,6 +12,7 @@
 #include <linux/lockdep.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -76,6 +77,21 @@ static struct rpmh_ctrlr *get_rpmh_ctrlr(const struct device *dev)
 	return &drv->client;
 }
 
+static struct rpmh_ctrlr *get_rpmh_ctrlr_from_dev(const struct device *ctrl_dev)
+{
+	struct rsc_drv *drv;
+
+	if (!ctrl_dev)
+		return ERR_PTR(-EINVAL);
+
+	drv = dev_get_drvdata(ctrl_dev);
+
+	if (!drv)
+		return ERR_PTR(-ENODEV);
+
+	return &drv->client;
+}
+
 void rpmh_tx_done(const struct tcs_request *msg)
 {
 	struct rpmh_request *rpm_msg = container_of(msg, struct rpmh_request,
@@ -156,23 +172,11 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr,
 	return req;
 }
 
-/**
- * __rpmh_write: Cache and send the RPMH request
- *
- * @dev: The device making the request
- * @state: Active/Sleep request type
- * @rpm_msg: The data that needs to be sent (cmds).
- *
- * Cache the RPMH request and send if the state is ACTIVE_ONLY.
- * SLEEP/WAKE_ONLY requests are not sent to the controller at
- * this time. Use rpmh_flush() to send them to the controller.
- */
-static int __rpmh_write(const struct device *dev, enum rpmh_state state,
-			struct rpmh_request *rpm_msg)
+static int __rpmh_write_direct(struct rpmh_ctrlr *ctrlr, enum rpmh_state state,
+			       struct rpmh_request *rpm_msg)
 {
-	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
-	int ret = -EINVAL;
 	struct cache_req *req;
+	int ret = -EINVAL;
 	int i;
 
 	/* Cache the request in our store and link the payload */
@@ -193,6 +197,25 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
 	return ret;
 }
 
+/**
+ * __rpmh_write: Cache and send the RPMH request
+ *
+ * @dev: The device making the request
+ * @state: Active/Sleep request type
+ * @rpm_msg: The data that needs to be sent (cmds).
+ *
+ * Cache the RPMH request and send if the state is ACTIVE_ONLY.
+ * SLEEP/WAKE_ONLY requests are not sent to the controller at
+ * this time. Use rpmh_flush() to send them to the controller.
+ */
+static int __rpmh_write(const struct device *dev, enum rpmh_state state,
+			struct rpmh_request *rpm_msg)
+{
+	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+
+	return __rpmh_write_direct(ctrlr, state, rpm_msg);
+}
+
 static int __fill_rpmh_msg(struct rpmh_request *req, enum rpmh_state state,
 		const struct tcs_cmd *cmd, u32 n)
 {
@@ -271,6 +294,114 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
 }
 EXPORT_SYMBOL_GPL(rpmh_write);
 
+/**
+ * rpmh_get_ctrlr_dev: Get RPMH controller device from device tree
+ *
+ * @dev: Device with "qcom,rpmh" phandle property
+ *
+ * Returns: Pointer to RPMH controller device, with a devm action registered
+ * on @dev to release the reference when @dev is unbound.
+ */
+struct device *rpmh_get_ctrlr_dev(struct device *dev)
+{
+	struct device_node *rpmh_np;
+	struct platform_device *pdev;
+	int ret;
+
+	rpmh_np = of_parse_phandle(dev->of_node, "qcom,rpmh", 0);
+	if (!rpmh_np)
+		return ERR_PTR(-ENODEV);
+
+	pdev = of_find_device_by_node(rpmh_np);
+	of_node_put(rpmh_np);
+
+	if (!pdev)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	ret = devm_add_action_or_reset(dev, (void (*)(void *))put_device,
+				       &pdev->dev);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return &pdev->dev;
+}
+EXPORT_SYMBOL_GPL(rpmh_get_ctrlr_dev);
+
+/**
+ * rpmh_write_async_ctrlr: Write RPMH commands with the controller device pointer
+ *
+ * @ctrl_dev: The RPMH controller device
+ * @state: Active/sleep set
+ * @cmd: The payload data
+ * @n: The number of elements in payload
+ *
+ * Write a set of RPMH commands, the order of commands is maintained
+ * and will be sent as a single shot.
+ */
+int rpmh_write_async_ctrlr(const struct device *ctrl_dev, enum rpmh_state state,
+			   const struct tcs_cmd *cmd, u32 n)
+{
+	struct rpmh_request *rpm_msg;
+	struct rpmh_ctrlr *ctrlr;
+	int ret;
+
+	ctrlr = get_rpmh_ctrlr_from_dev(ctrl_dev);
+	if (IS_ERR(ctrlr))
+		return PTR_ERR(ctrlr);
+
+	rpm_msg = kzalloc_obj(*rpm_msg, GFP_ATOMIC);
+	if (!rpm_msg)
+		return -ENOMEM;
+	rpm_msg->needs_free = true;
+
+	ret = __fill_rpmh_msg(rpm_msg, state, cmd, n);
+	if (ret) {
+		kfree(rpm_msg);
+		return ret;
+	}
+
+	return __rpmh_write_direct(ctrlr, state, rpm_msg);
+}
+EXPORT_SYMBOL_GPL(rpmh_write_async_ctrlr);
+
+/**
+ * rpmh_write_ctrlr: Write RPMH commands and block until response,
+ * with the controller device pointer
+ *
+ * @ctrlr_dev: The RPMH controller device
+ * @state: Active/sleep set
+ * @cmd: The payload data
+ * @n: The number of elements in @cmd
+ *
+ * May sleep. Do not call from atomic contexts.
+ */
+int rpmh_write_ctrlr(const struct device *ctrlr_dev, enum rpmh_state state,
+		     const struct tcs_cmd *cmd, u32 n)
+{
+	DECLARE_COMPLETION_ONSTACK(compl);
+	/* dev is unused in the synchronous non-batch path; pass NULL */
+	DEFINE_RPMH_MSG_ONSTACK(NULL, state, &compl, rpm_msg);
+	struct rpmh_ctrlr *ctrlr;
+	int ret;
+
+	ctrlr = get_rpmh_ctrlr_from_dev(ctrlr_dev);
+	if (IS_ERR(ctrlr))
+		return PTR_ERR(ctrlr);
+
+	ret = __fill_rpmh_msg(&rpm_msg, state, cmd, n);
+	if (ret)
+		return ret;
+
+	ret = __rpmh_write_direct(ctrlr, state, &rpm_msg);
+	if (ret)
+		return ret;
+
+	ret = wait_for_completion_timeout(&compl, RPMH_TIMEOUT_MS);
+	WARN_ON(!ret);
+	return (ret > 0) ? 0 : -ETIMEDOUT;
+}
+EXPORT_SYMBOL_GPL(rpmh_write_ctrlr);
+
 static void cache_batch(struct rpmh_ctrlr *ctrlr, struct batch_cache_req *req)
 {
 	unsigned long flags;
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index bdbee1a97d36..90ddcd7ca2fe 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -22,6 +22,14 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
 
 void rpmh_invalidate(const struct device *dev);
 
+struct device *rpmh_get_ctrlr_dev(struct device *dev);
+
+int rpmh_write_async_ctrlr(const struct device *ctrl_dev, enum rpmh_state state,
+			   const struct tcs_cmd *cmd, u32 n);
+
+int rpmh_write_ctrlr(const struct device *ctrlr_dev, enum rpmh_state state,
+		     const struct tcs_cmd *cmd, u32 n);
+
 #else
 
 static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
@@ -42,6 +50,19 @@ static inline void rpmh_invalidate(const struct device *dev)
 {
 }
 
+static inline struct device *rpmh_get_ctrlr_dev(struct device *dev)
+{ return ERR_PTR(-ENODEV); }
+
+static inline int rpmh_write_async_ctrlr(const struct device *ctrl_dev,
+					  enum rpmh_state state,
+					  const struct tcs_cmd *cmd, u32 n)
+{ return -ENODEV; }
+
+static inline int rpmh_write_ctrlr(const struct device *ctrlr_dev,
+				    enum rpmh_state state,
+				    const struct tcs_cmd *cmd, u32 n)
+{ return -ENODEV; }
+
 #endif /* CONFIG_QCOM_RPMH */
 
 #endif /* __SOC_QCOM_RPMH_H__ */

-- 
2.43.0

Re: [PATCH 1/4] soc: qcom: rpmh: Allow non-child devices to issue write commands

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Casting `put_device` to `void (*)(void *)` for `devm_add_action_or_reset` will trigger a Control-Flow Integrity (CFI) kernel panic.
- [High] Lack of device links allows non-child consumer devices to access the hardware after unbind or while suspended, leading to UAF or physical state machine violations.

Pre-existing issues:
- [High] If `wait_for_completion_timeout` times out, a dangling stack pointer is left for the hardware completion interrupt to access, causing stack corruption.
--

commit 033be046152076a7be483616e27b4cc044eaf8da
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

soc: qcom: rpmh: Allow non-child devices to issue write commands

This patch extends the RPMH driver to support write commands from any
device that has a pointer to the RPMH controller device by looking up
the controller via the qcom,rpmh device tree phandle.

> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> index ca37da3dc2b14..9c7844434e9a1 100644
> --- a/drivers/soc/qcom/rpmh.c
> +++ b/drivers/soc/qcom/rpmh.c

[ ... ]

> @@ -271,6 +294,114 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
>  }
>  EXPORT_SYMBOL_GPL(rpmh_write);
>  
> +/**
> + * rpmh_get_ctrlr_dev: Get RPMH controller device from device tree
> + *
> + * @dev: Device with "qcom,rpmh" phandle property
> + *
> + * Returns: Pointer to RPMH controller device, with a devm action registered
> + * on @dev to release the reference when @dev is unbound.
> + */
> +struct device *rpmh_get_ctrlr_dev(struct device *dev)
> +{
> +	struct device_node *rpmh_np;
> +	struct platform_device *pdev;
> +	int ret;
> +
> +	rpmh_np = of_parse_phandle(dev->of_node, "qcom,rpmh", 0);
> +	if (!rpmh_np)
> +		return ERR_PTR(-ENODEV);
> +
> +	pdev = of_find_device_by_node(rpmh_np);
> +	of_node_put(rpmh_np);
> +
> +	if (!pdev)
> +		return ERR_PTR(-EPROBE_DEFER);
> +

[Severity: High]
Since qcom,rpmh is a custom property, fw_devlink does not automatically
establish a device link. Without a device link created here (e.g., via
device_link_add()), is there any guarantee on PM or probe ordering?

Without a device link, a consumer could access the RPMH hardware while it is
runtime-suspended or after the RPMH driver has been unbound. If the RPMH
driver unbinds, its devres-managed struct rsc_drv is freed. The consumer's
devm action only pins the struct device, so calling dev_get_drvdata() later
could return a dangling pointer, leading to a Use-After-Free.

> +	ret = devm_add_action_or_reset(dev, (void (*)(void *))put_device,
> +				       &pdev->dev);

[Severity: High]
Will casting put_device to (void (*)(void *)) trigger a Control-Flow
Integrity (CFI) kernel panic?

The put_device function is defined as void put_device(struct device *dev).
Under KCFI, the mismatch between the expected function signature and the
actual signature of put_device will fail the hash check, resulting in a
kernel panic when the consumer device unbinds or its probe fails.

Could a wrapper function matching the void (*)(void *) signature be used
instead?

> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return &pdev->dev;
> +}
> +EXPORT_SYMBOL_GPL(rpmh_get_ctrlr_dev);

[ ... ]

> +/**
> + * rpmh_write_ctrlr: Write RPMH commands and block until response,
> + * with the controller device pointer
> + *
> + * @ctrlr_dev: The RPMH controller device
> + * @state: Active/sleep set
> + * @cmd: The payload data
> + * @n: The number of elements in @cmd
> + *
> + * May sleep. Do not call from atomic contexts.
> + */
> +int rpmh_write_ctrlr(const struct device *ctrlr_dev, enum rpmh_state state,
> +		     const struct tcs_cmd *cmd, u32 n)
> +{
> +	DECLARE_COMPLETION_ONSTACK(compl);
> +	/* dev is unused in the synchronous non-batch path; pass NULL */
> +	DEFINE_RPMH_MSG_ONSTACK(NULL, state, &compl, rpm_msg);
> +	struct rpmh_ctrlr *ctrlr;
> +	int ret;
> +
> +	ctrlr = get_rpmh_ctrlr_from_dev(ctrlr_dev);
> +	if (IS_ERR(ctrlr))
> +		return PTR_ERR(ctrlr);
> +
> +	ret = __fill_rpmh_msg(&rpm_msg, state, cmd, n);
> +	if (ret)
> +		return ret;
> +
> +	ret = __rpmh_write_direct(ctrlr, state, &rpm_msg);
> +	if (ret)
> +		return ret;
> +
> +	ret = wait_for_completion_timeout(&compl, RPMH_TIMEOUT_MS);
> +	WARN_ON(!ret);
> +	return (ret > 0) ? 0 : -ETIMEDOUT;

[Severity: High]
This isn't a bug introduced by this patch, but it appears to be a
pre-existing issue copied from rpmh_write(). If
wait_for_completion_timeout() expires, doesn't it leave a dangling stack
pointer for the hardware completion interrupt to access?

The rpm_msg and its completion are allocated on the stack. If the timeout
expires, the function returns and its stack frame is released, but the
hardware request is not canceled.

When the hardware eventually completes the transaction, the asynchronous
interrupt handler calls rpmh_tx_done(), which dereferences the now-invalid
rpm_msg pointer and attempts to call complete(), corrupting stack memory.

> +}
> +EXPORT_SYMBOL_GPL(rpmh_write_ctrlr);
> +
>  static void cache_batch(struct rpmh_ctrlr *ctrlr, struct batch_cache_req *req)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-pinctrl-level-shifter-v1-0-1965461d0a7c@oss.qualcomm.com?part=1

[PATCH 2/4] dt-bindings: pinctrl: qcom,pmic-gpio: Add level-shifter function

[Fenglin Wu]

Add the "level-shifter" function and add the required DT properties to
allow RPMh firmware to control the level-shifter. Introduce a custom
parameter "qcom,1p2v-1p8v-ls-en" for enabling or disabling the
level-shifter function.

Additionally, add the "groups" property with the allowed group names
that can be used to control the level-shifter function on pmh0101.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 .../bindings/pinctrl/qcom,pmic-gpio.yaml           | 69 +++++++++++++++++++++-
 include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |  1 +
 2 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
index b8109e6c2a10..016f4ad75033 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
@@ -119,6 +119,21 @@ properties:
       The first cell will be used to define gpio number and the
       second denotes the flags for this gpio
 
+  qcom,rpmh:
+    description:
+      Phandle to the RPMh controller device. Required for PMICs that support
+      bidirectional level shifters (e.g., pmh0101) to enable communication
+      with RPMh firmware for level shifter control.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  qcom,pmic-id:
+    description:
+      The ID of the PMIC which supports bidirectional level shifter function.
+      It is used as the RPMh resource name suffix to request control of the
+      level shifter to the RPMh firmware.
+    $ref: /schemas/types.yaml#/definitions/string
+    pattern: "^[A-N]_E[0-3]+$"
+
 additionalProperties: false
 
 required:
@@ -330,6 +345,25 @@ allOf:
           contains:
             enum:
               - qcom,pmh0101-gpio
+    then:
+      properties:
+        gpio-line-names:
+          minItems: 18
+          maxItems: 18
+        gpio-reserved-ranges:
+          minItems: 1
+          maxItems: 9
+        qcom,rpmh: true
+        qcom,pmic-id: true
+      required:
+        - qcom,rpmh
+        - qcom,pmic-id
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
               - qcom,pmih0108-gpio
     then:
       properties:
@@ -523,6 +557,19 @@ $defs:
         items:
           pattern: '^gpio([0-9]+)$'
 
+      groups:
+        $ref: /schemas/types.yaml#/definitions/string-array
+        description:
+          List of GPIO groups to apply properties to. Only valid for
+          function "level-shifter" on pmh0101. Valid groups are
+          gpio11, gpio12; gpio13, gpio14; gpio15, gpio16; gpio17, gpio18.
+        items:
+          enum:
+            - gpio11, gpio12
+            - gpio13, gpio14
+            - gpio15, gpio16
+            - gpio17, gpio18
+
       function:
         items:
           - enum:
@@ -536,6 +583,7 @@ $defs:
               - dtest4
               - func3  # supported by LV/MV GPIO subtypes
               - func4  # supported by LV/MV GPIO subtypes
+              - level-shifter  # supported only by pmh0101
 
       bias-disable: true
       bias-pull-down: true
@@ -592,9 +640,24 @@ $defs:
           configured as digital input.
         enum: [1, 2, 3, 4]
 
-    required:
-      - pins
-      - function
+      qcom,1p2v-1p8v-ls-en:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Enable or disable the bidirectional 1.2V/1.8V level shifter
+          associated with the specified GPIO group. When set to 1, an RPMh
+          vote is sent to AOP to enable the level shifter. When set to 0,
+          the vote is withdrawn. Only valid when function is "level-shifter"
+          and groups is a level-shifter GPIO pair (e.g., "gpio11, gpio12"
+          on pmh0101).
+        enum: [0, 1]
+
+    oneOf:
+      - required:
+          - pins
+          - function
+      - required:
+          - groups
+          - function
 
     additionalProperties: false
 
diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
index e5df5ce45a0f..b0824d5eb056 100644
--- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
+++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
@@ -105,6 +105,7 @@
 #define PMIC_GPIO_FUNC_DTEST2		"dtest2"
 #define PMIC_GPIO_FUNC_DTEST3		"dtest3"
 #define PMIC_GPIO_FUNC_DTEST4		"dtest4"
+#define PMIC_GPIO_FUNC_LEVEL_SHIFTER	"level-shifter"
 
 #define PM8038_GPIO1_2_LPG_DRV		PMIC_GPIO_FUNC_FUNC1
 #define PM8038_GPIO3_5V_BOOST_EN	PMIC_GPIO_FUNC_FUNC1

-- 
2.43.0

Re: [PATCH 2/4] dt-bindings: pinctrl: qcom,pmic-gpio: Add level-shifter function

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally requiring `qcom,rpmh` and `qcom,pmic-id` for the existing `qcom,pmh0101-gpio` compatible breaks device tree validation and backward compatibility.
--

commit 4ef6fb2ad4a4a7f20f7b6195e05c09b5f1ccf313
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

dt-bindings: pinctrl: qcom,pmic-gpio: Add level-shifter function

This commit adds the level-shifter function and related device tree
properties for RPMh firmware control. It introduces a parameter to enable
or disable the level-shifter and defines allowed groups for pmh0101.

> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml

[ ... ]

> @@ -330,6 +345,25 @@ allOf:
>            contains:
>              enum:
>                - qcom,pmh0101-gpio
> +    then:
> +      properties:
> +        gpio-line-names:
> +          minItems: 18
> +          maxItems: 18
> +        gpio-reserved-ranges:
> +          minItems: 1
> +          maxItems: 9
> +        qcom,rpmh: true
> +        qcom,pmic-id: true
> +      required:
> +        - qcom,rpmh
> +        - qcom,pmic-id

[Severity: High]
Does unconditionally requiring these properties break backward compatibility
with existing device trees?

Existing device trees using the qcom,pmh0101-gpio compatible might not have
the qcom,rpmh or qcom,pmic-id properties, which would cause validation
to fail for those older platforms.

Additionally, if the driver unconditionally relies on these properties
in subsequent changes (such as calling pmic_gpio_register_level_shifters()
where a missing qcom,rpmh causes rpmh_get_ctrlr_dev() to return -ENODEV),
this could cause the entire GPIO controller probe to fail on older device
trees.

Could these properties be made optional or strictly conditional on actual use
so that the driver can gracefully handle their absence?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-pinctrl-level-shifter-v1-0-1965461d0a7c@oss.qualcomm.com?part=2

[PATCH 3/4] pinctrl: qcom: spmi-gpio: Rearchitect for flexible group support

[Fenglin Wu]

Currently, the driver treats each pin as a group, and every pin or
group can function correctly in all existing functions. However,
this approach is no longer valid since some PMIC pins only operate
together in specific functions, which are limited to certain GPIO
groups. For example, in pmh0101, the level-shifter function is
supported only between GPIO pairs like GPIO11/12, GPIO13/14,
GPIO15/16, and GPIO17/18.

To better accommodate these new functions and restrict GPIOs to
those that support them, rearchitect the driver to enable flexible
pinctrl group configurations by utilizing the generic pinctrl group
and function APIs.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 367 +++++++++++++++++++++----------
 1 file changed, 249 insertions(+), 118 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index cdd61dae74cf..f159c56784b4 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -24,6 +24,7 @@
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 
 #include "../core.h"
+#include "../pinmux.h"
 #include "../pinctrl-utils.h"
 
 #define PMIC_GPIO_ADDRESS_RANGE			0x100
@@ -253,139 +254,124 @@ static int pmic_gpio_write(struct pmic_gpio_state *state,
 	return ret;
 }
 
-static int pmic_gpio_get_groups_count(struct pinctrl_dev *pctldev)
-{
-	/* Every PIN is a group */
-	return pctldev->desc->npins;
-}
-
-static const char *pmic_gpio_get_group_name(struct pinctrl_dev *pctldev,
-					    unsigned pin)
-{
-	return pctldev->desc->pins[pin].name;
-}
-
-static int pmic_gpio_get_group_pins(struct pinctrl_dev *pctldev, unsigned pin,
-				    const unsigned **pins, unsigned *num_pins)
-{
-	*pins = &pctldev->desc->pins[pin].number;
-	*num_pins = 1;
-	return 0;
-}
-
 static const struct pinctrl_ops pmic_gpio_pinctrl_ops = {
-	.get_groups_count	= pmic_gpio_get_groups_count,
-	.get_group_name		= pmic_gpio_get_group_name,
-	.get_group_pins		= pmic_gpio_get_group_pins,
+	.get_groups_count	= pinctrl_generic_get_group_count,
+	.get_group_name		= pinctrl_generic_get_group_name,
+	.get_group_pins		= pinctrl_generic_get_group_pins,
 	.dt_node_to_map		= pinconf_generic_dt_node_to_map_group,
 	.dt_free_map		= pinctrl_utils_free_map,
 };
 
-static int pmic_gpio_get_functions_count(struct pinctrl_dev *pctldev)
-{
-	return ARRAY_SIZE(pmic_gpio_functions);
-}
-
-static const char *pmic_gpio_get_function_name(struct pinctrl_dev *pctldev,
-					       unsigned function)
-{
-	return pmic_gpio_functions[function];
-}
-
-static int pmic_gpio_get_function_groups(struct pinctrl_dev *pctldev,
-					 unsigned function,
-					 const char *const **groups,
-					 unsigned *const num_qgroups)
-{
-	*groups = pmic_gpio_groups;
-	*num_qgroups = pctldev->desc->npins;
-	return 0;
-}
-
-static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned function,
-				unsigned pin)
+static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
+				unsigned int selector)
 {
 	struct pmic_gpio_state *state = pinctrl_dev_get_drvdata(pctldev);
 	struct pmic_gpio_pad *pad;
-	unsigned int val;
-	int ret;
+	struct group_desc *group;
+	unsigned int val, pin, func;
+	int ret, i;
 
 	if (function > PMIC_GPIO_FUNC_INDEX_DTEST4) {
 		pr_err("function: %d is not defined\n", function);
 		return -EINVAL;
 	}
 
-	pad = pctldev->desc->pins[pin].drv_data;
-	/*
-	 * Non-LV/MV subtypes only support 2 special functions,
-	 * offsetting the dtestx function values by 2
-	 */
-	if (!pad->lv_mv_type) {
-		if (function == PMIC_GPIO_FUNC_INDEX_FUNC3 ||
-				function == PMIC_GPIO_FUNC_INDEX_FUNC4) {
-			pr_err("LV/MV subtype doesn't have func3/func4\n");
-			return -EINVAL;
+	group = pinctrl_generic_get_group(pctldev, selector);
+	if (!group)
+		return -EINVAL;
+
+	/* For standard functions, iterate over all pins in the group */
+	for (i = 0; i < group->grp.npins; i++) {
+		pin = group->grp.pins[i];
+		pad = pctldev->desc->pins[pin].drv_data;
+
+		func = function;
+		/*
+		 * Non-LV/MV subtypes only support 2 special functions,
+		 * offsetting the dtestx function values by 2
+		 */
+		if (!pad->lv_mv_type) {
+			if (func == PMIC_GPIO_FUNC_INDEX_FUNC3 ||
+					func == PMIC_GPIO_FUNC_INDEX_FUNC4) {
+				dev_err(state->dev,
+					"pin%d: LV/MV subtype doesn't have func3/func4\n",
+					pin);
+				return -EINVAL;
+			}
+			if (func >= PMIC_GPIO_FUNC_INDEX_DTEST1)
+				func -= (PMIC_GPIO_FUNC_INDEX_DTEST1 -
+						PMIC_GPIO_FUNC_INDEX_FUNC3);
 		}
-		if (function >= PMIC_GPIO_FUNC_INDEX_DTEST1)
-			function -= (PMIC_GPIO_FUNC_INDEX_DTEST1 -
-					PMIC_GPIO_FUNC_INDEX_FUNC3);
-	}
 
-	pad->function = function;
+		pad->function = func;
 
-	if (pad->analog_pass)
-		val = PMIC_GPIO_MODE_ANALOG_PASS_THRU;
-	else if (pad->output_enabled && pad->input_enabled)
-		val = PMIC_GPIO_MODE_DIGITAL_INPUT_OUTPUT;
-	else if (pad->output_enabled)
-		val = PMIC_GPIO_MODE_DIGITAL_OUTPUT;
-	else
-		val = PMIC_GPIO_MODE_DIGITAL_INPUT;
+		if (pad->analog_pass)
+			val = PMIC_GPIO_MODE_ANALOG_PASS_THRU;
+		else if (pad->output_enabled && pad->input_enabled)
+			val = PMIC_GPIO_MODE_DIGITAL_INPUT_OUTPUT;
+		else if (pad->output_enabled)
+			val = PMIC_GPIO_MODE_DIGITAL_OUTPUT;
+		else
+			val = PMIC_GPIO_MODE_DIGITAL_INPUT;
 
-	if (pad->lv_mv_type) {
-		ret = pmic_gpio_write(state, pad,
-				PMIC_GPIO_REG_MODE_CTL, val);
-		if (ret < 0)
-			return ret;
+		if (pad->lv_mv_type) {
+			ret = pmic_gpio_write(state, pad,
+					PMIC_GPIO_REG_MODE_CTL, val);
+			if (ret < 0)
+				return ret;
 
-		val = pad->atest - 1;
-		ret = pmic_gpio_write(state, pad,
-				PMIC_GPIO_REG_LV_MV_ANA_PASS_THRU_SEL, val);
-		if (ret < 0)
-			return ret;
+			val = pad->atest - 1;
+			ret = pmic_gpio_write(state, pad,
+					PMIC_GPIO_REG_LV_MV_ANA_PASS_THRU_SEL, val);
+			if (ret < 0)
+				return ret;
+
+			val = pad->out_value
+				<< PMIC_GPIO_LV_MV_OUTPUT_INVERT_SHIFT;
+			val |= pad->function
+				& PMIC_GPIO_LV_MV_OUTPUT_SOURCE_SEL_MASK;
+			ret = pmic_gpio_write(state, pad,
+				PMIC_GPIO_REG_LV_MV_DIG_OUT_SOURCE_CTL, val);
+			if (ret < 0)
+				return ret;
+		} else {
+			val = val << PMIC_GPIO_REG_MODE_DIR_SHIFT;
+			val |= pad->function << PMIC_GPIO_REG_MODE_FUNCTION_SHIFT;
+			val |= pad->out_value & PMIC_GPIO_REG_MODE_VALUE_SHIFT;
 
-		val = pad->out_value
-			<< PMIC_GPIO_LV_MV_OUTPUT_INVERT_SHIFT;
-		val |= pad->function
-			& PMIC_GPIO_LV_MV_OUTPUT_SOURCE_SEL_MASK;
-		ret = pmic_gpio_write(state, pad,
-			PMIC_GPIO_REG_LV_MV_DIG_OUT_SOURCE_CTL, val);
-		if (ret < 0)
-			return ret;
-	} else {
-		val = val << PMIC_GPIO_REG_MODE_DIR_SHIFT;
-		val |= pad->function << PMIC_GPIO_REG_MODE_FUNCTION_SHIFT;
-		val |= pad->out_value & PMIC_GPIO_REG_MODE_VALUE_SHIFT;
+			ret = pmic_gpio_write(state, pad, PMIC_GPIO_REG_MODE_CTL, val);
+			if (ret < 0)
+				return ret;
+		}
 
-		ret = pmic_gpio_write(state, pad, PMIC_GPIO_REG_MODE_CTL, val);
+		val = pad->is_enabled << PMIC_GPIO_REG_MASTER_EN_SHIFT;
+
+		ret = pmic_gpio_write(state, pad, PMIC_GPIO_REG_EN_CTL, val);
 		if (ret < 0)
 			return ret;
 	}
 
-	val = pad->is_enabled << PMIC_GPIO_REG_MASTER_EN_SHIFT;
-
-	return pmic_gpio_write(state, pad, PMIC_GPIO_REG_EN_CTL, val);
+	return 0;
 }
 
 static const struct pinmux_ops pmic_gpio_pinmux_ops = {
-	.get_functions_count	= pmic_gpio_get_functions_count,
-	.get_function_name	= pmic_gpio_get_function_name,
-	.get_function_groups	= pmic_gpio_get_function_groups,
+	.get_functions_count	= pinmux_generic_get_function_count,
+	.get_function_name	= pinmux_generic_get_function_name,
+	.get_function_groups	= pinmux_generic_get_function_groups,
 	.set_mux		= pmic_gpio_set_mux,
 };
 
-static int pmic_gpio_config_get(struct pinctrl_dev *pctldev,
-				unsigned int pin, unsigned long *config)
+/**
+ * pmic_gpio_pinconf_pin_get() - Get configuration for a single pin
+ * @pctldev: pinctrl device
+ * @pin: Pin number
+ * @config: Configuration parameter to get
+ *
+ * Core function to read a single pin's configuration.
+ * Used by both per-pin and per-group config get operations.
+ */
+static int pmic_gpio_pinconf_pin_get(struct pinctrl_dev *pctldev,
+				     unsigned int pin, unsigned long *config)
 {
 	unsigned param = pinconf_to_config_param(*config);
 	struct pmic_gpio_pad *pad;
@@ -476,8 +462,48 @@ static int pmic_gpio_config_get(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
-static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
-				unsigned long *configs, unsigned nconfs)
+/**
+ * pmic_gpio_pinconf_group_get() - Get configuration for a pin group
+ * @pctldev: pinctrl device
+ * @selector: Group selector
+ * @config: Configuration parameter to get
+ *
+ * For multi-pin groups, we assume all pins have the same configuration,
+ * so we read the configuration from the first pin in the group.
+ */
+static int pmic_gpio_pinconf_group_get(struct pinctrl_dev *pctldev,
+				       unsigned int selector,
+				       unsigned long *config)
+{
+	const struct group_desc *group;
+	unsigned int pin;
+
+	group = pinctrl_generic_get_group(pctldev, selector);
+	if (!group)
+		return -EINVAL;
+
+	/*
+	 * For multi-pin groups, we assume all pins have the same configuration,
+	 * so we read the configuration from the first pin in the group.
+	 */
+	pin = group->grp.pins[0];
+
+	return pmic_gpio_pinconf_pin_get(pctldev, pin, config);
+}
+
+/**
+ * pmic_gpio_pinconf_pin_set() - Set configuration for a single pin
+ * @pctldev: pinctrl device
+ * @pin: Pin number
+ * @configs: Array of configuration parameters
+ * @nconfs: Number of configurations
+ *
+ * Core function to configure a single pin.
+ * Used by both per-pin and per-group config set operations.
+ */
+static int pmic_gpio_pinconf_pin_set(struct pinctrl_dev *pctldev,
+				     unsigned int pin,
+				     unsigned long *configs, unsigned int nconfs)
 {
 	struct pmic_gpio_state *state = pinctrl_dev_get_drvdata(pctldev);
 	struct pmic_gpio_pad *pad;
@@ -651,12 +677,58 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
 	val = pad->is_enabled << PMIC_GPIO_REG_MASTER_EN_SHIFT;
 
 	ret = pmic_gpio_write(state, pad, PMIC_GPIO_REG_EN_CTL, val);
+	if (ret < 0)
+		return ret;
 
-	return ret;
+	return 0;
+}
+
+/**
+ * pmic_gpio_pinconf_group_set() - Set configuration for a pin group
+ * @pctldev: pinctrl device
+ * @selector: Group selector
+ * @configs: Array of configuration parameters
+ * @nconfs: Number of configurations
+ *
+ * Iterates over all pins in the group and applies config to each.
+ */
+static int pmic_gpio_pinconf_group_set(struct pinctrl_dev *pctldev,
+				       unsigned int selector,
+				       unsigned long *configs,
+				       unsigned int nconfs)
+{
+	const struct group_desc *group;
+	unsigned int pin;
+	int i, ret;
+
+	group = pinctrl_generic_get_group(pctldev, selector);
+	if (!group)
+		return -EINVAL;
+
+	/* Iterate over all pins in the group and apply config to each */
+	for (i = 0; i < group->grp.npins; i++) {
+		pin = group->grp.pins[i];
+
+		ret = pmic_gpio_pinconf_pin_set(pctldev, pin, configs, nconfs);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
 }
 
-static void pmic_gpio_config_dbg_show(struct pinctrl_dev *pctldev,
-				      struct seq_file *s, unsigned pin)
+/**
+ * pmic_gpio_pinconf_pin_dbg_show() - Show configuration for a single pin
+ * @pctldev: pinctrl device
+ * @s: seq_file for output
+ * @pin: Pin number
+ *
+ * Core function that dumps the configuration of a single GPIO pin.
+ * Used by pinconf-pins debugfs, pinconf-groups debugfs, and gpio debugfs.
+ */
+static void pmic_gpio_pinconf_pin_dbg_show(struct pinctrl_dev *pctldev,
+					   struct seq_file *s,
+					   unsigned int pin)
 {
 	struct pmic_gpio_state *state = pinctrl_dev_get_drvdata(pctldev);
 	struct pmic_gpio_pad *pad;
@@ -716,11 +788,46 @@ static void pmic_gpio_config_dbg_show(struct pinctrl_dev *pctldev,
 	}
 }
 
+/**
+ * pmic_gpio_pinconf_group_dbg_show() - Show configuration for a pin group
+ * @pctldev: pinctrl device
+ * @s: seq_file for output
+ * @selector: Group selector
+ *
+ * Shows the configuration for all pins in a group.
+ * Used by the pinconf-groups debugfs interface.
+ */
+static void pmic_gpio_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
+					     struct seq_file *s,
+					     unsigned int selector)
+{
+	const struct group_desc *group;
+	unsigned int pin;
+	int i;
+
+	group = pinctrl_generic_get_group(pctldev, selector);
+	if (!group)
+		return;
+
+	/* Iterate over all pins in the group and show status for each */
+	for (i = 0; i < group->grp.npins; i++) {
+		pin = group->grp.pins[i];
+
+		if (i > 0)
+			seq_puts(s, "\n       ");
+
+		pmic_gpio_pinconf_pin_dbg_show(pctldev, s, pin);
+	}
+}
+
 static const struct pinconf_ops pmic_gpio_pinconf_ops = {
 	.is_generic			= true,
-	.pin_config_group_get		= pmic_gpio_config_get,
-	.pin_config_group_set		= pmic_gpio_config_set,
-	.pin_config_group_dbg_show	= pmic_gpio_config_dbg_show,
+	.pin_config_get			= pmic_gpio_pinconf_pin_get,
+	.pin_config_set			= pmic_gpio_pinconf_pin_set,
+	.pin_config_dbg_show		= pmic_gpio_pinconf_pin_dbg_show,
+	.pin_config_group_get		= pmic_gpio_pinconf_group_get,
+	.pin_config_group_set		= pmic_gpio_pinconf_group_set,
+	.pin_config_group_dbg_show	= pmic_gpio_pinconf_group_dbg_show,
 };
 
 static int pmic_gpio_get_direction(struct gpio_chip *chip, unsigned pin)
@@ -745,7 +852,7 @@ static int pmic_gpio_direction_input(struct gpio_chip *chip, unsigned pin)
 
 	config = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 1);
 
-	return pmic_gpio_config_set(state->ctrl, pin, &config, 1);
+	return pmic_gpio_pinconf_pin_set(state->ctrl, pin, &config, 1);
 }
 
 static int pmic_gpio_direction_output(struct gpio_chip *chip,
@@ -756,7 +863,7 @@ static int pmic_gpio_direction_output(struct gpio_chip *chip,
 
 	config = pinconf_to_config_packed(PIN_CONFIG_LEVEL, val);
 
-	return pmic_gpio_config_set(state->ctrl, pin, &config, 1);
+	return pmic_gpio_pinconf_pin_set(state->ctrl, pin, &config, 1);
 }
 
 static int pmic_gpio_get(struct gpio_chip *chip, unsigned pin)
@@ -788,7 +895,7 @@ static int pmic_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
 
 	config = pinconf_to_config_packed(PIN_CONFIG_LEVEL, value);
 
-	return pmic_gpio_config_set(state->ctrl, pin, &config, 1);
+	return pmic_gpio_pinconf_pin_set(state->ctrl, pin, &config, 1);
 }
 
 static int pmic_gpio_of_xlate(struct gpio_chip *chip,
@@ -810,7 +917,7 @@ static void pmic_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	unsigned i;
 
 	for (i = 0; i < chip->ngpio; i++) {
-		pmic_gpio_config_dbg_show(state->ctrl, s, i);
+		pmic_gpio_pinconf_pin_dbg_show(state->ctrl, s, i);
 		seq_puts(s, "\n");
 	}
 }
@@ -1129,11 +1236,11 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	pctrldesc->custom_conf_items = pmic_conf_items;
 #endif
 
-	for (i = 0; i < npins; i++, pindesc++) {
+	for (i = 0; i < npins; i++) {
 		pad = &pads[i];
-		pindesc->drv_data = pad;
-		pindesc->number = i;
-		pindesc->name = pmic_gpio_groups[i];
+		pindesc[i].drv_data = pad;
+		pindesc[i].number = i;
+		pindesc[i].name = pmic_gpio_groups[i];
 
 		pad->base = reg + i * PMIC_GPIO_ADDRESS_RANGE;
 
@@ -1154,6 +1261,30 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(state->ctrl))
 		return PTR_ERR(state->ctrl);
 
+	/* Register pin groups - each GPIO is a group for standard functions */
+	for (i = 0; i < npins; i++) {
+		ret = pinctrl_generic_add_group(state->ctrl,
+						pmic_gpio_groups[i],
+						&pindesc[i].number, 1, NULL);
+		if (ret < 0) {
+			dev_err(dev, "failed to register group %s\n",
+				pmic_gpio_groups[i]);
+			return ret;
+		}
+	}
+
+	/* Register standard functions - all GPIOs support these */
+	for (i = 0; i < ARRAY_SIZE(pmic_gpio_functions); i++) {
+		ret = pinmux_generic_add_function(state->ctrl,
+						  pmic_gpio_functions[i],
+						  pmic_gpio_groups, npins, NULL);
+		if (ret < 0) {
+			dev_err(dev, "failed to register function %s\n",
+				pmic_gpio_functions[i]);
+			return ret;
+		}
+	}
+
 	parent_node = of_irq_find_parent(state->dev->of_node);
 	if (!parent_node)
 		return -ENXIO;

-- 
2.43.0

Re: [PATCH 3/4] pinctrl: qcom: spmi-gpio: Rearchitect for flexible group support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect registration sequence breaks pinctrl hogs and exposes an uninitialized controller.
- [Low] Missing Kconfig dependencies for generic pinctrl APIs.
--

commit a1bdf02d93e7a07ac0395bf90452b7604f1cb0ac
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

pinctrl: qcom: spmi-gpio: Rearchitect for flexible group support

This commit transitions the spmi-gpio pinctrl driver to use generic pinctrl
groups and functions instead of treating every individual pin as a separate,
fully-featured group.

> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index cdd61dae74cf9..f159c56784b4f 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
[ ... ]
> @@ -253,139 +254,124 @@ static int pmic_gpio_write(struct pmic_gpio_state *state,
>  	return ret;
>  }
>  
> -static int pmic_gpio_get_groups_count(struct pinctrl_dev *pctldev)
> -{
[ ... ]
>  static const struct pinctrl_ops pmic_gpio_pinctrl_ops = {
> -	.get_groups_count	= pmic_gpio_get_groups_count,
> -	.get_group_name		= pmic_gpio_get_group_name,
> -	.get_group_pins		= pmic_gpio_get_group_pins,
> +	.get_groups_count	= pinctrl_generic_get_group_count,
> +	.get_group_name		= pinctrl_generic_get_group_name,
> +	.get_group_pins		= pinctrl_generic_get_group_pins,
>  	.dt_node_to_map		= pinconf_generic_dt_node_to_map_group,
>  	.dt_free_map		= pinctrl_utils_free_map,
>  };

[Severity: Low]
The driver was migrated to use generic pinctrl APIs, but does the Kconfig for
CONFIG_PINCTRL_QCOM_SPMI_PMIC select GENERIC_PINCTRL_GROUPS and
GENERIC_PINMUX_FUNCTIONS?

If these are not enabled by another driver during a randconfig build, could
this lead to unresolved symbol errors when using these generic APIs?

[ ... ]
> @@ -1129,11 +1236,11 @@ static int pmic_gpio_probe(struct platform_device *pdev)
[ ... ]
>  	state->chip = pmic_gpio_gpio_template;
>  	state->chip.parent = dev;
>  	state->chip.base = -1;
>  	state->chip.ngpio = npins;
>  	state->chip.label = dev_name(dev);
>  	state->chip.of_gpio_n_cells = 2;
>  	state->chip.can_sleep = false;
>  
>  	state->ctrl = devm_pinctrl_register(dev, pctrldesc, state);
>  	if (IS_ERR(state->ctrl))
>  		return PTR_ERR(state->ctrl);
>  
> +	/* Register pin groups - each GPIO is a group for standard functions */
> +	for (i = 0; i < npins; i++) {
> +		ret = pinctrl_generic_add_group(state->ctrl,

[Severity: High]
Does registering the pinctrl device before adding the generic groups and
functions break pinctrl hogs?

devm_pinctrl_register() internally invokes pinctrl_enable(), which attempts
to immediately parse and claim pinctrl hogs from the device tree via
pinctrl_claim_hogs(). Because the generic group and function radix trees are
empty at this exact moment, pinmux_func_name_to_selector() and
pinctrl_get_group_selector() will fail, silently dropping the hardware hogs.

Could this also expose the pinctrl device to concurrent consumers before it
is fully populated? Should the driver use devm_pinctrl_register_and_init(),
populate its generic structures, and then explicitly call pinctrl_enable()?

> +						pmic_gpio_groups[i],
> +						&pindesc[i].number, 1, NULL);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to register group %s\n",
> +				pmic_gpio_groups[i]);
> +			return ret;
> +		}
> +	}
> +
> +	/* Register standard functions - all GPIOs support these */
> +	for (i = 0; i < ARRAY_SIZE(pmic_gpio_functions); i++) {
> +		ret = pinmux_generic_add_function(state->ctrl,
> +						  pmic_gpio_functions[i],
> +						  pmic_gpio_groups, npins, NULL);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to register function %s\n",
> +				pmic_gpio_functions[i]);
> +			return ret;
> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-pinctrl-level-shifter-v1-0-1965461d0a7c@oss.qualcomm.com?part=3

[PATCH 4/4] pinctrl: qcom: spmi-gpio: Add level-shifter function support

[Fenglin Wu]

PMH0101 introduces bidirectional level shifter (BIDIR_LVL_SHIFTER)
modules that provide voltage translation between 1.2 V and 1.8 V power
domains for open-drain signals, such as connecting a camera sensor with
1.8 V I2C IO to a SoC I2C bus operating at 1.2 V IO.

Each BIDIR_LVL_SHIFTER shares its two physical pins with a corresponding
pair of GPIO modules. When used, the associated GPIOs need to keep
disabled, with no GPIO function or configuration enabled. The
BIDIR_LVL_SHIFTER then operates as a bidirectional open-drain voltage
translator, using fixed 1.2 V and 1.8 V reference voltages on each side
and relying on external board-level pull-up resistors for open-drain
drive control.

From a system perspective, a BIDIR_LVL_SHIFTER may be used on a serial
bus shared by multiple clients managed by different subsystems. As a
result, its enable control is shared and centrally managed by AOP,
with each subsystem voting for the enable state via an RPMh “XOB”
resource.

Add a new "level-shifter" function to the SPMI GPIO driver to support
BIDIR_LVL_SHIFTER operation considering that BIDIR_LVL_SHIFTER shares
physical pins with GPIOs, it must be mutually exclusive with all
existing functions and configurations. Additionally, limit the
"level-shifter" function to specific GPIO pairs that share physical
pins with the BIDIR_LVL_SHIFTER module by creating pingroups and
linking them to the "level-shifter" function.

With this change, the BIDIR_LVL_SHIFTER could be controlled with
following dtsi nodes:

    pmh0101-ls1-en {
    	groups = "gpio11, gpio12";
    	function = "level-shifter";
    	qcom,1p2v-1p8v-ls-en = <1>;
    };

    pmh0101-ls1-dis {
    	groups = "gpio11, gpio12";
    	function = "level-shifter";
    	qcom,1p2v-1p8v-ls-en = <0>;
    };

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 410 +++++++++++++++++++++++++------
 1 file changed, 335 insertions(+), 75 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f159c56784b4..d0144dbb7cfc 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -23,6 +23,9 @@
 
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 
+#include <soc/qcom/cmd-db.h>
+#include <soc/qcom/rpmh.h>
+
 #include "../core.h"
 #include "../pinmux.h"
 #include "../pinctrl-utils.h"
@@ -118,12 +121,16 @@
 /* PMIC_GPIO_REG_LV_MV_ANA_PASS_THRU_SEL */
 #define PMIC_GPIO_LV_MV_ANA_MUX_SEL_MASK		0x3
 
+/* Level shifter register offset */
+#define REG_LS_ENABLE_OFFSET			4
+
 /* Qualcomm specific pin configurations */
 #define PMIC_GPIO_CONF_PULL_UP			(PIN_CONFIG_END + 1)
 #define PMIC_GPIO_CONF_STRENGTH			(PIN_CONFIG_END + 2)
 #define PMIC_GPIO_CONF_ATEST			(PIN_CONFIG_END + 3)
 #define PMIC_GPIO_CONF_ANALOG_PASS		(PIN_CONFIG_END + 4)
 #define PMIC_GPIO_CONF_DTEST_BUFFER		(PIN_CONFIG_END + 5)
+#define PMIC_GPIO_CONF_LS_ENABLE		(PIN_CONFIG_END + 6)
 
 /* The index of each function in pmic_gpio_functions[] array */
 enum pmic_gpio_func_index {
@@ -137,6 +144,7 @@ enum pmic_gpio_func_index {
 	PMIC_GPIO_FUNC_INDEX_DTEST2,
 	PMIC_GPIO_FUNC_INDEX_DTEST3,
 	PMIC_GPIO_FUNC_INDEX_DTEST4,
+	PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER,
 };
 
 /**
@@ -183,25 +191,86 @@ struct pmic_gpio_state {
 	struct regmap	*map;
 	struct pinctrl_dev *ctrl;
 	struct gpio_chip chip;
+	struct device	*rpmh_dev;
+	const char	*pmic_id;
 	u8 usid;
 	u8 pid_base;
 };
 
+/**
+ * struct ls_config - Level shifter configuration
+ * @name: Pinctrl group name (e.g., "gpio11, gpio12")
+ * @rpmh_prefix: RPMh resource name prefix (e.g., "LS1")
+ * @pins: Array of pin numbers for this level shifter pair
+ */
+struct ls_config {
+	const char *name;
+	const char *rpmh_prefix;
+	const unsigned int *pins;
+};
+
+/**
+ * struct ls_pingroup_data - Level shifter pin group data
+ * @config: Pointer to level shifter configuration
+ * @level_shifter_addr: RPMh address for level shifter control
+ * @ls_enabled: Shadowed enable state, updated on each successful RPMh write
+ */
+struct ls_pingroup_data {
+	struct ls_config *config;
+	u32 level_shifter_addr;
+	bool ls_enabled;
+};
+
+/**
+ * struct pmic_gpio_hw_data - Hardware-specific data
+ * @npins: Number of GPIO pins
+ * @ls_config: Array of level shifter configurations
+ * @num_ls: Number of level shifters
+ */
+struct pmic_gpio_hw_data {
+	u32 npins;
+	struct ls_config *ls_config;
+	u32 num_ls;
+};
+
+/* Macro to create hardware data inline */
+#define PMIC_GPIO_HW_DATA(n, ls, num) \
+	(&(const struct pmic_gpio_hw_data) { \
+		.npins = n, \
+		.ls_config = ls, \
+		.num_ls = num, \
+	})
+
+/* Level shifter pin groups for PMH0101 */
+static const unsigned int ls1_pins[] = { 10, 11 }; /* GPIO11, GPIO12 */
+static const unsigned int ls2_pins[] = { 12, 13 }; /* GPIO13, GPIO14 */
+static const unsigned int ls3_pins[] = { 14, 15 }; /* GPIO15, GPIO16 */
+static const unsigned int ls4_pins[] = { 16, 17 }; /* GPIO17, GPIO18 */
+
+static struct ls_config pmh0101_ls_configs[] = {
+	{ "gpio11, gpio12", "LS1", ls1_pins },
+	{ "gpio13, gpio14", "LS2", ls2_pins },
+	{ "gpio15, gpio16", "LS3", ls3_pins },
+	{ "gpio17, gpio18", "LS4", ls4_pins },
+};
+
 static const struct pinconf_generic_params pmic_gpio_bindings[] = {
 	{"qcom,pull-up-strength",	PMIC_GPIO_CONF_PULL_UP,		0},
 	{"qcom,drive-strength",		PMIC_GPIO_CONF_STRENGTH,	0},
 	{"qcom,atest",			PMIC_GPIO_CONF_ATEST,		0},
 	{"qcom,analog-pass",		PMIC_GPIO_CONF_ANALOG_PASS,	0},
-	{"qcom,dtest-buffer",           PMIC_GPIO_CONF_DTEST_BUFFER,    0},
+	{"qcom,dtest-buffer",		PMIC_GPIO_CONF_DTEST_BUFFER,	0},
+	{"qcom,1p2v-1p8v-ls-en",	PMIC_GPIO_CONF_LS_ENABLE,	0},
 };
 
 #ifdef CONFIG_DEBUG_FS
 static const struct pin_config_item pmic_conf_items[ARRAY_SIZE(pmic_gpio_bindings)] = {
-	PCONFDUMP(PMIC_GPIO_CONF_PULL_UP,  "pull up strength", NULL, true),
-	PCONFDUMP(PMIC_GPIO_CONF_STRENGTH, "drive-strength", NULL, true),
-	PCONFDUMP(PMIC_GPIO_CONF_ATEST, "atest", NULL, true),
-	PCONFDUMP(PMIC_GPIO_CONF_ANALOG_PASS, "analog-pass", NULL, true),
-	PCONFDUMP(PMIC_GPIO_CONF_DTEST_BUFFER, "dtest-buffer", NULL, true),
+	PCONFDUMP(PMIC_GPIO_CONF_PULL_UP,      "pull up strength", NULL, true),
+	PCONFDUMP(PMIC_GPIO_CONF_STRENGTH,     "drive-strength",   NULL, true),
+	PCONFDUMP(PMIC_GPIO_CONF_ATEST,        "atest",            NULL, true),
+	PCONFDUMP(PMIC_GPIO_CONF_ANALOG_PASS,  "analog-pass",      NULL, true),
+	PCONFDUMP(PMIC_GPIO_CONF_DTEST_BUFFER, "dtest-buffer",     NULL, true),
+	PCONFDUMP(PMIC_GPIO_CONF_LS_ENABLE,    "ls-enable",        NULL, true),
 };
 #endif
 
@@ -262,6 +331,41 @@ static const struct pinctrl_ops pmic_gpio_pinctrl_ops = {
 	.dt_free_map		= pinctrl_utils_free_map,
 };
 
+/**
+ * pmic_gpio_set_ls_rpmh() - Send RPMh XOB vote for a level shifter
+ * @state: PMIC GPIO state
+ * @ls_data: Level shifter pin group data
+ * @enable: true to enable, false to disable
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int pmic_gpio_set_ls_rpmh(struct pmic_gpio_state *state,
+				  struct ls_pingroup_data *ls_data,
+				  bool enable)
+{
+	struct tcs_cmd cmd = { };
+	int ret;
+
+	if (!ls_data->level_shifter_addr) {
+		dev_err(state->dev, "Level shifter address not configured\n");
+		return -EINVAL;
+	}
+
+	if (!state->rpmh_dev) {
+		dev_err(state->dev, "RPMh device not available\n");
+		return -ENODEV;
+	}
+
+	cmd.addr = ls_data->level_shifter_addr + REG_LS_ENABLE_OFFSET;
+	cmd.data = enable ? 1 : 0;
+
+	ret = rpmh_write_ctrlr(state->rpmh_dev, RPMH_ACTIVE_ONLY_STATE, &cmd, 1);
+	if (!ret)
+		ls_data->ls_enabled = enable;
+
+	return ret;
+}
+
 static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 				unsigned int selector)
 {
@@ -271,7 +375,7 @@ static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 	unsigned int val, pin, func;
 	int ret, i;
 
-	if (function > PMIC_GPIO_FUNC_INDEX_DTEST4) {
+	if (function > PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER) {
 		pr_err("function: %d is not defined\n", function);
 		return -EINVAL;
 	}
@@ -280,6 +384,28 @@ static int pmic_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 	if (!group)
 		return -EINVAL;
 
+	if (function == PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER) {
+		if (!group->data)
+			return -EINVAL;
+
+		/*
+		 * Disable both GPIO pads in the pair. The RPMh XOB vote is
+		 * sent separately via the qcom,1p2v-1p8v-ls-en pinconf parameter.
+		 */
+		for (i = 0; i < group->grp.npins; i++) {
+			pin = group->grp.pins[i];
+			pad = pctldev->desc->pins[pin].drv_data;
+
+			ret = pmic_gpio_write(state, pad, PMIC_GPIO_REG_EN_CTL, 0);
+			if (ret < 0)
+				return ret;
+
+			pad->is_enabled = false;
+			pad->function = PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER;
+		}
+		return 0;
+	}
+
 	/* For standard functions, iterate over all pins in the group */
 	for (i = 0; i < group->grp.npins; i++) {
 		pin = group->grp.pins[i];
@@ -597,6 +723,9 @@ static int pmic_gpio_pinconf_pin_set(struct pinctrl_dev *pctldev,
 				return -EINVAL;
 			pad->dtest_buffer = arg;
 			break;
+		case PMIC_GPIO_CONF_LS_ENABLE:
+			/* Group-level only; handled in pmic_gpio_pinconf_group_set */
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -690,13 +819,14 @@ static int pmic_gpio_pinconf_pin_set(struct pinctrl_dev *pctldev,
  * @configs: Array of configuration parameters
  * @nconfs: Number of configurations
  *
- * Iterates over all pins in the group and applies config to each.
+ * Handles group-level parameters (e.g., LS enable) before iterating per-pin.
  */
 static int pmic_gpio_pinconf_group_set(struct pinctrl_dev *pctldev,
 				       unsigned int selector,
 				       unsigned long *configs,
 				       unsigned int nconfs)
 {
+	struct pmic_gpio_state *state = pinctrl_dev_get_drvdata(pctldev);
 	const struct group_desc *group;
 	unsigned int pin;
 	int i, ret;
@@ -705,7 +835,25 @@ static int pmic_gpio_pinconf_group_set(struct pinctrl_dev *pctldev,
 	if (!group)
 		return -EINVAL;
 
-	/* Iterate over all pins in the group and apply config to each */
+	/* Handle group-level LS_ENABLE before iterating per-pin configs */
+	for (i = 0; i < nconfs; i++) {
+		if (pinconf_to_config_param(configs[i]) != PMIC_GPIO_CONF_LS_ENABLE)
+			continue;
+
+		if (!group->data) {
+			dev_err(state->dev,
+				"qcom,1p2v-1p8v-ls-en is only valid for level-shifter groups\n");
+			return -EINVAL;
+		}
+
+		ret = pmic_gpio_set_ls_rpmh(state,
+					    (struct ls_pingroup_data *)group->data,
+					    !!pinconf_to_config_argument(configs[i]));
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Apply per-pin configs to each pin in the group */
 	for (i = 0; i < group->grp.npins; i++) {
 		pin = group->grp.pins[i];
 
@@ -794,8 +942,8 @@ static void pmic_gpio_pinconf_pin_dbg_show(struct pinctrl_dev *pctldev,
  * @s: seq_file for output
  * @selector: Group selector
  *
- * Shows the configuration for all pins in a group.
- * Used by the pinconf-groups debugfs interface.
+ * Shows the configuration for all pins in a group. For level-shifter groups,
+ * also prints the RPMh address as a preamble.
  */
 static void pmic_gpio_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
 					     struct seq_file *s,
@@ -809,6 +957,14 @@ static void pmic_gpio_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
 	if (!group)
 		return;
 
+	/* For level-shifter groups, print the enable status as preamble */
+	if (group->data) {
+		const struct ls_pingroup_data *ls_data = group->data;
+
+		seq_printf(s, ", level-shifter (%s)\n       ",
+			   ls_data->ls_enabled ? "enabled" : "disabled");
+	}
+
 	/* Iterate over all pins in the group and show status for each */
 	for (i = 0; i < group->grp.npins; i++) {
 		pin = group->grp.pins[i];
@@ -1177,6 +1333,96 @@ static const struct irq_chip spmi_gpio_irq_chip = {
 	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
+/**
+ * pmic_gpio_register_level_shifters() - Register level-shifter groups and function
+ * @state: PMIC GPIO state
+ * @hw_data: Hardware-specific data containing level-shifter configurations
+ *
+ * This function registers level-shifter support by:
+ * 1. Getting RPMh device reference and PMIC ID from device tree
+ * 2. Registering each level-shifter pair as a multi-pin group
+ * 3. Getting RPMh addresses from cmd_db for each level-shifter
+ * 4. Registering the level-shifter function once with all valid groups
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int pmic_gpio_register_level_shifters(struct pmic_gpio_state *state,
+					     const struct pmic_gpio_hw_data *hw_data)
+{
+	struct device *dev = state->dev;
+	const char **ls_group_names;
+	int ret, i;
+
+	/* Get RPMh device reference for level shifter control */
+	state->rpmh_dev = rpmh_get_ctrlr_dev(dev);
+	if (IS_ERR(state->rpmh_dev))
+		return dev_err_probe(dev, PTR_ERR(state->rpmh_dev),
+				     "Level shifter needs rpmh device\n");
+
+	/* Get PMIC ID from device tree for RPMh resource name composition */
+	ret = of_property_read_string(dev->of_node, "qcom,pmic-id",
+				       &state->pmic_id);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "get qcom,pmic-id failed for rpmh resource\n");
+
+	/* Allocate array to hold all level-shifter group names */
+	ls_group_names = devm_kcalloc(dev, hw_data->num_ls,
+				      sizeof(*ls_group_names), GFP_KERNEL);
+	if (!ls_group_names)
+		return -ENOMEM;
+
+	/* Register each level-shifter pair as a multi-pin group */
+	for (i = 0; i < hw_data->num_ls; i++) {
+		struct ls_config *ls = &hw_data->ls_config[i];
+		struct ls_pingroup_data *ls_group_data;
+		char rpmh_resource_name[32];
+
+		/* Allocate ls_pingroup_data for this level shifter */
+		ls_group_data = devm_kzalloc(dev, sizeof(*ls_group_data),
+					     GFP_KERNEL);
+		if (!ls_group_data)
+			return -ENOMEM;
+
+		ls_group_data->config = ls;
+
+		/* Compose RPMh resource name and get address from cmd_db */
+		if (state->pmic_id && ls->rpmh_prefix) {
+			snprintf(rpmh_resource_name,
+				 sizeof(rpmh_resource_name),
+				 "%s%s", ls->rpmh_prefix, state->pmic_id);
+			ls_group_data->level_shifter_addr =
+				cmd_db_read_addr(rpmh_resource_name);
+			if (!ls_group_data->level_shifter_addr)
+				return dev_err_probe(dev, -ENODEV,
+						     "RPMh resource %s not found in cmd_db\n",
+						     rpmh_resource_name);
+		}
+
+		/* Store group name for function registration */
+		ls_group_names[i] = ls->name;
+
+		/* Register the multi-pin group using the level-shifter name */
+		ret = pinctrl_generic_add_group(state->ctrl, ls->name,
+						ls->pins, 2, ls_group_data);
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "Failed to register level-shifter group %s\n",
+					     ls->name);
+	}
+
+	/* Register level-shifter function once with all valid groups */
+	ret = pinmux_generic_add_function(state->ctrl,
+					  PMIC_GPIO_FUNC_LEVEL_SHIFTER,
+					  ls_group_names, hw_data->num_ls,
+					  NULL);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Failed to register level-shifter function\n");
+
+	return 0;
+}
+
 static int pmic_gpio_probe(struct platform_device *pdev)
 {
 	struct irq_domain *parent_domain;
@@ -1188,6 +1434,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	struct pmic_gpio_state *state;
 	struct gpio_irq_chip *girq;
 	const struct spmi_device *parent_spmi_dev;
+	const struct pmic_gpio_hw_data *hw_data;
 	int ret, npins, i;
 	u32 reg;
 
@@ -1197,7 +1444,13 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	npins = (uintptr_t) device_get_match_data(&pdev->dev);
+	hw_data = device_get_match_data(&pdev->dev);
+	if (!hw_data) {
+		dev_err(dev, "no match data found\n");
+		return -EINVAL;
+	}
+
+	npins = hw_data->npins;
 
 	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
 	if (!state)
@@ -1285,6 +1538,13 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Register level-shifter groups and function if supported */
+	if (hw_data->ls_config && hw_data->num_ls) {
+		ret = pmic_gpio_register_level_shifters(state, hw_data);
+		if (ret < 0)
+			return ret;
+	}
+
 	parent_node = of_irq_find_parent(state->dev->of_node);
 	if (!parent_node)
 		return -ENXIO;
@@ -1345,80 +1605,80 @@ static void pmic_gpio_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id pmic_gpio_of_match[] = {
-	{ .compatible = "qcom,pm2250-gpio", .data = (void *) 10 },
+	{ .compatible = "qcom,pm2250-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
 	/* pm660 has 13 GPIOs with holes on 1, 5, 6, 7, 8 and 10 */
-	{ .compatible = "qcom,pm660-gpio", .data = (void *) 13 },
+	{ .compatible = "qcom,pm660-gpio", .data = PMIC_GPIO_HW_DATA(13, NULL, 0) },
 	/* pm660l has 12 GPIOs with holes on 1, 2, 10, 11 and 12 */
-	{ .compatible = "qcom,pm660l-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pm6125-gpio", .data = (void *) 9 },
-	{ .compatible = "qcom,pm6150-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pm6150l-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pm6350-gpio", .data = (void *) 9 },
-	{ .compatible = "qcom,pm6450-gpio", .data = (void *) 9 },
-	{ .compatible = "qcom,pm7250b-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pm7325-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pm7550-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pm7550ba-gpio", .data = (void *) 8},
-	{ .compatible = "qcom,pm8005-gpio", .data = (void *) 4 },
-	{ .compatible = "qcom,pm8010-gpio", .data = (void *) 2 },
-	{ .compatible = "qcom,pm8019-gpio", .data = (void *) 6 },
+	{ .compatible = "qcom,pm660l-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pm6125-gpio", .data = PMIC_GPIO_HW_DATA(9, NULL, 0) },
+	{ .compatible = "qcom,pm6150-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pm6150l-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pm6350-gpio", .data = PMIC_GPIO_HW_DATA(9, NULL, 0) },
+	{ .compatible = "qcom,pm6450-gpio", .data = PMIC_GPIO_HW_DATA(9, NULL, 0) },
+	{ .compatible = "qcom,pm7250b-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pm7325-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pm7550-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pm7550ba-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pm8005-gpio", .data = PMIC_GPIO_HW_DATA(4, NULL, 0) },
+	{ .compatible = "qcom,pm8010-gpio", .data = PMIC_GPIO_HW_DATA(2, NULL, 0) },
+	{ .compatible = "qcom,pm8019-gpio", .data = PMIC_GPIO_HW_DATA(6, NULL, 0) },
 	/* pm8150 has 10 GPIOs with holes on 2, 5, 7 and 8 */
-	{ .compatible = "qcom,pm8150-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pmc8180-gpio", .data = (void *) 10 },
+	{ .compatible = "qcom,pm8150-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pmc8180-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
 	/* pm8150b has 12 GPIOs with holes on 3, r and 7 */
-	{ .compatible = "qcom,pm8150b-gpio", .data = (void *) 12 },
+	{ .compatible = "qcom,pm8150b-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
 	/* pm8150l has 12 GPIOs with holes on 7 */
-	{ .compatible = "qcom,pm8150l-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pmc8180c-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pm8226-gpio", .data = (void *) 8 },
-	{ .compatible = "qcom,pm8350-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pm8350b-gpio", .data = (void *) 8 },
-	{ .compatible = "qcom,pm8350c-gpio", .data = (void *) 9 },
-	{ .compatible = "qcom,pm8450-gpio", .data = (void *) 4 },
-	{ .compatible = "qcom,pm8550-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pm8550b-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pm8550ve-gpio", .data = (void *) 8 },
-	{ .compatible = "qcom,pm8550vs-gpio", .data = (void *) 6 },
-	{ .compatible = "qcom,pm8916-gpio", .data = (void *) 4 },
+	{ .compatible = "qcom,pm8150l-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pmc8180c-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pm8226-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pm8350-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pm8350b-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pm8350c-gpio", .data = PMIC_GPIO_HW_DATA(9, NULL, 0) },
+	{ .compatible = "qcom,pm8450-gpio", .data = PMIC_GPIO_HW_DATA(4, NULL, 0) },
+	{ .compatible = "qcom,pm8550-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pm8550b-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pm8550ve-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pm8550vs-gpio", .data = PMIC_GPIO_HW_DATA(6, NULL, 0) },
+	{ .compatible = "qcom,pm8916-gpio", .data = PMIC_GPIO_HW_DATA(4, NULL, 0) },
 	/* pm8937 has 8 GPIOs with holes on 3, 4 and 6 */
-	{ .compatible = "qcom,pm8937-gpio", .data = (void *) 8 },
-	{ .compatible = "qcom,pm8941-gpio", .data = (void *) 36 },
+	{ .compatible = "qcom,pm8937-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pm8941-gpio", .data = PMIC_GPIO_HW_DATA(36, NULL, 0) },
 	/* pm8950 has 8 GPIOs with holes on 3 */
-	{ .compatible = "qcom,pm8950-gpio", .data = (void *) 8 },
+	{ .compatible = "qcom,pm8950-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
 	/* pm8953 has 8 GPIOs with holes on 3 and 6 */
-	{ .compatible = "qcom,pm8953-gpio", .data = (void *) 8 },
-	{ .compatible = "qcom,pm8994-gpio", .data = (void *) 22 },
-	{ .compatible = "qcom,pm8998-gpio", .data = (void *) 26 },
-	{ .compatible = "qcom,pma8084-gpio", .data = (void *) 22 },
-	{ .compatible = "qcom,pmc8380-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pmcx0102-gpio", .data = (void *)14 },
-	{ .compatible = "qcom,pmd8028-gpio", .data = (void *) 4 },
-	{ .compatible = "qcom,pmh0101-gpio", .data = (void *)18 },
-	{ .compatible = "qcom,pmh0104-gpio", .data = (void *)8 },
-	{ .compatible = "qcom,pmh0110-gpio", .data = (void *)14 },
-	{ .compatible = "qcom,pmi632-gpio", .data = (void *) 8 },
-	{ .compatible = "qcom,pmi8950-gpio", .data = (void *) 2 },
-	{ .compatible = "qcom,pmi8994-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pmi8998-gpio", .data = (void *) 14 },
-	{ .compatible = "qcom,pmih0108-gpio", .data = (void *) 18 },
-	{ .compatible = "qcom,pmiv0104-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pmk8350-gpio", .data = (void *) 4 },
-	{ .compatible = "qcom,pmk8550-gpio", .data = (void *) 6 },
-	{ .compatible = "qcom,pmk8850-gpio", .data = (void *)8 },
-	{ .compatible = "qcom,pmm8155au-gpio", .data = (void *) 10 },
-	{ .compatible = "qcom,pmm8654au-gpio", .data = (void *) 12 },
+	{ .compatible = "qcom,pm8953-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pm8994-gpio", .data = PMIC_GPIO_HW_DATA(22, NULL, 0) },
+	{ .compatible = "qcom,pm8998-gpio", .data = PMIC_GPIO_HW_DATA(26, NULL, 0) },
+	{ .compatible = "qcom,pma8084-gpio", .data = PMIC_GPIO_HW_DATA(22, NULL, 0) },
+	{ .compatible = "qcom,pmc8380-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pmcx0102-gpio", .data = PMIC_GPIO_HW_DATA(14, NULL, 0) },
+	{ .compatible = "qcom,pmd8028-gpio", .data = PMIC_GPIO_HW_DATA(4, NULL, 0) },
+	{ .compatible = "qcom,pmh0101-gpio", .data = PMIC_GPIO_HW_DATA(18, pmh0101_ls_configs, 4) },
+	{ .compatible = "qcom,pmh0104-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pmh0110-gpio", .data = PMIC_GPIO_HW_DATA(14, NULL, 0) },
+	{ .compatible = "qcom,pmi632-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pmi8950-gpio", .data = PMIC_GPIO_HW_DATA(2, NULL, 0) },
+	{ .compatible = "qcom,pmi8994-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pmi8998-gpio", .data = PMIC_GPIO_HW_DATA(14, NULL, 0) },
+	{ .compatible = "qcom,pmih0108-gpio", .data = PMIC_GPIO_HW_DATA(18, NULL, 0) },
+	{ .compatible = "qcom,pmiv0104-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pmk8350-gpio", .data = PMIC_GPIO_HW_DATA(4, NULL, 0) },
+	{ .compatible = "qcom,pmk8550-gpio", .data = PMIC_GPIO_HW_DATA(6, NULL, 0) },
+	{ .compatible = "qcom,pmk8850-gpio", .data = PMIC_GPIO_HW_DATA(8, NULL, 0) },
+	{ .compatible = "qcom,pmm8155au-gpio", .data = PMIC_GPIO_HW_DATA(10, NULL, 0) },
+	{ .compatible = "qcom,pmm8654au-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
 	/* pmp8074 has 12 GPIOs with holes on 1 and 12 */
-	{ .compatible = "qcom,pmp8074-gpio", .data = (void *) 12 },
-	{ .compatible = "qcom,pmr735a-gpio", .data = (void *) 4 },
-	{ .compatible = "qcom,pmr735b-gpio", .data = (void *) 4 },
-	{ .compatible = "qcom,pmr735d-gpio", .data = (void *) 2 },
+	{ .compatible = "qcom,pmp8074-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
+	{ .compatible = "qcom,pmr735a-gpio", .data = PMIC_GPIO_HW_DATA(4, NULL, 0) },
+	{ .compatible = "qcom,pmr735b-gpio", .data = PMIC_GPIO_HW_DATA(4, NULL, 0) },
+	{ .compatible = "qcom,pmr735d-gpio", .data = PMIC_GPIO_HW_DATA(2, NULL, 0) },
 	/* pms405 has 12 GPIOs with holes on 1, 9, and 10 */
-	{ .compatible = "qcom,pms405-gpio", .data = (void *) 12 },
+	{ .compatible = "qcom,pms405-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
 	/* pmx55 has 11 GPIOs with holes on 3, 7, 10, 11 */
-	{ .compatible = "qcom,pmx55-gpio", .data = (void *) 11 },
-	{ .compatible = "qcom,pmx65-gpio", .data = (void *) 16 },
-	{ .compatible = "qcom,pmx75-gpio", .data = (void *) 16 },
-	{ .compatible = "qcom,pmxr2230-gpio", .data = (void *) 12 },
+	{ .compatible = "qcom,pmx55-gpio", .data = PMIC_GPIO_HW_DATA(11, NULL, 0) },
+	{ .compatible = "qcom,pmx65-gpio", .data = PMIC_GPIO_HW_DATA(16, NULL, 0) },
+	{ .compatible = "qcom,pmx75-gpio", .data = PMIC_GPIO_HW_DATA(16, NULL, 0) },
+	{ .compatible = "qcom,pmxr2230-gpio", .data = PMIC_GPIO_HW_DATA(12, NULL, 0) },
 	{ },
 };
 

-- 
2.43.0

Re: [PATCH 4/4] pinctrl: qcom: spmi-gpio: Add level-shifter function support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Out-of-bounds array access on the `pmic_gpio_functions` array when reading pin configurations from debugfs.
- [High] Applying pin configurations to a level-shifter group incorrectly re-enables the disabled GPIO pads and writes out-of-bounds function values, corrupting hardware mode control registers.
- [High] The driver fails to properly defer probing if the command DB (`cmd-db`) is not yet initialized.
- [High] The unconditional registration of level shifters breaks the probe sequence for existing devices that do not utilize them.
--

commit 0ccd51f9c406d2406a12a93ef3944f4bac8c5bdb
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

pinctrl: qcom: spmi-gpio: Add level-shifter function support

This commit introduces support for bidirectional level shifter modules
(BIDIR_LVL_SHIFTER) to the SPMI GPIO driver. It adds a new "level-shifter"
function to manage GPIO pairs that share physical pins with the level shifter,
controlling their state via RPMh.

> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index f159c56784b4f..d0144dbb7cfc8 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c

[ ... ]

> @@ -137,6 +144,7 @@ enum pmic_gpio_func_index {
>  	PMIC_GPIO_FUNC_INDEX_DTEST2,
>  	PMIC_GPIO_FUNC_INDEX_DTEST3,
>  	PMIC_GPIO_FUNC_INDEX_DTEST4,
> +	PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER,
>  };

[Severity: High]
Does this code cause an out-of-bounds array access when reading pin
configurations from debugfs?

If a pin is configured as a level-shifter and its pad is enabled, reading the
pin's debugfs node will index pmic_gpio_functions[10] (or [12] for non-LV/MV
subtypes due to the DTEST offset). Since PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER was
added to the enum but pmic_gpio_functions was not updated, it seems this could
result in an out-of-bounds read that can crash the kernel or leak memory.

[ ... ]

> @@ -705,7 +835,25 @@ static int pmic_gpio_pinconf_group_set(struct pinctrl_dev *pctldev,
>  	if (!group)
>  		return -EINVAL;
>  
> -	/* Iterate over all pins in the group and apply config to each */
> +	/* Handle group-level LS_ENABLE before iterating per-pin configs */
> +	for (i = 0; i < nconfs; i++) {
> +		if (pinconf_to_config_param(configs[i]) != PMIC_GPIO_CONF_LS_ENABLE)
> +			continue;
> +
> +		if (!group->data) {
> +			dev_err(state->dev,
> +				"qcom,1p2v-1p8v-ls-en is only valid for level-shifter groups\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = pmic_gpio_set_ls_rpmh(state,
> +					    (struct ls_pingroup_data *)group->data,
> +					    !!pinconf_to_config_argument(configs[i]));
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* Apply per-pin configs to each pin in the group */
>  	for (i = 0; i < group->grp.npins; i++) {
>  		pin = group->grp.pins[i];
>  
> +		ret = pmic_gpio_pinconf_pin_set(pctldev, pin, configs, nconfs);
> +		if (ret < 0)
> +			return ret;
>  	}

[Severity: High]
Can applying pin configurations to a level-shifter group incorrectly re-enable
the disabled GPIO pads and write out-of-bounds function values?

In pmic_gpio_set_mux(), level-shifter pins correctly have their pads disabled.
However, when group-level pin configurations are applied
(e.g., qcom,1p2v-1p8v-ls-en), pmic_gpio_pinconf_group_set() delegates to
pmic_gpio_pinconf_pin_set().

The per-pin function unconditionally sets pad->is_enabled = true and writes it
to PMIC_GPIO_REG_EN_CTL, re-enabling the pads. Furthermore, it shifts
pad->function (10) left by 1 (PMIC_GPIO_REG_MODE_FUNCTION_SHIFT), which
overflows the 3-bit mode mask and overwrites adjacent bits in
PMIC_GPIO_REG_MODE_CTL.

[ ... ]

> @@ -1177,6 +1333,96 @@ static const struct irq_chip spmi_gpio_irq_chip = {
>  	GPIOCHIP_IRQ_RESOURCE_HELPERS,
>  };
>  
> +/**
> + * pmic_gpio_register_level_shifters() - Register level-shifter groups and function
> + * @state: PMIC GPIO state
> + * @hw_data: Hardware-specific data containing level-shifter configurations
> + *
> + * This function registers level-shifter support by:
> + * 1. Getting RPMh device reference and PMIC ID from device tree
> + * 2. Registering each level-shifter pair as a multi-pin group
> + * 3. Getting RPMh addresses from cmd_db for each level-shifter
> + * 4. Registering the level-shifter function once with all valid groups
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int pmic_gpio_register_level_shifters(struct pmic_gpio_state *state,
> +					     const struct pmic_gpio_hw_data *hw_data)
> +{
> +	struct device *dev = state->dev;
> +	const char **ls_group_names;
> +	int ret, i;
> +
> +	/* Get RPMh device reference for level shifter control */
> +	state->rpmh_dev = rpmh_get_ctrlr_dev(dev);
> +	if (IS_ERR(state->rpmh_dev))
> +		return dev_err_probe(dev, PTR_ERR(state->rpmh_dev),
> +				     "Level shifter needs rpmh device\n");
> +
> +	/* Get PMIC ID from device tree for RPMh resource name composition */
> +	ret = of_property_read_string(dev->of_node, "qcom,pmic-id",
> +				       &state->pmic_id);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "get qcom,pmic-id failed for rpmh resource\n");

[Severity: High]
Does the unconditional registration of level shifters break the probe sequence
for existing devices that do not utilize them?

pmic_gpio_probe() unconditionally invokes pmic_gpio_register_level_shifters()
for any device matching qcom,pmh0101-gpio. If rpmh_get_ctrlr_dev(),
qcom,pmic-id property parsing, or cmd_db_read_addr() fails, the function
returns an error that aborts the entire GPIO controller probe. Existing devices
do not possess the newly introduced qcom,rpmh or qcom,pmic-id properties,
causing them to encounter a fatal -ENODEV during probe.

> +
> +	/* Allocate array to hold all level-shifter group names */
> +	ls_group_names = devm_kcalloc(dev, hw_data->num_ls,
> +				      sizeof(*ls_group_names), GFP_KERNEL);
> +	if (!ls_group_names)
> +		return -ENOMEM;
> +
> +	/* Register each level-shifter pair as a multi-pin group */
> +	for (i = 0; i < hw_data->num_ls; i++) {
> +		struct ls_config *ls = &hw_data->ls_config[i];
> +		struct ls_pingroup_data *ls_group_data;
> +		char rpmh_resource_name[32];
> +
> +		/* Allocate ls_pingroup_data for this level shifter */
> +		ls_group_data = devm_kzalloc(dev, sizeof(*ls_group_data),
> +					     GFP_KERNEL);
> +		if (!ls_group_data)
> +			return -ENOMEM;
> +
> +		ls_group_data->config = ls;
> +
> +		/* Compose RPMh resource name and get address from cmd_db */
> +		if (state->pmic_id && ls->rpmh_prefix) {
> +			snprintf(rpmh_resource_name,
> +				 sizeof(rpmh_resource_name),
> +				 "%s%s", ls->rpmh_prefix, state->pmic_id);
> +			ls_group_data->level_shifter_addr =
> +				cmd_db_read_addr(rpmh_resource_name);
> +			if (!ls_group_data->level_shifter_addr)
> +				return dev_err_probe(dev, -ENODEV,
> +						     "RPMh resource %s not found in cmd_db\n",
> +						     rpmh_resource_name);

[Severity: High]
Does this code fail to properly defer probing if the command DB (cmd-db) is
not yet initialized?

If the command database is not yet ready, cmd_db_read_addr() swallows the
-EPROBE_DEFER error and returns 0. The driver misinterprets this 0 as a missing
resource and returns a permanent -ENODEV via dev_err_probe(). The driver must
explicitly check cmd_db_ready() to correctly defer probing. Otherwise, it races
with cmd-db initialization and breaks on early probes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-pinctrl-level-shifter-v1-0-1965461d0a7c@oss.qualcomm.com?part=4