[PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()

Maxwell Doose posted 1 patch 1 month ago
drivers/iio/adc/xilinx-ams.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
[PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()
Posted by Maxwell Doose 1 month ago
Include linux/cleanup.h to take advantage of RAII macros.

Replace spin_lock() and unlock() calls with their RAII macro
counterparts, which modernizes the code and increases readability.

Remove "flags" variables where spin_lock_irqsave() has been replaced
with guard(spinlock_irqsave)().

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 v2:
 - Replace guard(spinlock_irq)() in ams_unmask_worker() with
   scoped_guard() per Jonathan's suggestion.

 drivers/iio/adc/xilinx-ams.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index 124470c92529..05bf1444ca7d 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -10,6 +10,7 @@
 
 #include <linux/bits.h>
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/devm-helpers.h>
@@ -414,18 +415,16 @@ static void ams_unmask(struct ams *ams)
 
 static void ams_update_alarm(struct ams *ams, unsigned long alarm_mask)
 {
-	unsigned long flags;
-
 	if (ams->ps_base)
 		ams_update_ps_alarm(ams, alarm_mask);
 
 	if (ams->pl_base)
 		ams_update_pl_alarm(ams, alarm_mask);
 
-	spin_lock_irqsave(&ams->intr_lock, flags);
+	guard(spinlock_irqsave)(&ams->intr_lock);
+
 	ams_update_intrmask(ams, AMS_ISR0_ALARM_MASK, ~alarm_mask);
 	ams_unmask(ams);
-	spin_unlock_irqrestore(&ams->intr_lock, flags);
 }
 
 static void ams_enable_channel_sequence(struct iio_dev *indio_dev)
@@ -1060,9 +1059,8 @@ static void ams_unmask_worker(struct work_struct *work)
 {
 	struct ams *ams = container_of(work, struct ams, ams_unmask_work.work);
 
-	spin_lock_irq(&ams->intr_lock);
-	ams_unmask(ams);
-	spin_unlock_irq(&ams->intr_lock);
+	scoped_guard(spinlock_irq, &ams->intr_lock)
+		ams_unmask(ams);
 
 	/* If still pending some alarm re-trigger the timer */
 	if (ams->current_masked_alarm)
@@ -1076,16 +1074,14 @@ static irqreturn_t ams_irq(int irq, void *data)
 	struct ams *ams = iio_priv(indio_dev);
 	u32 isr0;
 
-	spin_lock(&ams->intr_lock);
+	guard(spinlock)(&ams->intr_lock);
 
 	isr0 = readl(ams->base + AMS_ISR_0);
 
 	/* Only process alarms that are not masked */
 	isr0 &= ~((ams->intr_mask & AMS_ISR0_ALARM_MASK) | ams->current_masked_alarm);
-	if (!isr0) {
-		spin_unlock(&ams->intr_lock);
+	if (!isr0)
 		return IRQ_NONE;
-	}
 
 	/* Clear interrupt */
 	writel(isr0, ams->base + AMS_ISR_0);
@@ -1099,8 +1095,6 @@ static irqreturn_t ams_irq(int irq, void *data)
 	schedule_delayed_work(&ams->ams_unmask_work,
 			      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
 
-	spin_unlock(&ams->intr_lock);
-
 	return IRQ_HANDLED;
 }
 
-- 
2.54.0
Re: [PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()
Posted by Salih Erim 1 month ago
Hi Maxwell,

On 5/8/2026 1:45 PM, Maxwell Doose wrote:

> Include linux/cleanup.h to take advantage of RAII macros.
> 
> Replace spin_lock() and unlock() calls with their RAII macro
> counterparts, which modernizes the code and increases readability.
> 
> Remove "flags" variables where spin_lock_irqsave() has been replaced
> with guard(spinlock_irqsave)().
> 
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>   v2:
>   - Replace guard(spinlock_irq)() in ams_unmask_worker() with
>     scoped_guard() per Jonathan's suggestion.

Nice cleanup, thanks for taking care of this.

Reviewed-by: Salih Erim <salih.erim@amd.com>

Salih.
Re: [PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()
Posted by Andy Shevchenko 1 month ago
On Fri, May 08, 2026 at 07:45:13AM -0500, Maxwell Doose wrote:
> Include linux/cleanup.h to take advantage of RAII macros.
> 
> Replace spin_lock() and unlock() calls with their RAII macro
> counterparts, which modernizes the code and increases readability.

> Remove "flags" variables where spin_lock_irqsave() has been replaced
> with guard(spinlock_irqsave)().

Unneeded detail, everyone can see that in the guard()() implementation
for the spin locks. Otherwise, LGTM.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()
Posted by Sanjay Chitroda 1 month ago

On 8 May 2026 6:15:13 pm IST, Maxwell Doose <m32285159@gmail.com> wrote:
>Include linux/cleanup.h to take advantage of RAII macros.
>
>Replace spin_lock() and unlock() calls with their RAII macro
>counterparts, which modernizes the code and increases readability.
>
>Remove "flags" variables where spin_lock_irqsave() has been replaced
>with guard(spinlock_irqsave)().
>
>Signed-off-by: Maxwell Doose <m32285159@gmail.com>
>---
> v2:
> - Replace guard(spinlock_irq)() in ams_unmask_worker() with
>   scoped_guard() per Jonathan's suggestion.
>
> drivers/iio/adc/xilinx-ams.c | 20 +++++++-------------
> 1 file changed, 7 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
>index 124470c92529..05bf1444ca7d 100644
>--- a/drivers/iio/adc/xilinx-ams.c
>+++ b/drivers/iio/adc/xilinx-ams.c
>@@ -10,6 +10,7 @@
> 
> #include <linux/bits.h>
> #include <linux/bitfield.h>
>+#include <linux/cleanup.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/devm-helpers.h>
>@@ -414,18 +415,16 @@ static void ams_unmask(struct ams *ams)
> 
> static void ams_update_alarm(struct ams *ams, unsigned long alarm_mask)
> {
>-	unsigned long flags;
>-
> 	if (ams->ps_base)
> 		ams_update_ps_alarm(ams, alarm_mask);
> 
> 	if (ams->pl_base)
> 		ams_update_pl_alarm(ams, alarm_mask);
> 
>-	spin_lock_irqsave(&ams->intr_lock, flags);
>+	guard(spinlock_irqsave)(&ams->intr_lock);
>+
> 	ams_update_intrmask(ams, AMS_ISR0_ALARM_MASK, ~alarm_mask);
> 	ams_unmask(ams);
>-	spin_unlock_irqrestore(&ams->intr_lock, flags);
> }
> 
> static void ams_enable_channel_sequence(struct iio_dev *indio_dev)
>@@ -1060,9 +1059,8 @@ static void ams_unmask_worker(struct work_struct *work)
> {
> 	struct ams *ams = container_of(work, struct ams, ams_unmask_work.work);
> 
>-	spin_lock_irq(&ams->intr_lock);
>-	ams_unmask(ams);
>-	spin_unlock_irq(&ams->intr_lock);
>+	scoped_guard(spinlock_irq, &ams->intr_lock)
>+		ams_unmask(ams);
> 
> 	/* If still pending some alarm re-trigger the timer */
> 	if (ams->current_masked_alarm)
>@@ -1076,16 +1074,14 @@ static irqreturn_t ams_irq(int irq, void *data)
> 	struct ams *ams = iio_priv(indio_dev);
> 	u32 isr0;
> 
>-	spin_lock(&ams->intr_lock);
>+	guard(spinlock)(&ams->intr_lock);
> 
> 	isr0 = readl(ams->base + AMS_ISR_0);
> 
> 	/* Only process alarms that are not masked */
> 	isr0 &= ~((ams->intr_mask & AMS_ISR0_ALARM_MASK) | ams->current_masked_alarm);
>-	if (!isr0) {
>-		spin_unlock(&ams->intr_lock);
>+	if (!isr0)
> 		return IRQ_NONE;
>-	}
> 
> 	/* Clear interrupt */
> 	writel(isr0, ams->base + AMS_ISR_0);
>@@ -1099,8 +1095,6 @@ static irqreturn_t ams_irq(int irq, void *data)
> 	schedule_delayed_work(&ams->ams_unmask_work,
> 			      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
> 
>-	spin_unlock(&ams->intr_lock);
>-
> 	return IRQ_HANDLED;
> 
Hi Maxwell,

Thanks for the resource cleanup change.

Overall looks good.

Also, there is opportunity for guard(mutex)().

Thanks,
Sanjay Chitroda

> 
Re: [PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()
Posted by Maxwell Doose 1 month ago
On Fri, May 8, 2026 at 1:50 PM Sanjay Chitroda
<sanjayembeddedse@gmail.com> wrote:
>
> Hi Maxwell,
>
> Thanks for the resource cleanup change.
>
> Overall looks good.
>
> Also, there is opportunity for guard(mutex)().
>

Thanks, of course I'd also be happy to do the transition to
guard(mutex)() but it seems like something for tomorrow evening since
I've already submitted a lot to the mailing list.

best regards,
max




>
> Thanks,
> Sanjay Chitroda
>
>
Re: [PATCH v2] iio: adc: xilinx-ams: Replace spin_lock() and unlock() calls with guard(spinlock*)()
Posted by Jonathan Cameron 1 month ago
On Fri, 8 May 2026 13:53:11 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> On Fri, May 8, 2026 at 1:50 PM Sanjay Chitroda
> <sanjayembeddedse@gmail.com> wrote:
> >
> > Hi Maxwell,
> >
> > Thanks for the resource cleanup change.
> >
> > Overall looks good.
> >
> > Also, there is opportunity for guard(mutex)().
> >  
> 
> Thanks, of course I'd also be happy to do the transition to
> guard(mutex)() but it seems like something for tomorrow evening since
> I've already submitted a lot to the mailing list.

Nice patch  and well noted by Sanjay - I'd have just applied this
without checking for that :)  I think sensible to do one patch that
deals with guard() for both mutex and spinlock.

Sometimes the Xilinx folk do review so I'll leave this a while longer
anyway in the hope they have time to take a look.
It's a safe change set but none the less nice to give time!

Thanks,

Jonathan

> 
> best regards,
> max
> 
> 
> 
> 
> >
> > Thanks,
> > Sanjay Chitroda
> >
> >