On Fri, Mar 13, 2020 at 10:05:58PM +0100, Thomas Gleixner wrote: > Bjorn Helgaas <helgaas@xxxxxxxxxx> writes: > > On Thu, Mar 12, 2020 at 10:53:06AM +0100, Marc Gonzalez wrote: > >> Last time around, my understanding was that, going forward, > >> the best solution was: > >> > >> virq = platform_get_irq(...) > >> if (virq <= 0) > >> return virq ? : -ENODEV; > >> > >> i.e. map 0 to -ENODEV, pass other errors as-is, remove the dev_err > >> > >> @Bjorn/Lorenzo did you have a change of heart? > > > > Yes. In 10006651 (Oct 20, 2017), I thought: > > > > irq = platform_get_irq(pdev, 0); > > if (irq <= 0) > > return -ENODEV; > > > > was fine. In 11066455 (Aug 7, 2019), I said I thought I was wrong and > > that: > > > > platform_get_irq() is a generic interface and we have to be able to > > interpret return values consistently. The overwhelming consensus > > among platform_get_irq() callers is to treat "irq < 0" as an error, > > and I think we should follow suit. > > ... > > I think the best pattern is: > > > > irq = platform_get_irq(pdev, i); > > if (irq < 0) > > return irq; > > Careful. 0 is not a valid interrupt. Should callers of platform_get_irq() check for a 0 return value? About 900 of them do not. Or should platform_get_irq() return a negative error instead of 0? If 0 is not a valid interrupt, I think it would be easier to use the interface if we made it so platform_get_irq() could never return 0, which I think would also fit the interface documentation better: * Return: IRQ number on success, negative error number on failure. Bjorn
