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

[John Madieu]

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.

Regards,
John

> Cheers,
> Biju
> 
> >
> >  		fsleep(275);
> >
> > --
> > 2.25.1