[PATCH v1 0/3] pinctrl: qcom: lpass-lpi: PM clock framework cleanup and fixes

[PATCH v1 0/3] pinctrl: qcom: lpass-lpi: PM clock framework cleanup and fixes

[Ajay Kumar Nandam]

This series updates the Qualcomm LPASS LPI pinctrl driver to use the
generic PM clock framework and fixes clock handling around GPIO register
access.

The changes ensure clocks are resumed before register access and
properly suspended afterward, avoiding unsafe register reads when the
device is runtime suspended.

Ajay Kumar Nandam (3):
  pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM
  pinctrl: qcom: lpass-lpi: Fix GPIO register access helper return types
  pinctrl: qcom: lpass-lpi: Resume clocks for GPIO access

 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c      | 120 +++++++++++++-----
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h      |   2 +
 .../pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c   |   5 +
 3 files changed, 97 insertions(+), 30 deletions(-)


base-commit: 66672af7a095d89f082c5327f3b15bc2f93d558e
-- 
2.34.1

[PATCH v1 1/3] pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM

[Ajay Kumar Nandam]

Convert the LPASS LPI pinctrl driver to use the PM clock framework for
runtime power management.

This allows the LPASS LPI pinctrl driver to drop clock votes when idle,
improves power efficiency on platforms using LPASS LPI island mode, and
aligns the driver with common runtime PM patterns used across Qualcomm
LPASS subsystems.

Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c      | 36 +++++++++++++------
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h      |  2 ++
 .../pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c   |  5 +++
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 76aed3296..6d50e06ef 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -14,15 +14,16 @@
 
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinconf.h>
+#include <linux/pm_runtime.h>
 #include <linux/pinctrl/pinmux.h>
 
 #include "../pinctrl-utils.h"
 
 #include "pinctrl-lpass-lpi.h"
+#include <linux/pm_clock.h>
 
 #define MAX_NR_GPIO		32
 #define GPIO_FUNC		0
-#define MAX_LPI_NUM_CLKS	2
 
 struct lpi_pinctrl {
 	struct device *dev;
@@ -31,7 +32,6 @@ struct lpi_pinctrl {
 	struct pinctrl_desc desc;
 	char __iomem *tlmm_base;
 	char __iomem *slew_base;
-	struct clk_bulk_data clks[MAX_LPI_NUM_CLKS];
 	/* Protects from concurrent register updates */
 	struct mutex lock;
 	DECLARE_BITMAP(ever_gpio, MAX_NR_GPIO);
@@ -480,9 +480,6 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
 	pctrl->data = data;
 	pctrl->dev = &pdev->dev;
 
-	pctrl->clks[0].id = "core";
-	pctrl->clks[1].id = "audio";
-
 	pctrl->tlmm_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pctrl->tlmm_base))
 		return dev_err_probe(dev, PTR_ERR(pctrl->tlmm_base),
@@ -495,13 +492,17 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
 					     "Slew resource not provided\n");
 	}
 
-	ret = devm_clk_bulk_get_optional(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
+	ret = devm_pm_clk_create(dev);
 	if (ret)
 		return ret;
 
-	ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
-	if (ret)
-		return dev_err_probe(dev, ret, "Can't enable clocks\n");
+	ret = of_pm_clk_add_clks(dev);
+	if (ret < 0)
+		return ret;
+
+	pm_runtime_set_autosuspend_delay(dev, 100);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
 
 	pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
 	pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
@@ -539,20 +540,33 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
 	return 0;
 
 err_pinctrl:
+	pm_runtime_disable(dev);
 	mutex_destroy(&pctrl->lock);
-	clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(lpi_pinctrl_probe);
 
+int lpi_pinctrl_runtime_suspend(struct device *dev)
+{
+	return pm_clk_suspend(dev);
+}
+EXPORT_SYMBOL_GPL(lpi_pinctrl_runtime_suspend);
+
+int lpi_pinctrl_runtime_resume(struct device *dev)
+{
+	return pm_clk_resume(dev);
+}
+EXPORT_SYMBOL_GPL(lpi_pinctrl_runtime_resume);
+
 void lpi_pinctrl_remove(struct platform_device *pdev)
 {
 	struct lpi_pinctrl *pctrl = platform_get_drvdata(pdev);
 	int i;
 
+	pm_runtime_disable(pctrl->dev);
+
 	mutex_destroy(&pctrl->lock);
-	clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);
 
 	for (i = 0; i < pctrl->data->npins; i++)
 		pinctrl_generic_remove_group(pctrl->ctrl, i);
diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
index f48368492..ae94ef48d 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
@@ -107,5 +107,7 @@ struct lpi_pinctrl_variant_data {
 
 int lpi_pinctrl_probe(struct platform_device *pdev);
 void lpi_pinctrl_remove(struct platform_device *pdev);
+int lpi_pinctrl_runtime_suspend(struct device *dev);
+int lpi_pinctrl_runtime_resume(struct device *dev);
 
 #endif /*__PINCTRL_LPASS_LPI_H__*/
diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
index 750f41031..2d955643d 100644
--- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
@@ -139,10 +139,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
 
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+	RUNTIME_PM_OPS(lpi_pinctrl_runtime_suspend, lpi_pinctrl_runtime_resume, NULL)
+};
+
 static struct platform_driver lpi_pinctrl_driver = {
 	.driver = {
 		   .name = "qcom-sc7280-lpass-lpi-pinctrl",
 		   .of_match_table = lpi_pinctrl_of_match,
+		   .pm = pm_ptr(&lpi_pinctrl_pm_ops),
 	},
 	.probe = lpi_pinctrl_probe,
 	.remove = lpi_pinctrl_remove,
-- 
2.34.1

Re: [PATCH v1 1/3] pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM

[Konrad Dybcio]

On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
> Convert the LPASS LPI pinctrl driver to use the PM clock framework for
> runtime power management.
> 
> This allows the LPASS LPI pinctrl driver to drop clock votes when idle,
> improves power efficiency on platforms using LPASS LPI island mode, and
> aligns the driver with common runtime PM patterns used across Qualcomm
> LPASS subsystems.
> 
> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
> ---

[...]

> +int lpi_pinctrl_runtime_suspend(struct device *dev)
> +{
> +	return pm_clk_suspend(dev);
> +}
> +EXPORT_SYMBOL_GPL(lpi_pinctrl_runtime_suspend);
> +
> +int lpi_pinctrl_runtime_resume(struct device *dev)
> +{
> +	return pm_clk_resume(dev);
> +}
> +EXPORT_SYMBOL_GPL(lpi_pinctrl_runtime_resume);

You can do:

SET_RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)

instead

Konrad

Re: [PATCH v1 1/3] pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM

[Ajay Kumar Nandam]

On 4/14/2026 2:03 PM, Konrad Dybcio wrote:
> On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
>> Convert the LPASS LPI pinctrl driver to use the PM clock framework for
>> runtime power management.
>>
>> This allows the LPASS LPI pinctrl driver to drop clock votes when idle,
>> improves power efficiency on platforms using LPASS LPI island mode, and
>> aligns the driver with common runtime PM patterns used across Qualcomm
>> LPASS subsystems.
>>
>> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +int lpi_pinctrl_runtime_suspend(struct device *dev)
>> +{
>> +	return pm_clk_suspend(dev);
>> +}
>> +EXPORT_SYMBOL_GPL(lpi_pinctrl_runtime_suspend);
>> +
>> +int lpi_pinctrl_runtime_resume(struct device *dev)
>> +{
>> +	return pm_clk_resume(dev);
>> +}
>> +EXPORT_SYMBOL_GPL(lpi_pinctrl_runtime_resume);
> 
> You can do:
> 
> SET_RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
> 
> instead

ACK, will post accordingly in v2

> 
> Konrad

Re: [PATCH v1 1/3] pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM

[Konrad Dybcio]

On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
> Convert the LPASS LPI pinctrl driver to use the PM clock framework for
> runtime power management.
> 
> This allows the LPASS LPI pinctrl driver to drop clock votes when idle,
> improves power efficiency on platforms using LPASS LPI island mode, and
> aligns the driver with common runtime PM patterns used across Qualcomm
> LPASS subsystems.
> 
> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
> ---
>  drivers/pinctrl/qcom/pinctrl-lpass-lpi.c      | 36 +++++++++++++------
>  drivers/pinctrl/qcom/pinctrl-lpass-lpi.h      |  2 ++
>  .../pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c   |  5 +++
>  3 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> index 76aed3296..6d50e06ef 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> @@ -14,15 +14,16 @@
>  
>  #include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/pinctrl/pinconf.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/pinctrl/pinmux.h>
>  
>  #include "../pinctrl-utils.h"
>  
>  #include "pinctrl-lpass-lpi.h"
> +#include <linux/pm_clock.h>

Please move it up, together with other non-local includes

[...]

> +	pm_runtime_set_autosuspend_delay(dev, 100);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_enable(dev);

devm_pm_runtime_enable() will let you drop the manual disablement below and
in .remove()

>  
>  	pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
>  	pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
> @@ -539,20 +540,33 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_pinctrl:
> +	pm_runtime_disable(dev);
>  	mutex_destroy(&pctrl->lock);
> -	clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);
>  
>  	return ret;
>  }

