On Thu, Mar 18, 2021 at 1:12 PM Alexander Sverdlin
[off-list ref] wrote:

On 18/03/2021 11:51, Andy Shevchenko wrote:

quoted

On Wednesday, March 17, 2021, Alexander A Sverdlin <alexander.sverdlin@nokia.com <mailto:alexander.sverdlin@nokia.com>> wrote:

...

quoted

    Existing (irq < 0) condition is always false because adev->irq has unsigned
    type and contains 0 in case of failed irq_of_parse_and_map(). Up to now all
    the mapping errors were silently ignored.

    Seems that repairing this check would be backwards-incompatible and might
    break the probe() for the implementations without IRQ support. Therefore
    warn the user instead.

...

quoted

    -       if (irq < 0) {
    -               dev_err(&adev->dev, "invalid IRQ\n");
    -               return -ENODEV;
    -       }
    +       if (!irq)
    +               dev_warn(&adev->dev, "IRQ support disabled\n");



I guess you need to preserve bailing out. Seems nobody hit this error path.

Do you mean preserve "return -ENODEV;"?
This never ever happened, because the "if" is "always false", irqs coming from irq[] cannot be
negative.
And there is another use-case actually: there are legal PL061 configurations without IRQs at all,
which simply work even trying to instantiate irq chip, but as devm_gpiochip_add_data() doesn't
fail with irq==0, this goes completely unnoticed and such a gpio bank works fine.

The proper way would be not even try to instantiate any irq chip in such case.
Let me know if I shall rework the patch this way.

Yes, please, rewrite it like
if (irq > 0) {
 ... instantiate an IRQ chip ...
} else {
 // nothing. No warning is needed (as it wasn't ever before), perhaps
just a debug message
}
--
With Best Regards,
Andy Shevchenko