[PATCH] iio: adc: ingenic-adc: use guard(mutex)(&lock) to handle synchronisation

[PATCH] iio: adc: ingenic-adc: use guard(mutex)(&lock) to handle synchronisation

[Felipe Ribeiro de Souza]

Replace mutex_lock(&lock) and mutex_unlock(&lock) calls with
guard(mutex)(&lock) in functions ingenic_adc_set_adcmd,
ingenic_adc_set_config, ingenic_adc_enable, ingenic_adc_capture
and 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>
---
 drivers/iio/adc/ingenic-adc.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 1e802c877..bb7abdcd8 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/cleanup.h>
 
 #define JZ_ADC_REG_ENABLE		0x00
 #define JZ_ADC_REG_CFG			0x04
@@ -115,7 +116,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 +163,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 +171,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_unlocked(struct ingenic_adc *adc,
@@ -201,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_unlocked(adc, engine, enabled);
-	mutex_unlock(&adc->lock);
 }
 
 static int ingenic_adc_capture(struct ingenic_adc *adc,
@@ -218,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);
 
@@ -229,7 +225,6 @@ static int ingenic_adc_capture(struct ingenic_adc *adc,
 		ingenic_adc_enable_unlocked(adc, engine, false);
 
 	writel(cfg, adc->base + JZ_ADC_REG_CFG);
-	mutex_unlock(&adc->lock);
 
 	return ret;
 }
