drivers/iio/adc/xilinx-ams.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
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
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.
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
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
>
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 > >
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 > > > >
© 2016 - 2026 Red Hat, Inc.