[PATCH v8 0/5] dwc3: omap: fixes and dual-role preparation

[PATCH v8 0/5] dwc3: omap: fixes and dual-role preparation

[Roger Quadros]

Hi Felipe,

I've removed the patches that add dual role support. These are just
some fixes can go in before any actual dual role support.

About the OTG software mailbox, I tried checking internally in TI but
nobody was aware of any constraints. The events can be set as they are
obtained from the PMIC OTG comparators. My gut feeling is that we didn't have
VBUS and ID events working properly back then and so resulting
in a hackish software OTG mailbox implementation.

I tested this on omap5-uevm, dra7-evm and am572x-evm and didn't
observe any regressions.

Based on balbi/next commit 2a58f9c12bb360f38fb39e470bb5ff94014356e6.

v8:
- implement proper top and bottom half irq handler for dwc3-omap.
- remove otg_irq code, this can come later with dual-role support.
- use dwc3->gadget_irq while free_irq() in dwc3_gadget_stop().

v7:
- remove patches adding dual-role support.
- split out shared irq conversion from threaded irq conversion patch.
- added a new patch about not touching POWERPRESENT bit.

v6:
- use just otg irq to get otg events and don't depend on extcon at all.
- follow OTG flow in TRM strictly.
- use tracepoints instead of dev_dbg().
- match IRQ flags in dwc3_omap and core.c for shared otg interrupt.

v5: Internal revision. Not sent to mailing list.

v4: first version that was reviewed.

cheers,
-roger

Roger Quadros (5):
  usb: dwc3: omap: use request_threaded_irq()
  usb: dwc3: omap: Mark the interrupt handler as shared
  usb: dwc3: omap: Don't set POWERPRESENT
  usb: dwc3: omap: Pass VBUS and ID events transparently
  usb: dwc3: core: cleanup IRQ resources

 drivers/usb/dwc3/core.c      | 10 ---------
 drivers/usb/dwc3/core.h      |  3 +++
 drivers/usb/dwc3/dwc3-omap.c | 53 ++++++++++++++++++++++++++++----------------
 drivers/usb/dwc3/gadget.c    | 23 +++++++++++++++----
 drivers/usb/dwc3/host.c      | 19 ++++++++++++++++
 5 files changed, 75 insertions(+), 33 deletions(-)

-- 
2.7.4

[PATCH v8 1/5] usb: dwc3: omap: use request_threaded_irq()

[Roger Quadros]

We intend to share this interrupt with the OTG driver an to ensure
that irqflags match for the shared interrupt handlers we use
request_threaded_irq()

If we don't use request_treaded_irq() then forced threaded irq will
set IRQF_ONESHOT and this won't match with the OTG IRQ handler's
IRQ flags.

NOTE: OTG IRQ handler is yet to be added. This is a preparatory step.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 59ea35f..7d49da1 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -165,7 +165,7 @@ static void dwc3_omap_write_utmi_ctrl(struct dwc3_omap *omap, u32 value)
 
 static u32 dwc3_omap_read_irq0_status(struct dwc3_omap *omap)
 {
-	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_0 -
+	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_RAW_0 -
 						omap->irq0_offset);
 }
 
@@ -178,7 +178,7 @@ static void dwc3_omap_write_irq0_status(struct dwc3_omap *omap, u32 value)
 
 static u32 dwc3_omap_read_irqmisc_status(struct dwc3_omap *omap)
 {
-	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_MISC +
+	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_RAW_MISC +
 						omap->irqmisc_offset);
 }
 
@@ -268,19 +268,38 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
 	}
 }
 
+static void dwc3_omap_enable_irqs(struct dwc3_omap *omap);
+static void dwc3_omap_disable_irqs(struct dwc3_omap *omap);
+
 static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
 {
 	struct dwc3_omap	*omap = _omap;
+
+	if (dwc3_omap_read_irqmisc_status(omap) ||
+	    dwc3_omap_read_irq0_status(omap)) {
+		/* mask irqs */
+		dwc3_omap_disable_irqs(omap);
+		return IRQ_WAKE_THREAD;
+	}
+
+	return IRQ_NONE;
+}
+
+static irqreturn_t dwc3_omap_interrupt_thread(int irq, void *_omap)
+{
+	struct dwc3_omap	*omap = _omap;
 	u32			reg;
 
+	/* clear irq status flags */
 	reg = dwc3_omap_read_irqmisc_status(omap);
-
 	dwc3_omap_write_irqmisc_status(omap, reg);
 
 	reg = dwc3_omap_read_irq0_status(omap);
-
 	dwc3_omap_write_irq0_status(omap, reg);
 
+	/* unmask irqs */
+	dwc3_omap_enable_irqs(omap);
+
 	return IRQ_HANDLED;
 }
 
