[PATCH v2 01/22] usb: host: ehci-exynos: deny IRQ0

[PATCH v2 01/22] usb: host: ehci-exynos: deny IRQ0

[Sergey Shtylyov]

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 <s.shtylyov@omp.ru>
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;
+	}
 
 	err = exynos_ehci_phy_enable(&pdev->dev);
 	if (err) {
-- 
2.26.3


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

Re: [PATCH v2 01/22] usb: host: ehci-exynos: deny IRQ0

[Greg Kroah-Hartman]

On Tue, Oct 26, 2021 at 08:39:22PM +0300, Sergey Shtylyov wrote:
> 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 <s.shtylyov@omp.ru>
> 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.

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.  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.

thanks,

greg k-h

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

Re: [PATCH v2 01/22] usb: host: ehci-exynos: deny IRQ0

[Sergey Shtylyov]

Hello!

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

>> 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 <s.shtylyov@omp.ru>
>> 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).

> 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...

> 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}...

> thanks,
> 
> greg k-h

MBR, Sergey

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

Re: [PATCH v2 01/22] usb: host: ehci-exynos: deny IRQ0

[Greg Kroah-Hartman]

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:
> 
> >> 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 <s.shtylyov@omp.ru>
> >> 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.

> > 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.

> > 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

Re: [PATCH v2 01/22] usb: host: ehci-exynos: deny IRQ0

[Sergey Shtylyov]

HEllo!

On 11/2/21 4:55 PM, Greg Kroah-Hartman wrote:

[...]
>>>> 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 <s.shtylyov@omp.ru>
>>>> Acked-by: Alan Stern <stern@rowland.harvard.edu>
[...]

>>> 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}...

   "And the latter is called with 0 by", I meant to type... :-/
   The arguments I've heard for this ambiguity to continue were that IRQ0 is requested only with
setup_irq() (no longer true) and that these calls are confined to the arch code (still true).

> 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?

   These drivers call usb_add_hcd() that only calls request_irq() (via usb_hcd_request_irqs())
if IRQ # is non-zero -- otherwise the driver is supposed to handle the interrupt itself (if it
needs one?) --thus calling usb_add_hcd() with IRQ0 results in non-working HCD without IRQs...
   (For libata, ata_host_activate() (called in the end of the driver's probe() methods) checks
if the 'irq' parameter is 0 early and in this case warns about the 'irq_handler' parameter being
non-NULL and calls ata_host_register() without registering the IRQ handler and expects the driver
to set the polling flag, thus if IRQ0 is passed from an (unsuspecting) ATA driver, one gets not
fully functional host driver).)

> thanks,
> 
> greg k-h

MBR, Sergey

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

[PATCH v2 05/22] usb: host: ehci-platform: deny IRQ0

[Sergey Shtylyov]

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: 7a7a4a592f42 ("USB: EHCI: Add a generic platform device driver")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes in version 2:
- added Alan's ACK.

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

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index c70f2d0b4aaf..1b20af65d799 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -265,6 +265,8 @@ static int ehci_platform_probe(struct platform_device *dev)
 	irq = platform_get_irq(dev, 0);
 	if (irq < 0)
 		return irq;
+	if (!irq)
+		return -EINVAL;
 
 	hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
 			     dev_name(&dev->dev));
-- 
2.26.3


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

[PATCH v2 07/22] usb: host: ehci-st: deny IRQ0

[Sergey Shtylyov]

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: e47c5a0906f9 ("usb: host: ehci-st: Add EHCI support for ST STB devices")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes in version 2:
- added Alan's ACK.

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

diff --git a/drivers/usb/host/ehci-st.c b/drivers/usb/host/ehci-st.c
index f74433aac948..5f53c313f943 100644
--- a/drivers/usb/host/ehci-st.c
+++ b/drivers/usb/host/ehci-st.c
@@ -160,6 +160,8 @@ static int st_ehci_platform_probe(struct platform_device *dev)
 	irq = platform_get_irq(dev, 0);
 	if (irq < 0)
 		return irq;
+	if (!irq)
+		return -EINVAL;
 	res_mem = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	if (!res_mem) {
 		dev_err(&dev->dev, "no memory resource provided");
-- 
2.26.3


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

Re: [PATCH v2 07/22] usb: host: ehci-st: deny IRQ0

[Patrice CHOTARD]

Hi Sergey

On 10/26/21 7:39 PM, Sergey Shtylyov wrote:
> 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: e47c5a0906f9 ("usb: host: ehci-st: Add EHCI support for ST STB devices")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> ---
> Changes in version 2:
> - added Alan's ACK.
> 
>  drivers/usb/host/ehci-st.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/host/ehci-st.c b/drivers/usb/host/ehci-st.c
> index f74433aac948..5f53c313f943 100644
> --- a/drivers/usb/host/ehci-st.c
> +++ b/drivers/usb/host/ehci-st.c
> @@ -160,6 +160,8 @@ static int st_ehci_platform_probe(struct platform_device *dev)
>  	irq = platform_get_irq(dev, 0);
>  	if (irq < 0)
>  		return irq;
> +	if (!irq)
> +		return -EINVAL;
>  	res_mem = platform_get_resource(dev, IORESOURCE_MEM, 0);
>  	if (!res_mem) {
>  		dev_err(&dev->dev, "no memory resource provided");
> 
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

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

[PATCH v2 08/22] usb: host: ohci-at91: deny IRQ0

[Sergey Shtylyov]

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: fb5f1834c322 ("usb: ohci-at91: fix irq and iomem resource retrieval")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes in version 2:
- added Alan's ACK.

 drivers/usb/host/ohci-at91.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index a24aea3d2759..6a7276a730d1 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -181,6 +181,8 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 		dev_dbg(dev, "hcd probe: missing irq resource\n");
 		return irq;
 	}
+	if (!irq)
+		return -EINVAL;
 
 	hcd = usb_create_hcd(driver, dev, "at91");
 	if (!hcd)
-- 
2.26.3


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

[PATCH v2 10/22] usb: host: ohci-exynos: deny IRQ0

[Sergey Shtylyov]

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: 1d4169834628 ("usb: host: ohci-exynos: Fix error handling in exynos_ohci_probe()")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes in version 2:
- added Alan's ACK.

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

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 5f5e8a64c8e2..f28f28e42f43 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -175,6 +175,10 @@ static int exynos_ohci_probe(struct platform_device *pdev)
 		err = irq;
 		goto fail_io;
 	}
+	if (!irq) {
+		err = -EINVAL;
+		goto fail_io;
+	}
 
 	platform_set_drvdata(pdev, hcd);
 
-- 
2.26.3


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

[PATCH v2 11/22] usb: host: ohci-nxp: deny IRQ0

[Sergey Shtylyov]

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: 60bbfc84b6d9 ("USB OHCI controller support for PNX4008")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes in version 2:
- fixed the subject to match the patch;
- added Alan's ACK.

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

diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
index 85878e8ad331..afb9c2fc85c3 100644
--- a/drivers/usb/host/ohci-nxp.c
+++ b/drivers/usb/host/ohci-nxp.c
@@ -215,6 +215,10 @@ static int ohci_hcd_nxp_probe(struct platform_device *pdev)
 		ret = -ENXIO;
 		goto fail_resource;
 	}
+	if (!irq) {
+		ret = -EINVAL;
+		goto fail_resource;
+	}
 
 	ohci_nxp_start_hc();
 	platform_set_drvdata(pdev, hcd);
-- 
2.26.3


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

Re: [PATCH v2 11/22] usb: host: ohci-nxp: deny IRQ0

[Vladimir Zapolskiy]

Hi Sergey,

On 10/26/21 8:39 PM, Sergey Shtylyov wrote:
> 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: 60bbfc84b6d9 ("USB OHCI controller support for PNX4008")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> ---
> Changes in version 2:
> - fixed the subject to match the patch;
> - added Alan's ACK.
> 
>   drivers/usb/host/ohci-nxp.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
> index 85878e8ad331..afb9c2fc85c3 100644
> --- a/drivers/usb/host/ohci-nxp.c
> +++ b/drivers/usb/host/ohci-nxp.c
> @@ -215,6 +215,10 @@ static int ohci_hcd_nxp_probe(struct platform_device *pdev)
>   		ret = -ENXIO;
>   		goto fail_resource;
>   	}
> +	if (!irq) {
> +		ret = -EINVAL;
> +		goto fail_resource;
> +	}
>   
>   	ohci_nxp_start_hc();
>   	platform_set_drvdata(pdev, hcd);
> 

