* [PATCH v3 0/2] use guard()() to handle synchronisation
@ 2026-04-21 22:14 Felipe Ribeiro de Souza
2026-04-21 22:14 ` [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw() Felipe Ribeiro de Souza
2026-04-21 22:14 ` [PATCH v3 2/2] iio: adc: ingenic-adc: use guard()() to handle synchronisation Felipe Ribeiro de Souza
0 siblings, 2 replies; 6+ messages in thread
From: Felipe Ribeiro de Souza @ 2026-04-21 22:14 UTC (permalink / raw)
To: andy, dlechner, jic23, nuno.sa, paul
Cc: Felipe Ribeiro de Souza, linux-iio, linux-mips
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()
2026-04-21 22:14 [PATCH v3 0/2] use guard()() to handle synchronisation Felipe Ribeiro de Souza
@ 2026-04-21 22:14 ` Felipe Ribeiro de Souza
2026-04-22 8:41 ` Andy Shevchenko
2026-04-21 22:14 ` [PATCH v3 2/2] iio: adc: ingenic-adc: use guard()() to handle synchronisation Felipe Ribeiro de Souza
1 sibling, 1 reply; 6+ messages in thread
From: Felipe Ribeiro de Souza @ 2026-04-21 22:14 UTC (permalink / raw)
To: andy, dlechner, jic23, nuno.sa, paul
Cc: Felipe Ribeiro de Souza, Lucas Ivars Cadima Ciziks, linux-iio,
linux-mips
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] iio: adc: ingenic-adc: use guard()() to handle synchronisation
2026-04-21 22:14 [PATCH v3 0/2] use guard()() to handle synchronisation Felipe Ribeiro de Souza
2026-04-21 22:14 ` [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw() Felipe Ribeiro de Souza
@ 2026-04-21 22:14 ` Felipe Ribeiro de Souza
2026-04-22 8:48 ` Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Felipe Ribeiro de Souza @ 2026-04-21 22:14 UTC (permalink / raw)
To: andy, dlechner, jic23, nuno.sa, paul
Cc: Felipe Ribeiro de Souza, Lucas Ivars Cadima Ciziks, linux-iio,
linux-mips
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()
2026-04-21 22:14 ` [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw() Felipe Ribeiro de Souza
@ 2026-04-22 8:41 ` Andy Shevchenko
2026-04-22 13:28 ` Sergey Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-04-22 8:41 UTC (permalink / raw)
To: Felipe Ribeiro de Souza
Cc: andy, dlechner, jic23, nuno.sa, paul, Lucas Ivars Cadima Ciziks,
linux-iio, linux-mips
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] iio: adc: ingenic-adc: use guard()() to handle synchronisation
2026-04-21 22:14 ` [PATCH v3 2/2] iio: adc: ingenic-adc: use guard()() to handle synchronisation Felipe Ribeiro de Souza
@ 2026-04-22 8:48 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2026-04-22 8:48 UTC (permalink / raw)
To: Felipe Ribeiro de Souza
Cc: andy, dlechner, jic23, nuno.sa, paul, Lucas Ivars Cadima Ciziks,
linux-iio, linux-mips
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw()
2026-04-22 8:41 ` Andy Shevchenko
@ 2026-04-22 13:28 ` Sergey Shtylyov
0 siblings, 0 replies; 6+ messages in thread
From: Sergey Shtylyov @ 2026-04-22 13:28 UTC (permalink / raw)
To: Andy Shevchenko, Felipe Ribeiro de Souza
Cc: andy, dlechner, jic23, nuno.sa, paul, Lucas Ivars Cadima Ciziks,
linux-iio, linux-mips
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-22 13:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 22:14 [PATCH v3 0/2] use guard()() to handle synchronisation Felipe Ribeiro de Souza
2026-04-21 22:14 ` [PATCH v3 1/2] iio: adc: ingenic-adc: refactor ingenic_adc_read_chan_info_raw() Felipe Ribeiro de Souza
2026-04-22 8:41 ` Andy Shevchenko
2026-04-22 13:28 ` Sergey Shtylyov
2026-04-21 22:14 ` [PATCH v3 2/2] iio: adc: ingenic-adc: use guard()() to handle synchronisation Felipe Ribeiro de Souza
2026-04-22 8:48 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox