On Wed, Jan 12, 2022 at 11:27:02AM +0100, Geert Uytterhoeven wrote:
On Wed, Jan 12, 2022 at 9:51 AM Uwe Kleine-König u.kleine-koenig@pengutronix.de wrote:
On Wed, Jan 12, 2022 at 09:33:48AM +0100, Geert Uytterhoeven wrote:
On Mon, Jan 10, 2022 at 10:20 PM Andrew Lunn andrew@lunn.ch wrote:
On Mon, Jan 10, 2022 at 09:10:14PM +0100, Uwe Kleine-König wrote:
On Mon, Jan 10, 2022 at 10:54:48PM +0300, Sergey Shtylyov wrote:
This patch is based on the former Andy Shevchenko's patch:
https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux....
Currently platform_get_irq_optional() returns an error code even if IRQ resource simply has not been found. It prevents the callers from being error code agnostic in their error handling:
ret = platform_get_irq_optional(...); if (ret < 0 && ret != -ENXIO) return ret; // respect deferred probe if (ret > 0) ...we get an IRQ...All other *_optional() APIs seem to return 0 or NULL in case an optional resource is not available. Let's follow this good example, so that the callers would look like:
ret = platform_get_irq_optional(...); if (ret < 0) return ret; if (ret > 0) ...we get an IRQ...The difference to gpiod_get_optional (and most other *_optional) is that you can use the NULL value as if it were a valid GPIO.
As this isn't given with for irqs, I don't think changing the return value has much sense.
We actually want platform_get_irq_optional() to look different to all the other _optional() methods because it is not equivalent. If it looks the same, developers will assume it is the same, and get themselves into trouble.
Developers already assume it is the same, and thus forget they have to check against -ENXIO instead of zero.
Is this an ack for renaming platform_get_irq_optional() to platform_get_irq_silent()?
No it isn't ;-)
If an optional IRQ is not present, drivers either just ignore it (e.g. for devices that can have multiple interrupts or a single muxed IRQ), or they have to resort to polling. For the latter, fall-back handling is needed elsewhere in the driver. To me it sounds much more logical for the driver to check if an optional irq is non-zero (available) or zero (not available), than to sprinkle around checks for -ENXIO. In addition, you have to remember that this one returns -ENXIO, while other APIs use -ENOENT or -ENOSYS (or some other error code) to indicate absence. I thought not having to care about the actual error code was the main reason behind the introduction of the *_optional() APIs.
For the record, I'm on the same page with Geert.