@@ -643,7 +638,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);
+	guard(mutex)(&adc->lock);
+
 	if (adc->soc_data->has_aux_md && engine == 0) {
 		switch (chan->channel) {
 		case INGENIC_ADC_AUX0:
@@ -677,7 +673,6 @@ static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
 
 	ret = IIO_VAL_INT;
 out:
-	mutex_unlock(&adc->aux_lock);
 	clk_disable(adc->clk);
 
 	return ret;
-- 
2.43.0

Re: [PATCH] iio: adc: ingenic-adc: use guard(mutex)(&lock) to handle synchronisation

[Paul Cercueil]

Hi Felipe,

Le mercredi 15 avril 2026 à 22:18 -0300, Felipe Ribeiro de Souza a
écrit :
> Replace mutex_lock(&lock) and mutex_unlock(&lock) calls with
> guard(mutex)(&lock) in functions ingenic_adc_set_adcmd,
> ingenic_adc_set_config, ingenic_adc_enable, ingenic_adc_capture
> and 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>

I see a slight behaviour change in ingenic_adc_read_chan_info_raw()
where the mutex is now unlocked after the clk_disable(). I think that's
fine though. So:

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/iio/adc/ingenic-adc.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-
> adc.c
> index 1e802c877..bb7abdcd8 100644
> --- a/drivers/iio/adc/ingenic-adc.c
> +++ b/drivers/iio/adc/ingenic-adc.c
> @@ -20,6 +20,7 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
> +#include <linux/cleanup.h>
>  
>  #define JZ_ADC_REG_ENABLE		0x00
>  #define JZ_ADC_REG_CFG			0x04
> @@ -115,7 +116,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 +163,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 +171,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_unlocked(struct ingenic_adc *adc,
> @@ -201,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_unlocked(adc, engine, enabled);
> -	mutex_unlock(&adc->lock);
>  }
>  
>  static int ingenic_adc_capture(struct ingenic_adc *adc,
> @@ -218,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);
>  
> @@ -229,7 +225,6 @@ static int ingenic_adc_capture(struct ingenic_adc
> *adc,
>  		ingenic_adc_enable_unlocked(adc, engine, false);
>  
>  	writel(cfg, adc->base + JZ_ADC_REG_CFG);
> -	mutex_unlock(&adc->lock);
>  
>  	return ret;
>  }
> @@ -643,7 +638,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);
> +	guard(mutex)(&adc->lock);
> +
>  	if (adc->soc_data->has_aux_md && engine == 0) {
>  		switch (chan->channel) {
>  		case INGENIC_ADC_AUX0:
> @@ -677,7 +673,6 @@ static int ingenic_adc_read_chan_info_raw(struct
> iio_dev *iio_dev,
>  
>  	ret = IIO_VAL_INT;
>  out:
> -	mutex_unlock(&adc->aux_lock);
>  	clk_disable(adc->clk);
>  
>  	return ret;

Re: [PATCH] iio: adc: ingenic-adc: use guard(mutex)(&lock) to handle synchronisation

[Rong Zhang]

Hi Felipe,

On Wed, 2026-04-15 at 22:18 -0300, Felipe Ribeiro de Souza wrote:
> Replace mutex_lock(&lock) and mutex_unlock(&lock) calls with
> guard(mutex)(&lock) in functions ingenic_adc_set_adcmd,
> ingenic_adc_set_config, ingenic_adc_enable, ingenic_adc_capture
> and 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>
> ---
>  drivers/iio/adc/ingenic-adc.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
> index 1e802c877..bb7abdcd8 100644
> --- a/drivers/iio/adc/ingenic-adc.c
> +++ b/drivers/iio/adc/ingenic-adc.c
> @@ -20,6 +20,7 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
> +#include <linux/cleanup.h>

Can you keep #include sorted?

Thanks,
Rong

>  
>  #define JZ_ADC_REG_ENABLE		0x00
>  #define JZ_ADC_REG_CFG			0x04
> @@ -115,7 +116,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 +163,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 +171,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_unlocked(struct ingenic_adc *adc,
> @@ -201,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_unlocked(adc, engine, enabled);
> -	mutex_unlock(&adc->lock);
>  }
>  
>  static int ingenic_adc_capture(struct ingenic_adc *adc,
> @@ -218,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);
>  
> @@ -229,7 +225,6 @@ static int ingenic_adc_capture(struct ingenic_adc *adc,
>  		ingenic_adc_enable_unlocked(adc, engine, false);
>  
>  	writel(cfg, adc->base + JZ_ADC_REG_CFG);
> -	mutex_unlock(&adc->lock);
>  
>  	return ret;
>  }
> @@ -643,7 +638,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);
> +	guard(mutex)(&adc->lock);
> +
>  	if (adc->soc_data->has_aux_md && engine == 0) {
>  		switch (chan->channel) {
>  		case INGENIC_ADC_AUX0:
> @@ -677,7 +673,6 @@ static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
>  
>  	ret = IIO_VAL_INT;
>  out:
> -	mutex_unlock(&adc->aux_lock);
>  	clk_disable(adc->clk);
>  
>  	return ret;

Re: [PATCH] iio: adc: ingenic-adc: use guard(mutex)(&lock) to handle synchronisation

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 10:18:11PM -0300, Felipe Ribeiro de Souza wrote:
> Replace mutex_lock(&lock) and mutex_unlock(&lock) calls with
> guard(mutex)(&lock) in functions ingenic_adc_set_adcmd,
> ingenic_adc_set_config, ingenic_adc_enable, ingenic_adc_capture
> and ingenic_adc_read_chan_info_raw.

We refer to the functions as func().

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

...

>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
> +#include <linux/cleanup.h>

Preserve ordering.

...

>  	/* We cannot sample the aux channels in parallel. */
> -	mutex_lock(&adc->aux_lock);
> +	guard(mutex)(&adc->lock);

	...

>  	ret = IIO_VAL_INT;
>  out:
> -	mutex_unlock(&adc->aux_lock);
>  	clk_disable(adc->clk);

Not sure about this. At bare minimum you should elaborate on this change in the
commit message.

>  	return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: adc: ingenic-adc: use guard(mutex)(&lock) to handle synchronisation

[Jonathan Cameron]

On Fri, 17 Apr 2026 11:01:48 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Apr 15, 2026 at 10:18:11PM -0300, Felipe Ribeiro de Souza wrote:
> > Replace mutex_lock(&lock) and mutex_unlock(&lock) calls with
> > guard(mutex)(&lock) in functions ingenic_adc_set_adcmd,
> > ingenic_adc_set_config, ingenic_adc_enable, ingenic_adc_capture
> > and ingenic_adc_read_chan_info_raw.  
> 
> We refer to the functions as func().
> 
> > 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.  
> 
> ...
> 
> >  #include <linux/of.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/property.h>
> > +#include <linux/cleanup.h>  
> 
> Preserve ordering.
> 
> ...
> 
> >  	/* We cannot sample the aux channels in parallel. */
> > -	mutex_lock(&adc->aux_lock);
> > +	guard(mutex)(&adc->lock);  
> 
> 	...
> 
> >  	ret = IIO_VAL_INT;
> >  out:
Holding the lock a bit longer might be fine, but please go read the
comments in cleanup.h.   The short version is don't combine use
of guard() with any gotos in the same function.
It's too high risk given different compiler behaviour and the chance of
code getting reorganized in the future

I wonder if anyone would be interested in applying ACQUIRE() and ACQUIRE_ERR()
to clock management.  Might be worth seeing if that is of interest and general
enough as then you could handle the clk_disable() as well.

> > -	mutex_unlock(&adc->aux_lock);
> >  	clk_disable(adc->clk);  
> 
> Not sure about this. At bare minimum you should elaborate on this change in the
> commit message.
> 
> >  	return ret;  
>