Re: [PATCH v1] iio: health: max30102: fix NULL dereference in interrupt handler

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 2 Aug 2026 10:21:41 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 8/1/26 9:10 PM, Jonathan Cameron wrote:
> > On Sat, 1 Aug 2026 10:18:42 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >   
> >> On 7/31/26 1:41 PM, Marco Chen wrote:  
> >>> The interrupt is requested in max30102_probe() and stays enabled    
> >>
> >> I've always wondered why most IIO drivers seem to do this. It seems to
> >> me that requesting the irq with IRQF_NO_AUTOEN and only enabling it
> >> when everything is set up for reading data would avoid spurious interrupts
> >> like this.  
> > 
> > It teaches discipline.  More serious from what I recall the no autoen thing
> > used to be done via a racey bit of trying to turn it off as fast as you can.
> > 
> > Most of the time we should be able to write drivers that don't need to
> > use that flag and don't touch enable_irq() at all.
> > 
> > Obviously that's not always true.
> >   
> 
> Hmm... so I should go back and change [1] since it isn't strictly
> required there?
> 
> [1]: https://lore.kernel.org/linux-iio/20260731-iio-adc-ti-ads112c14-continuous-mode-v2-1-eb13da38e8fc@baylibre.com/
> 
> 

Ah. I was assuming that was spurious irq territory or a device
where we couldn't disable it device end.

I remember a discussion a long time back with the irq maintainers
(probably tglx) about the fact that they really advise drivers not
to turn interrupts on or off because it can be a very expensive
operation on some systems and such control belongs at the device
end when possible. What a host does with a disabled irq is just
too variable. The only guarantee is it won't immediately report it.

Maybe that has changed over time - I have no idea!

Anyhow, yes - I would drop that enable / disable dance if it
was about spurious prevention.  If we want to prevent spurious then
better thing to do is to check the software interrupt status register.
Looks like for that part there is a drdy status bit.   Bit annoying that you'd
have to drop to a thread to read it but we probably should do that
before the complete.  Maybe we could speculatively complete and
check it in the main thread.  

I note you do use that for polling.  I'd treat the irq as a way
to ensure you only need to read it once rather than not read it at all.

Jonathan