Re: [PATCH] iio: adc: sophgo-saradc: Handle errors from optional IRQ lookup

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 23 Aug 2026 05:27:21 +0700
Bui Duc Phuc <phucduc.bui@gmail.com> wrote:

> Hi Jonathan,
> 
> Thank you for your feedback.
> 
> > >
> > > platform_get_irq_optional() returns a positive IRQ number on success or
> > > a negative error code on failure. For an optional IRQ, -ENXIO indicates
> > > that no optional IRQ is available. Other errors, such as -EPROBE_DEFER
> > > and -EINVAL, should be propagated so that the caller can handle them
> > > appropriately.  
> >
> > That function is very much undocumented other than not printing a
> > message when it returns an error.  However I think you analysis is
> > correct.
> >  
> 
> Yes, I agree. The documentation for this function may not be clear enough,
> which could have led to error handling being implemented incorrectly
> in some places.
> 
> There is also an inconsistency in this driver: if devm_request_irq() fails,
> the error is returned and the probe fails. But if platform_get_irq_optional()
> fails, the error is ignored.
> I'm not sure whether the author misunderstood and assumed that any
> negative return value simply means that there is no IRQ.

If the platform irq get fails because there isn't one in firmware, we expect
to just carry on (no interrupt support).  What we are missing is failing when
there is one but we get an error anyway.

For the later devm_request_irq() that is only called if we have an irq
from firmware, but something else goes wrong. That one should definitely 
always fail probe as it indicates a probe (rather than lack of interrupt
support)

> 
> > I'm not going to rush this is because it is not known to have
> > been a problem in the wild (only odd loading orders should have
> > caused deferal).
> >  
> 
> I understand your point. However, in this case the error can be
> completely hidden:
> There is no error message or log, the error is not returned, and the
> driver falls back
> to polling:
> -----------------------------------
> if (saradc->irq < 0) {
>         u32 reg;
> 
>         return readl_poll_timeout(saradc->regs + CV1800B_ADC_STATUS_REG,
>                                                  reg, !(reg & CV1800B_ADC_BUSY),
>                                                  500, CV1800B_READ_TIMEOUT_US);
> }
> ------------------------------------
> 
> So there may never be an obvious failure for a user to report.

Without evidence that there are real setups where interrupt controller
loads late enough to result in a deferral + are used with this chip
I'm fine with the small risk of just using polling in kernels prior
to the fix.  Everything still works, just potentially less efficiently.

Jonathan

> 
> Best regards,
> Phuc