On Wed, Jan 12, 2022 at 02:38:37PM +0100, Andrew Lunn wrote:
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.
The *_optional() functions return an error code if there has been a real error which should be reported up the call stack. This excludes whatever error code indicates the requested resource does not exist, which can be -ENODEV etc. If the device does not exist, a magic cookie is returned which appears to be a valid resources but in fact is not. So the users of these functions just need to check for an error code, and fail the probe if present.
You seems to be suggesting in binary return value: non-zero (available) or zero (not available)
No, what is suggested is to (besides the API changes): - do not treat ENXIO as something special in platform_get_irq*() - allow platform_get_irq*() to return other error codes
This discards the error code when something goes wrong. That is useful information to have, so we should not be discarding it.
IRQ don't currently have a magic cookie value. One option would be to add such a magic cookie to the subsystem. Otherwise, since 0 is invalid, return 0 to indicate the IRQ does not exist.
The request for a script checking this then makes sense. However, i don't know how well coccinelle/sparse can track values across function calls. They probably can check for:
ret = irq_get_optional() if (ret < 0) return ret;
A missing if < 0 statement somewhere later is very likely to be an error. A comparison of <= 0 is also likely to be an error. A check for
0 before calling any other IRQ functions would be good. I'm
surprised such a check does not already existing in the IRQ API, but there are probably historical reasons for that.
Andrew