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

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.

> 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.

Best regards,
Phuc