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

[PATCH v2] 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 with scoped_guard(mutex, &lock) 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.
---
 drivers/iio/adc/ingenic-adc.c | 69 ++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 1e802c8779a4..5f914d5e5362 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -7,6 +7,7 @@
  */
 
 #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 +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,41 +638,41 @@ 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);
-	if (adc->soc_data->has_aux_md && engine == 0) {
+	scoped_guard(mutex, &adc->lock) {
+		if (adc->soc_data->has_aux_md && engine == 0) {
+			switch (chan->channel) {
+			case INGENIC_ADC_AUX0:
+				cmd = 0;
+				break;
+			case INGENIC_ADC_AUX:
+				cmd = 1;
+				break;
+			case INGENIC_ADC_AUX2:
+				cmd = 2;
+				break;
+			}
+	
+			ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_AUX_MD, cmd);
+		}
+	
+		ret = ingenic_adc_capture(adc, engine);
+		if (ret)
+			goto out;
+	
 		switch (chan->channel) {
 		case INGENIC_ADC_AUX0:
-			cmd = 0;
-			break;
 		case INGENIC_ADC_AUX:
-			cmd = 1;
-			break;
 		case INGENIC_ADC_AUX2:
-			cmd = 2;
+			*val = readw(adc->base + JZ_ADC_REG_ADSDAT);
+			break;
+		case INGENIC_ADC_BATTERY:
+			*val = readw(adc->base + JZ_ADC_REG_ADBDAT);
 			break;
 		}
-
-		ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_AUX_MD, cmd);
-	}
-
-	ret = ingenic_adc_capture(adc, engine);
-	if (ret)
-		goto out;
-
-	switch (chan->channel) {
-	case INGENIC_ADC_AUX0:
-	case INGENIC_ADC_AUX:
-	case INGENIC_ADC_AUX2:
-		*val = readw(adc->base + JZ_ADC_REG_ADSDAT);
-		break;
-	case INGENIC_ADC_BATTERY:
-		*val = readw(adc->base + JZ_ADC_REG_ADBDAT);
-		break;
+	
+		ret = IIO_VAL_INT;
 	}
-
-	ret = IIO_VAL_INT;
 out:
-	mutex_unlock(&adc->aux_lock);
 	clk_disable(adc->clk);
 
 	return ret;
-- 
2.43.0

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

[Andy Shevchenko]

On Sun, Apr 19, 2026 at 12:27:34PM -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 with scoped_guard(mutex, &lock) 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.

...

>  	/* We cannot sample the aux channels in parallel. */
> -	mutex_lock(&adc->aux_lock);
> -	if (adc->soc_data->has_aux_md && engine == 0) {
> +	scoped_guard(mutex, &adc->lock) {

This makes a bit noisy refactoring, hard to follow. Perhaps, split out
the helper function first (in a separate change) and then use guard()()
there.

> +		if (adc->soc_data->has_aux_md && engine == 0) {
> +			switch (chan->channel) {
> +			case INGENIC_ADC_AUX0:
> +				cmd = 0;
> +				break;
> +			case INGENIC_ADC_AUX:
> +				cmd = 1;
> +				break;
> +			case INGENIC_ADC_AUX2:
> +				cmd = 2;
> +				break;
> +			}
> +	
> +			ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_AUX_MD, cmd);
> +		}
> +	
> +		ret = ingenic_adc_capture(adc, engine);
> +		if (ret)
> +			goto out;
> +	
>  		switch (chan->channel) {
>  		case INGENIC_ADC_AUX0:
> -			cmd = 0;
> -			break;
>  		case INGENIC_ADC_AUX:
> -			cmd = 1;
> -			break;
>  		case INGENIC_ADC_AUX2:
> -			cmd = 2;
> +			*val = readw(adc->base + JZ_ADC_REG_ADSDAT);
> +			break;
> +		case INGENIC_ADC_BATTERY:
> +			*val = readw(adc->base + JZ_ADC_REG_ADBDAT);
>  			break;
>  		}
> -
> -		ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_AUX_MD, cmd);
> -	}
> -
> -	ret = ingenic_adc_capture(adc, engine);
> -	if (ret)
> -		goto out;
> -
> -	switch (chan->channel) {
> -	case INGENIC_ADC_AUX0:
> -	case INGENIC_ADC_AUX:
> -	case INGENIC_ADC_AUX2:
> -		*val = readw(adc->base + JZ_ADC_REG_ADSDAT);
> -		break;
> -	case INGENIC_ADC_BATTERY:
> -		*val = readw(adc->base + JZ_ADC_REG_ADBDAT);
> -		break;
> +	
> +		ret = IIO_VAL_INT;
>  	}
> -
> -	ret = IIO_VAL_INT;
>  out:
> -	mutex_unlock(&adc->aux_lock);
>  	clk_disable(adc->clk);
>  
>  	return ret;

Alternatively, propose the respective ACQUIRE*() macros to CCF.

-- 
With Best Regards,
Andy Shevchenko