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

[PATCH v2 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 {
    	qcom,rpmh = <&apps_rsc>;
    	qcom,pmic-id = "B_E0";

    	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>
---
Changes in v2:
Add following change to address review comments from Sashiko AI:

- PATCH 1, in drivers/soc/qcom/rpmh.c
  1. Add rpmh_put_device() wrapper to avoid casting put_device directly to
     void (*)(void *) in devm_add_action_or_reset(), which would trigger a
     CFI kernel panic on indirect call type mismatch.
  2. Add device_link_add() with DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_PM_RUNTIME
     in rpmh_get_ctrlr_dev() to enforce probe/remove ordering and runtime PM
     suspend/resume ordering between the RPMH controller and non-child consumer
     devices, preventing use-after-free and hardware state machine violations on unbind.

- PATCH 2, in Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
  1. Make qcom,rpmh and qcom,pmic-id optional for qcom,pmh0101-gpio by removing
     them from the required list in the if/then conditional block. Update property
     descriptions to clarify they are only needed when the level-shifter function is used.

- PATCH 3, in drivers/pinctrl/qcom/Kconfig
  1. Add select GENERIC_PINCTRL_GROUPS and select GENERIC_PINMUX_FUNCTIONS to
     PINCTRL_QCOM_SPMI_PMIC to declare explicit dependencies on the generic pinctrl
     APIs (pinctrl_generic_add_group(), pinmux_generic_add_function(), etc.) used by the driver.

- PATCH 3, in drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
  1. Switch from devm_pinctrl_register() to devm_pinctrl_register_and_init() + pinctrl_enable()
     to defer hog application until after all pin groups, functions, are fully registered,
     fixing a probe sequence bug where hogs would be applied against an incomplete pinctrl state.

- PATCH 4, in drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
  1. Guard pad->is_enabled = true in pmic_gpio_pinconf_pin_set() to skip the assignment
     when pad->function == PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER, preventing pinconf
     application from re-enabling GPIO pads that were explicitly disabled by the
     level-shifter mux path.
  2. Call cmd_db_ready() before cmd_db_read_addr() in pmic_gpio_register_level_shifters()
     to propagate -EPROBE_DEFER when the command DB is not yet initialized, instead of
     incorrectly returning -ENODEV which causes a permanent probe failure.
  3. Add qcom,rpmh / qcom,pmic-id DT property check before calling pmic_gpio_register_level_shifters()
     add make the level shifter function registration optional to match with the DT binding change.

- Link to v1: https://patch.msgid.link/20260527-pinctrl-level-shifter-v1-0-1965461d0a7c@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           |  66 +-
 drivers/pinctrl/qcom/Kconfig                       |   2 +
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c           | 796 ++++++++++++++++-----
 drivers/soc/qcom/rpmh.c                            | 173 ++++-
 include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |   1 +
 include/soc/qcom/rpmh.h                            |  21 +
 6 files changed, 845 insertions(+), 214 deletions(-)
---
base-commit: b8f192cec7dcb2e4f04ee57ab78d51777b0a5729
change-id: 20260527-pinctrl-level-shifter-929b286f583d

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

[PATCH v2 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 | 173 +++++++++++++++++++++++++++++++++++++++++++-----
 include/soc/qcom/rpmh.h |  21 ++++++
 2 files changed, 179 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index ca37da3dc2b1..9dbc42b775d9 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,126 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
 }
 EXPORT_SYMBOL_GPL(rpmh_write);
 
+static void rpmh_put_device(void *data)
+{
+	put_device(data);
+}
+
+/**
+ * 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;
+	struct device_link *link;
+	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);
+
+	link = device_link_add(dev, &pdev->dev,
+			       DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_PM_RUNTIME);
+	if (!link) {
+		put_device(&pdev->dev);
+		return ERR_PTR(-EINVAL);
+	}
+
+	ret = devm_add_action_or_reset(dev, rpmh_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 v2 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] Returning `-ENODEV` instead of `-EPROBE_DEFER` when the RPMH controller hasn't fully probed causes premature and permanent probe failures for consumers.
- [High] Missing validation of the device tree phandle target can lead to type confusion and memory corruption.

Pre-existing issues:
- [Critical] Stack variables accessed asynchronously after stack frame is destroyed on timeout, leading to Use-After-Free.
--

commit 4f9cc5ff213e0a26acf5f3c77494a0af70926c9e
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 allow any device with a pointer to
the RPMH controller device to issue write commands. It introduces
rpmh_get_ctrlr_dev() to look up the controller via a device tree phandle,
and new APIs to accept the controller device pointer directly.

> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> index ca37da3dc2b14..9dbc42b775d9f 100644
> --- a/drivers/soc/qcom/rpmh.c
> +++ b/drivers/soc/qcom/rpmh.c
[ ... ]
> @@ -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);

[Severity: High]
If the RPMH controller device is found by of_find_device_by_node() but hasn't
fully probed yet, dev_get_drvdata() will return NULL. Does returning -ENODEV
here cause a premature and permanent probe failure for consumers instead of
allowing them to gracefully defer by returning -EPROBE_DEFER?

> +
> +	return &drv->client;
> +}
[ ... ]
> +/**
> + * 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;
> +	struct device_link *link;
> +	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);

[Severity: High]
If a device tree maliciously or accidentally points the "qcom,rpmh" phandle
to a non-RPMH device, is it possible for drvdata to point to a different
driver's structure when get_rpmh_ctrlr_from_dev() evaluates it?

Would this cause type confusion and memory corruption when the consumer
subsequently casts it to struct rsc_drv? Should there be a validation
check to ensure the resolved device is actually an RPMH controller?

> +	of_node_put(rpmh_np);
> +
> +	if (!pdev)
> +		return ERR_PTR(-EPROBE_DEFER);
[ ... ]
> +/**
> + * 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: Critical]
This is a pre-existing issue, but can this lead to a stack use-after-free
if the timeout expires?

If wait_for_completion_timeout() times out, the function returns -ETIMEDOUT
and the local stack frame is destroyed. However, rpm_msg and compl are
allocated on the stack and the hardware transaction isn't explicitly
canceled.

When the hardware eventually completes the request, the interrupt handler
would call rpmh_tx_done() which accesses the stale rpm_msg->completion
pointer and calls complete(), writing to unmapped or reused stack memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-pinctrl-level-shifter-v2-0-3a6a025392bf@oss.qualcomm.com?part=1

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

[Bartosz Golaszewski]

On Fri, 29 May 2026 03:05:35 +0200, Fenglin Wu
<fenglin.wu@oss.qualcomm.com> said:
> 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 | 173 +++++++++++++++++++++++++++++++++++++++++++-----
>  include/soc/qcom/rpmh.h |  21 ++++++
>  2 files changed, 179 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> index ca37da3dc2b1..9dbc42b775d9 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,126 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
>  }
>  EXPORT_SYMBOL_GPL(rpmh_write);
>
> +static void rpmh_put_device(void *data)
> +{

Can you cast it here to struct device * to make it clear what is being
released?

> +	put_device(data);
> +}
> +
> +/**
> + * 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.

Then I believe it should be called devm_rpmh_get_ctrlr_dev() with the non-devm
variant not registering the devres action.

> + */
> +struct device *rpmh_get_ctrlr_dev(struct device *dev)
> +{
> +	struct device_node *rpmh_np;
> +	struct platform_device *pdev;
> +	struct device_link *link;
> +	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);
> +
> +	link = device_link_add(dev, &pdev->dev,
> +			       DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_PM_RUNTIME);

This should already be covered by driver core as there already exists a phandle
link from dev->of_node to rpmh_np.

> +	if (!link) {
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	ret = devm_add_action_or_reset(dev, rpmh_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.

s/shot/batch/?

> + */
> +int rpmh_write_async_ctrlr(const struct device *ctrl_dev, enum rpmh_state state,
> +			   const struct tcs_cmd *cmd, u32 n)
> +{

The signature of this is the same as that of rpmh_write_async() and the two
functions share a lot of code. Can we not rework rpmh_write_async() to
internally check if it needs to call get_rpmh_ctrlr_from_dev() and avoid
introducing new interfaces?

> +	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.

Add might_sleep() at the top of the function?

> + */
> +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
>
>

Bartosz

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

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
> 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 replicates the story of the PMIC regulators. There are two drivers,
one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
driver targeting only those paired GPIOs (and we don't even need to
represent them as a pair, it might be just one pin).

> 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 | 173 +++++++++++++++++++++++++++++++++++++++++++-----
>  include/soc/qcom/rpmh.h |  21 ++++++
>  2 files changed, 179 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> index ca37da3dc2b1..9dbc42b775d9 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);

This is racy, because the drvdata can disappear at any point. Having the
handle of the device doesn't help because the driver can be unbound even
if we have the reference on the device. 
It's not the case for RPMh (which if unbound will break a lot of
devices), but it's still a antipattern.

> +
> +	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,

> @@ -271,6 +294,126 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
>  }
>  EXPORT_SYMBOL_GPL(rpmh_write);
>  
> +static void rpmh_put_device(void *data)
> +{
> +	put_device(data);
> +}
> +
> +/**
> + * 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;
> +	struct device_link *link;
> +	int ret;
> +
> +	rpmh_np = of_parse_phandle(dev->of_node, "qcom,rpmh", 0);

Ugh. I think this kind of phandle properties are not recommended.

> +	if (!rpmh_np)
> +		return ERR_PTR(-ENODEV);
> +
> +	pdev = of_find_device_by_node(rpmh_np);
> +	of_node_put(rpmh_np);
> +

-- 
With best wishes
Dmitry

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

[Fenglin Wu]

On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
> On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
>> 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 replicates the story of the PMIC regulators. There are two drivers,
> one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
> driver targeting only those paired GPIOs (and we don't even need to
> represent them as a pair, it might be just one pin).

Thanks for the suggestion.

I agree that adding a separate, RPMh-based GPIO driver would be more 
straightforward from RPMh control perspective. It makes the new device 
as a child of the RSC device then it can naturally use the APIs for RPMh 
commands. The main challenge here is, we need to make the level-shifter 
mutually exclusive with other GPIO functions when the GPIO pairs are 
used in level-shifter function, which means we need to write SPMI 
commands to disable the associated GPIO modules. I am not sure if AOP 
already handles this; as far as I know, AOP only manages the 
BIDIR_LVL_SHIFTER module registers. Let me double check on this 
internally, if the GPIO modules could be controlled along 
with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.

Also, I would still insist on the pin group configuration, as the 
level-shifter would only work between 2 pins. Do you see any concerns if 
we represent the level-shifter as following even after move to a new driver?

     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>;
     };

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

[Fenglin Wu]

On 6/2/2026 3:29 PM, Fenglin Wu wrote:
>
> On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
>> On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
>>> 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 replicates the story of the PMIC regulators. There are two drivers,
>> one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
>> driver targeting only those paired GPIOs (and we don't even need to
>> represent them as a pair, it might be just one pin).
>
> Thanks for the suggestion.
>
> I agree that adding a separate, RPMh-based GPIO driver would be more 
> straightforward from RPMh control perspective. It makes the new device 
> as a child of the RSC device then it can naturally use the APIs for 
> RPMh commands. The main challenge here is, we need to make the 
> level-shifter mutually exclusive with other GPIO functions when the 
> GPIO pairs are used in level-shifter function, which means we need to 
> write SPMI commands to disable the associated GPIO modules. I am not 
> sure if AOP already handles this; as far as I know, AOP only manages 
> the BIDIR_LVL_SHIFTER module registers. Let me double check on this 
> internally, if the GPIO modules could be controlled along 
> with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.
>
I checked on this internally, AOP only handles BIDIR_LVL_SHIFTER module 
registers, it doesn't disable the associated GPIO modules. Also, I still 
have no idea how could we make the "level-shifter" function to be 
mutually exclusive with other GPIO functions after moved it into a 
separate driver. Do you have further suggestions?

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

[Dmitry Baryshkov]

On Thu, Jun 04, 2026 at 10:02:43AM +0800, Fenglin Wu wrote:
> 
> On 6/2/2026 3:29 PM, Fenglin Wu wrote:
> > 
> > On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
> > > On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
> > > > 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 replicates the story of the PMIC regulators. There are two drivers,
> > > one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
> > > driver targeting only those paired GPIOs (and we don't even need to
> > > represent them as a pair, it might be just one pin).
> > 
> > Thanks for the suggestion.
> > 
> > I agree that adding a separate, RPMh-based GPIO driver would be more
> > straightforward from RPMh control perspective. It makes the new device
> > as a child of the RSC device then it can naturally use the APIs for RPMh
> > commands. The main challenge here is, we need to make the level-shifter
> > mutually exclusive with other GPIO functions when the GPIO pairs are
> > used in level-shifter function, which means we need to write SPMI
> > commands to disable the associated GPIO modules. I am not sure if AOP
> > already handles this; as far as I know, AOP only manages the
> > BIDIR_LVL_SHIFTER module registers. Let me double check on this
> > internally, if the GPIO modules could be controlled along
> > with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.
> > 
> I checked on this internally, AOP only handles BIDIR_LVL_SHIFTER module
> registers, it doesn't disable the associated GPIO modules. Also, I still
> have no idea how could we make the "level-shifter" function to be mutually
> exclusive with other GPIO functions after moved it into a separate driver.
> Do you have further suggestions?

So, for my understanding, we still need to write SPMI registers to
configure the pins and only then AOP can handle the level shifter?

I was thinking of using gpio-reserved-ranges to prevent those GPIOs from
being used by the normal SPMI driver.

-- 
With best wishes
Dmitry

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

[Fenglin Wu]

On 6/8/2026 5:21 AM, Dmitry Baryshkov wrote:
> On Thu, Jun 04, 2026 at 10:02:43AM +0800, Fenglin Wu wrote:
>> On 6/2/2026 3:29 PM, Fenglin Wu wrote:
>>> On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
>>>> On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
>>>>> 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 replicates the story of the PMIC regulators. There are two drivers,
>>>> one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
>>>> driver targeting only those paired GPIOs (and we don't even need to
>>>> represent them as a pair, it might be just one pin).
>>> Thanks for the suggestion.
>>>
>>> I agree that adding a separate, RPMh-based GPIO driver would be more
>>> straightforward from RPMh control perspective. It makes the new device
>>> as a child of the RSC device then it can naturally use the APIs for RPMh
>>> commands. The main challenge here is, we need to make the level-shifter
>>> mutually exclusive with other GPIO functions when the GPIO pairs are
>>> used in level-shifter function, which means we need to write SPMI
>>> commands to disable the associated GPIO modules. I am not sure if AOP
>>> already handles this; as far as I know, AOP only manages the
>>> BIDIR_LVL_SHIFTER module registers. Let me double check on this
>>> internally, if the GPIO modules could be controlled along
>>> with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.
>>>
>> I checked on this internally, AOP only handles BIDIR_LVL_SHIFTER module
>> registers, it doesn't disable the associated GPIO modules. Also, I still
>> have no idea how could we make the "level-shifter" function to be mutually
>> exclusive with other GPIO functions after moved it into a separate driver.
>> Do you have further suggestions?
> So, for my understanding, we still need to write SPMI registers to
> configure the pins and only then AOP can handle the level shifter?
>
> I was thinking of using gpio-reserved-ranges to prevent those GPIOs from
> being used by the normal SPMI driver.

More background: "level-shifter" module is actually an independent 
hardware which is not part of the GPIO module. However, they are sharing 
the physical pins. Which means, from PMIC chip perspective, these pins 
can be configured to either a GPIO function or the "level-shifter" 
function. So in PMIC base dtsi file, for example, pmh0101.dtsi, these 
pins should not be restricted in the GPIO nodes in "gpio-reserved-ranges".

Also, we need to make the GPIO modules are disabled when the 
"level-shifter" is enabled, to ensure that the "level-shifter" circuitry 
is not impacted by the GPIO modules internal circuitry. So it is 
supposed to write GPIO EN_CTL register (offset 0x46) to 0 through SPMI 
bus when the "level-shifter" is enabled.

That's why we have the requirement to access both RPMh and SPMI bus in 
the same driver.

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

[Konrad Dybcio]

On 6/9/26 3:28 AM, Fenglin Wu wrote:
> 
> On 6/8/2026 5:21 AM, Dmitry Baryshkov wrote:
>> On Thu, Jun 04, 2026 at 10:02:43AM +0800, Fenglin Wu wrote:
>>> On 6/2/2026 3:29 PM, Fenglin Wu wrote:
>>>> On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
>>>>> On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
>>>>>> 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 replicates the story of the PMIC regulators. There are two drivers,
>>>>> one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
>>>>> driver targeting only those paired GPIOs (and we don't even need to
>>>>> represent them as a pair, it might be just one pin).
>>>> Thanks for the suggestion.
>>>>
>>>> I agree that adding a separate, RPMh-based GPIO driver would be more
>>>> straightforward from RPMh control perspective. It makes the new device
>>>> as a child of the RSC device then it can naturally use the APIs for RPMh
>>>> commands. The main challenge here is, we need to make the level-shifter
>>>> mutually exclusive with other GPIO functions when the GPIO pairs are
>>>> used in level-shifter function, which means we need to write SPMI
>>>> commands to disable the associated GPIO modules. I am not sure if AOP
>>>> already handles this; as far as I know, AOP only manages the
>>>> BIDIR_LVL_SHIFTER module registers. Let me double check on this
>>>> internally, if the GPIO modules could be controlled along
>>>> with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.
>>>>
>>> I checked on this internally, AOP only handles BIDIR_LVL_SHIFTER module
>>> registers, it doesn't disable the associated GPIO modules. Also, I still
>>> have no idea how could we make the "level-shifter" function to be mutually
>>> exclusive with other GPIO functions after moved it into a separate driver.
>>> Do you have further suggestions?
>> So, for my understanding, we still need to write SPMI registers to
>> configure the pins and only then AOP can handle the level shifter?
>>
>> I was thinking of using gpio-reserved-ranges to prevent those GPIOs from
>> being used by the normal SPMI driver.
> 
> More background: "level-shifter" module is actually an independent hardware which is not part of the GPIO module. However, they are sharing the physical pins. Which means, from PMIC chip perspective, these pins can be configured to either a GPIO function or the "level-shifter" function. So in PMIC base dtsi file, for example, pmh0101.dtsi, these pins should not be restricted in the GPIO nodes in "gpio-reserved-ranges".
> 
> Also, we need to make the GPIO modules are disabled when the "level-shifter" is enabled, to ensure that the "level-shifter" circuitry is not impacted by the GPIO modules internal circuitry. So it is supposed to write GPIO EN_CTL register (offset 0x46) to 0 through SPMI bus when the "level-shifter" is enabled.
> 
> That's why we have the requirement to access both RPMh and SPMI bus in the same driver.

I was thinking about other ways to solve it.. maybe someting like:

&pmh0101_gpios {
	pmh0101_ls_pins1_2: foo-bar {
		pins = "gpio1", "gpio2";
		// appropriate pinctrl config
	};
};

&rpmh_rsc {
	// should this be a gpio controller? a mux provider?
	// is there another class that would better suit this?
	rpmh_level_shifter: rpmh-foo-bar {
		pinctrl-0 = <&>;
		pinctrl-names = "default";
	};
};

// but where would it make sense to describe?
// fixed-regulator or something akin to that?
&some_consumer {
	someclass = <&rpmh_level_shifter 1>;
};

i.e. the "rpmh level shifter" driver would consume a reference to the
pins, configure them as necessary (just like any other pinctrl consumer)
upon request

Konrad

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

[Dmitry Baryshkov]

On Thu, Jun 11, 2026 at 12:36:43PM +0200, Konrad Dybcio wrote:
> On 6/9/26 3:28 AM, Fenglin Wu wrote:
> > 
> > On 6/8/2026 5:21 AM, Dmitry Baryshkov wrote:
> >> On Thu, Jun 04, 2026 at 10:02:43AM +0800, Fenglin Wu wrote:
> >>> On 6/2/2026 3:29 PM, Fenglin Wu wrote:
> >>>> On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
> >>>>> On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
> >>>>>> 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 replicates the story of the PMIC regulators. There are two drivers,
> >>>>> one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
> >>>>> driver targeting only those paired GPIOs (and we don't even need to
> >>>>> represent them as a pair, it might be just one pin).
> >>>> Thanks for the suggestion.
> >>>>
> >>>> I agree that adding a separate, RPMh-based GPIO driver would be more
> >>>> straightforward from RPMh control perspective. It makes the new device
> >>>> as a child of the RSC device then it can naturally use the APIs for RPMh
> >>>> commands. The main challenge here is, we need to make the level-shifter
> >>>> mutually exclusive with other GPIO functions when the GPIO pairs are
> >>>> used in level-shifter function, which means we need to write SPMI
> >>>> commands to disable the associated GPIO modules. I am not sure if AOP
> >>>> already handles this; as far as I know, AOP only manages the
> >>>> BIDIR_LVL_SHIFTER module registers. Let me double check on this
> >>>> internally, if the GPIO modules could be controlled along
> >>>> with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.
> >>>>
> >>> I checked on this internally, AOP only handles BIDIR_LVL_SHIFTER module
> >>> registers, it doesn't disable the associated GPIO modules. Also, I still
> >>> have no idea how could we make the "level-shifter" function to be mutually
> >>> exclusive with other GPIO functions after moved it into a separate driver.
> >>> Do you have further suggestions?
> >> So, for my understanding, we still need to write SPMI registers to
> >> configure the pins and only then AOP can handle the level shifter?
> >>
> >> I was thinking of using gpio-reserved-ranges to prevent those GPIOs from
> >> being used by the normal SPMI driver.
> > 
> > More background: "level-shifter" module is actually an independent hardware which is not part of the GPIO module. However, they are sharing the physical pins. Which means, from PMIC chip perspective, these pins can be configured to either a GPIO function or the "level-shifter" function. So in PMIC base dtsi file, for example, pmh0101.dtsi, these pins should not be restricted in the GPIO nodes in "gpio-reserved-ranges".
> > 
> > Also, we need to make the GPIO modules are disabled when the "level-shifter" is enabled, to ensure that the "level-shifter" circuitry is not impacted by the GPIO modules internal circuitry. So it is supposed to write GPIO EN_CTL register (offset 0x46) to 0 through SPMI bus when the "level-shifter" is enabled.
> > 
> > That's why we have the requirement to access both RPMh and SPMI bus in the same driver.
> 
> I was thinking about other ways to solve it.. maybe someting like:
> 
> &pmh0101_gpios {
> 	pmh0101_ls_pins1_2: foo-bar {
> 		pins = "gpio1", "gpio2";
> 		// appropriate pinctrl config
> 	};
> };
> 
> &rpmh_rsc {
> 	// should this be a gpio controller? a mux provider?
> 	// is there another class that would better suit this?
> 	rpmh_level_shifter: rpmh-foo-bar {
> 		pinctrl-0 = <&>;
> 		pinctrl-names = "default";
> 	};
> };
> 
> // but where would it make sense to describe?
> // fixed-regulator or something akin to that?
> &some_consumer {
> 	someclass = <&rpmh_level_shifter 1>;
> };
> 
> i.e. the "rpmh level shifter" driver would consume a reference to the
> pins, configure them as necessary (just like any other pinctrl consumer)
> upon request

SGTM.

> 
> Konrad

-- 
With best wishes
Dmitry

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

[Fenglin Wu]

On 6/12/2026 8:27 AM, Dmitry Baryshkov wrote:
> On Thu, Jun 11, 2026 at 12:36:43PM +0200, Konrad Dybcio wrote:
>> On 6/9/26 3:28 AM, Fenglin Wu wrote:
>>> On 6/8/2026 5:21 AM, Dmitry Baryshkov wrote:
>>>> On Thu, Jun 04, 2026 at 10:02:43AM +0800, Fenglin Wu wrote:
>>>>> On 6/2/2026 3:29 PM, Fenglin Wu wrote:
>>>>>> On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
>>>>>>> On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
>>>>>>>> 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 replicates the story of the PMIC regulators. There are two drivers,
>>>>>>> one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
>>>>>>> driver targeting only those paired GPIOs (and we don't even need to
>>>>>>> represent them as a pair, it might be just one pin).
>>>>>> Thanks for the suggestion.
>>>>>>
>>>>>> I agree that adding a separate, RPMh-based GPIO driver would be more
>>>>>> straightforward from RPMh control perspective. It makes the new device
>>>>>> as a child of the RSC device then it can naturally use the APIs for RPMh
>>>>>> commands. The main challenge here is, we need to make the level-shifter
>>>>>> mutually exclusive with other GPIO functions when the GPIO pairs are
>>>>>> used in level-shifter function, which means we need to write SPMI
>>>>>> commands to disable the associated GPIO modules. I am not sure if AOP
>>>>>> already handles this; as far as I know, AOP only manages the
>>>>>> BIDIR_LVL_SHIFTER module registers. Let me double check on this
>>>>>> internally, if the GPIO modules could be controlled along
>>>>>> with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.
>>>>>>
>>>>> I checked on this internally, AOP only handles BIDIR_LVL_SHIFTER module
>>>>> registers, it doesn't disable the associated GPIO modules. Also, I still
>>>>> have no idea how could we make the "level-shifter" function to be mutually
>>>>> exclusive with other GPIO functions after moved it into a separate driver.
>>>>> Do you have further suggestions?
>>>> So, for my understanding, we still need to write SPMI registers to
>>>> configure the pins and only then AOP can handle the level shifter?
>>>>
>>>> I was thinking of using gpio-reserved-ranges to prevent those GPIOs from
>>>> being used by the normal SPMI driver.
>>> More background: "level-shifter" module is actually an independent hardware which is not part of the GPIO module. However, they are sharing the physical pins. Which means, from PMIC chip perspective, these pins can be configured to either a GPIO function or the "level-shifter" function. So in PMIC base dtsi file, for example, pmh0101.dtsi, these pins should not be restricted in the GPIO nodes in "gpio-reserved-ranges".
>>>
>>> Also, we need to make the GPIO modules are disabled when the "level-shifter" is enabled, to ensure that the "level-shifter" circuitry is not impacted by the GPIO modules internal circuitry. So it is supposed to write GPIO EN_CTL register (offset 0x46) to 0 through SPMI bus when the "level-shifter" is enabled.
>>>
>>> That's why we have the requirement to access both RPMh and SPMI bus in the same driver.
>> I was thinking about other ways to solve it.. maybe someting like:
>>
>> &pmh0101_gpios {
>> 	pmh0101_ls_pins1_2: foo-bar {
>> 		pins = "gpio1", "gpio2";
>> 		// appropriate pinctrl config
>> 	};
>> };
>>
>> &rpmh_rsc {
>> 	// should this be a gpio controller? a mux provider?
>> 	// is there another class that would better suit this?
>> 	rpmh_level_shifter: rpmh-foo-bar {
>> 		pinctrl-0 = <&>;
>> 		pinctrl-names = "default";
>> 	};
>> };
>>
>> // but where would it make sense to describe?
>> // fixed-regulator or something akin to that?
>> &some_consumer {
>> 	someclass = <&rpmh_level_shifter 1>;
>> };
>>
>> i.e. the "rpmh level shifter" driver would consume a reference to the
>> pins, configure them as necessary (just like any other pinctrl consumer)
>> upon request
> SGTM.
Thanks for the suggestion, Konrad and Dmitry!
Using the pinctrl state in the new driver to disable GPIO pairs is a 
good idea. I’ve been considering which class would best support the PMIC 
level-shifter, especially since we’re moving it to a new driver and it’s 
no longer restricted by the pinctrl framework. Functionally, the driver 
should provide following capabilities:
1. Enable and disable the level-shifter at runtime. Consumers, likely 
I2C client devices, will enable it when active and disable it when not, 
mainly to save power.
2. Allow sharing the level-shifter between multiple consumers, even 
across different subsystems (currently managed by AOP).

I see flaws in each of the following approaches and haven’t decided 
which to use:
A. Using the mux subsystem: The level-shifter acts as a switch, so it 
fits the mux subsystem physically. It can be enabled/disabled via 
‘mux_control_select()’ and ‘mux_control_deselect()’. However, with 
multiple consumers, a second call to ‘mux_control_select()’ is blocked 
until ‘mux_control_deselect()’ is called, so votes from multiple 
consumers are not allowed and can’t be aggregated.
B. Using the GPIO/pinctrl subsystem: After moving to a new driver, the 
level-shifter doesn’t fit the GPIO controller or pinctrl device concept. 
It has only one pinmux, and each level-shifter works with two pins. 
Also, both GPIO and pinctrl frameworks require exclusive control, and 
couldn't shared between consumers.
C. Using the regulator framework: The level-shifter is controlled via 
the RPMh XOB resource at the AOP side, which was adopted from the idea 
of power rails sharing between subsystems. The regulator framework’s 
APIs and reference counting fit the requirements for sharing between 
multiple consumers. The problem is, the level-shifter isn’t a power rail 
so it is conceptually not a regulator.

Please let me know your thoughts. If there isn’t a suitable approach for 
supporting the PMIC level-shifter right now, I’ll stop chasing on this 
until there is a better idea.

Thanks


>> Konrad

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

[Konrad Dybcio]

On 6/18/26 8:39 AM, Fenglin Wu wrote:
> 
> On 6/12/2026 8:27 AM, Dmitry Baryshkov wrote:
>> On Thu, Jun 11, 2026 at 12:36:43PM +0200, Konrad Dybcio wrote:
>>> On 6/9/26 3:28 AM, Fenglin Wu wrote:
>>>> On 6/8/2026 5:21 AM, Dmitry Baryshkov wrote:
>>>>> On Thu, Jun 04, 2026 at 10:02:43AM +0800, Fenglin Wu wrote:
>>>>>> On 6/2/2026 3:29 PM, Fenglin Wu wrote:
>>>>>>> On 6/1/2026 9:37 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Thu, May 28, 2026 at 06:05:35PM -0700, Fenglin Wu wrote:
>>>>>>>>> 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 replicates the story of the PMIC regulators. There are two drivers,
>>>>>>>> one SPMI and one RPMh. Why don't we add a separate, RPMh-based GPIO
>>>>>>>> driver targeting only those paired GPIOs (and we don't even need to
>>>>>>>> represent them as a pair, it might be just one pin).
>>>>>>> Thanks for the suggestion.
>>>>>>>
>>>>>>> I agree that adding a separate, RPMh-based GPIO driver would be more
>>>>>>> straightforward from RPMh control perspective. It makes the new device
>>>>>>> as a child of the RSC device then it can naturally use the APIs for RPMh
>>>>>>> commands. The main challenge here is, we need to make the level-shifter
>>>>>>> mutually exclusive with other GPIO functions when the GPIO pairs are
>>>>>>> used in level-shifter function, which means we need to write SPMI
>>>>>>> commands to disable the associated GPIO modules. I am not sure if AOP
>>>>>>> already handles this; as far as I know, AOP only manages the
>>>>>>> BIDIR_LVL_SHIFTER module registers. Let me double check on this
>>>>>>> internally, if the GPIO modules could be controlled along
>>>>>>> with BIDIR_LVL_SHIFTER module registers at AOP side, and get back.
>>>>>>>
>>>>>> I checked on this internally, AOP only handles BIDIR_LVL_SHIFTER module
>>>>>> registers, it doesn't disable the associated GPIO modules. Also, I still
>>>>>> have no idea how could we make the "level-shifter" function to be mutually
>>>>>> exclusive with other GPIO functions after moved it into a separate driver.
>>>>>> Do you have further suggestions?
>>>>> So, for my understanding, we still need to write SPMI registers to
>>>>> configure the pins and only then AOP can handle the level shifter?
>>>>>
>>>>> I was thinking of using gpio-reserved-ranges to prevent those GPIOs from
>>>>> being used by the normal SPMI driver.
>>>> More background: "level-shifter" module is actually an independent hardware which is not part of the GPIO module. However, they are sharing the physical pins. Which means, from PMIC chip perspective, these pins can be configured to either a GPIO function or the "level-shifter" function. So in PMIC base dtsi file, for example, pmh0101.dtsi, these pins should not be restricted in the GPIO nodes in "gpio-reserved-ranges".
>>>>
>>>> Also, we need to make the GPIO modules are disabled when the "level-shifter" is enabled, to ensure that the "level-shifter" circuitry is not impacted by the GPIO modules internal circuitry. So it is supposed to write GPIO EN_CTL register (offset 0x46) to 0 through SPMI bus when the "level-shifter" is enabled.
>>>>
>>>> That's why we have the requirement to access both RPMh and SPMI bus in the same driver.
>>> I was thinking about other ways to solve it.. maybe someting like:
>>>
>>> &pmh0101_gpios {
>>>     pmh0101_ls_pins1_2: foo-bar {
>>>         pins = "gpio1", "gpio2";
>>>         // appropriate pinctrl config
>>>     };
>>> };
>>>
>>> &rpmh_rsc {
>>>     // should this be a gpio controller? a mux provider?
>>>     // is there another class that would better suit this?
>>>     rpmh_level_shifter: rpmh-foo-bar {
>>>         pinctrl-0 = <&>;
>>>         pinctrl-names = "default";
>>>     };
>>> };
>>>
>>> // but where would it make sense to describe?
>>> // fixed-regulator or something akin to that?
>>> &some_consumer {
>>>     someclass = <&rpmh_level_shifter 1>;
>>> };
>>>
>>> i.e. the "rpmh level shifter" driver would consume a reference to the
>>> pins, configure them as necessary (just like any other pinctrl consumer)
>>> upon request
>> SGTM.
> Thanks for the suggestion, Konrad and Dmitry!
> Using the pinctrl state in the new driver to disable GPIO pairs is a good idea. I’ve been considering which class would best support the PMIC level-shifter, especially since we’re moving it to a new driver and it’s no longer restricted by the pinctrl framework. Functionally, the driver should provide following capabilities:
> 1. Enable and disable the level-shifter at runtime. Consumers, likely I2C client devices, will enable it when active and disable it when not, mainly to save power.
> 2. Allow sharing the level-shifter between multiple consumers, even across different subsystems (currently managed by AOP).
> 
> I see flaws in each of the following approaches and haven’t decided which to use:
> A. Using the mux subsystem: The level-shifter acts as a switch, so it fits the mux subsystem physically. It can be enabled/disabled via ‘mux_control_select()’ and ‘mux_control_deselect()’. However, with multiple consumers, a second call to ‘mux_control_select()’ is blocked until ‘mux_control_deselect()’ is called, so votes from multiple consumers are not allowed and can’t be aggregated.
> B. Using the GPIO/pinctrl subsystem: After moving to a new driver, the level-shifter doesn’t fit the GPIO controller or pinctrl device concept. It has only one pinmux, and each level-shifter works with two pins. Also, both GPIO and pinctrl frameworks require exclusive control, and couldn't shared between consumers.
> C. Using the regulator framework: The level-shifter is controlled via the RPMh XOB resource at the AOP side, which was adopted from the idea of power rails sharing between subsystems. The regulator framework’s APIs and reference counting fit the requirements for sharing between multiple consumers. The problem is, the level-shifter isn’t a power rail so it is conceptually not a regulator.

(keeping all the context above)

+Mark, would you accept not-quite-a-regulator driver?

Otherwise, I don't really have a good idea either, perhaps the mux
maintainers could be open to adding refcounting for 'shared muxes'

Konrad
> 
> Please let me know your thoughts. If there isn’t a suitable approach for supporting the PMIC level-shifter right now, I’ll stop chasing on this until there is a better idea.
> 
> Thanks
> 
> 
>>> Konrad

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

[Mark Brown]

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

On Tue, Jun 30, 2026 at 04:28:54PM +0200, Konrad Dybcio wrote:

> +Mark, would you accept not-quite-a-regulator driver?

Probably not, but I'm having a hard time telling what the problem is -
the quoting level is rather deep and multiple levels of it don't use any
word wrapping within paragraphs so it's all excessively hard to read.
Frankly I very nearly just deleted the mail unread.  Could someone
summarise what's going on here please?

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

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

[Fenglin Wu]

On 6/30/2026 10:37 PM, Mark Brown wrote:
> On Tue, Jun 30, 2026 at 04:28:54PM +0200, Konrad Dybcio wrote:
> 
>> +Mark, would you accept not-quite-a-regulator driver?
> 
> Probably not, but I'm having a hard time telling what the problem is -
> the quoting level is rather deep and multiple levels of it don't use any
> word wrapping within paragraphs so it's all excessively hard to read.
> Frankly I very nearly just deleted the mail unread.  Could someone
> summarise what's going on here please?

Hi Mark,

Please see a short summary below:

Qcom PMH0101 includes bi-directional level-shifter (BIDIR_LVL_SHIFTER)
modules that act as open-drain voltage translators, mainly to support
1.2 to 1.8V voltage translation on the I2C bus between the SoC with 1.2V
IOs and other I2C clients with 1.8V IOs. Each module shares two physical
pins with two GPIO modules, and control of the BIDIR_LVL_SHIFTER module
is centralized on the AOP side with 'XOB' resources so it can be shared
between different subsystems.

When a BIDIR_LVL_SHIFTER is enabled, it needs to send RPMh commands to
enable it and SPMI commands to disable the related two GPIO modules, to
make sure their circuitry does not interfere with it. For now, the
suggestion is to write a new driver for BIDIR_LVL_SHIFTER and place it
as a child of the rpmh_rsc device, and refer to pinstate nodes to
disable these two GPIO modules. However, we are now facing a difficulty
in deciding which subsystem the new driver should belong to. We expected
that the new driver should provide following capabilities:

1. Enable and disable the level-shifter at runtime. Consumers, likely
I2C client devices, will enable it when active and disable it when not,
mainly to save power.
2. Allow sharing the level-shifter between multiple consumers, even
across different subsystems (currently managed by AOP).

Following are the approaches that we are considered, and it seems only a
regulator device could satisfy the requirement the best but we want to
check with you if you are fine to put it in the regulator framework.

A. Using the mux subsystem: The level-shifter acts as a switch, so it
fits the mux subsystem physically. It can be enabled/disabled via
‘mux_control_select()’ and ‘mux_control_deselect()’. However, with
multiple consumers, a second call to ‘mux_control_select()’ is blocked
until ‘mux_control_deselect()’ is called, so votes from multiple
consumers are not allowed and can’t be aggregated.

B. Using the GPIO/pinctrl subsystem: After moving to a new driver, the
level-shifter doesn’t fit the GPIO controller or pinctrl device concept.
It has only one pinmux, and each level-shifter works with two pins.
Also, both GPIO and pinctrl frameworks require exclusive control, and
couldn't shared between consumers.

C. Using the regulator framework: The level-shifter is controlled via
the RPMh XOB resource at the AOP side, which was adopted from the idea
of power rails sharing between subsystems. The regulator framework’s
APIs and reference counting fit the requirements for sharing between
multiple consumers. The problem is, the level-shifter isn’t a power rail
so it is conceptually not a regulator.

Thanks
Fenglin Wu

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

[Mark Brown]

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

On Wed, Jul 01, 2026 at 10:43:01AM +0800, Fenglin Wu wrote:

> Qcom PMH0101 includes bi-directional level-shifter (BIDIR_LVL_SHIFTER)
> modules that act as open-drain voltage translators, mainly to support
> 1.2 to 1.8V voltage translation on the I2C bus between the SoC with 1.2V
> IOs and other I2C clients with 1.8V IOs. Each module shares two physical
> pins with two GPIO modules, and control of the BIDIR_LVL_SHIFTER module
> is centralized on the AOP side with 'XOB' resources so it can be shared
> between different subsystems.

> When a BIDIR_LVL_SHIFTER is enabled, it needs to send RPMh commands to
> enable it and SPMI commands to disable the related two GPIO modules, to
> make sure their circuitry does not interfere with it. For now, the
> suggestion is to write a new driver for BIDIR_LVL_SHIFTER and place it
> as a child of the rpmh_rsc device, and refer to pinstate nodes to
> disable these two GPIO modules. However, we are now facing a difficulty
> in deciding which subsystem the new driver should belong to. We expected
> that the new driver should provide following capabilities:

To the extent any of this is comprehensible (I don't know what things
like AOP, XOB and so on are) nothing about this is screaming regulator.
It sounds like you just want to turn on and off a device to make the bus
you're talking through work?

> 1. Enable and disable the level-shifter at runtime. Consumers, likely
> I2C client devices, will enable it when active and disable it when not,
> mainly to save power.
> 2. Allow sharing the level-shifter between multiple consumers, even
> across different subsystems (currently managed by AOP).

> Following are the approaches that we are considered, and it seems only a
> regulator device could satisfy the requirement the best but we want to
> check with you if you are fine to put it in the regulator framework.

Why would this be a regulator?  It's not providing power to anything
which is a fairly key element of being a regulator, it fits about as
well with clocks (which also provide reference counting which is AFAICT
what you're looking for here).  You'll have a hard time writing DTs for
this that actually refelect the hardware I think.  Possibly it's a power
domain (or a resource controlled by a power domain, I know they manage
clocks and regulators)? 

Possibly work out a DT binding for this and then work back from there.

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

[PATCH v2 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
pinconf 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           | 66 +++++++++++++++++++++-
 include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |  1 +
 2 files changed, 64 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..19dc61ddff2d 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 when the
+      bidirectional level shifters is used (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,22 @@ 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
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
               - qcom,pmih0108-gpio
     then:
       properties:
@@ -523,6 +554,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 +580,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 +637,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 v2 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:
- [Medium] The `qcom,rpmh` and `qcom,pmic-id` properties are globally permitted for all PMIC compatibles instead of being restricted to variants that support level-shifters.
--

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

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

This commit adds a new level-shifter function to the pmic-gpio binding.
It introduces qcom,rpmh and qcom,pmic-id properties for the pmh0101 variant
to enable communication with the RPMh firmware, along with a custom pinconf
parameter.

> 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
> @@ -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 when the
> +      bidirectional level shifters is used (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]+$"

[ ... ]

> @@ -330,6 +345,22 @@ 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

[Severity: Medium]
Does this schema actually restrict qcom,rpmh and qcom,pmic-id to the pmh0101
variant as intended?

Because these properties are defined in the top-level properties block, they
are globally permitted for all PMIC compatibles. Setting them to true in this
then block is a no-op in JSON schema, as affirming a property is allowed
does not forbid it from appearing in unmatched conditionals.

Should the schema explicitly forbid these properties for unsupported variants
so they are not implicitly valid on all PMICs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-pinctrl-level-shifter-v2-0-3a6a025392bf@oss.qualcomm.com?part=2

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

[Krzysztof Kozlowski]

On Thu, May 28, 2026 at 06:05:36PM -0700, Fenglin Wu wrote:
> Add the "level-shifter" function and add the required DT properties to
> allow RPMh firmware to control the level-shifter. Introduce a custom
> pinconf parameter "qcom,1p2v-1p8v-ls-en" for enabling or disabling the
> level-shifter function.

I don't get how PMIC, which is not a child of RPMh at all or not
talking with RPMh RSC, needs to configure its pin via RPMh. It feels it
is misrepresented.

> 
> 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           | 66 +++++++++++++++++++++-
>  include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |  1 +
>  2 files changed, 64 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..19dc61ddff2d 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 when the
> +      bidirectional level shifters is used (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]+$"

You do not get instance IDs (it's explcitly documented in docs).

> +
>  additionalProperties: false
>  
>  required:
> @@ -330,6 +345,22 @@ 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
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
>                - qcom,pmih0108-gpio
>      then:
>        properties:
> @@ -523,6 +554,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 +580,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 +637,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).

And there are no generic pinconf properties defining the voltage?

Best regards,
Krzysztof

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

[Fenglin Wu]

On 5/30/2026 6:29 PM, Krzysztof Kozlowski wrote:
> On Thu, May 28, 2026 at 06:05:36PM -0700, Fenglin Wu wrote:
>> Add the "level-shifter" function and add the required DT properties to
>> allow RPMh firmware to control the level-shifter. Introduce a custom
>> pinconf parameter "qcom,1p2v-1p8v-ls-en" for enabling or disabling the
>> level-shifter function.
> I don't get how PMIC, which is not a child of RPMh at all or not
> talking with RPMh RSC, needs to configure its pin via RPMh. It feels it
> is misrepresented.

The control for enabling or disabling the bi-directional level shifter 
has been centralized in AOP, similar to how regulator resources are 
managed. This allows it to be used on a serial bus shared by multiple 
clients from different subsystems. Each subsystem can vote for its 
enable state through RPMh commands, and AOP determines the final status 
to turn the BIDIR_LVL_SHIFTER PMIC modules on or off. Additionally, each 
bi-directional level shifter shares its physical pins with a pair of 
PMIC GPIO modules and is mutually exclusive with other PMIC GPIO 
functions, which means those PMIC GPIO functions must be disabled.

For these reasons, adding bi-directional level shifter software support 
to the pinctrl-spmi-gpio driver is considered the best approach. Let me 
know if you have a better suggestion.

>> 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           | 66 +++++++++++++++++++++-
>>   include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |  1 +
>>   2 files changed, 64 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..19dc61ddff2d 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 when the
>> +      bidirectional level shifters is used (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]+$"
> You do not get instance IDs (it's explcitly documented in docs).

Okay. This is primarily for creating the resource names used to obtain 
the rpmh addresses from the cmd-db for the level-shifter.

I can change it to a different name if you still agree to add the 
support in the pinctrl driver.

>> +
>>   additionalProperties: false
>>   
>>   required:
>> @@ -330,6 +345,22 @@ 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
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>>                 - qcom,pmih0108-gpio
>>       then:
>>         properties:
>> @@ -523,6 +554,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 +580,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 +637,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).
> And there are no generic pinconf properties defining the voltage?

The 1.2V and 1.8V voltages on each side of the bidirectional level 
shifter are not configurable. They are fixed in the hardware with 
built-in reference voltages at each side of the pins. I am adding this 
custom pinconf parameter mainly to control its enabling status. Also, I 
am adding "1p2v-1p8v" in the parameter name to provide additional 
clarity for users about the "level-shifter" function.

>
> Best regards,
> Krzysztof
>

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

[Krzysztof Kozlowski]

On 01/06/2026 07:00, Fenglin Wu wrote:
> 
> On 5/30/2026 6:29 PM, Krzysztof Kozlowski wrote:
>> On Thu, May 28, 2026 at 06:05:36PM -0700, Fenglin Wu wrote:
>>> Add the "level-shifter" function and add the required DT properties to
>>> allow RPMh firmware to control the level-shifter. Introduce a custom
>>> pinconf parameter "qcom,1p2v-1p8v-ls-en" for enabling or disabling the
>>> level-shifter function.
>> I don't get how PMIC, which is not a child of RPMh at all or not
>> talking with RPMh RSC, needs to configure its pin via RPMh. It feels it
>> is misrepresented.
> 
> The control for enabling or disabling the bi-directional level shifter 
> has been centralized in AOP, similar to how regulator resources are 
> managed. This allows it to be used on a serial bus shared by multiple 
> clients from different subsystems. Each subsystem can vote for its 
> enable state through RPMh commands, and AOP determines the final status 
> to turn the BIDIR_LVL_SHIFTER PMIC modules on or off. Additionally, each 
> bi-directional level shifter shares its physical pins with a pair of 
> PMIC GPIO modules and is mutually exclusive with other PMIC GPIO 
> functions, which means those PMIC GPIO functions must be disabled.

So two completely independent hardware devices - PMIC and RPMh -
configure the same hardware - level shifter and pin function?

> 
> For these reasons, adding bi-directional level shifter software support 
> to the pinctrl-spmi-gpio driver is considered the best approach. Let me 
> know if you have a better suggestion.
> 
>>> 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           | 66 +++++++++++++++++++++-
>>>   include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |  1 +
>>>   2 files changed, 64 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..19dc61ddff2d 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 when the
>>> +      bidirectional level shifters is used (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]+$"
>> You do not get instance IDs (it's explcitly documented in docs).
> 
> Okay. This is primarily for creating the resource names used to obtain 
> the rpmh addresses from the cmd-db for the level-shifter.
> 
> I can change it to a different name if you still agree to add the 
> support in the pinctrl driver.

ID or name, same thing. Still not allowed.

> 
>>> +
>>>   additionalProperties: false
>>>   
>>>   required:
>>> @@ -330,6 +345,22 @@ 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
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>>                 - qcom,pmih0108-gpio
>>>       then:
>>>         properties:
>>> @@ -523,6 +554,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 +580,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 +637,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).
>> And there are no generic pinconf properties defining the voltage?
> 
> The 1.2V and 1.8V voltages on each side of the bidirectional level 
> shifter are not configurable. They are fixed in the hardware with 
> built-in reference voltages at each side of the pins. I am adding this 
> custom pinconf parameter mainly to control its enabling status. Also, I 
> am adding "1p2v-1p8v" in the parameter name to provide additional 
> clarity for users about the "level-shifter" function.

So there are or there are not?

Best regards,
Krzysztof

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

[Fenglin Wu]

On 6/1/2026 7:30 PM, Krzysztof Kozlowski wrote:
> On 01/06/2026 07:00, Fenglin Wu wrote:
>> On 5/30/2026 6:29 PM, Krzysztof Kozlowski wrote:
>>> On Thu, May 28, 2026 at 06:05:36PM -0700, Fenglin Wu wrote:
>>>> Add the "level-shifter" function and add the required DT properties to
>>>> allow RPMh firmware to control the level-shifter. Introduce a custom
>>>> pinconf parameter "qcom,1p2v-1p8v-ls-en" for enabling or disabling the
>>>> level-shifter function.
>>> I don't get how PMIC, which is not a child of RPMh at all or not
>>> talking with RPMh RSC, needs to configure its pin via RPMh. It feels it
>>> is misrepresented.
>> The control for enabling or disabling the bi-directional level shifter
>> has been centralized in AOP, similar to how regulator resources are
>> managed. This allows it to be used on a serial bus shared by multiple
>> clients from different subsystems. Each subsystem can vote for its
>> enable state through RPMh commands, and AOP determines the final status
>> to turn the BIDIR_LVL_SHIFTER PMIC modules on or off. Additionally, each
>> bi-directional level shifter shares its physical pins with a pair of
>> PMIC GPIO modules and is mutually exclusive with other PMIC GPIO
>> functions, which means those PMIC GPIO functions must be disabled.
> So two completely independent hardware devices - PMIC and RPMh -
> configure the same hardware - level shifter and pin function?

Yes if we consider level shifter as a mux function of the GPIO pairs.

>
>> For these reasons, adding bi-directional level shifter software support
>> to the pinctrl-spmi-gpio driver is considered the best approach. Let me
>> know if you have a better suggestion.
>>
>>>> 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           | 66 +++++++++++++++++++++-
>>>>    include/dt-bindings/pinctrl/qcom,pmic-gpio.h       |  1 +
>>>>    2 files changed, 64 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..19dc61ddff2d 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 when the
>>>> +      bidirectional level shifters is used (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]+$"
>>> You do not get instance IDs (it's explcitly documented in docs).
>> Okay. This is primarily for creating the resource names used to obtain
>> the rpmh addresses from the cmd-db for the level-shifter.
>>
>> I can change it to a different name if you still agree to add the
>> support in the pinctrl driver.
> ID or name, same thing. Still not allowed.

Okay. Then I don't know how could the driver be able to get the rpmh 
address from the cmd-db.

Maybe just like what Dmitry suggested, register a separate, RPMh-based 
GPIO driver for those GPIOs associated with the level shifter function, 
would be a better approach?

Re: [PATCH v2 1/4] soc: qcom: rpmh: Allow non-child devices to issue 
write commands - Dmitry Baryshkov 
<https://lore.kernel.org/linux-arm-msm/4ac5hjmr6divqs4myhcw5sveuboj265sw2jwslbivrfwh5e7ce@6d7ajvgikkgt/>

>
>>>> +
>>>>    additionalProperties: false
>>>>    
>>>>    required:
>>>> @@ -330,6 +345,22 @@ 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
>>>> +
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>>                  - qcom,pmih0108-gpio
>>>>        then:
>>>>          properties:
>>>> @@ -523,6 +554,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 +580,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 +637,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).
>>> And there are no generic pinconf properties defining the voltage?
>> The 1.2V and 1.8V voltages on each side of the bidirectional level
>> shifter are not configurable. They are fixed in the hardware with
>> built-in reference voltages at each side of the pins. I am adding this
>> custom pinconf parameter mainly to control its enabling status. Also, I
>> am adding "1p2v-1p8v" in the parameter name to provide additional
>> clarity for users about the "level-shifter" function.
> So there are or there are not?

There is not such generic pinconf parameter based what I saw here:

https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/tree/drivers/pinctrl/pinconf-generic.c#n173

>
> Best regards,
> Krzysztof

[PATCH v2 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/Kconfig             |   2 +
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 377 +++++++++++++++++++++----------
 2 files changed, 258 insertions(+), 121 deletions(-)

diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index a09e840a01c6..80bcec33b9b8 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -25,6 +25,8 @@ config PINCTRL_QCOM_SPMI_PMIC
 	select PINMUX
 	select PINCONF
 	select GENERIC_PINCONF
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
 	select IRQ_DOMAIN_HIERARCHY
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index cdd61dae74cf..268cfae706a8 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;
 }
 
-static void pmic_gpio_config_dbg_show(struct pinctrl_dev *pctldev,
-				      struct seq_file *s, unsigned pin)
+/**
+ * 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;
+}
+
+/**
+ * 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;
 
@@ -1150,9 +1257,33 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	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);
+	ret = devm_pinctrl_register_and_init(dev, pctrldesc, state, &state->ctrl);
+	if (ret)
+		return ret;
+
+	/* 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)
@@ -1174,6 +1305,10 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	girq->child_offset_to_irq = pmic_gpio_child_offset_to_irq;
 	girq->child_irq_domain_ops.translate = pmic_gpio_domain_translate;
 
+	ret = pinctrl_enable(state->ctrl);
+	if (ret)
+		return ret;
+
 	ret = gpiochip_add_data(&state->chip, state);
 	if (ret) {
 		dev_err(state->dev, "can't add gpio chip\n");

-- 
2.43.0

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

[Krzysztof Kozlowski]

On Thu, May 28, 2026 at 06:05:37PM -0700, Fenglin Wu wrote:
> 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/Kconfig             |   2 +
>  drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 377 +++++++++++++++++++++----------

And now make this built-in and RPMH as module, then build it. Does
it build?

Best regards,
Krzysztof

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

[Fenglin Wu]

On 5/30/2026 6:31 PM, Krzysztof Kozlowski wrote:
> On Thu, May 28, 2026 at 06:05:37PM -0700, Fenglin Wu wrote:
>> 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/Kconfig             |   2 +
>>   drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 377 +++++++++++++++++++++----------
> And now make this built-in and RPMH as module, then build it. Does
> it build?
Thanks. I missed to add the dependency in patch4 which really adds the 
rpmh control. I will update the Kconfig to make it depends on rpmh and 
cmd-db drivers.
> Best regards,
> Krzysztof
>

[PATCH v2 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 | 441 +++++++++++++++++++++++++------
 1 file changed, 355 insertions(+), 86 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 268cfae706a8..741795ae261e 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
 
@@ -214,16 +283,16 @@ static const char *const pmic_gpio_groups[] = {
 };
 
 static const char *const pmic_gpio_functions[] = {
-	[PMIC_GPIO_FUNC_INDEX_NORMAL]	= PMIC_GPIO_FUNC_NORMAL,
-	[PMIC_GPIO_FUNC_INDEX_PAIRED]	= PMIC_GPIO_FUNC_PAIRED,
-	[PMIC_GPIO_FUNC_INDEX_FUNC1]	= PMIC_GPIO_FUNC_FUNC1,
-	[PMIC_GPIO_FUNC_INDEX_FUNC2]	= PMIC_GPIO_FUNC_FUNC2,
-	[PMIC_GPIO_FUNC_INDEX_FUNC3]	= PMIC_GPIO_FUNC_FUNC3,
-	[PMIC_GPIO_FUNC_INDEX_FUNC4]	= PMIC_GPIO_FUNC_FUNC4,
-	[PMIC_GPIO_FUNC_INDEX_DTEST1]	= PMIC_GPIO_FUNC_DTEST1,
-	[PMIC_GPIO_FUNC_INDEX_DTEST2]	= PMIC_GPIO_FUNC_DTEST2,
-	[PMIC_GPIO_FUNC_INDEX_DTEST3]	= PMIC_GPIO_FUNC_DTEST3,
-	[PMIC_GPIO_FUNC_INDEX_DTEST4]	= PMIC_GPIO_FUNC_DTEST4,
+	[PMIC_GPIO_FUNC_INDEX_NORMAL]		= PMIC_GPIO_FUNC_NORMAL,
+	[PMIC_GPIO_FUNC_INDEX_PAIRED]		= PMIC_GPIO_FUNC_PAIRED,
+	[PMIC_GPIO_FUNC_INDEX_FUNC1]		= PMIC_GPIO_FUNC_FUNC1,
+	[PMIC_GPIO_FUNC_INDEX_FUNC2]		= PMIC_GPIO_FUNC_FUNC2,
+	[PMIC_GPIO_FUNC_INDEX_FUNC3]		= PMIC_GPIO_FUNC_FUNC3,
+	[PMIC_GPIO_FUNC_INDEX_FUNC4]		= PMIC_GPIO_FUNC_FUNC4,
+	[PMIC_GPIO_FUNC_INDEX_DTEST1]		= PMIC_GPIO_FUNC_DTEST1,
+	[PMIC_GPIO_FUNC_INDEX_DTEST2]		= PMIC_GPIO_FUNC_DTEST2,
+	[PMIC_GPIO_FUNC_INDEX_DTEST3]		= PMIC_GPIO_FUNC_DTEST3,
+	[PMIC_GPIO_FUNC_INDEX_DTEST4]		= PMIC_GPIO_FUNC_DTEST4,
 };
 
 static int pmic_gpio_read(struct pmic_gpio_state *state,
@@ -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];
@@ -513,7 +639,8 @@ static int pmic_gpio_pinconf_pin_set(struct pinctrl_dev *pctldev,
 
 	pad = pctldev->desc->pins[pin].drv_data;
 
-	pad->is_enabled = true;
+	if (pad->function != PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER)
+		pad->is_enabled = true;
 	for (i = 0; i < nconfs; i++) {
 		param = pinconf_to_config_param(configs[i]);
 		arg = pinconf_to_config_argument(configs[i]);
@@ -597,6 +724,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 +820,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 +836,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 +943,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 +958,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 +1334,102 @@ 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) {
+			int ret_ready = cmd_db_ready();
+
+			if (ret_ready)
+				return dev_err_probe(dev, ret_ready,
+						     "Command DB not available\n");
+
+			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 +1441,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 +1451,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 +1545,15 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Register level-shifter groups and function if hardware and DT both opt in */
+	if (hw_data->ls_config && hw_data->num_ls &&
+	    of_property_present(dev->of_node, "qcom,rpmh") &&
+	    of_property_present(dev->of_node, "qcom,pmic-id")) {
+		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;
@@ -1349,80 +1618,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 v2 4/4] pinctrl: qcom: spmi-gpio: Add level-shifter function support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Writing the software-defined `LEVEL_SHIFTER` function index to hardware registers causes register corruption.
- [High] Out-of-bounds array access when displaying pinconf debugfs for a level-shifter pin.
- [High] Use-After-Free of level shifter resources during driver unbind due to reversed devm initialization order.
- [High] Failure to enforce mutual exclusivity between the bidirectional level shifter and physical GPIO functions.
- [Medium] Missing GET support for the newly added `qcom,1p2v-1p8v-ls-en` pin configuration.
--

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

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

This commit introduces bidirectional level shifter support for open-drain
signals. It adds a level-shifter function to the SPMI GPIO driver for specific
GPIO pairs managed via RPMh.

> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index 268cfae706a82..741795ae261ef 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,
>  };

[ ... ]

> @@ -183,25 +191,86 @@ struct pmic_gpio_state {
>  	{"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},
>  };

[Severity: Medium]
Since this commit adds the PMIC_GPIO_CONF_LS_ENABLE property and handles it
in pmic_gpio_pinconf_group_set(), does it also need corresponding read
support?

The driver uses generic pinconf dumps but lacks a pin_config_group_get
implementation, and pmic_gpio_pinconf_pin_get() lacks a case for the new
parameter. This appears to break the symmetry of the pinctrl API, which
might cause programmatic reads of this configuration to fail with -EINVAL.

[ ... ]

> @@ -214,16 +283,16 @@ static const char *const pmic_gpio_groups[] = {
>  };
>  
>  static const char *const pmic_gpio_functions[] = {
> -	[PMIC_GPIO_FUNC_INDEX_NORMAL]	= PMIC_GPIO_FUNC_NORMAL,
> -	[PMIC_GPIO_FUNC_INDEX_PAIRED]	= PMIC_GPIO_FUNC_PAIRED,
> -	[PMIC_GPIO_FUNC_INDEX_FUNC1]	= PMIC_GPIO_FUNC_FUNC1,
> -	[PMIC_GPIO_FUNC_INDEX_FUNC2]	= PMIC_GPIO_FUNC_FUNC2,
> -	[PMIC_GPIO_FUNC_INDEX_FUNC3]	= PMIC_GPIO_FUNC_FUNC3,
> -	[PMIC_GPIO_FUNC_INDEX_FUNC4]	= PMIC_GPIO_FUNC_FUNC4,
> -	[PMIC_GPIO_FUNC_INDEX_DTEST1]	= PMIC_GPIO_FUNC_DTEST1,
> -	[PMIC_GPIO_FUNC_INDEX_DTEST2]	= PMIC_GPIO_FUNC_DTEST2,
> -	[PMIC_GPIO_FUNC_INDEX_DTEST3]	= PMIC_GPIO_FUNC_DTEST3,
> -	[PMIC_GPIO_FUNC_INDEX_DTEST4]	= PMIC_GPIO_FUNC_DTEST4,
> +	[PMIC_GPIO_FUNC_INDEX_NORMAL]		= PMIC_GPIO_FUNC_NORMAL,
> +	[PMIC_GPIO_FUNC_INDEX_PAIRED]		= PMIC_GPIO_FUNC_PAIRED,
> +	[PMIC_GPIO_FUNC_INDEX_FUNC1]		= PMIC_GPIO_FUNC_FUNC1,
> +	[PMIC_GPIO_FUNC_INDEX_FUNC2]		= PMIC_GPIO_FUNC_FUNC2,
> +	[PMIC_GPIO_FUNC_INDEX_FUNC3]		= PMIC_GPIO_FUNC_FUNC3,
> +	[PMIC_GPIO_FUNC_INDEX_FUNC4]		= PMIC_GPIO_FUNC_FUNC4,
> +	[PMIC_GPIO_FUNC_INDEX_DTEST1]		= PMIC_GPIO_FUNC_DTEST1,
> +	[PMIC_GPIO_FUNC_INDEX_DTEST2]		= PMIC_GPIO_FUNC_DTEST2,
> +	[PMIC_GPIO_FUNC_INDEX_DTEST3]		= PMIC_GPIO_FUNC_DTEST3,
> +	[PMIC_GPIO_FUNC_INDEX_DTEST4]		= PMIC_GPIO_FUNC_DTEST4,
>  };

[Severity: High]
Should PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER be added to this array?

If pmic_gpio_pinconf_pin_dbg_show() formats the status of a level-shifter
pin, it dereferences pmic_gpio_functions[function]. For a level-shifter
pin, this would be index 10 (or potentially 12 for non-LV/MV pins), while
the array only has 10 elements.

Could this cause an out-of-bounds memory access that crashes the kernel when
reading pinctrl debugfs?

[ ... ]

> @@ -513,7 +639,8 @@ static int pmic_gpio_pinconf_pin_set(struct pinctrl_dev *pctldev,
>  
>  	pad = pctldev->desc->pins[pin].drv_data;
>  
> -	pad->is_enabled = true;
> +	if (pad->function != PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER)
> +		pad->is_enabled = true;

[Severity: High]
The commit message states that the level shifter must be mutually exclusive
with all existing functions and configurations. However, does this code
enforce that exclusivity?

If a device tree simultaneously assigns a normal function to a pin and
enables the level shifter on its associated group, pad->function will be
NORMAL.

Since the check above will pass, it will set pad->is_enabled = true,
potentially causing both the physical GPIO output buffer and the external
level shifter to drive the pin concurrently. Could this result in
electrical overstress?

[Severity: High]
Is there a risk of corrupting hardware registers with the LEVEL_SHIFTER
index?

When setting PMIC_GPIO_REG_MODE_CTL for non-LV/MV pins later in
pmic_gpio_pinconf_pin_set(), the driver does this:

        val = val << PMIC_GPIO_REG_MODE_DIR_SHIFT;
        val |= pad->function << PMIC_GPIO_REG_MODE_FUNCTION_SHIFT;

If pad->function is PMIC_GPIO_FUNC_INDEX_LEVEL_SHIFTER (10, 0b1010),
shifting it by 1 results in 20 (0x14). Since the hardware function field is
only 3 bits, this overflows and overwrites bit 4
(PMIC_GPIO_REG_MODE_DIR_SHIFT).

Since LEVEL_SHIFTER is a software-defined pseudo-function that physically
operates by keeping the pad disabled, should the driver explicitly bypass
hardware register writes for level-shifter pins?

[ ... ]

> @@ -1285,6 +1545,15 @@ static int pmic_gpio_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/* Register level-shifter groups and function if hardware and DT both opt in */
> +	if (hw_data->ls_config && hw_data->num_ls &&
> +	    of_property_present(dev->of_node, "qcom,rpmh") &&
> +	    of_property_present(dev->of_node, "qcom,pmic-id")) {
> +		ret = pmic_gpio_register_level_shifters(state, hw_data);
> +		if (ret < 0)
> +			return ret;
> +	}

[Severity: High]
Will this devres allocation order cause a Use-After-Free during driver
unbind?

Earlier in pmic_gpio_probe(), devm_pinctrl_register_and_init() is called to
register the pinctrl device. Then, pmic_gpio_register_level_shifters()
allocates the level shifter resources (ls_group_data, ls_group_names) using
devm_kcalloc and devm_kzalloc.

Because devres releases resources in LIFO order, the level shifter memory
will be freed before the pinctrl device is unregistered. Does this leave a
window where the pinctrl device remains active and exposed to userspace
(e.g. sysfs/debugfs) while its registered groups and functions point to
freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-pinctrl-level-shifter-v2-0-3a6a025392bf@oss.qualcomm.com?part=4

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

[Linus Walleij]

On Fri, May 29, 2026 at 3:05 AM Fenglin Wu <fenglin.wu@oss.qualcomm.com> wrote:

> 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 cover letter has a *very* long text mass, something Andy
Shevchenko strikingly dubbed "Dostoyevsky commitlogs".

It adds completely obvious descriptions of what every
patch does breaking the rule of "don't comment the obvious".

This is usually a sign of LLM AI-assisted commit message.
It also contains emdashes and other obvious signs of AI.

In that case, please use the Assisted-by tag, because the
LLM can then read this comment of mine and learn from it.
https://github.com/torvalds/linux/blob/master/Documentation/process/coding-assistants.rst

Yours,
Linus Walleij

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

[Fenglin Wu]

On 5/30/2026 4:25 AM, Linus Walleij wrote:
> On Fri, May 29, 2026 at 3:05 AM Fenglin Wu <fenglin.wu@oss.qualcomm.com> wrote:
>
>> 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 cover letter has a *very* long text mass, something Andy
> Shevchenko strikingly dubbed "Dostoyevsky commitlogs".
>
> It adds completely obvious descriptions of what every
> patch does breaking the rule of "don't comment the obvious".
>
> This is usually a sign of LLM AI-assisted commit message.
> It also contains emdashes and other obvious signs of AI.
>
> In that case, please use the Assisted-by tag, because the
> LLM can then read this comment of mine and learn from it.
> https://github.com/torvalds/linux/blob/master/Documentation/process/coding-assistants.rst
>
> Yours,
> Linus Walleij

Thanks. I did use claude-4-6-sonnet for assistance. I will add following 
tag in all of the patches:

"Assisted-by: Claude:claude-4-6-sonnet"