On Fri, Dec 10, 2021 at 08:15:43PM +0300, Sergei Shtylyov wrote:

On 12/10/21 2:36 PM, Andy Shevchenko wrote:

quoted

quoted

quoted

quoted

quoted

quoted

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.

quoted

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?

   No (as if you need to really see this while it's obvious from the code review).

quoted

If no, what we are discussing about then? The workaround is redundant and

   I don't know. :-) Your arguments so far seem bogus (sorry! :-))...

It seems you haven't got them at all. The problems of platform_get_irq() et al
shouldn't be worked around in the callers.

quoted

no need to have a dead code in the driver, really.

  "Jazz isn't dead, it just smells funny". :-)

quoted

quoted

quoted

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

   The drivers would be broken in not quite obvious ways. With IRQ0 check, they just
don't probe anymore. See the explanation of the IRQ0 check (in the drivers) in my
previous mail...

quoted

and you get the big WARN() even before this patch.

   As if that was enough...
   The IRQ0 problem exists for at least 15 (if not 20) years...

-- 
With Best Regards,
Andy Shevchenko