thank you for the change.

Acked-by: Vladimir Zapolskiy <vz@mleia.com>

--
Best wishes,
Vladimir

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

[PATCH v2 17/22] usb: host: ohci-st: deny IRQ0

[Sergey Shtylyov]

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: d115837259ad ("usb: host: ohci-st: Add OHCI driver support for ST STB devices")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
Changes in version 2:
- added Alan's ACK.

 drivers/usb/host/ohci-st.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ohci-st.c b/drivers/usb/host/ohci-st.c
index ac796ccd93ef..b9200071ce6c 100644
--- a/drivers/usb/host/ohci-st.c
+++ b/drivers/usb/host/ohci-st.c
@@ -140,6 +140,8 @@ static int st_ohci_platform_probe(struct platform_device *dev)
 	irq = platform_get_irq(dev, 0);
 	if (irq < 0)
 		return irq;
+	if (!irq)
+		return -EINVAL;
 
 	res_mem = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	if (!res_mem) {
-- 
2.26.3


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

Re: [PATCH v2 17/22] usb: host: ohci-st: deny IRQ0

[Patrice CHOTARD]

Hi Sergey

On 10/26/21 7:39 PM, Sergey Shtylyov wrote:
> 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: d115837259ad ("usb: host: ohci-st: Add OHCI driver support for ST STB devices")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> ---
> Changes in version 2:
> - added Alan's ACK.
> 
>  drivers/usb/host/ohci-st.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/host/ohci-st.c b/drivers/usb/host/ohci-st.c
> index ac796ccd93ef..b9200071ce6c 100644
> --- a/drivers/usb/host/ohci-st.c
> +++ b/drivers/usb/host/ohci-st.c
> @@ -140,6 +140,8 @@ static int st_ohci_platform_probe(struct platform_device *dev)
>  	irq = platform_get_irq(dev, 0);
>  	if (irq < 0)
>  		return irq;
> +	if (!irq)
> +		return -EINVAL;
>  
>  	res_mem = platform_get_resource(dev, IORESOURCE_MEM, 0);
>  	if (!res_mem) {
> 
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

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

[PATCH v2 20/22] usb: host: xhci-mtk: deny IRQ0

[Sergey Shtylyov]

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: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
 drivers/usb/host/xhci-mtk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index c53f6f276d5c..d2dc8d9863ee 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -495,7 +495,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return ret;
 
 	irq = platform_get_irq_byname_optional(pdev, "host");
-	if (irq < 0) {
+	if (irq <= 0) {
 		if (irq == -EPROBE_DEFER)
 			return irq;
 
@@ -503,6 +503,8 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		irq = platform_get_irq(pdev, 0);
 		if (irq < 0)
 			return irq;
+		if (!irq)
+			return -EINVAL;
 	}
 
 	wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
-- 
2.26.3

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

Re: [PATCH v2 20/22] usb: host: xhci-mtk: deny IRQ0

[Chunfeng Yun]

On Tue, 2021-10-26 at 20:39 +0300, Sergey Shtylyov wrote:
> 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: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host
> controller")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> ---
>  drivers/usb/host/xhci-mtk.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-
> mtk.c
> index c53f6f276d5c..d2dc8d9863ee 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -495,7 +495,7 @@ static int xhci_mtk_probe(struct platform_device
> *pdev)
>  		return ret;
>  
>  	irq = platform_get_irq_byname_optional(pdev, "host");
> -	if (irq < 0) {
> +	if (irq <= 0) {
>  		if (irq == -EPROBE_DEFER)
>  			return irq;
>  
> @@ -503,6 +503,8 @@ static int xhci_mtk_probe(struct platform_device
> *pdev)
>  		irq = platform_get_irq(pdev, 0);
>  		if (irq < 0)
>  			return irq;
> +		if (!irq)
It is "if (irq <= 0)"?


> +			return -EINVAL;
>  	}
>  
>  	wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 20/22] usb: host: xhci-mtk: deny IRQ0

[Chunfeng Yun]

On Tue, 2021-10-26 at 20:39 +0300, Sergey Shtylyov wrote:
> 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: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host
> controller")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> ---
>  drivers/usb/host/xhci-mtk.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-
> mtk.c
> index c53f6f276d5c..d2dc8d9863ee 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -495,7 +495,7 @@ static int xhci_mtk_probe(struct platform_device
> *pdev)
>  		return ret;
>  
>  	irq = platform_get_irq_byname_optional(pdev, "host");
> -	if (irq < 0) {
> +	if (irq <= 0) {
>  		if (irq == -EPROBE_DEFER)
>  			return irq;
>  
> @@ -503,6 +503,8 @@ static int xhci_mtk_probe(struct platform_device
> *pdev)
>  		irq = platform_get_irq(pdev, 0);
>  		if (irq < 0)
>  			return irq;
> +		if (!irq)
it's "if (irq <= 0)"?

> +			return -EINVAL;
>  	}
>  
>  	wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 20/22] usb: host: xhci-mtk: deny IRQ0

[Sergey Shtylyov]

On 27.10.2021 12:18, Chunfeng Yun wrote:

>> 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: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host
>> controller")
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>> ---
>>   drivers/usb/host/xhci-mtk.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-
>> mtk.c
>> index c53f6f276d5c..d2dc8d9863ee 100644
>> --- a/drivers/usb/host/xhci-mtk.c
>> +++ b/drivers/usb/host/xhci-mtk.c
>> @@ -495,7 +495,7 @@ static int xhci_mtk_probe(struct platform_device
>> *pdev)
>>   		return ret;
>>   
>>   	irq = platform_get_irq_byname_optional(pdev, "host");
>> -	if (irq < 0) {
>> +	if (irq <= 0) {
>>   		if (irq == -EPROBE_DEFER)
>>   			return irq;
>>   
>> @@ -503,6 +503,8 @@ static int xhci_mtk_probe(struct platform_device
>> *pdev)
>>   		irq = platform_get_irq(pdev, 0);
>>   		if (irq < 0)
>>   			return irq;
>> +		if (!irq)
> it's "if (irq <= 0)"?

    No, if (irq == 0).

>> +			return -EINVAL;
>>   	}
>>   
>>   	wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");

MBR, Sergey

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

Re: [PATCH v2 20/22] usb: host: xhci-mtk: deny IRQ0

[Chunfeng Yun]

On Wed, 2021-10-27 at 12:25 +0300, Sergey Shtylyov wrote:
> On 27.10.2021 12:18, Chunfeng Yun wrote:
> 
> > > 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: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host
> > > controller")
> > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> > > ---
> > >   drivers/usb/host/xhci-mtk.c | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-
> > > mtk.c
> > > index c53f6f276d5c..d2dc8d9863ee 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -495,7 +495,7 @@ static int xhci_mtk_probe(struct
> > > platform_device
> > > *pdev)
> > >   		return ret;
> > >   
> > >   	irq = platform_get_irq_byname_optional(pdev, "host");
> > > -	if (irq < 0) {
> > > +	if (irq <= 0) {
> > >   		if (irq == -EPROBE_DEFER)
> > >   			return irq;
> > >   
> > > @@ -503,6 +503,8 @@ static int xhci_mtk_probe(struct
> > > platform_device
> > > *pdev)
> > >   		irq = platform_get_irq(pdev, 0);
Here, irq is updated again

> > >   		if (irq < 0)
> > >   			return irq;
> > > +		if (!irq)
> > 
> > it's "if (irq <= 0)"?
> 
>     No, if (irq == 0).
> 
> > > +			return -EINVAL;
add "else if (irq < 0), return irq;"?  but not -EINVAL only, this may
drop EPROBE_DEFER

> > >   	}
> > >   
> > >   	wakeup_irq = platform_get_irq_byname_optional(pdev,
> > > "wakeup");
> 
> MBR, Sergey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel