[PATCH v5 0/3] iio: adc: ingenic-adc: use guard()() to handle synchronisation and refactor lock-held helpers

[PATCH v5 0/3] iio: adc: ingenic-adc: use guard()() to handle synchronisation and refactor lock-held helpers

[Felipe Ribeiro de Souza]

In drivers/iio/adc/ingenic-adc.c:
- Refactor ingenic_adc_read_chan_info_raw() and ingenic_adc_enable_unlocked()
- Replace mutex_lock() and mutex_unlock() calls with guard()()

Felipe Ribeiro de Souza (3):
  iio: adc: ingenic-adc: rename ingenic_adc_enable_unlocked() function
  iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()
  iio: adc: ingenic-adc: use guard()() and scoped_guard() to handle
    synchronisation

 drivers/iio/adc/ingenic-adc.c | 68 ++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 33 deletions(-)

-- 
2.43.0

[PATCH v5 1/3] iio: adc: ingenic-adc: rename ingenic_adc_enable_unlocked() function

[Felipe Ribeiro de Souza]

Rename ingenic_adc_enable_unlocked() to __ingenic_adc_enable() to
better reflect that this helper must be called with the lock held.

Signed-off-by: Felipe Ribeiro de Souza <felipers@ime.usp.br>
Co-developed-by: Lucas Ivars Cadima Ciziks <lucas@ciziks.com>
Signed-off-by: Lucas Ivars Cadima Ciziks <lucas@ciziks.com>

---
v2:
  - Adjust order of #include.
  - Replace guard() in ingenic_adc_read_chan_info_raw() by scoped_guard()
    to preserve original behavior.
v3:
  - Split out ingenic_adc_read_chan_info_raw() logic to helper function.
v4:
  - Adjust code style for better readability.
v5:
  - Rename ingenic_adc_enable_unlocked() to __ingenic_adc_enable() and
    ingenic_adc_read_chan_locked() to __ingenic_adc_read_chan() to keep
    the convention about the locked functions name.
  - Move the mutex lock back to ingenic_adc_read_chan_info_raw().
---
 drivers/iio/adc/ingenic-adc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 1e802c8779a4c..231ff8d584c2d 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -181,9 +181,8 @@ static void ingenic_adc_set_config(struct ingenic_adc *adc,
 	mutex_unlock(&adc->lock);
 }
 
