On Mon, Jan 10, 2022 at 09:10:14PM +0100, Uwe Kleine-König wrote:

Hello,

On Mon, Jan 10, 2022 at 10:54:48PM +0300, Sergey Shtylyov wrote:

quoted

This patch is based on the former Andy Shevchenko's patch:

https://lore.kernel.org/lkml/20210331144526.19439-1-andriy.shevchenko@linux.intel.com/ (local)

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.

My suggestion would be to keep the return value of
platform_get_irq_optional() as is, but rename it to
platform_get_irq_silent() to get rid of the expectation invoked by
the naming similarity that motivated you to change
platform_get_irq_optional().

This is a good idea.

     Andrew