@@ -497,8 +516,9 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	/* check the DMA Status */
 	reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
 
-	ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0,
-			"dwc3-omap", omap);
+	ret = devm_request_threaded_irq(dev, omap->irq, dwc3_omap_interrupt,
+					dwc3_omap_interrupt_thread, 0,
+					"dwc3-omap", omap);
 	if (ret) {
 		dev_err(dev, "failed to request IRQ #%d --> %d\n",
 				omap->irq, ret);
-- 
2.7.4

[PATCH v8 2/5] usb: dwc3: omap: Mark the interrupt handler as shared

[Roger Quadros]

On OMAPs, OTG events come on the same IRQ so we need to share
this IRQ with the OTG device driver.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 7d49da1..b58546c 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -517,7 +517,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
 
 	ret = devm_request_threaded_irq(dev, omap->irq, dwc3_omap_interrupt,
-					dwc3_omap_interrupt_thread, 0,
+					dwc3_omap_interrupt_thread, IRQF_SHARED,
 					"dwc3-omap", omap);
 	if (ret) {
 		dev_err(dev, "failed to request IRQ #%d --> %d\n",
-- 
2.7.4

[PATCH v8 3/5] usb: dwc3: omap: Don't set POWERPRESENT

[Roger Quadros]

TRM [1] recommends that POWERPRESENT bit must not be
set and left at it's default value of 0.

[1] OMAP542x TRM - http://www.ti.com/lit/pdf/swpu249
Section 23.11.4.5.1 Mailbox VBUS/ID Management

"Because PIPE powerpresent has a different meaning in host and in device mode,
and because of the redundancy with the UTMI signals, the controller ORes
together the appropriate PIPE and UTMI inputs to create its internal
VBUS status. For that reason, it is recommended to leave field
USBOTGSS_UTMI_OTG_STATUS[9] POWERPRESENT at its default value (=0), and only to
fill in the USB2 VBUS status fields in the same register."

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index b58546c..dd55e08 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -234,8 +234,7 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
 		val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG
 				| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID
 				| USBOTGSS_UTMI_OTG_CTRL_SESSEND);
-		val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID
-				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
+		val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID;
 		dwc3_omap_write_utmi_ctrl(omap, val);
 		break;
 
@@ -244,8 +243,7 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
 		val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND;
 		val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG
 				| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID
-				| USBOTGSS_UTMI_OTG_CTRL_SESSVALID
-				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
+				| USBOTGSS_UTMI_OTG_CTRL_SESSVALID;
 		dwc3_omap_write_utmi_ctrl(omap, val);
 		break;
 
@@ -256,8 +254,7 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
 	case OMAP_DWC3_VBUS_OFF:
 		val = dwc3_omap_read_utmi_ctrl(omap);
 		val &= ~(USBOTGSS_UTMI_OTG_CTRL_SESSVALID
-				| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID
-				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT);
+				| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID);
 		val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND
 				| USBOTGSS_UTMI_OTG_CTRL_IDDIG;
 		dwc3_omap_write_utmi_ctrl(omap, val);
-- 
2.7.4

[PATCH v8 4/5] usb: dwc3: omap: Pass VBUS and ID events transparently

[Roger Quadros]

Don't make any decisions regarding VBUS session based on ID
status. That is best left to the OTG core.

Pass ID and VBUS events independent of each other so that OTG
core knows exactly what to do.

This makes dual-role with extcon work with OTG irq on OMAP platforms.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index dd55e08..3cebf9b 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -231,18 +231,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
 		}
 
 		val = dwc3_omap_read_utmi_ctrl(omap);
-		val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG
-				| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID
-				| USBOTGSS_UTMI_OTG_CTRL_SESSEND);
-		val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID;
+		val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG;
 		dwc3_omap_write_utmi_ctrl(omap, val);
 		break;
 
 	case OMAP_DWC3_VBUS_VALID:
 		val = dwc3_omap_read_utmi_ctrl(omap);
 		val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND;
-		val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG
-				| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID
+		val |= USBOTGSS_UTMI_OTG_CTRL_VBUSVALID
 				| USBOTGSS_UTMI_OTG_CTRL_SESSVALID;
 		dwc3_omap_write_utmi_ctrl(omap, val);
 		break;
@@ -250,13 +246,15 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
 	case OMAP_DWC3_ID_FLOAT:
 		if (omap->vbus_reg)
 			regulator_disable(omap->vbus_reg);
+		val = dwc3_omap_read_utmi_ctrl(omap);
+		val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG;
+		dwc3_omap_write_utmi_ctrl(omap, val);
 
 	case OMAP_DWC3_VBUS_OFF:
 		val = dwc3_omap_read_utmi_ctrl(omap);
 		val &= ~(USBOTGSS_UTMI_OTG_CTRL_SESSVALID
 				| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID);
