On Mon, Nov 01, 2021 at 11:39:13PM +0300, Sergey Shtylyov wrote:

Hello!

On 10/30/21 11:54 AM, Greg Kroah-Hartman wrote:

quoted

quoted

If platform_get_irq() returns IRQ0 (considered invalid according to Linus)
the driver blithely passes it to usb_add_hcd() that treats IRQ0 as no IRQ
at all. Deny IRQ0 right away, returning -EINVAL from the probe() method...

Fixes: 44ed240d6273 ("usb: host: ehci-exynos: Fix error check in exynos_ehci_probe()")
Signed-off-by: Sergey Shtylyov <redacted>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes in version 2:
- added Alan's ACK.

 drivers/usb/host/ehci-exynos.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 1a9b7572e17f..ff4e1261801a 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c

@@ -207,6 +207,10 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 		err = irq;
 		goto fail_io;
 	}
+	if (!irq) {
+		err = -EINVAL;
+		goto fail_io;
+	}

This is a huge sign that the api being used here is broken.

   And you're telling me that after I've wasted  time on v2? :-( Well, at least the series had
couple blunders, so it couldn't merged for 5.16-rc1 anyway (not sure why these weren't detected
in v1).

I thought about it some more and noticed it on your v2 submission.

quoted

Please fix the root cause here, if returning a 0 is an error, then have
the function you called to get this irq return an error.

   Well, technically not, although that doesn't match the kernel-doc for the function now.
I only don't understand why returning IRQ0 hasn't been replaced still...

Then please work on that.

quoted

Otherwise you
will have to fix ALL callers, and people will always get it wrong.
Fix the root cause here, don't paper it over.

   As I have already told you, I won't have to do it as filtering out is only needed iff 0 is
used as an indication for something special. IRQ0 is still perfectly valid for request_irq()
and is even called by arch/{aplha|mips|x86}...

If it is valid, then why can it not be a valid irq for all of these
drivers that you are changing here?  What is preventing them from
running on the platforms where 0 is a valid irq value?

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel