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

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

Posted by Maxwell Doose 1 month, 1 week ago

Include linux/cleanup.h to take advantage of RAII macros.

Replace spin_lock() and unlock() calls with their RAII macro
counterparts. This will help modernize the code and increase readability.

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

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/adc/xilinx-ams.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index 124470c92529..2b98f94d90bc 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,15 @@ 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 +1058,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);
+	guard(spinlock_irq)(&ams->intr_lock);
 	ams_unmask(ams);
-	spin_unlock_irq(&ams->intr_lock);
 
 	/* If still pending some alarm re-trigger the timer */
 	if (ams->current_masked_alarm)
@@ -1076,16 +1073,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 +1094,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;
 }
 

base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
-- 
2.54.0

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

Posted by Jonathan Cameron 1 month, 1 week ago

On Mon,  4 May 2026 16:46:12 -0500
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. This will help modernize the code and increase readability.
> 
> Remove "flags" variables where spin_lock_irqsave() has been replaced
> with guard(spinlock_irqsave)()
> 
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>

Hi Maxwell

One thing inline.

Thanks,

Jonathan

>  static void ams_enable_channel_sequence(struct iio_dev *indio_dev)
> @@ -1060,9 +1058,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);
> +	guard(spinlock_irq)(&ams->intr_lock);

This increases the scope to include scheduling delayed work. Seems unwise
in general and should definitely be mentioned in the commit message.
Use a scoped_guard() here probably or leave it alone. 

>  	ams_unmask(ams);
> -	spin_unlock_irq(&ams->intr_lock);
>  
>  	/* If still pending some alarm re-trigger the timer */
>  	if (ams->current_masked_alarm)

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

Posted by Maxwell Doose 1 month, 1 week ago

On Wed, May 6, 2026 at 10:51 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon,  4 May 2026 16:46:12 -0500
> 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. This will help modernize the code and increase readability.
> >
> > Remove "flags" variables where spin_lock_irqsave() has been replaced
> > with guard(spinlock_irqsave)()
> >
> > Signed-off-by: Maxwell Doose <m32285159@gmail.com>
>
> Hi Maxwell
>
> One thing inline.
>
> Thanks,
>
> Jonathan
>
> >  static void ams_enable_channel_sequence(struct iio_dev *indio_dev)
> > @@ -1060,9 +1058,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);
> > +     guard(spinlock_irq)(&ams->intr_lock);
>
> This increases the scope to include scheduling delayed work. Seems unwise
> in general and should definitely be mentioned in the commit message.
> Use a scoped_guard() here probably or leave it alone.
>

Will add tonight.

best regards,
max



> >       ams_unmask(ams);
> > -     spin_unlock_irq(&ams->intr_lock);
> >
> >       /* If still pending some alarm re-trigger the timer */
> >       if (ams->current_masked_alarm)
>
>