[...]

> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> @@ -139,10 +139,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
>  
> +static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
> +	RUNTIME_PM_OPS(lpi_pinctrl_runtime_suspend, lpi_pinctrl_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver lpi_pinctrl_driver = {
>  	.driver = {
>  		   .name = "qcom-sc7280-lpass-lpi-pinctrl",
>  		   .of_match_table = lpi_pinctrl_of_match,
> +		   .pm = pm_ptr(&lpi_pinctrl_pm_ops),

I believe SoCs other than kodiak also require this change to avoid
regressions.

Konrad

Re: [PATCH v1 1/3] pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM

[Ajay Kumar Nandam]

On 4/14/2026 2:07 PM, Konrad Dybcio wrote:
> On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
>> Convert the LPASS LPI pinctrl driver to use the PM clock framework for
>> runtime power management.
>>
>> This allows the LPASS LPI pinctrl driver to drop clock votes when idle,
>> improves power efficiency on platforms using LPASS LPI island mode, and
>> aligns the driver with common runtime PM patterns used across Qualcomm
>> LPASS subsystems.
>>
>> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
>> ---
>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c      | 36 +++++++++++++------
>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.h      |  2 ++
>>   .../pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c   |  5 +++
>>   3 files changed, 32 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> index 76aed3296..6d50e06ef 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> @@ -14,15 +14,16 @@
>>   
>>   #include <linux/pinctrl/pinconf-generic.h>
>>   #include <linux/pinctrl/pinconf.h>
>> +#include <linux/pm_runtime.h>
>>   #include <linux/pinctrl/pinmux.h>
>>   
>>   #include "../pinctrl-utils.h"
>>   
>>   #include "pinctrl-lpass-lpi.h"
>> +#include <linux/pm_clock.h>
> 
> Please move it up, together with other non-local includes
> 
> [...]

ACK, will correct in the next version

Thanks
Ajay Kumar Nandam

> 
>> +	pm_runtime_set_autosuspend_delay(dev, 100);
>> +	pm_runtime_use_autosuspend(dev);
>> +	pm_runtime_enable(dev);
> 
> devm_pm_runtime_enable() will let you drop the manual disablement below and
> in .remove()

ACK, will switch in the next version.

Thanks
Ajay Kumar Nandam

> 
>>   
>>   	pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
>>   	pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
>> @@ -539,20 +540,33 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
>>   	return 0;
>>   
>>   err_pinctrl:
>> +	pm_runtime_disable(dev);
>>   	mutex_destroy(&pctrl->lock);
>> -	clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);
>>   
>>   	return ret;
>>   }
> 
> [...]
> 
>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> @@ -139,10 +139,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
>>   };
>>   MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
>>   
>> +static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
>> +	RUNTIME_PM_OPS(lpi_pinctrl_runtime_suspend, lpi_pinctrl_runtime_resume, NULL)
>> +};
>> +
>>   static struct platform_driver lpi_pinctrl_driver = {
>>   	.driver = {
>>   		   .name = "qcom-sc7280-lpass-lpi-pinctrl",
>>   		   .of_match_table = lpi_pinctrl_of_match,
>> +		   .pm = pm_ptr(&lpi_pinctrl_pm_ops),
> 
> I believe SoCs other than kodiak also require this change to avoid
> regressions.

This patch is currently limited to SC7280, where the runtime PM behavior
has been validated.

Based on this feedback, I’ll revisit the series and extend the same
runtime PM support to the other LPASS‑LPI SoC drivers that share this
block, so the behavior remains consistent across platforms and avoids
potential regressions.

Thanks
Ajay Kumar Nandam



> 
> Konrad

[PATCH v1 2/3] pinctrl: qcom: lpass-lpi: Fix GPIO register access helper return types

[Ajay Kumar Nandam]

The LPI GPIO register access helpers previously returned the value from
ioread32(), even though their return type was int. This mixes data
return with status and is inconsistent with common kernel helper
conventions.

Rework lpi_gpio_read() and lpi_gpio_write() to return an int status and
use output parameters to pass register values. Update all callers to
match the new helper interface.

This change fixes the helper API and resulting call sites without
intending any functional change in GPIO or pinctrl behavior.

Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 66 +++++++++++++++++-------
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 6d50e06ef..d108e7321 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -39,22 +39,26 @@ struct lpi_pinctrl {
 };
 
 static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
-			 unsigned int addr)
+			 unsigned int addr, u32 *val)
 {
 	u32 pin_offset;
+	int ret;
 
 	if (state->data->flags & LPI_FLAG_USE_PREDEFINED_PIN_OFFSET)
 		pin_offset = state->data->groups[pin].pin_offset;
 	else
 		pin_offset = LPI_TLMM_REG_OFFSET * pin;
 
-	return ioread32(state->tlmm_base + pin_offset + addr);
+	*val = ioread32(state->tlmm_base + pin_offset + addr);
+
+	return 0;
 }
 
 static int lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin,
 			  unsigned int addr, unsigned int val)
 {
 	u32 pin_offset;
+	int ret;
 
 	if (state->data->flags & LPI_FLAG_USE_PREDEFINED_PIN_OFFSET)
 		pin_offset = state->data->groups[pin].pin_offset;
@@ -107,7 +111,8 @@ static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 {
 	struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 	const struct lpi_pingroup *g = &pctrl->data->groups[group];
-	u32 val;
+	u32 val, io_val;
+	int ret;
 	int i, pin = g->pin;
 
 	for (i = 0; i < g->nfuncs; i++) {
@@ -119,7 +124,9 @@ static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 		return -EINVAL;
 
 	mutex_lock(&pctrl->lock);
-	val = lpi_gpio_read(pctrl, pin, LPI_GPIO_CFG_REG);
+	ret = lpi_gpio_read(pctrl, pin, LPI_GPIO_CFG_REG, &val);
+	if (ret)
+		goto out_unlock;
 
 	/*
 	 * If this is the first time muxing to GPIO and the direction is
@@ -129,24 +136,28 @@ static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 	 */
 	if (i == GPIO_FUNC && (val & LPI_GPIO_OE_MASK) &&
 	    !test_and_set_bit(group, pctrl->ever_gpio)) {
-		u32 io_val = lpi_gpio_read(pctrl, group, LPI_GPIO_VALUE_REG);
+		ret = lpi_gpio_read(pctrl, group, LPI_GPIO_VALUE_REG, &io_val);
+		if (ret)
+			goto out_unlock;
 
 		if (io_val & LPI_GPIO_VALUE_IN_MASK) {
 			if (!(io_val & LPI_GPIO_VALUE_OUT_MASK))
-				lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
-					       io_val | LPI_GPIO_VALUE_OUT_MASK);
+				ret = lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
+						     io_val | LPI_GPIO_VALUE_OUT_MASK);
 		} else {
 			if (io_val & LPI_GPIO_VALUE_OUT_MASK)
-				lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
-					       io_val & ~LPI_GPIO_VALUE_OUT_MASK);
+				ret = lpi_gpio_write(pctrl, group, LPI_GPIO_VALUE_REG,
+						     io_val & ~LPI_GPIO_VALUE_OUT_MASK);
 		}
 	}
 
 	u32p_replace_bits(&val, i, LPI_GPIO_FUNCTION_MASK);
-	lpi_gpio_write(pctrl, pin, LPI_GPIO_CFG_REG, val);
+	ret = lpi_gpio_write(pctrl, pin, LPI_GPIO_CFG_REG, val);
+
+out_unlock:
 	mutex_unlock(&pctrl->lock);
 
-	return 0;
+	return ret;
 }
 
 static const struct pinmux_ops lpi_gpio_pinmux_ops = {
@@ -165,8 +176,11 @@ static int lpi_config_get(struct pinctrl_dev *pctldev,
 	int is_out;
 	int pull;
 	u32 ctl_reg;
+	int ret;
 
-	ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_CFG_REG);
+	ret = lpi_gpio_read(state, pin, LPI_GPIO_CFG_REG, &ctl_reg);
+	if (ret)
+		return ret;
 	is_out = ctl_reg & LPI_GPIO_OE_MASK;
 	pull = FIELD_GET(LPI_GPIO_PULL_MASK, ctl_reg);
 
@@ -293,17 +307,22 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
 	}
 
 	mutex_lock(&pctrl->lock);
-	val = lpi_gpio_read(pctrl, group, LPI_GPIO_CFG_REG);
+	ret = lpi_gpio_read(pctrl, group, LPI_GPIO_CFG_REG, &val);
+	if (ret) {
+		mutex_unlock(&pctrl->lock);
+		goto out_unlock;
+	}
 
 	u32p_replace_bits(&val, pullup, LPI_GPIO_PULL_MASK);
 	u32p_replace_bits(&val, LPI_GPIO_DS_TO_VAL(strength),
 			  LPI_GPIO_OUT_STRENGTH_MASK);
 	u32p_replace_bits(&val, output_enabled, LPI_GPIO_OE_MASK);
 
