[PATCH v3 0/2] use guard()() to handle synchronisation

[PATCH v3 0/2] use guard()() to handle synchronisation

[Felipe Ribeiro de Souza]

Refactor ingenic_adc_read_chan_info_raw() and replace mutex_lock()
and mutex_unlock() calls with guard()() in drivers/iio/adc/ingenic-adc.c

Felipe Ribeiro de Souza (2):
  iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()
  iio: adc: ingenic-adc: use guard()() to handle synchronisation

 drivers/iio/adc/ingenic-adc.c | 57 +++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 26 deletions(-)

-- 
2.43.0

[PATCH v3 1/2] 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_locked() to improve code
readability and modularity.

The helper handles the mutex-protected section for sampling channels,
while the main function manages 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
---
 drivers/iio/adc/ingenic-adc.c | 36 ++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 1e802c8779a4c..81ed92f4490cd 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -628,22 +628,15 @@ 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_locked(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:
@@ -678,6 +671,27 @@ static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
 	ret = IIO_VAL_INT;
 out:
 	mutex_unlock(&adc->aux_lock);
+
+	return ret;
+}
+
+static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
+					  struct iio_chan_spec const *chan,
+					  int *val)
+{
+	int ret;
+	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;
+	}
+
+	ret = ingenic_adc_read_chan_locked(adc, chan, val);
+
 	clk_disable(adc->clk);
 
 	return ret;
-- 
2.43.0

Re: [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()

[Andy Shevchenko]

On Tue, Apr 21, 2026 at 07:14:58PM -0300, Felipe Ribeiro de Souza wrote:
> Extract the sample logic from ingenic_adc_read_chan_info_raw() into
> a new helper function ingenic_adc_read_chan_locked() to improve code
> readability and modularity.
> 
> The helper handles the mutex-protected section for sampling channels,
> while the main function manages clock enabling/disabling.

...

> +static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
> +					  struct iio_chan_spec const *chan,
> +					  int *val)
> +{
> +	int ret;
> +	struct ingenic_adc *adc = iio_priv(iio_dev);

Please, keep it as in reversed xmas tree order.

> +	ret = clk_enable(adc->clk);

> +

Redundant blank line.

> +	if (ret) {
> +		dev_err(iio_dev->dev.parent, "Failed to enable clock: %d\n",
> +			ret);

Make it a single line. It's fine to go a bit over 80 for readability purposes.

> +		return ret;
> +	}
> +
> +	ret = ingenic_adc_read_chan_locked(adc, chan, val);
> +
>  	clk_disable(adc->clk);
>  
>  	return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()

[Sergey Shtylyov]

On 4/22/26 11:41 AM, Andy Shevchenko wrote:
[...]

>> Extract the sample logic from ingenic_adc_read_chan_info_raw() into
>> a new helper function ingenic_adc_read_chan_locked() to improve code
>> readability and modularity.
>>
>> The helper handles the mutex-protected section for sampling channels,
>> while the main function manages clock enabling/disabling.
> 
> ...
> 
>> +static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
>> +					  struct iio_chan_spec const *chan,
>> +					  int *val)
>> +{
>> +	int ret;
>> +	struct ingenic_adc *adc = iio_priv(iio_dev);
> 
> Please, keep it as in reversed xmas tree order.
> 
>> +	ret = clk_enable(adc->clk);
> 
>> +
> 
> Redundant blank line.

   Yep.

> 
>> +	if (ret) {
>> +		dev_err(iio_dev->dev.parent, "Failed to enable clock: %d\n",
>> +			ret);
> 
> Make it a single line. It's fine to go a bit over 80 for readability purposes.

   And BTW the new checkpatch limit is 100 columns. :-)

[...]

MBR, Sergey

Re: [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 04:28:24PM +0300, Sergey Shtylyov wrote:
> On 4/22/26 11:41 AM, Andy Shevchenko wrote:

[...]

> >> +		dev_err(iio_dev->dev.parent, "Failed to enable clock: %d\n",
> >> +			ret);
> > 
> > Make it a single line. It's fine to go a bit over 80 for readability purposes.
> 
>    And BTW the new checkpatch limit is 100 columns. :-)

Yes. But checkpatch has lower priority over documentation. Documentation
clearly says 80 and IIO tries to follow this.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 2/2] iio: adc: ingenic-adc: use 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
ingenic_adc_read_chan_locked().

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
---
 drivers/iio/adc/ingenic-adc.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 81ed92f4490cd..733f0276bb0b1 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;
 }
@@ -635,7 +630,7 @@ static int ingenic_adc_read_chan_locked(struct ingenic_adc *adc,
 	int cmd, ret, engine = (chan->channel == INGENIC_ADC_BATTERY);
 
 	/* We cannot sample the aux channels in parallel. */
-	mutex_lock(&adc->aux_lock);
+	guard(mutex)(&adc->aux_lock);
 
 	if (adc->soc_data->has_aux_md && engine == 0) {
 		switch (chan->channel) {
@@ -655,7 +650,7 @@ static int ingenic_adc_read_chan_locked(struct ingenic_adc *adc,
 
 	ret = ingenic_adc_capture(adc, engine);
 	if (ret)
-		goto out;
+		return ret;
 
 	switch (chan->channel) {
 	case INGENIC_ADC_AUX0:
@@ -668,11 +663,7 @@ static int ingenic_adc_read_chan_locked(struct ingenic_adc *adc,
 		break;
 	}
 
-	ret = IIO_VAL_INT;
-out:
-	mutex_unlock(&adc->aux_lock);
-
-	return ret;
+	return IIO_VAL_INT;
 }
 
 static int ingenic_adc_read_chan_info_raw(struct iio_dev *iio_dev,
-- 
2.43.0

Re: [PATCH v3 2/2] iio: adc: ingenic-adc: use guard()() to handle synchronisation

[Andy Shevchenko]

On Tue, Apr 21, 2026 at 07:14:59PM -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
> ingenic_adc_read_chan_locked().
> 
> 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 <dt-bindings/iio/adc/ingenic,adc.h>

For better readability I would make the above to be a group by adding a blank
line here. 

> +#include <linux/cleanup.h>
>  #include <linux/clk.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/iio.h>

-- 
With Best Regards,
Andy Shevchenko