-static void ingenic_adc_enable_unlocked(struct ingenic_adc *adc,
-					int engine,
-					bool enabled)
+static void __ingenic_adc_enable(struct ingenic_adc *adc, int engine,
+			      bool enabled)
 {
 	u8 val;
 
@@ -202,7 +201,7 @@ static void ingenic_adc_enable(struct ingenic_adc *adc,
 			       bool enabled)
 {
 	mutex_lock(&adc->lock);
-	ingenic_adc_enable_unlocked(adc, engine, enabled);
+	__ingenic_adc_enable(adc, engine, enabled);
 	mutex_unlock(&adc->lock);
 }
 
@@ -222,11 +221,11 @@ static int ingenic_adc_capture(struct ingenic_adc *adc,
 	cfg = readl(adc->base + JZ_ADC_REG_CFG);
 	writel(cfg & ~JZ_ADC_REG_CFG_CMD_SEL, adc->base + JZ_ADC_REG_CFG);
 
-	ingenic_adc_enable_unlocked(adc, engine, true);
+	__ingenic_adc_enable(adc, engine, true);
 	ret = readb_poll_timeout(adc->base + JZ_ADC_REG_ENABLE, val,
 				 !(val & BIT(engine)), 250, 1000);
 	if (ret)
-		ingenic_adc_enable_unlocked(adc, engine, false);
+		__ingenic_adc_enable(adc, engine, false);
 
 	writel(cfg, adc->base + JZ_ADC_REG_CFG);
 	mutex_unlock(&adc->lock);
-- 
2.43.0

[PATCH v5 2/3] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()

[Felipe Ribeiro de Souza]

Extract the sample logic from ingenic_adc_read_chan_info_raw() into
a new helper function __ingenic_adc_read_chan() to improve code
readability and modularity.

The helper handles the mutex-protected section for sampling channels,
while the main function manages mutex and clock enabling/disabling.

Signed-off-by: Felipe Ribeiro de Souza <felipers@ime.usp.br>
Co-developed-by: Lucas Ivars Cadima Ciziks <lucas@ciziks.com>
Signed-off-by: Lucas Ivars Cadima Ciziks <lucas@ciziks.com>

---
v2:
  - Adjust order of #include.
  - Replace guard() in ingenic_adc_read_chan_info_raw() by scoped_guard()
    to preserve original behavior.
v3:
  - Split out ingenic_adc_read_chan_info_raw() logic to helper function.
v4:
  - Adjust code style for better readability.
v5:
  - Rename ingenic_adc_enable_unlocked() to __ingenic_adc_enable() and
    ingenic_adc_read_chan_locked() to __ingenic_adc_read_chan() to keep
    the convention about the locked functions name.
  - Move the mutex lock back to ingenic_adc_read_chan_info_raw().
---
 drivers/iio/adc/ingenic-adc.c | 40 +++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 231ff8d584c2d..91e3ea6615942 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -627,22 +627,12 @@ static int ingenic_adc_read_avail(struct iio_dev *iio_dev,
 	}
 }
 
-static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
-					  struct iio_chan_spec const *chan,
-					  int *val)
+static int __ingenic_adc_read_chan(struct ingenic_adc *adc,
+				   struct iio_chan_spec const *chan,
+				   int *val)
 {
 	int cmd, ret, engine = (chan->channel == INGENIC_ADC_BATTERY);
-	struct ingenic_adc *adc = iio_priv(iio_dev);
-
-	ret = clk_enable(adc->clk);
-	if (ret) {
-		dev_err(iio_dev->dev.parent, "Failed to enable clock: %d\n",
-			ret);
-		return ret;
-	}
 
-	/* We cannot sample the aux channels in parallel. */
-	mutex_lock(&adc->aux_lock);
 	if (adc->soc_data->has_aux_md && engine == 0) {
 		switch (chan->channel) {
 		case INGENIC_ADC_AUX0:
@@ -661,7 +651,7 @@ static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
 
 	ret = ingenic_adc_capture(adc, engine);
 	if (ret)
-		goto out;
+		return ret;
 
 	switch (chan->channel) {
 	case INGENIC_ADC_AUX0:
@@ -674,9 +664,27 @@ static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
 		break;
 	}
 
-	ret = IIO_VAL_INT;
-out:
+	return IIO_VAL_INT;
+}
+
+static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
+					  struct iio_chan_spec const *chan,
+					  int *val)
+{
+	struct ingenic_adc *adc = iio_priv(iio_dev);
+	int ret;
+
+	ret = clk_enable(adc->clk);
+	if (ret) {
+		dev_err(iio_dev->dev.parent, "Failed to enable clock: %d\n", ret);
+		return ret;
+	}
+
+	/* We cannot sample the aux channels in parallel. */
+	mutex_lock(&adc->aux_lock);
+	ret = __ingenic_adc_read_chan(adc, chan, val);
 	mutex_unlock(&adc->aux_lock);
+
 	clk_disable(adc->clk);
 
 	return ret;
-- 
2.43.0

[PATCH v5 3/3] iio: adc: ingenic-adc: use guard()() and scoped_guard() to handle synchronisation

[Felipe Ribeiro de Souza]

Replace mutex_lock() and mutex_unlock() calls with guard()() in
functions ingenic_adc_set_adcmd(), ingenic_adc_set_config(),
ingenic_adc_enable(), ingenic_adc_capture(), and with scoped_guard()
in function ingenic_adc_read_chan_info_raw().

This removes the need to call the unlock function, as the lock is
automatically released when the function return or the scope exits
for any other case.

Signed-off-by: Felipe Ribeiro de Souza <felipers@ime.usp.br>
Co-developed-by: Lucas Ivars Cadima Ciziks <lucas@ciziks.com>
Signed-off-by: Lucas Ivars Cadima Ciziks <lucas@ciziks.com>

---
v2:
  - Adjust order of #include.
  - Replace guard() in ingenic_adc_read_chan_info_raw() by scoped_guard()
    to preserve original behavior.
v3:
  - Split out ingenic_adc_read_chan_info_raw() logic to helper function.
v4:
  - Adjust code style for better readability.
v5:
  - Rename ingenic_adc_enable_unlocked() to __ingenic_adc_enable() and
    ingenic_adc_read_chan_locked() to __ingenic_adc_read_chan() to keep
    the convention about the locked functions name.
  - Move the mutex lock back to ingenic_adc_read_chan_info_raw().
---
 drivers/iio/adc/ingenic-adc.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 91e3ea6615942..414f69acab7b3 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -7,6 +7,8 @@
  */
 
 #include <dt-bindings/iio/adc/ingenic,adc.h>
+
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
@@ -115,7 +117,7 @@ static void ingenic_adc_set_adcmd(struct iio_dev *iio_dev, unsigned long mask)
 {
 	struct ingenic_adc *adc = iio_priv(iio_dev);
 
-	mutex_lock(&adc->lock);
+	guard(mutex)(&adc->lock);
 
 	/* Init ADCMD */
 	readl(adc->base + JZ_ADC_REG_ADCMD);
@@ -162,8 +164,6 @@ static void ingenic_adc_set_adcmd(struct iio_dev *iio_dev, unsigned long mask)
 
 	/* We're done */
 	writel(0, adc->base + JZ_ADC_REG_ADCMD);
-
-	mutex_unlock(&adc->lock);
 }
 
 static void ingenic_adc_set_config(struct ingenic_adc *adc,
@@ -172,13 +172,11 @@ static void ingenic_adc_set_config(struct ingenic_adc *adc,
 {
 	uint32_t cfg;
 
-	mutex_lock(&adc->lock);
+	guard(mutex)(&adc->lock);
 
 	cfg = readl(adc->base + JZ_ADC_REG_CFG) & ~mask;
 	cfg |= val;
 	writel(cfg, adc->base + JZ_ADC_REG_CFG);
-
-	mutex_unlock(&adc->lock);
 }
 
 static void __ingenic_adc_enable(struct ingenic_adc *adc, int engine,
@@ -200,9 +198,8 @@ static void ingenic_adc_enable(struct ingenic_adc *adc,
 			       int engine,
 			       bool enabled)
 {
-	mutex_lock(&adc->lock);
+	guard(mutex)(&adc->lock);
 	__ingenic_adc_enable(adc, engine, enabled);
-	mutex_unlock(&adc->lock);
 }
 
 static int ingenic_adc_capture(struct ingenic_adc *adc,
@@ -217,7 +214,7 @@ static int ingenic_adc_capture(struct ingenic_adc *adc,
 	 * probably due to the switch of VREF. We must keep the lock here to
 	 * avoid races with the buffer enable/disable functions.
 	 */
-	mutex_lock(&adc->lock);
+	guard(mutex)(&adc->lock);
 	cfg = readl(adc->base + JZ_ADC_REG_CFG);
 	writel(cfg & ~JZ_ADC_REG_CFG_CMD_SEL, adc->base + JZ_ADC_REG_CFG);
 
@@ -228,7 +225,6 @@ static int ingenic_adc_capture(struct ingenic_adc *adc,
 		__ingenic_adc_enable(adc, engine, false);
 
 	writel(cfg, adc->base + JZ_ADC_REG_CFG);
-	mutex_unlock(&adc->lock);
 
 	return ret;
 }
@@ -681,9 +677,8 @@ static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
 	}
 
 	/* We cannot sample the aux channels in parallel. */
-	mutex_lock(&adc->aux_lock);
-	ret = __ingenic_adc_read_chan(adc, chan, val);
-	mutex_unlock(&adc->aux_lock);
+	scoped_guard(mutex, &adc->aux_lock)
+		ret = __ingenic_adc_read_chan(adc, chan, val);
 
 	clk_disable(adc->clk);
 
-- 
2.43.0

Re: [PATCH v5 3/3] iio: adc: ingenic-adc: use guard()() and scoped_guard() to handle synchronisation

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:24:30PM -0300, Felipe Ribeiro de Souza wrote:
> Replace mutex_lock() and mutex_unlock() calls with guard()() in
> functions ingenic_adc_set_adcmd(), ingenic_adc_set_config(),
> ingenic_adc_enable(), ingenic_adc_capture(), and with scoped_guard()
> in function ingenic_adc_read_chan_info_raw().
> 
> This removes the need to call the unlock function, as the lock is
> automatically released when the function return or the scope exits
> for any other case.

...

>  static void ingenic_adc_set_config(struct ingenic_adc *adc,
>  {
>  	uint32_t cfg;

Side note: We use uXX types in the kernel (you can do a separate change
if you want to).

>  
> -	mutex_lock(&adc->lock);
> +	guard(mutex)(&adc->lock);
>  
>  	cfg = readl(adc->base + JZ_ADC_REG_CFG) & ~mask;
>  	cfg |= val;
>  	writel(cfg, adc->base + JZ_ADC_REG_CFG);

Side note (in case you want another patch): this is not so robust against
arbitrary values. The traditional pattern of custom FIELD_MODIFY() is this

	cfg = readl(adc->base + JZ_ADC_REG_CFG);
	cfg = (cfg & ~mask) | (val & mask);
	writel(cfg, adc->base + JZ_ADC_REG_CFG);

> -	mutex_unlock(&adc->lock);
>  }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/3] iio: adc: ingenic-adc: use guard()() to handle synchronisation and refactor lock-held helpers

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:24:27PM -0300, Felipe Ribeiro de Souza wrote:
> In drivers/iio/adc/ingenic-adc.c:
> - Refactor ingenic_adc_read_chan_info_raw() and ingenic_adc_enable_unlocked()
> - Replace mutex_lock() and mutex_unlock() calls with guard()()

This version LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

In case you want to cook more cleanups, I provided a couple of side notes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/3] iio: adc: ingenic-adc: use guard()() to handle synchronisation and refactor lock-held helpers

[Jonathan Cameron]

On Wed, 6 May 2026 12:20:27 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, May 05, 2026 at 11:24:27PM -0300, Felipe Ribeiro de Souza wrote:
> > In drivers/iio/adc/ingenic-adc.c:
> > - Refactor ingenic_adc_read_chan_info_raw() and ingenic_adc_enable_unlocked()
> > - Replace mutex_lock() and mutex_unlock() calls with guard()()  
> 
> This version LGTM,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
> In case you want to cook more cleanups, I provided a couple of side notes.
> 

Applied to the testing branch of iio.git

thanks

Jonathan