RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler

[Biju Das]

Hi John,

> -----Original Message-----
> From: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Sent: 26 April 2026 18:48
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>; alexandre.belloni@xxxxxxxxxxx
> Cc: ryan@xxxxxxxxxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx; m.grzeschik@xxxxxxxxxxxxxx;
> Denis.Osterland@xxxxxxxxx; linux-rtc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> john.madieu@xxxxxxxxx
> Subject: RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
> 
> Hi Biju,
> 
> Thanks fort he review.
> 
> > -----Original Message-----
> > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Sent: Samstag, 25. April 2026 19:16
> > To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>;
> > alexandre.belloni@xxxxxxxxxxx
> > Subject: RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as
> > irqreturn_t in IRQ handler
> >
> > Hi John,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > > Sent: 25 April 2026 16:50
> > > Subject: [PATCH 1/2] rtc: isl1208: Fix returning errno as
> > > irqreturn_t in IRQ handler
> > >
> > > isl1208_rtc_interrupt() is of irqreturn_t type but two paths return
> > > a negative i2c errno instead of an
> > > IRQ_* value:
> > >
> > >   - The SR-poll loop on timeout: `return sr;`
> > >   - The post-alarm cleanup path: `return err;`
> > >
> > > genirq's note_interrupt() casts the return to unsigned int and flags
> > > any value above IRQ_HANDLED|IRQ_WAKE_THREAD as a bogus return,
> > > logging "irq event N: bogus return value X" each time it happens.
> > >
> > > Return IRQ_NONE when the SR read failed (no progress, can't claim
> > > the
> > > interrupt) and IRQ_HANDLED when toggle_alarm failed.
> > >
> > > Fixes: cf044f0ed526 ("drivers/rtc/rtc-isl1208.c: add alarm support")
> > > Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > > ---
> > >  drivers/rtc/rtc-isl1208.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> > > index f71a6bb77b2a..c93998c53e7a
> > > 100644
> > > --- a/drivers/rtc/rtc-isl1208.c
> > > +++ b/drivers/rtc/rtc-isl1208.c
> > > @@ -654,7 +654,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> > >  		if (time_after(jiffies, timeout)) {
> > >  			dev_err(&client->dev, "%s: reading SR failed\n",
> > >  				__func__);
> > > -			return sr;
> > > +			return IRQ_NONE;
> >
> > Maybe you can use a goto statement?? that will take care of handled
> > IRQ's
> >
> > 		goto err_irq:
> >
> > err_irq:
> > 	return handled ? IRQ_HANDLED : IRQ_NONE;
> 
> Agreed. I'll do it your way in v2.


> 
> >
> > >  		}
> > >  	}
> > >
> > > @@ -666,7 +666,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> > >  		/* Disable the alarm */
> > >  		err = isl1208_rtc_toggle_alarm(client, 0);
> > >  		if (err)
> > > -			return err;
> > > +			return IRQ_HANDLED;
> >
> > Same as above.
> >
> 
> I'll set handled = 1 so that goto can return IRQ_HANDLED.

Looks this is change in behaviour compared to original code,
previous code does not set, handled = 1 for this path.

Cheers,
Biju