The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] iio: adc: sophgo-saradc: Handle errors from optional IRQ lookup
@ 2026-08-13  7:48 phucduc.bui
  2026-08-22  1:39 ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: phucduc.bui @ 2026-08-13  7:48 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, nuno.sa, Chen Wang,
	Inochi Amaoto, Takashi Sakamoto, u.kleine-koenig,
	Danilo Krummrich, Bjorn Helgaas, linux-iio, sophgo
  Cc: linux-kernel, bui duc phuc

From: bui duc phuc <phucduc.bui@gmail.com>

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.

Propagate negative errors other than -ENXIO.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/iio/adc/sophgo-cv1800b-adc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/adc/sophgo-cv1800b-adc.c b/drivers/iio/adc/sophgo-cv1800b-adc.c
index bdc3e1326a9a..c6eed09acfcd 100644
--- a/drivers/iio/adc/sophgo-cv1800b-adc.c
+++ b/drivers/iio/adc/sophgo-cv1800b-adc.c
@@ -182,6 +182,8 @@ static int cv1800b_adc_probe(struct platform_device *pdev)
 		return PTR_ERR(saradc->regs);
 
 	saradc->irq = platform_get_irq_optional(pdev, 0);
+	if (saradc->irq < 0 && saradc->irq != -ENXIO)
+		return saradc->irq;
 	if (saradc->irq > 0) {
 		init_completion(&saradc->completion);
 		ret = devm_request_irq(dev, saradc->irq,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: adc: sophgo-saradc: Handle errors from optional IRQ lookup
  2026-08-13  7:48 [PATCH] iio: adc: sophgo-saradc: Handle errors from optional IRQ lookup phucduc.bui
@ 2026-08-22  1:39 ` Jonathan Cameron
  2026-08-22 22:27   ` Bui Duc Phuc
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2026-08-22  1:39 UTC (permalink / raw)
  To: phucduc.bui
  Cc: David Lechner, nuno.sa, Chen Wang, Inochi Amaoto,
	Takashi Sakamoto, u.kleine-koenig, Danilo Krummrich,
	Bjorn Helgaas, linux-iio, sophgo, linux-kernel

On Thu, 13 Aug 2026 14:48:10 +0700
phucduc.bui@gmail.com wrote:

> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> 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.

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


> 
> Propagate negative errors other than -ENXIO.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>  drivers/iio/adc/sophgo-cv1800b-adc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/adc/sophgo-cv1800b-adc.c b/drivers/iio/adc/sophgo-cv1800b-adc.c
> index bdc3e1326a9a..c6eed09acfcd 100644
> --- a/drivers/iio/adc/sophgo-cv1800b-adc.c
> +++ b/drivers/iio/adc/sophgo-cv1800b-adc.c
> @@ -182,6 +182,8 @@ static int cv1800b_adc_probe(struct platform_device *pdev)
>  		return PTR_ERR(saradc->regs);
>  
>  	saradc->irq = platform_get_irq_optional(pdev, 0);
> +	if (saradc->irq < 0 && saradc->irq != -ENXIO)
> +		return saradc->irq;
>  	if (saradc->irq > 0) {
>  		init_completion(&saradc->completion);
>  		ret = devm_request_irq(dev, saradc->irq,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: adc: sophgo-saradc: Handle errors from optional IRQ lookup
  2026-08-22  1:39 ` Jonathan Cameron
@ 2026-08-22 22:27   ` Bui Duc Phuc
  2026-08-23 19:02     ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Bui Duc Phuc @ 2026-08-22 22:27 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, nuno.sa, Chen Wang, Inochi Amaoto,
	Takashi Sakamoto, u.kleine-koenig, Danilo Krummrich,
	Bjorn Helgaas, linux-iio, sophgo, linux-kernel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: adc: sophgo-saradc: Handle errors from optional IRQ lookup
  2026-08-22 22:27   ` Bui Duc Phuc
@ 2026-08-23 19:02     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-08-23 19:02 UTC (permalink / raw)
  To: Bui Duc Phuc
  Cc: David Lechner, nuno.sa, Chen Wang, Inochi Amaoto,
	Takashi Sakamoto, u.kleine-koenig, Danilo Krummrich,
	Bjorn Helgaas, linux-iio, sophgo, linux-kernel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-23 19:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  7:48 [PATCH] iio: adc: sophgo-saradc: Handle errors from optional IRQ lookup phucduc.bui
2026-08-22  1:39 ` Jonathan Cameron
2026-08-22 22:27   ` Bui Duc Phuc
2026-08-23 19:02     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox