dwc3: omap: fixes and dual-role preparation

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 <[email protected]>

Hi,

Quoted 10 lines · Roger Quadros

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

Quoted 5 lines

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 <[email protected]>

fails to apply, unfortunately:

checking file drivers/usb/dwc3/core.c
Hunk #1 succeeded at 845 (offset 31 lines). checking file drivers/usb/dwc3/core.h
Hunk #1 succeeded at 738 (offset 22 lines).
Hunk #2 FAILED at 818.
1 out of 2 hunks FAILED
checking file drivers/usb/dwc3/gadget.c
Hunk #1 succeeded at 1710 with fuzz 2 (offset 105 lines). Hunk #2 FAILED at 1728.
Hunk #3 FAILED at 1740.
Hunk #4 succeeded at 2835 (offset 54 lines).
2 out of 4 hunks FAILED
checking file drivers/usb/dwc3/host.c

I'll update my testing/next shortly, if you could rebase the remaining of these patches on that, I'd be glad.

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 <[email protected]>

---
v9: rebased on top of balbi/testing/next

Hi,

Quoted 10 lines · Roger Quadros

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

Quoted 7 lines

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 <[email protected]>

---
v9: rebased on top of balbi/testing/next

breaks dwc3:

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

please test

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 <[email protected]>

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

drivers/usb/dwc3/core.c +0 -10

index 9c4e1d8d..5cedf3d 100644
@@ -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");

drivers/usb/dwc3/gadget.c +1 -2

index c37168d..c18c72f 100644
@@ -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) {

drivers/usb/dwc3/gadget.c +7 -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);

drivers/usb/dwc3/host.c +5 -0

index c679f63..f2b60a4 100644
@@ -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) {

drivers/usb/dwc3/host.c +5 -0

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

Quoted 4 lines · Roger Quadros

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

Quoted 5 lines

Following are the interrupt names

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

or "dwc_usb3"??

Quoted 5 lines

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

Quoted 51 lines

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -10

index 9c4e1d8d..5cedf3d 100644
@@ -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");

drivers/usb/dwc3/gadget.c +1 -2

index c37168d..c18c72f 100644
@@ -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;
 

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 11 lines

drivers/usb/dwc3/gadget.c +5 -0

      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?

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2864

Quoted 6 lines

drivers/usb/dwc3/gadget.c +5 -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);

It's better to use platform_get_irq().

Quoted 30 lines

drivers/usb/dwc3/gadget.c +7 -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);

drivers/usb/dwc3/host.c +11 -0

index c679f63..f2b60a4 100644
@@ -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)

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 41

Quoted 3 lines

drivers/usb/dwc3/host.c +3 -0

+                return -ENOMEM;
+        }
+    }

Is it expected to have more than one IRQ here?

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

Quoted 8 lines

drivers/usb/dwc3/host.c +5 -0

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

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 49

Quoted 3 lines

drivers/usb/dwc3/host.c +0 -0

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

Quoted 7 lines · Grygorii Strashko

Quoted 6 lines · Roger Quadros

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

Quoted 5 lines

Quoted 3 lines

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.

Quoted 8 lines

Quoted 6 lines

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.

Quoted 51 lines

Quoted 51 lines

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -10

index 9c4e1d8d..5cedf3d 100644
@@ -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");

drivers/usb/dwc3/gadget.c +1 -2

index c37168d..c18c72f 100644
@@ -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;
 

Quoted 13 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 11 lines

drivers/usb/dwc3/gadget.c +5 -0

       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.

Quoted 9 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2864

Quoted 7 lines

drivers/usb/dwc3/gadget.c +5 -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);

It's better to use platform_get_irq().

OK.

Quoted 31 lines

Quoted 30 lines

drivers/usb/dwc3/gadget.c +7 -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);

drivers/usb/dwc3/host.c +11 -0

index c679f63..f2b60a4 100644
@@ -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)

Quoted 5 lines

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 41

Quoted 3 lines

drivers/usb/dwc3/host.c +3 -0

+                return -ENOMEM;
+        }
+    }

Is it expected to have more than one IRQ here?

No.

Quoted 2 lines

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

OK.

Quoted 8 lines

Quoted 8 lines

drivers/usb/dwc3/host.c +5 -0

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

Quoted 5 lines

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 49

Quoted 3 lines

drivers/usb/dwc3/host.c +0 -0

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

cheers,
-roger

Quoted 12 lines · Roger Quadros

Quoted 11 lines · Grygorii Strashko

Quoted 9 lines · Roger Quadros

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

Quoted 4 lines

Quoted 1 line

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.

Quoted 75 lines

Quoted 8 lines

Quoted 6 lines

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.

Quoted 51 lines

Quoted 51 lines

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -10