-		val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND
-				| USBOTGSS_UTMI_OTG_CTRL_IDDIG;
+		val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND;
 		dwc3_omap_write_utmi_ctrl(omap, val);
 		break;
 
-- 
2.7.4

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

[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

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>
---
 drivers/usb/dwc3/core.c   | 10 ----------
 drivers/usb/dwc3/core.h   |  3 +++
 drivers/usb/dwc3/gadget.c | 23 +++++++++++++++++++----
 drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
 4 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c050a88..4e8b154 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -814,16 +814,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/core.h b/drivers/usb/dwc3/core.h
index 186a886..ab0d615 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -716,6 +716,7 @@ struct dwc3_scratchpad_array {
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
  * @revision: revision register contents
  * @dr_mode: requested mode of operation
+ * @gadget_irq: IRQ number for Peripheral IRQs
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
@@ -817,6 +818,8 @@ struct dwc3 {
 
 	enum usb_dr_mode	dr_mode;
 
+	int			gadget_irq;
+
 	/* used for suspend/resume */
 	u32			dcfg;
 	u32			gctl;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c3b0d01..f493c32 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1605,7 +1605,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
 	int			irq;
 	u32			reg;
 
-	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
+	irq = dwc->gadget_irq;
 	ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
 			IRQF_SHARED, "dwc3", dwc->ev_buf);
 	if (ret) {
@@ -1728,7 +1728,6 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
 {
 	struct dwc3		*dwc = gadget_to_dwc(g);
 	unsigned long		flags;
-	int			irq;
 
 	spin_lock_irqsave(&dwc->lock, flags);
 
@@ -1740,8 +1739,7 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
-	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
-	free_irq(irq, dwc->ev_buf);
+	free_irq(dwc->gadget_irq, dwc->ev_buf);
 
 	return 0;
 }
@@ -2781,6 +2779,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->gadget_irq = platform_get_irq_byname(dwc3_pdev, "peripheral");
+	if (dwc->gadget_irq <= 0) {
+		dwc->gadget_irq = platform_get_irq_byname(dwc3_pdev,
+							  "dwc_usb3");
+		if (dwc->gadget_irq <= 0) {
+			res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ,
+						    0);
+			if (!res) {
+				dev_err(dwc->dev, "missing peripheral IRQ\n");
+				return -ENODEV;
+			}
+			dwc->gadget_irq = 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

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]


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>

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.

Thanks

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

[Roger Quadros]

On 24/05/16 12:35, 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>
> 
> 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.

No Problem Felipe. I'll rebase and re-send this series.

cheers,
-roger

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

[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

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

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

[Grygorii Strashko]

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

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

[Roger Quadros]

[-- Attachment #1.1: Type: text/plain, Size: 922 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Roger Quadros]

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

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

[Grygorii Strashko]

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

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

[Roger Quadros]

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

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

[Roger Quadros]

[-- Attachment #1.1: Type: text/plain, Size: 1711 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Roger Quadros]

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

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

[Roger Quadros]

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

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

[Roger Quadros]

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

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 220 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

[Roger Quadros]

[-- Attachment #1.1: Type: text/plain, Size: 1604 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1767 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

[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

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

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

[Sergei Shtylyov]

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

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

[Roger Quadros]

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

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

[Sergei Shtylyov]

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

> --
> cheers,
> -roger

MBR, Sergei

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

[Roger Quadros]

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

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

[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

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>
---
v11:
- consider irq == 0 as error case and return -EINVAL

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 | 31 ++++++++++++++++++++++++++++---
 drivers/usb/dwc3/host.c   | 43 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 78 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..1ade5e8 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,33 @@ 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");
+				}
+				if (!irq)
+					irq = -EINVAL;
+				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..2e960ed 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -24,7 +24,48 @@ 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");
+				}
+				if (!irq)
+					irq = -EINVAL;
+				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

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

[Sergei Shtylyov]

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

>>>>> Implementations might use different IRQs for
>>>>> host, gadget so use named interrupt resources
>>>>> to allow device tree to specify the interrupts.
>>>>>
>>>>> Following are the interrupt names
>>>>>
>>>>> Peripheral Interrupt - peripheral
>>>>> HOST Interrupt - host
>>>>>
>>>>> Maintain backward compatibility for a single named
>>>>> interrupt ("dwc3_usb3") for all interrupts as well as
>>>>> unnamed interrupt at index 0 for all interrupts.
>>>>>
>>>>> As platform_get_irq_() variants are used, tackle
>>>>
>>>>    platform_get_irq().
>>>
>>> OK.
>>>>
>>>>> the -EPROBE_DEFER case as well.
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@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?

[...]

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

    Returning -EINVAL is fine then.

> cheers,
> -roger

MBR, Sergei