Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1410894 > unrolled thread

[PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-01 09:50 +0200
    Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Felipe Balbi <balbi@kernel.org> - 2016-06-01 10:10 +0200
      Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-07 11:40 +0200
    Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Grygorii Strashko <grygorii.strashko@ti.com> - 2016-06-02 14:00 +0200
      Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-07 11:40 +0200
        Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Grygorii Strashko <grygorii.strashko@ti.com> - 2016-06-07 14:00 +0200
          Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-07 14:50 +0200
            Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Felipe Balbi <balbi@kernel.org> - 2016-06-07 15:10 +0200
              Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-07 16:10 +0200
          Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 10:00 +0200
      Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 10:10 +0200
        Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 10:10 +0200
          Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Felipe Balbi <balbi@kernel.org> - 2016-06-10 10:20 +0200
            Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 10:40 +0200
              Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Felipe Balbi <balbi@kernel.org> - 2016-06-10 11:20 +0200
        Re: [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources Felipe Balbi <balbi@kernel.org> - 2016-06-10 10:20 +0200
    [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 12:00 +0200
      Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-06-10 12:40 +0200
        Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 13:40 +0200
          Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 13:50 +0200
            Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-06-10 14:30 +0200
          Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-06-10 13:50 +0200
      [PATCH v11 5/5] usb: dwc3: core: cleanup IRQ resources Roger Quadros <rogerq@ti.com> - 2016-06-10 13:50 +0200

Page 1 of 2  [1] 2  Next page →

#1410894 — [PATCH v9 5/5] usb: dwc3: core: cleanup IRQ resources

Implementations might use different IRQs for
host, gadget and OTG so use named interrupt resources
to allow Device tree to specify the 3 interrupts.

Following are the interrupt names

Peripheral Interrupt - peripheral
HOST Interrupt - host
OTG Interrupt - otg

We still maintain backward compatibility for a single named
interrupt for all 3 interrupts (e.g. for dwc3-pci) and
single unnamed interrupt for all 3 interrupts (e.g. old DT).

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
v9: rebased on top of balbi/testing/next

 drivers/usb/dwc3/core.c   | 10 ----------
 drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
 drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9c4e1d8d..5cedf3d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc->mem = mem;
 	dwc->dev = dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(dev, "missing IRQ\n");
-		return -ENODEV;
-	}
-	dwc->xhci_resources[1].start = res->start;
-	dwc->xhci_resources[1].end = res->end;
-	dwc->xhci_resources[1].flags = res->flags;
-	dwc->xhci_resources[1].name = res->name;
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(dev, "missing memory resource\n");
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c37168d..c18c72f 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
 	int			ret = 0;
 	int			irq;
 
-	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
+	irq = dwc->irq_gadget;
 	ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
 			IRQF_SHARED, "dwc3", dwc->ev_buf);
 	if (ret) {
@@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
 				irq, ret);
 		goto err0;
 	}
-	dwc->irq_gadget = irq;
 
 	spin_lock_irqsave(&dwc->lock, flags);
 	if (dwc->gadget_driver) {
@@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
 int dwc3_gadget_init(struct dwc3 *dwc)
 {
 	int					ret;
+	struct resource *res;
+	struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
+
+	dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
+	if (dwc->irq_gadget <= 0) {
+		dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev,
+							  "dwc_usb3");
+		if (dwc->irq_gadget <= 0) {
+			res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
+						    0);
+			if (!res) {
+				dev_err(dwc->dev, "missing peripheral IRQ\n");
+				return -ENODEV;
+			}
+			dwc->irq_gadget = res->start;
+		}
+	}
 
 	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..f2b60a4 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc)
 	struct platform_device	*xhci;
 	struct usb_xhci_pdata	pdata;
 	int			ret;
