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@xxxxxx>
>> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>> ---
>> 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
