Re: [PATCH v2] iio: adc: ad7944: Add support for "3-wire mode"

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 18 Mar 2024 09:33:44 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On Sun, Mar 17, 2024 at 3:23 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Sun, Mar 17, 2024 at 1:10 AM David Lechner <dlechner@baylibre.com> wrote:  
> > > On Sat, Mar 16, 2024 at 2:57 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:  
> > > > Thu, Mar 14, 2024 at 12:43:38PM -0500, David Lechner kirjoitti:  
> >  
> 
> ...
> 
> > > > > +     case AD7944_SPI_MODE_SINGLE:
> > > > > +             ret = ad7944_3wire_cs_mode_conversion(adc, &indio_dev->channels[0]);
> > > > > +             if (ret)
> > > > > +                     goto out;
> > > > > +
> > > > > +             break;
> > > > > +     default:
> > > > > +             /* not supported */  
> > > >
> > > > No error code set?  
> > >
> > > This is in an interrupt handler, so I didn't think there was anything
> > > we can do with an error.  
> >
> > return IRQ_NONE?
> >  
> 
> Wouldn't this just cause the interrupt handler to trigger again
> immediately resulting in very high CPU load? I don't see any other IIO
> ADC drivers using the generic triggered buffer returning anything
> other than IRQ_HANDLED and I always assumed this was the reason.
> 
To me, this is a long running 'open question' of what to do on error
in an interrupt.  I my mind at least there isn't a good solution so we tend
to just paper over it.

IRQ_NONE indicates it wasn't our interrupt - so if we check a status
register and none of the interrupt bits are set, then it is either
a shared interrupt in which case someone else will handle it, or it
is spurious and we want the spurious interrupt handler to deal with it.

If we get an error talking tot he device during an interrupt handler
the question is do we want to trigger again?  If it's a level interrupt
and we haven't cleared it we will anyway, but for edge interrupts we've
handled it as well as we can perhaps. 

Anyhow, it's all a bit unclear so given we don't expect to get these
errors I tend to more prefer IRQ_HANDLED but if someone argues
strongly for IRQ_NONE I don't push back too hard.

If anyone has clear guidance on this then please link to it!

Jonathan