linux-kernel - Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <42bc2e3b-9282-4bbd-d9ed-237ea4367ea4@cogentembedded.com>
Date:	Fri, 10 Jun 2016 15:22:30 +0300
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Roger Quadros <rogerq@...com>, balbi@...nel.org,
	grygorii.strashko@...com
Cc:	tony@...mide.com, Joao.Pinto@...opsys.com,
	peter.chen@...escale.com, jun.li@...escale.com,
	yoshihiro.shimoda.uh@...esas.com, nsekhar@...com, b-liu@...com,
	linux-usb@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources

On 6/10/2016 2:46 PM, Roger Quadros wrote:

>>>>> Implementations might use different IRQs for
>>>>> host, gadget so use named interrupt resources
>>>>> to allow device tree to specify the interrupts.
>>>>>
>>>>> Following are the interrupt names
>>>>>
>>>>> Peripheral Interrupt - peripheral
>>>>> HOST Interrupt - host
>>>>>
>>>>> Maintain backward compatibility for a single named
>>>>> interrupt ("dwc3_usb3") for all interrupts as well as
>>>>> unnamed interrupt at index 0 for all interrupts.
>>>>>
>>>>> As platform_get_irq_() variants are used, tackle
>>>>
>>>>    platform_get_irq().
>>>
>>> OK.
>>>>
>>>>> the -EPROBE_DEFER case as well.
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@...com>
>>>>> ---
>>>>> v10:
>>>>> - don't mention otg irq since we are not using it yet
>>>>> - use platform_get_irq() and friends and check -EPROBE_DEFER case.
>>>>>
>>>>>  drivers/usb/dwc3/core.c   | 22 ++++++++--------------
>>>>>  drivers/usb/dwc3/gadget.c | 29 ++++++++++++++++++++++++++---
>>>>>  drivers/usb/dwc3/host.c   | 41 ++++++++++++++++++++++++++++++++++++++++-
>>>>>  3 files changed, 74 insertions(+), 18 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>> index 8fceeb1..131e7eb 100644
>>>>> --- a/drivers/usb/dwc3/core.c
>>>>> +++ b/drivers/usb/dwc3/core.c
>>>> [...]
>>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>>> index 0f6fb8e..774a0d8 100644
>>>>> --- a/drivers/usb/dwc3/gadget.c
>>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> [...]
>>>>> @@ -2866,7 +2865,31 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>>>>   */
>>>>>  int dwc3_gadget_init(struct dwc3 *dwc)
>>>>>  {
>>>>> -    int                    ret;
>>>>> +    int ret, irq;
>>>>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>>>>> +
>>>>> +    irq = platform_get_irq_byname(dwc3_pdev, "peripheral");
>>>>> +    if (irq == -EPROBE_DEFER)
>>>>> +        return irq;
>>>>> +
>>>>> +    if (irq <= 0) {
>>>>> +        irq = platform_get_irq_byname(dwc3_pdev, "dwc_usb3");
>>>>> +        if (irq == -EPROBE_DEFER)
>>>>> +            return irq;
>>>>> +
>>>>> +        if (irq <= 0) {
>>>>> +            irq = platform_get_irq(dwc3_pdev, 0);
>>>>> +            if (irq <= 0) {
>>>>> +                if (irq != -EPROBE_DEFER) {
>>>>> +                    dev_err(dwc->dev,
>>>>> +                        "missing peripheral IRQ\n");
>>>>> +                }
>>>>> +                return irq;
>>>>
>>>>    Iff irq == 0, you'll return success despite IRQ was "invalid". Was that intended?
>>>
>>> good catch. It wasn't intended. I guess i'll return -EINVAL then?

[...]

>>    I'd just consider 0 a valid IRQ, that's simpler. FYI, I've submitted to Greg KH a patch fixing platform_get_irq[_byname]() to not return 0 on failure. No reaction so far...
>
> Maybe till your patch is in we can't really differentiate if it is error or not
> so it is safer to consider it as error IMO.

    Returning -EINVAL is fine then.

> cheers,
> -roger

MBR, Sergei