+	struct resource		*res;
+	struct platform_device	*dwc3_pdev = to_platform_device(dwc->dev);
+
+	res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host");
+	if (!res) {
+		res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ,
+						   "dwc_usb3");
+		if (!res) {
+			res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
+						    0);
+			if (!res)
+				return -ENOMEM;
+		}
+	}
+
+	dwc->xhci_resources[1].start = res->start;
+	dwc->xhci_resources[1].end = res->end;
+	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) {
-- 
2.7.4

[toc] | [next] | [standalone]

#1410923

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
> Implementations might use different IRQs for
> host, gadget and OTG so use named interrupt resources
> to allow Device tree to specify the 3 interrupts.
>
> Following are the interrupt names
>
> Peripheral Interrupt - peripheral
> HOST Interrupt - host
> OTG Interrupt - otg
>
> We still maintain backward compatibility for a single named
> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
> single unnamed interrupt for all 3 interrupts (e.g. old DT).
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> v9: rebased on top of balbi/testing/next

breaks dwc3:

[  222.776504] dwc3 dwc3.0.auto: failed to request irq #-6 --> -22

please test

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1415933

[Multipart message — attachments visible in raw view] — view raw

Felipe,

On 01/06/16 11:06, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> Implementations might use different IRQs for
>> host, gadget and OTG so use named interrupt resources
>> to allow Device tree to specify the 3 interrupts.
>>
>> Following are the interrupt names
>>
>> Peripheral Interrupt - peripheral
>> HOST Interrupt - host
>> OTG Interrupt - otg
>>
>> We still maintain backward compatibility for a single named
>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
>> single unnamed interrupt for all 3 interrupts (e.g. old DT).
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> v9: rebased on top of balbi/testing/next
> 
> breaks dwc3:
> 
> [  222.776504] dwc3 dwc3.0.auto: failed to request irq #-6 --> -22
> 
> please test
> 

I couldn't reproduce the failure at my end.
Could it be specific to your setup?

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1412163

On 06/01/2016 10:46 AM, Roger Quadros wrote:
> Implementations might use different IRQs for
> host, gadget and OTG so use named interrupt resources
> to allow Device tree to specify the 3 interrupts.
>
> Following are the interrupt names
>
> Peripheral Interrupt - peripheral
> HOST Interrupt - host
> OTG Interrupt - otg

or "dwc_usb3"??

>
> We still maintain backward compatibility for a single named
> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
> single unnamed interrupt for all 3 interrupts (e.g. old DT).

bindings

>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> v9: rebased on top of balbi/testing/next
>
>   drivers/usb/dwc3/core.c   | 10 ----------
>   drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
>   drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
>   3 files changed, 37 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 9c4e1d8d..5cedf3d 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
>   	dwc->mem = mem;
>   	dwc->dev = dev;
>
> -	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -	if (!res) {
> -		dev_err(dev, "missing IRQ\n");
> -		return -ENODEV;
> -	}
> -	dwc->xhci_resources[1].start = res->start;
> -	dwc->xhci_resources[1].end = res->end;
> -	dwc->xhci_resources[1].flags = res->flags;
> -	dwc->xhci_resources[1].name = res->name;
> -
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	if (!res) {
>   		dev_err(dev, "missing memory resource\n");
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index c37168d..c18c72f 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>   	int			ret = 0;
>   	int			irq;
>
> -	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
> +	irq = dwc->irq_gadget;
>   	ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
>   			IRQF_SHARED, "dwc3", dwc->ev_buf);
>   	if (ret) {
> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>   				irq, ret);
>   		goto err0;
>   	}
> -	dwc->irq_gadget = irq;
>
>   	spin_lock_irqsave(&dwc->lock, flags);
>   	if (dwc->gadget_driver) {
> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>   int dwc3_gadget_init(struct dwc3 *dwc)
>   {
>   	int					ret;
> +	struct resource *res;
> +	struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
> +
> +	dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
> +	if (dwc->irq_gadget <= 0) {

Is it expected to get -EPROBE_DEFER here?

> +		dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev,
> +							  "dwc_usb3");
> +		if (dwc->irq_gadget <= 0) {
> +			res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
> +						    0);

It's better to use platform_get_irq().

> +			if (!res) {
> +				dev_err(dwc->dev, "missing peripheral IRQ\n");
> +				return -ENODEV;
> +			}
> +			dwc->irq_gadget = res->start;
> +		}
> +	}
>
>   	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..f2b60a4 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc)
>   	struct platform_device	*xhci;
>   	struct usb_xhci_pdata	pdata;
>   	int			ret;
> +	struct resource		*res;
> +	struct platform_device	*dwc3_pdev = to_platform_device(dwc->dev);
> +
> +	res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host");
> +	if (!res) {
> +		res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ,
> +						   "dwc_usb3");
> +		if (!res) {
> +			res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
> +						    0);
> +			if (!res)

> +				return -ENOMEM;
> +		}
> +	}

Is it expected to have more than one IRQ here?

if not - it will better to use platform_get_irq[_byname]().


> +
> +	dwc->xhci_resources[1].start = res->start;
> +	dwc->xhci_resources[1].end = res->end;
> +	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) {
>


-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1415936

On 02/06/16 14:52, Grygorii Strashko wrote:
> On 06/01/2016 10:46 AM, Roger Quadros wrote:
>> Implementations might use different IRQs for
>> host, gadget and OTG so use named interrupt resources
>> to allow Device tree to specify the 3 interrupts.
>>
>> Following are the interrupt names
>>
>> Peripheral Interrupt - peripheral
>> HOST Interrupt - host
>> OTG Interrupt - otg
> 
> or "dwc_usb3"??

That is for backward compatibility only. I could explicitly
mention it in the next line.

> 
>>
>> We still maintain backward compatibility for a single named
>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
>> single unnamed interrupt for all 3 interrupts (e.g. old DT).
> 
> bindings

OK.
> 
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> v9: rebased on top of balbi/testing/next
>>
>>   drivers/usb/dwc3/core.c   | 10 ----------
>>   drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
>>   drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
>>   3 files changed, 37 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 9c4e1d8d..5cedf3d 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
>>       dwc->mem = mem;
>>       dwc->dev = dev;
>>
>> -    res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> -    if (!res) {
>> -        dev_err(dev, "missing IRQ\n");
>> -        return -ENODEV;
>> -    }
>> -    dwc->xhci_resources[1].start = res->start;
>> -    dwc->xhci_resources[1].end = res->end;
>> -    dwc->xhci_resources[1].flags = res->flags;
>> -    dwc->xhci_resources[1].name = res->name;
>> -
>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       if (!res) {
>>           dev_err(dev, "missing memory resource\n");
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index c37168d..c18c72f 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>       int            ret = 0;
>>       int            irq;
>>
>> -    irq = platform_get_irq(to_platform_device(dwc->dev), 0);
>> +    irq = dwc->irq_gadget;
>>       ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
>>               IRQF_SHARED, "dwc3", dwc->ev_buf);
>>       if (ret) {
>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>                   irq, ret);
>>           goto err0;
>>       }
>> -    dwc->irq_gadget = irq;
>>
>>       spin_lock_irqsave(&dwc->lock, flags);
>>       if (dwc->gadget_driver) {
>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>   int dwc3_gadget_init(struct dwc3 *dwc)
>>   {
>>       int                    ret;
>> +    struct resource *res;
>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>> +
>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>> +    if (dwc->irq_gadget <= 0) {
> 
> Is it expected to get -EPROBE_DEFER here?

Probably not as we don't have any chance of deferring probe here. We've already
probed successfully and are just turning on the gadget mode here.

> 
>> +        dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev,
>> +                              "dwc_usb3");
>> +        if (dwc->irq_gadget <= 0) {
>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>> +                            0);
> 
> It's better to use platform_get_irq().

OK.
> 
>> +            if (!res) {
>> +                dev_err(dwc->dev, "missing peripheral IRQ\n");
>> +                return -ENODEV;
>> +            }
>> +            dwc->irq_gadget = res->start;
>> +        }
>> +    }
>>
>>       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..f2b60a4 100644
>> --- a/drivers/usb/dwc3/host.c
>> +++ b/drivers/usb/dwc3/host.c
>> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc)
>>       struct platform_device    *xhci;
>>       struct usb_xhci_pdata    pdata;
>>       int            ret;
>> +    struct resource        *res;
>> +    struct platform_device    *dwc3_pdev = to_platform_device(dwc->dev);
>> +
>> +    res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host");
>> +    if (!res) {
>> +        res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ,
>> +                           "dwc_usb3");
>> +        if (!res) {
>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>> +                            0);
>> +            if (!res)
> 
>> +                return -ENOMEM;
>> +        }
>> +    }
> 
> Is it expected to have more than one IRQ here?

No.
> 
> if not - it will better to use platform_get_irq[_byname]().

OK.
> 
> 
>> +
>> +    dwc->xhci_resources[1].start = res->start;
>> +    dwc->xhci_resources[1].end = res->end;
>> +    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

[toc] | [prev] | [next] | [standalone]

#1416082

On 06/07/2016 12:34 PM, Roger Quadros wrote:
> On 02/06/16 14:52, Grygorii Strashko wrote:
>> On 06/01/2016 10:46 AM, Roger Quadros wrote:
>>> Implementations might use different IRQs for
>>> host, gadget and OTG so use named interrupt resources
>>> to allow Device tree to specify the 3 interrupts.
>>>
>>> Following are the interrupt names
>>>
>>> Peripheral Interrupt - peripheral
>>> HOST Interrupt - host
>>> OTG Interrupt - otg
>>
>> or "dwc_usb3"??
> 
> That is for backward compatibility only. I could explicitly
> mention it in the next line.

yes pls, this confuses.
 Also I don't see how "otg" irq name is used in code.

> 
>>
>>>
>>> We still maintain backward compatibility for a single named
>>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
>>> single unnamed interrupt for all 3 interrupts (e.g. old DT).
>>
>> bindings
> 
> OK.
>>
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>> v9: rebased on top of balbi/testing/next
>>>
>>>    drivers/usb/dwc3/core.c   | 10 ----------
>>>    drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
>>>    drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
>>>    3 files changed, 37 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 9c4e1d8d..5cedf3d 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
>>>        dwc->mem = mem;
>>>        dwc->dev = dev;
>>>
>>> -    res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>> -    if (!res) {
>>> -        dev_err(dev, "missing IRQ\n");
>>> -        return -ENODEV;
>>> -    }
>>> -    dwc->xhci_resources[1].start = res->start;
>>> -    dwc->xhci_resources[1].end = res->end;
>>> -    dwc->xhci_resources[1].flags = res->flags;
>>> -    dwc->xhci_resources[1].name = res->name;
>>> -
>>>        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>        if (!res) {
>>>            dev_err(dev, "missing memory resource\n");
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index c37168d..c18c72f 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>        int            ret = 0;
>>>        int            irq;
>>>
>>> -    irq = platform_get_irq(to_platform_device(dwc->dev), 0);
>>> +    irq = dwc->irq_gadget;
>>>        ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
>>>                IRQF_SHARED, "dwc3", dwc->ev_buf);
>>>        if (ret) {
>>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>                    irq, ret);
>>>            goto err0;
>>>        }
>>> -    dwc->irq_gadget = irq;
>>>
>>>        spin_lock_irqsave(&dwc->lock, flags);
>>>        if (dwc->gadget_driver) {
>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>>    int dwc3_gadget_init(struct dwc3 *dwc)
>>>    {
>>>        int                    ret;
>>> +    struct resource *res;
>>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>>> +
>>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>>> +    if (dwc->irq_gadget <= 0) {
>>
>> Is it expected to get -EPROBE_DEFER here?
> 
> Probably not as we don't have any chance of deferring probe here. We've already
> probed successfully and are just turning on the gadget mode here.

In general, you can't say that you've been probed successfully if not all resources are ready,
and irq is a resource :)
It's expected that all resources will be requested in probe, but here you are trying to get
resource outside of probe. As result, it will be perfectly possible to get -EPROBE_DEFER here
if on some HW GPIO IRQ will be used as peripheral, or host or otg irq (for example), because 
GPIO IRQ controller might not be ready at the moment when IRQ resource is requested.

> 
>>
>>> +        dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev,
>>> +                              "dwc_usb3");
>>> +        if (dwc->irq_gadget <= 0) {
>>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>>> +                            0);
>>
>> It's better to use platform_get_irq().
> 
> OK.
>>
>>> +            if (!res) {
>>> +                dev_err(dwc->dev, "missing peripheral IRQ\n");
>>> +                return -ENODEV;
>>> +            }
>>> +            dwc->irq_gadget = res->start;
>>> +        }
>>> +    }
>>>
>>>        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..f2b60a4 100644
>>> --- a/drivers/usb/dwc3/host.c
>>> +++ b/drivers/usb/dwc3/host.c
>>> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc)
>>>        struct platform_device    *xhci;
>>>        struct usb_xhci_pdata    pdata;
>>>        int            ret;
>>> +    struct resource        *res;
>>> +    struct platform_device    *dwc3_pdev = to_platform_device(dwc->dev);
>>> +
>>> +    res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host");
>>> +    if (!res) {
>>> +        res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ,
>>> +                           "dwc_usb3");
>>> +        if (!res) {
>>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>>> +                            0);
>>> +            if (!res)
>>
>>> +                return -ENOMEM;
>>> +        }
>>> +    }
>>
>> Is it expected to have more than one IRQ here?
> 
> No.
>>
>> if not - it will better to use platform_get_irq[_byname]().
> 
> OK.
>>
>>
>>> +
>>> +    dwc->xhci_resources[1].start = res->start;
>>> +    dwc->xhci_resources[1].end = res->end;
>>> +    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
> 