-	lpi_gpio_write(pctrl, group, LPI_GPIO_CFG_REG, val);
-	mutex_unlock(&pctrl->lock);
+	ret = lpi_gpio_write(pctrl, group, LPI_GPIO_CFG_REG, val);
 
-	return 0;
+out_unlock:
+	mutex_unlock(&pctrl->lock);
+	return ret;
 }
 
 static const struct pinconf_ops lpi_gpio_pinconf_ops = {
@@ -352,9 +371,13 @@ static int lpi_gpio_direction_output(struct gpio_chip *chip,
 static int lpi_gpio_get(struct gpio_chip *chip, unsigned int pin)
 {
 	struct lpi_pinctrl *state = gpiochip_get_data(chip);
+	u32 val;
+	int ret;
 
-	return lpi_gpio_read(state, pin, LPI_GPIO_VALUE_REG) &
-		LPI_GPIO_VALUE_IN_MASK;
+	ret = lpi_gpio_read(state, pin, LPI_GPIO_VALUE_REG, &val);
+	if (ret)
+		return ret;
+	return val & LPI_GPIO_VALUE_IN_MASK;
 }
 
 static int lpi_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
@@ -387,6 +410,7 @@ static void lpi_gpio_dbg_show_one(struct seq_file *s,
 	int drive;
 	int pull;
 	u32 ctl_reg;
+	int ret;
 
 	static const char * const pulls[] = {
 		"no pull",
@@ -397,7 +421,11 @@ static void lpi_gpio_dbg_show_one(struct seq_file *s,
 
 	pctldev = pctldev ? : state->ctrl;
 	pindesc = pctldev->desc->pins[offset];
-	ctl_reg = lpi_gpio_read(state, offset, LPI_GPIO_CFG_REG);
+	ret = lpi_gpio_read(state, offset, LPI_GPIO_CFG_REG, &ctl_reg);
+	if (ret) {
+		seq_printf(s, " %-8s: <read error %d>", pindesc.name, ret);
+		return;
+	}
 	is_out = ctl_reg & LPI_GPIO_OE_MASK;
 
 	func = FIELD_GET(LPI_GPIO_FUNCTION_MASK, ctl_reg);
-- 
2.34.1

Re: [PATCH v1 2/3] pinctrl: qcom: lpass-lpi: Fix GPIO register access helper return types

[Konrad Dybcio]

On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
> The LPI GPIO register access helpers previously returned the value from
> ioread32(), even though their return type was int. This mixes data
> return with status and is inconsistent with common kernel helper
> conventions.
> 
> Rework lpi_gpio_read() and lpi_gpio_write() to return an int status and
> use output parameters to pass register values. Update all callers to
> match the new helper interface.

lpi_gpio_read/write() can never fail, let's just make _read() return
a u32 and _write() a void

Konrad

Re: [PATCH v1 2/3] pinctrl: qcom: lpass-lpi: Fix GPIO register access helper return types

[Ajay Kumar Nandam]

On 4/14/2026 2:09 PM, Konrad Dybcio wrote:
> On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
>> The LPI GPIO register access helpers previously returned the value from
>> ioread32(), even though their return type was int. This mixes data
>> return with status and is inconsistent with common kernel helper
>> conventions.
>>
>> Rework lpi_gpio_read() and lpi_gpio_write() to return an int status and
>> use output parameters to pass register values. Update all callers to
>> match the new helper interface.
> 
> lpi_gpio_read/write() can never fail, let's just make _read() return
> a u32 and _write() a void

ACK, I’ll post v2 with this adjustment.
> 
> Konrad

[PATCH v1 3/3] pinctrl: qcom: lpass-lpi: Resume clocks for GPIO access

[Ajay Kumar Nandam]

Ensure the LPI pinctrl device clocks are runtime resumed
before accessing GPIO registers and autosuspended after
the access completes.

Guard GPIO register read and write helpers with synchronous
runtime PM calls so the device is active during MMIO
operations.

Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index d108e7321..4275f2734 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -49,8 +49,17 @@ static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
 	else
 		pin_offset = LPI_TLMM_REG_OFFSET * pin;
 
+	ret = pm_runtime_get_sync(state->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(state->dev);
+		return ret;
+	}
+
 	*val = ioread32(state->tlmm_base + pin_offset + addr);
 
+	pm_runtime_mark_last_busy(state->dev);
+	pm_runtime_put_autosuspend(state->dev);
+
 	return 0;
 }
 
@@ -65,8 +74,17 @@ static int lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin,
 	else
 		pin_offset = LPI_TLMM_REG_OFFSET * pin;
 
+	ret = pm_runtime_get_sync(state->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(state->dev);
+		return ret;
+	}
+
 	iowrite32(val, state->tlmm_base + pin_offset + addr);
 
+	pm_runtime_mark_last_busy(state->dev);
+	pm_runtime_put_autosuspend(state->dev);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH v1 3/3] pinctrl: qcom: lpass-lpi: Resume clocks for GPIO access

[Konrad Dybcio]

On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
> Ensure the LPI pinctrl device clocks are runtime resumed
> before accessing GPIO registers and autosuspended after
> the access completes.
> 
> Guard GPIO register read and write helpers with synchronous
> runtime PM calls so the device is active during MMIO
> operations.
> 
> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
> ---
>  drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> index d108e7321..4275f2734 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> @@ -49,8 +49,17 @@ static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
>  	else
>  		pin_offset = LPI_TLMM_REG_OFFSET * pin;
>  
> +	ret = pm_runtime_get_sync(state->dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(state->dev);
> +		return ret;
> +	}

Okay that's how they can fail..

Please move from pm_runtime_get_sync() to pm_runtime_resume_and_get() or
someone will come around next week to "improve" it

> +
>  	*val = ioread32(state->tlmm_base + pin_offset + addr);
>  
> +	pm_runtime_mark_last_busy(state->dev);
> +	pm_runtime_put_autosuspend(state->dev);

put_autosuspend() already does mark_last_busy() nowadays

Konrad

Re: [PATCH v1 3/3] pinctrl: qcom: lpass-lpi: Resume clocks for GPIO access

[Ajay Kumar Nandam]

On 4/14/2026 2:14 PM, Konrad Dybcio wrote:
> On 4/13/26 2:22 PM, Ajay Kumar Nandam wrote:
>> Ensure the LPI pinctrl device clocks are runtime resumed
>> before accessing GPIO registers and autosuspended after
>> the access completes.
>>
>> Guard GPIO register read and write helpers with synchronous
>> runtime PM calls so the device is active during MMIO
>> operations.
>>
>> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
>> ---
>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> index d108e7321..4275f2734 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> @@ -49,8 +49,17 @@ static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
>>   	else
>>   		pin_offset = LPI_TLMM_REG_OFFSET * pin;
>>   
>> +	ret = pm_runtime_get_sync(state->dev);
>> +	if (ret < 0) {
>> +		pm_runtime_put_noidle(state->dev);
>> +		return ret;
>> +	}
> 
> Okay that's how they can fail..
> 
> Please move from pm_runtime_get_sync() to pm_runtime_resume_and_get() or
> someone will come around next week to "improve" it

ACK, Will update the patch accordingly and post v2

> 
>> +
>>   	*val = ioread32(state->tlmm_base + pin_offset + addr);
>>   
>> +	pm_runtime_mark_last_busy(state->dev);
>> +	pm_runtime_put_autosuspend(state->dev);
> 
> put_autosuspend() already does mark_last_busy() nowadays

ACK, Will update the patch accordingly and post v2

Thanks
Ajay Kumar Nandam
> 
> Konrad