[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250825122319.6faf8782@jic23-huawei>
Date: Mon, 25 Aug 2025 12:23:19 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: "O'Griofa, Conall" <conall.ogriofa@....com>
Cc: Sean Anderson <sean.anderson@...ux.dev>, Anand Ashok Dumbre
<anand.ashok.dumbre@...inx.com>, "linux-iio@...r.kernel.org"
<linux-iio@...r.kernel.org>, David Lechner <dlechner@...libre.com>, Nuno
Sá <nuno.sa@...log.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "Simek, Michal"
<michal.simek@....com>, Andy Shevchenko <andy@...nel.org>, Manish Narani
<manish.narani@...inx.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iio: xilinx-ams: Unmask interrupts after updating
alarms
On Wed, 20 Aug 2025 11:28:27 +0000
"O'Griofa, Conall" <conall.ogriofa@....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@...ux.dev>
> > Sent: 15 July 2025 01:29
> > To: Anand Ashok Dumbre <anand.ashok.dumbre@...inx.com>; Jonathan Cameron
> > <jic23@...nel.org>; linux-iio@...r.kernel.org
> > Cc: David Lechner <dlechner@...libre.com>; Nuno Sá <nuno.sa@...log.com>;
> > linux-arm-kernel@...ts.infradead.org; Simek, Michal <michal.simek@....com>;
> > Andy Shevchenko <andy@...nel.org>; Manish Narani
> > <manish.narani@...inx.com>; linux-kernel@...r.kernel.org; Sean Anderson
> > <sean.anderson@...ux.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@...ux.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@....com>
Powered by blists - more mailing lists