Re: [PATCH] iio: xilinx-ams: Unmask interrupts after updating alarms

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 20 Aug 2025 11:28:27 +0000
"O'Griofa, Conall" <conall.ogriofa@amd.com> wrote:

> Hi,
> 
> Looks good, thank you for taking the time to submit this patch.
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> 
> Cheers,
> Conall.
> 
> > -----Original Message-----
> > From: Sean Anderson <sean.anderson@linux.dev>
> > Sent: 15 July 2025 01:29
> > To: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>; Jonathan Cameron
> > <jic23@kernel.org>; linux-iio@vger.kernel.org
> > Cc: David Lechner <dlechner@baylibre.com>; Nuno Sá <nuno.sa@analog.com>;
> > linux-arm-kernel@lists.infradead.org; Simek, Michal <michal.simek@amd.com>;
> > Andy Shevchenko <andy@kernel.org>; Manish Narani
> > <manish.narani@xilinx.com>; linux-kernel@vger.kernel.org; Sean Anderson
> > <sean.anderson@linux.dev>
> > Subject: [PATCH] iio: xilinx-ams: Unmask interrupts after updating alarms
> > 
> > To convert level-triggered alarms into edge-triggered IIO events, alarms are masked
> > when they are triggered. To ensure we catch subsequent alarms, we then
> > periodically poll to see if the alarm is still active.
> > If it isn't, we unmask it. Active but masked alarms are stored in
> > current_masked_alarm.
> > 
> > If an active alarm is disabled, it will remain set in current_masked_alarm until
> > ams_unmask_worker clears it. If the alarm is re-enabled before
> > ams_unmask_worker runs, then it will never be cleared from
> > current_masked_alarm. This will prevent the alarm event from being pushed even if
> > the alarm is still active.
> > 
> > Fix this by recalculating current_masked_alarm immediately when enabling or
> > disabling alarms.
> > 
> > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> > ---
> > 
> >  drivers/iio/adc/xilinx-ams.c | 45 ++++++++++++++++++++----------------
> >  1 file changed, 25 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c index
> > 76dd0343f5f7..180d4140993d 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -389,6 +389,29 @@ static void ams_update_pl_alarm(struct ams *ams,
> > unsigned long alarm_mask)
> >  	ams_pl_update_reg(ams, AMS_REG_CONFIG3,
> > AMS_REGCFG3_ALARM_MASK, cfg);  }
> > 
> > +static void ams_unmask(struct ams *ams) {
> > +	unsigned int status, unmask;
> > +
> > +	status = readl(ams->base + AMS_ISR_0);
> > +
> > +	/* Clear those bits which are not active anymore */
> > +	unmask = (ams->current_masked_alarm ^ status) &
> > +ams->current_masked_alarm;
> > +
> > +	/* Clear status of disabled alarm */
> > +	unmask |= ams->intr_mask;
> > +
> > +	ams->current_masked_alarm &= status;
> > +
> > +	/* Also clear those which are masked out anyway */
> > +	ams->current_masked_alarm &= ~ams->intr_mask;
> > +
> > +	/* Clear the interrupts before we unmask them */
> > +	writel(unmask, ams->base + AMS_ISR_0);
> > +
> > +	ams_update_intrmask(ams, ~AMS_ALARM_MASK,
> > ~AMS_ALARM_MASK); }
> > +
> >  static void ams_update_alarm(struct ams *ams, unsigned long alarm_mask)  {
> >  	unsigned long flags;
> > @@ -401,6 +424,7 @@ static void ams_update_alarm(struct ams *ams, unsigned
> > long alarm_mask)
> > 
> >  	spin_lock_irqsave(&ams->intr_lock, flags);
> >  	ams_update_intrmask(ams, AMS_ISR0_ALARM_MASK, ~alarm_mask);
> > +	ams_unmask(ams);
> >  	spin_unlock_irqrestore(&ams->intr_lock, flags);  }
> > 
> > @@ -1035,28 +1059,9 @@ static void ams_handle_events(struct iio_dev
> > *indio_dev, unsigned long events)  static void ams_unmask_worker(struct
> > work_struct *work)  {
> >  	struct ams *ams = container_of(work, struct ams, ams_unmask_work.work);
> > -	unsigned int status, unmask;
> > 
> >  	spin_lock_irq(&ams->intr_lock);
> > -
> > -	status = readl(ams->base + AMS_ISR_0);
> > -
> > -	/* Clear those bits which are not active anymore */
> > -	unmask = (ams->current_masked_alarm ^ status) & ams-  
> > >current_masked_alarm;  
> > -
> > -	/* Clear status of disabled alarm */
> > -	unmask |= ams->intr_mask;
> > -
> > -	ams->current_masked_alarm &= status;
> > -
> > -	/* Also clear those which are masked out anyway */
> > -	ams->current_masked_alarm &= ~ams->intr_mask;
> > -
> > -	/* Clear the interrupts before we unmask them */
> > -	writel(unmask, ams->base + AMS_ISR_0);
> > -
> > -	ams_update_intrmask(ams, ~AMS_ALARM_MASK,
> > ~AMS_ALARM_MASK);
> > -
> > +	ams_unmask(ams);
> >  	spin_unlock_irq(&ams->intr_lock);
> > 
> >  	/* If still pending some alarm re-trigger the timer */
> > --
> > 2.35.1.1320.gc452695387.dirty  
> 
> Reviewed-by: O'Griofa, Conall <conall.ogriofa@amd.com>