index 9c4e1d8d..5cedf3d 100644
@@ -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");

drivers/usb/dwc3/gadget.c +1 -2

index c37168d..c18c72f 100644
@@ -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;
 

Quoted 13 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 11 lines

drivers/usb/dwc3/gadget.c +5 -0

       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?

Quoted 2 lines

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.

Quoted 15 lines

Quoted 12 lines

Quoted 11 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 9 lines

drivers/usb/dwc3/gadget.c +5 -0

@@ -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?

Quoted 2 lines

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.

Quoted 8 lines

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?

Hi,

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

Quoted 24 lines · Roger Quadros

Quoted 23 lines

Quoted 12 lines

Quoted 11 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 9 lines

drivers/usb/dwc3/gadget.c +5 -0

@@ -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?

Quoted 2 lines

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.

Quoted 3 lines

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.

Quoted 12 lines · Roger Quadros

Quoted 11 lines · Grygorii Strashko

Quoted 9 lines · Roger Quadros

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

Quoted 4 lines

Quoted 1 line

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.

Quoted 77 lines

Quoted 8 lines

Quoted 6 lines

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.

Quoted 51 lines

Quoted 51 lines

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -10

index 9c4e1d8d..5cedf3d 100644
@@ -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");

drivers/usb/dwc3/gadget.c +1 -2

index c37168d..c18c72f 100644
@@ -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;
 

Quoted 13 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 11 lines

drivers/usb/dwc3/gadget.c +5 -0

       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.

Quoted 9 lines · Roger Quadros

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"??

Quoted 4 lines

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

Quoted 53 lines

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -10

index 9c4e1d8d..5cedf3d 100644
@@ -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");

drivers/usb/dwc3/gadget.c +1 -2

index c37168d..c18c72f 100644
@@ -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) {

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 8 lines

drivers/usb/dwc3/gadget.c +5 -0

@@ -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?

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2864

Quoted 5 lines

drivers/usb/dwc3/gadget.c +5 -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);

It's better to use platform_get_irq().

Quoted 29 lines

drivers/usb/dwc3/gadget.c +7 -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);

drivers/usb/dwc3/host.c +11 -0

index c679f63..f2b60a4 100644
@@ -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)

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 41

Quoted 3 lines

drivers/usb/dwc3/host.c +3 -0

+                return -ENOMEM;
+        }
+    }

Is it expected to have more than one IRQ here?

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

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 46

Quoted 6 lines

drivers/usb/dwc3/host.c +5 -0

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

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc)

Quoted 4 lines

drivers/usb/dwc3/host.c +0 -0

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

Grygorii,

Quoted 122 lines · Grygorii Strashko

Quoted 9 lines · Roger Quadros

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"??

Quoted 4 lines

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

Quoted 53 lines

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -10

index 9c4e1d8d..5cedf3d 100644
@@ -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");

drivers/usb/dwc3/gadget.c +1 -2

index c37168d..c18c72f 100644
@@ -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) {

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2859

Quoted 8 lines

drivers/usb/dwc3/gadget.c +5 -0

@@ -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?

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2864

Quoted 5 lines

drivers/usb/dwc3/gadget.c +5 -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);

It's better to use platform_get_irq().

Quoted 29 lines

drivers/usb/dwc3/gadget.c +7 -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);

drivers/usb/dwc3/host.c +11 -0

index c679f63..f2b60a4 100644
@@ -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)

Quoted 8 lines

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 41

Quoted 3 lines

drivers/usb/dwc3/host.c +3 -0

+                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.

Quoted 6 lines

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 46

Quoted 6 lines

drivers/usb/dwc3/host.c +5 -0

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

Quoted 7 lines

Quoted 5 lines

drivers/usb/dwc3/host.c +0 -0

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

Quoted 1 line

	dwc->xhci_resource[1] = *res;

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

Hi,

Quoted 3 lines · Roger Quadros

Quoted 1 line

	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

Quoted 10 lines · Felipe Balbi

Hi,

Quoted 3 lines · Roger Quadros

Quoted 1 line

	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.

(quoted diff) +30 -0

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

Quoted 4 lines

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.

Quoted 6 lines

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 46

Quoted 6 lines

drivers/usb/dwc3/host.c +5 -0

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

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 <[email protected]>

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

drivers/usb/dwc3/core.c +0 -0

index 8fceeb1..131e7eb 100644

drivers/usb/dwc3/gadget.c +0 -0

index 0f6fb8e..774a0d8 100644

drivers/usb/dwc3/gadget.c +10 -1

@@ -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)

drivers/usb/dwc3/gadget.c +5 -0

+			}
+		}
+	}
+
+	dwc->irq_gadget = irq;
 
 	dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
 			&dwc->ctrl_req_addr, GFP_KERNEL);

