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] [thread-next>] [day] [month] [year] [list]
Message-ID: <575AA5F8.90305@ti.com>
Date:	Fri, 10 Jun 2016 14:35:20 +0300
From:	Roger Quadros <rogerq@...com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.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 10/06/16 13:39, Sergei Shtylyov wrote:
> Hello.
> 
> On 6/10/2016 12:56 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?

> 
>> +            }
>> +        }
>> +    }
>> +
>> +    dwc->irq_gadget = irq;
>>
>>      dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
>>              &dwc->ctrl_req_addr, GFP_KERNEL);
>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
>> index c679f63..eb5e8f9 100644
>> --- a/drivers/usb/dwc3/host.c
>> +++ b/drivers/usb/dwc3/host.c
>> @@ -24,7 +24,46 @@ int dwc3_host_init(struct dwc3 *dwc)
>>  {
>>      struct platform_device    *xhci;
>>      struct usb_xhci_pdata    pdata;
>> -    int            ret;
>> +    int            ret, irq;
>> +    struct resource        *res;
>> +    struct platform_device    *dwc3_pdev = to_platform_device(dwc->dev);
>> +
>> +    irq = platform_get_irq_byname(dwc3_pdev, "host");
>> +    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 host IRQ\n");
>> +                }
>> +                return irq;
> 
>    Iff irq == 0, you'll return success despite IRQ was "invalid". Was that intended?
> 
>> +            } else {
>> +                res = platform_get_resource(dwc3_pdev,
>> +                                IORESOURCE_IRQ, 0);
>> +            }
>> +        } else {
>> +            res = platform_get_resource_byname(dwc3_pdev,
>> +                               IORESOURCE_IRQ,
>> +                               "dwc_usb3");
>> +        }
>> +
>> +    } else {
>> +        res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ,
>> +                           "host");
>> +    }
>> +
>> +    dwc->xhci_resources[1].start = irq;
>> +    dwc->xhci_resources[1].end = irq;
>> +    dwc->xhci_resources[1].flags = res->flags;
>> +    dwc->xhci_resources[1].name = res->name;
>>
>>      xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO);
>>      if (!xhci) {
> 

--
cheers,
-roger