-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1416137

On 07/06/16 14:49, Grygorii Strashko wrote:
> On 06/07/2016 12:34 PM, Roger Quadros wrote:
>> On 02/06/16 14:52, Grygorii Strashko wrote:
>>> On 06/01/2016 10:46 AM, Roger Quadros wrote:
>>>> Implementations might use different IRQs for
>>>> host, gadget and OTG so use named interrupt resources
>>>> to allow Device tree to specify the 3 interrupts.
>>>>
>>>> Following are the interrupt names
>>>>
>>>> Peripheral Interrupt - peripheral
>>>> HOST Interrupt - host
>>>> OTG Interrupt - otg
>>>
>>> or "dwc_usb3"??
>>
>> That is for backward compatibility only. I could explicitly
>> mention it in the next line.
> 
> yes pls, this confuses.
>  Also I don't see how "otg" irq name is used in code.
> 

OK. I'll remove it from the commit message.
>>
>>>
>>>>
>>>> We still maintain backward compatibility for a single named
>>>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
>>>> single unnamed interrupt for all 3 interrupts (e.g. old DT).
>>>
>>> bindings
>>
>> OK.
>>>
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>> v9: rebased on top of balbi/testing/next
>>>>
>>>>    drivers/usb/dwc3/core.c   | 10 ----------
>>>>    drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
>>>>    drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
>>>>    3 files changed, 37 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 9c4e1d8d..5cedf3d 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
>>>>        dwc->mem = mem;
>>>>        dwc->dev = dev;
>>>>
>>>> -    res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>>> -    if (!res) {
>>>> -        dev_err(dev, "missing IRQ\n");
>>>> -        return -ENODEV;
>>>> -    }
>>>> -    dwc->xhci_resources[1].start = res->start;
>>>> -    dwc->xhci_resources[1].end = res->end;
>>>> -    dwc->xhci_resources[1].flags = res->flags;
>>>> -    dwc->xhci_resources[1].name = res->name;
>>>> -
>>>>        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>        if (!res) {
>>>>            dev_err(dev, "missing memory resource\n");
>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index c37168d..c18c72f 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>>        int            ret = 0;
>>>>        int            irq;
>>>>
>>>> -    irq = platform_get_irq(to_platform_device(dwc->dev), 0);
>>>> +    irq = dwc->irq_gadget;
>>>>        ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
>>>>                IRQF_SHARED, "dwc3", dwc->ev_buf);
>>>>        if (ret) {
>>>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>>                    irq, ret);
>>>>            goto err0;
>>>>        }
>>>> -    dwc->irq_gadget = irq;
>>>>
>>>>        spin_lock_irqsave(&dwc->lock, flags);
>>>>        if (dwc->gadget_driver) {
>>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>>>    int dwc3_gadget_init(struct dwc3 *dwc)
>>>>    {
>>>>        int                    ret;
>>>> +    struct resource *res;
>>>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>>>> +
>>>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>>>> +    if (dwc->irq_gadget <= 0) {
>>>
>>> Is it expected to get -EPROBE_DEFER here?
>>
>> Probably not as we don't have any chance of deferring probe here. We've already
>> probed successfully and are just turning on the gadget mode here.
> 
> In general, you can't say that you've been probed successfully if not all resources are ready,
> and irq is a resource :)
> It's expected that all resources will be requested in probe, but here you are trying to get
> resource outside of probe. As result, it will be perfectly possible to get -EPROBE_DEFER here
> if on some HW GPIO IRQ will be used as peripheral, or host or otg irq (for example), because 
> GPIO IRQ controller might not be ready at the moment when IRQ resource is requested.

I agree with you.

Felipe, are you ok with moving the IRQ resource obtaining code to probe?

--
cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1416146

[Multipart message — attachments visible in raw view] — view raw

Hi,

(guys, please make sure to break lines at 80-columns)

Roger Quadros <rogerq@ti.com> writes:
>>>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>>>>    int dwc3_gadget_init(struct dwc3 *dwc)
>>>>>    {
>>>>>        int                    ret;
>>>>> +    struct resource *res;
>>>>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>>>>> +
>>>>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>>>>> +    if (dwc->irq_gadget <= 0) {
>>>>
>>>> Is it expected to get -EPROBE_DEFER here?
>>>
>>> Probably not as we don't have any chance of deferring probe here. We've already
>>> probed successfully and are just turning on the gadget mode here.
>> 
>> In general, you can't say that you've been probed successfully if not
>> all resources are ready, and irq is a resource :) It's expected that
>> all resources will be requested in probe, but here you are trying to
>> get resource outside of probe. As result, it will be perfectly
>> possible to get -EPROBE_DEFER here if on some HW GPIO IRQ will be
>> used as peripheral, or host or otg irq (for example), because GPIO
>> IRQ controller might not be ready at the moment when IRQ resource is
>> requested.
>
> I agree with you.
>
> Felipe, are you ok with moving the IRQ resource obtaining code to probe?

You mean that probe() would setup all gadget_irq, otg_irq and host_irq
while the other pieces (otg.c, gadget.c and host.c) only use it?

yeah, that should be fine. No problems.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1416240

[Multipart message — attachments visible in raw view] — view raw

On 07/06/16 16:09, Felipe Balbi wrote:
> 
> Hi,
> 
> (guys, please make sure to break lines at 80-columns)
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>>>>>    int dwc3_gadget_init(struct dwc3 *dwc)
>>>>>>    {
>>>>>>        int                    ret;
>>>>>> +    struct resource *res;
>>>>>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>>>>>> +
>>>>>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>>>>>> +    if (dwc->irq_gadget <= 0) {
>>>>>
>>>>> Is it expected to get -EPROBE_DEFER here?
>>>>
>>>> Probably not as we don't have any chance of deferring probe here. We've already
>>>> probed successfully and are just turning on the gadget mode here.
>>>
>>> In general, you can't say that you've been probed successfully if not
>>> all resources are ready, and irq is a resource :) It's expected that
>>> all resources will be requested in probe, but here you are trying to
>>> get resource outside of probe. As result, it will be perfectly
>>> possible to get -EPROBE_DEFER here if on some HW GPIO IRQ will be
>>> used as peripheral, or host or otg irq (for example), because GPIO
>>> IRQ controller might not be ready at the moment when IRQ resource is
>>> requested.
>>
>> I agree with you.
>>
>> Felipe, are you ok with moving the IRQ resource obtaining code to probe?
> 
> You mean that probe() would setup all gadget_irq, otg_irq and host_irq
> while the other pieces (otg.c, gadget.c and host.c) only use it?
> 
> yeah, that should be fine. No problems.
> 
OK great. I'll fix this up then. Thanks.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1419107

On 07/06/16 14:49, Grygorii Strashko wrote:
> On 06/07/2016 12:34 PM, Roger Quadros wrote:
>> On 02/06/16 14:52, Grygorii Strashko wrote:
>>> On 06/01/2016 10:46 AM, Roger Quadros wrote:
>>>> Implementations might use different IRQs for
>>>> host, gadget and OTG so use named interrupt resources
>>>> to allow Device tree to specify the 3 interrupts.
>>>>
>>>> Following are the interrupt names
>>>>
>>>> Peripheral Interrupt - peripheral
>>>> HOST Interrupt - host
>>>> OTG Interrupt - otg
>>>
>>> or "dwc_usb3"??
>>
>> That is for backward compatibility only. I could explicitly
>> mention it in the next line.
> 
> yes pls, this confuses.
>  Also I don't see how "otg" irq name is used in code.
> 
>>
>>>
>>>>
>>>> We still maintain backward compatibility for a single named
>>>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
>>>> single unnamed interrupt for all 3 interrupts (e.g. old DT).
>>>
>>> bindings
>>
>> OK.
>>>
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>> v9: rebased on top of balbi/testing/next
>>>>
>>>>    drivers/usb/dwc3/core.c   | 10 ----------
>>>>    drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
>>>>    drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
>>>>    3 files changed, 37 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 9c4e1d8d..5cedf3d 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
>>>>        dwc->mem = mem;
>>>>        dwc->dev = dev;
>>>>
>>>> -    res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>>> -    if (!res) {
>>>> -        dev_err(dev, "missing IRQ\n");
>>>> -        return -ENODEV;
>>>> -    }
>>>> -    dwc->xhci_resources[1].start = res->start;
>>>> -    dwc->xhci_resources[1].end = res->end;
>>>> -    dwc->xhci_resources[1].flags = res->flags;
>>>> -    dwc->xhci_resources[1].name = res->name;
>>>> -
>>>>        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>        if (!res) {
>>>>            dev_err(dev, "missing memory resource\n");
>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index c37168d..c18c72f 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>>        int            ret = 0;
>>>>        int            irq;
>>>>
>>>> -    irq = platform_get_irq(to_platform_device(dwc->dev), 0);
>>>> +    irq = dwc->irq_gadget;
>>>>        ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
>>>>                IRQF_SHARED, "dwc3", dwc->ev_buf);
>>>>        if (ret) {
>>>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>>                    irq, ret);
>>>>            goto err0;
>>>>        }
>>>> -    dwc->irq_gadget = irq;
>>>>
>>>>        spin_lock_irqsave(&dwc->lock, flags);
>>>>        if (dwc->gadget_driver) {
>>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>>>    int dwc3_gadget_init(struct dwc3 *dwc)
>>>>    {
>>>>        int                    ret;
>>>> +    struct resource *res;
>>>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>>>> +
>>>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>>>> +    if (dwc->irq_gadget <= 0) {
>>>
>>> Is it expected to get -EPROBE_DEFER here?
>>
>> Probably not as we don't have any chance of deferring probe here. We've already
>> probed successfully and are just turning on the gadget mode here.
> 

I was mistaken here.

dwc3_gadget_init() and dwc3_host_init() get called during dwc3_core_init_mode() which is
in fact called during probe().

So I'll add take care -EPROBE_DEFER in the next revision.

> In general, you can't say that you've been probed successfully if not all resources are ready,
> and irq is a resource :)
> It's expected that all resources will be requested in probe, but here you are trying to get
> resource outside of probe. As result, it will be perfectly possible to get -EPROBE_DEFER here
> if on some HW GPIO IRQ will be used as peripheral, or host or otg irq (for example), because 
> GPIO IRQ controller might not be ready at the moment when IRQ resource is requested.
> 

--
cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1419110

Grygorii,

On 02/06/16 14:52, Grygorii Strashko wrote:
> On 06/01/2016 10:46 AM, Roger Quadros wrote:
>> Implementations might use different IRQs for
>> host, gadget and OTG so use named interrupt resources
>> to allow Device tree to specify the 3 interrupts.
>>
>> Following are the interrupt names
>>
>> Peripheral Interrupt - peripheral
>> HOST Interrupt - host
>> OTG Interrupt - otg
> 
> or "dwc_usb3"??
> 
>>
>> We still maintain backward compatibility for a single named
>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
>> single unnamed interrupt for all 3 interrupts (e.g. old DT).
> 
> bindings
> 
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> v9: rebased on top of balbi/testing/next
>>
>>   drivers/usb/dwc3/core.c   | 10 ----------
>>   drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
>>   drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
>>   3 files changed, 37 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 9c4e1d8d..5cedf3d 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
>>       dwc->mem = mem;
>>       dwc->dev = dev;
>>
>> -    res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> -    if (!res) {
>> -        dev_err(dev, "missing IRQ\n");
>> -        return -ENODEV;
>> -    }
>> -    dwc->xhci_resources[1].start = res->start;
>> -    dwc->xhci_resources[1].end = res->end;
>> -    dwc->xhci_resources[1].flags = res->flags;
>> -    dwc->xhci_resources[1].name = res->name;
>> -
>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       if (!res) {
>>           dev_err(dev, "missing memory resource\n");
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index c37168d..c18c72f 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>       int            ret = 0;
>>       int            irq;
>>
>> -    irq = platform_get_irq(to_platform_device(dwc->dev), 0);
>> +    irq = dwc->irq_gadget;
>>       ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
>>               IRQF_SHARED, "dwc3", dwc->ev_buf);
>>       if (ret) {
>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>                   irq, ret);
>>           goto err0;
>>       }
>> -    dwc->irq_gadget = irq;
>>
>>       spin_lock_irqsave(&dwc->lock, flags);
>>       if (dwc->gadget_driver) {
>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>   int dwc3_gadget_init(struct dwc3 *dwc)
>>   {
>>       int                    ret;
>> +    struct resource *res;
>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>> +
>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>> +    if (dwc->irq_gadget <= 0) {
> 
> Is it expected to get -EPROBE_DEFER here?
> 
>> +        dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev,
>> +                              "dwc_usb3");
>> +        if (dwc->irq_gadget <= 0) {
>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>> +                            0);
> 
> It's better to use platform_get_irq().
> 
>> +            if (!res) {
>> +                dev_err(dwc->dev, "missing peripheral IRQ\n");
>> +                return -ENODEV;
>> +            }
>> +            dwc->irq_gadget = res->start;
>> +        }
>> +    }
>>
>>       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..f2b60a4 100644
>> --- a/drivers/usb/dwc3/host.c
>> +++ b/drivers/usb/dwc3/host.c
>> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc)
>>       struct platform_device    *xhci;
>>       struct usb_xhci_pdata    pdata;
>>       int            ret;
>> +    struct resource        *res;
>> +    struct platform_device    *dwc3_pdev = to_platform_device(dwc->dev);
>> +
>> +    res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host");
>> +    if (!res) {
>> +        res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ,
>> +                           "dwc_usb3");
>> +        if (!res) {
>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>> +                            0);
>> +            if (!res)
> 
>> +                return -ENOMEM;
>> +        }
>> +    }
> 
> Is it expected to have more than one IRQ here?
> 
> if not - it will better to use platform_get_irq[_byname]().
> 

The reason I used platform_get_resource variant is that i'm passing the
resource directly to the XHCI platform device below.
> 
>> +
>> +    dwc->xhci_resources[1].start = res->start;
>> +    dwc->xhci_resources[1].end = res->end;
>> +    dwc->xhci_resources[1].flags = res->flags;
>> +    dwc->xhci_resources[1].name = res->name;

This could just change to

	dwc->xhci_resource[1] = *res;

>>
>>       xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO);
>>       if (!xhci) {
>>
> 
> 

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1419112

On 10/06/16 11:02, Roger Quadros wrote:
> Grygorii,
> 
> On 02/06/16 14:52, Grygorii Strashko wrote:
>> On 06/01/2016 10:46 AM, Roger Quadros wrote:
>>> Implementations might use different IRQs for
>>> host, gadget and OTG so use named interrupt resources
>>> to allow Device tree to specify the 3 interrupts.
>>>
>>> Following are the interrupt names
>>>
>>> Peripheral Interrupt - peripheral
>>> HOST Interrupt - host
>>> OTG Interrupt - otg
>>
>> or "dwc_usb3"??
>>
>>>
>>> We still maintain backward compatibility for a single named
>>> interrupt for all 3 interrupts (e.g. for dwc3-pci) and
>>> single unnamed interrupt for all 3 interrupts (e.g. old DT).
>>
>> bindings
>>
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>> v9: rebased on top of balbi/testing/next
>>>
>>>   drivers/usb/dwc3/core.c   | 10 ----------
>>>   drivers/usb/dwc3/gadget.c | 20 ++++++++++++++++++--
>>>   drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
>>>   3 files changed, 37 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 9c4e1d8d..5cedf3d 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -843,16 +843,6 @@ static int dwc3_probe(struct platform_device *pdev)
>>>       dwc->mem = mem;
>>>       dwc->dev = dev;
>>>
>>> -    res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>> -    if (!res) {
>>> -        dev_err(dev, "missing IRQ\n");
>>> -        return -ENODEV;
>>> -    }
>>> -    dwc->xhci_resources[1].start = res->start;
>>> -    dwc->xhci_resources[1].end = res->end;
>>> -    dwc->xhci_resources[1].flags = res->flags;
>>> -    dwc->xhci_resources[1].name = res->name;
>>> -
>>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>       if (!res) {
>>>           dev_err(dev, "missing memory resource\n");
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index c37168d..c18c72f 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1726,7 +1726,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>       int            ret = 0;
>>>       int            irq;
>>>
>>> -    irq = platform_get_irq(to_platform_device(dwc->dev), 0);
>>> +    irq = dwc->irq_gadget;
>>>       ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
>>>               IRQF_SHARED, "dwc3", dwc->ev_buf);
>>>       if (ret) {
>>> @@ -1734,7 +1734,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>>                   irq, ret);
>>>           goto err0;
>>>       }
>>> -    dwc->irq_gadget = irq;
>>>
>>>       spin_lock_irqsave(&dwc->lock, flags);
>>>       if (dwc->gadget_driver) {
>>> @@ -2853,6 +2852,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_evt)
>>>   int dwc3_gadget_init(struct dwc3 *dwc)
>>>   {
>>>       int                    ret;
>>> +    struct resource *res;
>>> +    struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
>>> +
>>> +    dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev, "peripheral");
>>> +    if (dwc->irq_gadget <= 0) {
>>
>> Is it expected to get -EPROBE_DEFER here?
>>
>>> +        dwc->irq_gadget = platform_get_irq_byname(dwc3_pdev,
>>> +                              "dwc_usb3");
>>> +        if (dwc->irq_gadget <= 0) {
>>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>>> +                            0);
>>
>> It's better to use platform_get_irq().
>>
>>> +            if (!res) {
>>> +                dev_err(dwc->dev, "missing peripheral IRQ\n");
>>> +                return -ENODEV;
>>> +            }
>>> +            dwc->irq_gadget = res->start;
>>> +        }
>>> +    }
>>>
>>>       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..f2b60a4 100644
>>> --- a/drivers/usb/dwc3/host.c
>>> +++ b/drivers/usb/dwc3/host.c
>>> @@ -25,6 +25,25 @@ int dwc3_host_init(struct dwc3 *dwc)
>>>       struct platform_device    *xhci;
>>>       struct usb_xhci_pdata    pdata;
>>>       int            ret;
>>> +    struct resource        *res;
>>> +    struct platform_device    *dwc3_pdev = to_platform_device(dwc->dev);
>>> +
>>> +    res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ, "host");
>>> +    if (!res) {
>>> +        res = platform_get_resource_byname(dwc3_pdev, IORESOURCE_IRQ,
>>> +                           "dwc_usb3");
>>> +        if (!res) {
>>> +            res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
>>> +                            0);
>>> +            if (!res)
>>
>>> +                return -ENOMEM;
>>> +        }
>>> +    }
>>
>> Is it expected to have more than one IRQ here?
>>
>> if not - it will better to use platform_get_irq[_byname]().
>>
> 
> The reason I used platform_get_resource variant is that i'm passing the
> resource directly to the XHCI platform device below.
>>
>>> +
>>> +    dwc->xhci_resources[1].start = res->start;
>>> +    dwc->xhci_resources[1].end = res->end;
>>> +    dwc->xhci_resources[1].flags = res->flags;
>>> +    dwc->xhci_resources[1].name = res->name;
> 
> This could just change to
> 
> 	dwc->xhci_resource[1] = *res;

