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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux for Synopsys ARC Processors]    
  • [Linux on Unisoc (RDA Micro) SoCs]     [Linux Actions SoC]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  •   Powered by Linux