RE: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup

[John Madieu]

Hi Biju, Alexandre,

Thanks for the feedback.

> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> Sent: Samstag, 25. April 2026 20:37
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with
> disable_irq_wake() on cleanup
> 
> On 25/04/2026 16:39:16+0000, Biju Das wrote:
> > Hi John,
> >
> > > -----Original Message-----
> > > From: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > > Sent: 25 April 2026 16:50
> > > Subject: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with
> > > disable_irq_wake() on cleanup
> > >
> > > isl1208_setup_irq() calls enable_irq_wake() after a successful IRQ
> > > request, but the driver has no remove path that balances it.
> > > The driver is devm-only, so on unbind devm releases the IRQ - but
> > > enable_irq_wake() is not undone by IRQ release, so the wake count for
> that IRQ stays incremented.
> > >
> > > Each rebind therefore leaks one wake reference; the leak doubles for
> > > the chip variant that has a separate evdet IRQ, since
> > > isl1208_setup_irq() is then called twice during probe.
> >
> > Is removal of RTC device possible [1]?
> >

This patch addresses the per-IRQ wake refcount leak, which I
think is independent of whether alarmtimer is holding the RTC.
Found this by code inspection while working on patch [1/2]'s
issue.

> > [1]
> > https://patchwork.ozlabs.org/project/rtc-linux/patch/20230922081208.26
> > 334-1-biju.das.jz@xxxxxxxxxxxxxx/#3195765
> >
> 
> I'd say yes if this is not the RTC that is backing alarmtimer or
> alarmtimer is not compiled in the kernel.
> 

Thanks for the clarification. That said, looking around I noticed
most RTC drivers don't bother balancing enable_irq_wake() either.

Alexandre, do you want this patch as-is, or would you rather I
drop it? I'm fine either way.

Regards,
John