Probably not as we don't want to change parent/child members.

> 
>>>
>>>       xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO);
>>>       if (!xhci) {
>>>
>>
>>
> 

--
cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1419116

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
>> 	dwc->xhci_resource[1] = *res;
>
> Probably not as we don't want to change parent/child members.

oh, you had already replied. Sorry. This is correct

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1419126

[Multipart message — attachments visible in raw view] — view raw

On 10/06/16 11:18, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>> 	dwc->xhci_resource[1] = *res;
>>
>> Probably not as we don't want to change parent/child members.
> 
> oh, you had already replied. Sorry. This is correct
> 
np :).

So what i'll do is get the irq via platform_get_irq() and friends
and if it was a success use platform_get_resource() and friends
to get struct resource and just edit the relevant parts for the
XHCI irq resource.

Sounds OK?

something like this.

+	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;
+			} 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;
 


--
cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1419140

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
> On 10/06/16 11:18, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Roger Quadros <rogerq@ti.com> writes:
>>>> 	dwc->xhci_resource[1] = *res;
>>>
>>> Probably not as we don't want to change parent/child members.
>> 
>> oh, you had already replied. Sorry. This is correct
>> 
> np :).
>
> So what i'll do is get the irq via platform_get_irq() and friends
> and if it was a success use platform_get_resource() and friends
> to get struct resource and just edit the relevant parts for the
> XHCI irq resource.
>
> Sounds OK?
>
> something like this.
>
> +	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;
> +			} 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;

looks okay to me.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1419115

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
>> Is it expected to have more than one IRQ here?
>> 
>> if not - it will better to use platform_get_irq[_byname]().
>> 
>
> The reason I used platform_get_resource variant is that i'm passing the
> resource directly to the XHCI platform device below.
>> 
>>> +
>>> +    dwc->xhci_resources[1].start = res->start;
>>> +    dwc->xhci_resources[1].end = res->end;
>>> +    dwc->xhci_resources[1].flags = res->flags;
>>> +    dwc->xhci_resources[1].name = res->name;
>
> This could just change to
>
> 	dwc->xhci_resource[1] = *res;

no, it cannot. Look at the definition of struct resource and how it's
used, then you'll see we don't want to copy everything.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1419178 — [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources

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
the -EPROBE_DEFER case as well.

Signed-off-by: Roger Quadros <rogerq@ti.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
@@ -766,7 +766,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 		ret = dwc3_gadget_init(dwc);
 		if (ret) {
-			dev_err(dev, "failed to initialize gadget\n");
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "failed to initialize gadget\n");
 			return ret;
 		}
 		break;
@@ -774,7 +775,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
 		ret = dwc3_host_init(dwc);
 		if (ret) {
-			dev_err(dev, "failed to initialize host\n");
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "failed to initialize host\n");
 			return ret;
 		}
 		break;