drivers/usb/dwc3/host.c +11 -1

index c679f63..eb5e8f9 100644
@@ -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)

drivers/usb/dwc3/host.c +12 -0

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

Hello.

Quoted 9 lines · Roger Quadros

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

Quoted 5 lines

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

Quoted 18 lines

the -EPROBE_DEFER case as well.

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -0

index 8fceeb1..131e7eb 100644

[...]

Quoted 4 lines

drivers/usb/dwc3/gadget.c +0 -0

index 0f6fb8e..774a0d8 100644

[...]

Quoted 19 lines

drivers/usb/dwc3/gadget.c +14 -1

@@ -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);

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2887

Quoted 6 lines

drivers/usb/dwc3/gadget.c +6 -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?

Quoted 29 lines

drivers/usb/dwc3/gadget.c +5 -0

+            }
+        }
+    }
+
+    dwc->irq_gadget = irq;
 
     dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
             &dwc->ctrl_req_addr, GFP_KERNEL);

drivers/usb/dwc3/host.c +11 -1

index c679f63..eb5e8f9 100644
@@ -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)

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 47

Quoted 10 lines

drivers/usb/dwc3/host.c +10 -0

+            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?

Quoted 22 lines

drivers/usb/dwc3/host.c +19 -0

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

Quoted 13 lines · Sergei Shtylyov

Hello.

Quoted 9 lines · Roger Quadros

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

Quoted 7 lines

Quoted 5 lines

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.

Quoted 43 lines

Quoted 18 lines

the -EPROBE_DEFER case as well.

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -0

index 8fceeb1..131e7eb 100644

[...]

Quoted 4 lines

drivers/usb/dwc3/gadget.c +0 -0

index 0f6fb8e..774a0d8 100644

[...]

Quoted 19 lines

drivers/usb/dwc3/gadget.c +14 -1

@@ -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);

Quoted 8 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2887

Quoted 6 lines

drivers/usb/dwc3/gadget.c +6 -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?

Quoted 41 lines

Quoted 29 lines

drivers/usb/dwc3/gadget.c +5 -0

+            }
+        }
+    }
+
+    dwc->irq_gadget = irq;
 
     dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
             &dwc->ctrl_req_addr, GFP_KERNEL);

drivers/usb/dwc3/host.c +11 -1

index c679f63..eb5e8f9 100644
@@ -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)

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 47

Quoted 10 lines

drivers/usb/dwc3/host.c +10 -0

+            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?

--
cheers,
-roger

Quoted 75 lines · Roger Quadros

Quoted 13 lines · Sergei Shtylyov

Hello.

Quoted 9 lines · Roger Quadros

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

Quoted 7 lines

Quoted 5 lines

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.

Quoted 43 lines

Quoted 18 lines

the -EPROBE_DEFER case as well.

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -0

index 8fceeb1..131e7eb 100644

[...]

Quoted 4 lines

drivers/usb/dwc3/gadget.c +0 -0

index 0f6fb8e..774a0d8 100644

[...]

Quoted 19 lines

drivers/usb/dwc3/gadget.c +14 -1

@@ -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);

Quoted 8 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2887

Quoted 6 lines

drivers/usb/dwc3/gadget.c +6 -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?

Quoted 43 lines

good catch. It wasn't intended. I guess i'll return -EINVAL then?

Quoted 41 lines

Quoted 29 lines

drivers/usb/dwc3/gadget.c +5 -0

+            }
+        }
+    }
+
+    dwc->irq_gadget = irq;
 
     dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
             &dwc->ctrl_req_addr, GFP_KERNEL);

drivers/usb/dwc3/host.c +11 -1

index c679f63..eb5e8f9 100644
@@ -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)

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 47

Quoted 10 lines

drivers/usb/dwc3/host.c +10 -0

+            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...

Quoted 71 lines

Quoted 71 lines

Quoted 9 lines

Quoted 9 lines

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

Quoted 7 lines

Quoted 5 lines

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.

Quoted 43 lines

Quoted 18 lines

the -EPROBE_DEFER case as well.

Signed-off-by Roger Quadros <[email protected]>

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

drivers/usb/dwc3/core.c +0 -0

index 8fceeb1..131e7eb 100644

[...]

Quoted 4 lines

drivers/usb/dwc3/gadget.c +0 -0

index 0f6fb8e..774a0d8 100644

[...]

Quoted 19 lines

drivers/usb/dwc3/gadget.c +14 -1

@@ -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);

Quoted 8 lines

drivers/usb/dwc3/gadget.c static irqreturn_t dwc3_interrupt(int irq, void *_evt) · line 2887

Quoted 6 lines

drivers/usb/dwc3/gadget.c +6 -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?

Quoted 1 line

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