On Fri, Dec 10, 2021 at 02:19:52PM +0300, Sergey Shtylyov wrote: > On 12/10/21 1:46 PM, Andy Shevchenko wrote: > > >>>> platform_get_irq() will print a message when it fails. > >>>> No need to repeat this. > >>>> > >>>> While at it, drop redundant check for 0 as platform_get_irq() spills > >>>> out a big WARN() in such case. > >>> > >>> The reason you should be able to remove the "if (!irq)" test is that > >>> platform_get_irq() never returns 0. At least, that is what the function kdoc > >>> says. But looking at platform_get_irq_optional(), which is called by > >>> platform_get_irq(), the out label is: > >>> > >>> WARN(ret == 0, "0 is an invalid IRQ number\n"); > >>> return ret; > >>> > >>> So 0 will be returned as-is. That is rather weird. That should be fixed to > >>> return -ENXIO: > >>> > >>> if (WARN(ret == 0, "0 is an invalid IRQ number\n")) > >>> return -ENXIO; > >>> return ret; > >> > >> My unmerged patch (https://marc.info/?l=linux-kernel&m=163623041902285) does this > >> but returns -EINVAL instead. > >> > >>> Otherwise, I do not think that removing the "if (!irq)" hunk is safe. no ? > >> > >> Of course it isn't... > > > > It's unsubstantiated statement. The vIRQ 0 shouldn't be returned by any of > > those API calls. > > We do _not_ know what needs to be fixed, that's the problem, and that's why the WARN() > is there... So, have you seen this warning (being reported) related to libahci_platform? If no, what we are discussing about then? The workaround is redundant and no need to have a dead code in the driver, really. > > If it is the case, go and fix them, no need to workaround > > in each of the callers. > > There's a need to work around as long as IRQ0 ican be returned, otherwise > we get partly functioning or non-functioning drivers... You get them unfunctioning anyways and you get the big WARN() even before this patch. -- With Best Regards, Andy Shevchenko