@@ -782,13 +784,15 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
 		ret = dwc3_host_init(dwc);
 		if (ret) {
-			dev_err(dev, "failed to initialize host\n");
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "failed to initialize host\n");
 			return ret;
 		}
 
 		ret = dwc3_gadget_init(dwc);
 		if (ret) {
-			dev_err(dev, "failed to initialize gadget\n");
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "failed to initialize gadget\n");
 			return ret;
 		}
 		break;
@@ -843,16 +847,6 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc->mem = mem;
 	dwc->dev = dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(dev, "missing IRQ\n");
-		return -ENODEV;
-	}
-	dwc->xhci_resources[1].start = res->start;
-	dwc->xhci_resources[1].end = res->end;
-	dwc->xhci_resources[1].flags = res->flags;
-	dwc->xhci_resources[1].name = res->name;
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(dev, "missing memory resource\n");
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
@@ -1738,7 +1738,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
 	int			ret = 0;
 	int			irq;
 
-	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
+	irq = dwc->irq_gadget;
 	ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
 			IRQF_SHARED, "dwc3", dwc->ev_buf);
 	if (ret) {
@@ -1746,7 +1746,6 @@ static int dwc3_gadget_start(struct usb_gadget *g,
 				irq, ret);
 		goto err0;
 	}
-	dwc->irq_gadget = irq;
 
 	spin_lock_irqsave(&dwc->lock, flags);
 	if (dwc->gadget_driver) {
@@ -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;
+			}
+		}
+	}
+
+	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;
+			} 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) {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1419251 — Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources

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

> the -EPROBE_DEFER case as well.
>
> Signed-off-by: Roger Quadros <rogerq@ti.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?

> +			}
> +		}
> +	}
> +
> +	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) {

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1419294 — 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@ti.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

[toc] | [prev] | [next] | [standalone]

#1419304 — Re: [PATCH v10 5/5] usb: dwc3: core: cleanup IRQ resources

On 10/06/16 14:44, Sergei Shtylyov wrote:
> On 6/10/2016 2:35 PM, Roger Quadros wrote:
>> 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@ti.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?
> 
>    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.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web