[PATCH v6 00/10] usb: dwc3: add dual-role support

[PATCH v6 00/10] usb: dwc3: add dual-role support

[Roger Quadros]

Hi,

This series adds dual role support to dwc3 controller driver.
Series depends on the OTG/dual-role framework [1].

[1] - https://lkml.org/lkml/2016/4/5/492

Patches are based on v4.6-rc1.

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

Felipe Balbi (2):
  usb: dwc3: omap: fix up error path on probe()
  usb: dwc3: core: fix PHY handling during suspend

Roger Quadros (8):
  usb: dwc3: core.h: add some register definitions
  usb: dwc3: omap: Make the wrapper interrupt shared
  usb: dwc3: omap: Pass VBUS and ID events transparently
  usb: dwc3: core: cleanup IRQ resources
  usb: dwc3: add dual-role support
  usb: dwc3: gadget: Fix suspend/resume during dual-role mode
  ARM: dts: dra7*-evm: Enable dual-role for usb1
  ARM: dts: am43xx: Enable dual-role on USB1

 arch/arm/boot/dts/am437x-gp-evm.dts  |   2 +-
 arch/arm/boot/dts/am437x-sk-evm.dts  |   2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts |   2 +-
 arch/arm/boot/dts/dra7-evm.dts       |   2 +-
 arch/arm/boot/dts/dra72-evm.dts      |   2 +-
 drivers/usb/dwc3/core.c              | 565 +++++++++++++++++++++++++++++++++--
 drivers/usb/dwc3/core.h              | 109 ++++++-
 drivers/usb/dwc3/dwc3-omap.c         |  39 ++-
 drivers/usb/dwc3/gadget.c            |  31 +-
 drivers/usb/dwc3/host.c              |  21 ++
 10 files changed, 729 insertions(+), 46 deletions(-)

-- 
2.5.0

[PATCH v6 01/10] usb: dwc3: core.h: add some register definitions

[Roger Quadros]

Add OTG and GHWPARAMS6 register definitions

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/core.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6254b2f..81039f7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -176,6 +176,15 @@
 #define DWC3_GCTL_GBLHIBERNATIONEN	(1 << 1)
 #define DWC3_GCTL_DSBLCLKGTNG		(1 << 0)
 
+/* Global Status Register */
+#define DWC3_GSTS_OTG_IP	BIT(10)
+#define DWC3_GSTS_BC_IP		BIT(9)
+#define DWC3_GSTS_ADP_IP	BIT(8)
+#define DWC3_GSTS_HOST_IP	BIT(7)
+#define DWC3_GSTS_DEVICE_IP	BIT(6)
+#define DWC3_GSTS_CSR_TIMEOUT	BIT(5)
+#define DWC3_GSTS_BUS_ERR_ADDR_VLD	BIT(4)
+
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST	(1 << 31)
 #define DWC3_GUSB2PHYCFG_SUSPHY		(1 << 6)
@@ -239,7 +248,12 @@
 #define DWC3_MAX_HIBER_SCRATCHBUFS		15
 
 /* Global HWPARAMS6 Register */
-#define DWC3_GHWPARAMS6_EN_FPGA			(1 << 7)
+#define DWC3_GHWPARAMS6_BCSUPPORT		BIT(14)
+#define DWC3_GHWPARAMS6_OTG3SUPPORT		BIT(13)
+#define DWC3_GHWPARAMS6_ADPSUPPORT		BIT(12)
+#define DWC3_GHWPARAMS6_HNPSUPPORT		BIT(11)
+#define DWC3_GHWPARAMS6_SRPSUPPORT		BIT(10)
+#define DWC3_GHWPARAMS6_EN_FPGA			BIT(7)
 
 /* Global Frame Length Adjustment Register */
 #define DWC3_GFLADJ_30MHZ_SDBND_SEL		(1 << 7)
@@ -404,6 +418,74 @@
 #define DWC3_DEPCMD_TYPE_BULK		2
 #define DWC3_DEPCMD_TYPE_INTR		3
 
+/* OTG Configuration Register */
+#define DWC3_OCFG_DISPWRCUTTOFF		BIT(5)
+#define DWC3_OCFG_HIBDISMASK		BIT(4)
+#define DWC3_OCFG_SFTRSTMASK		BIT(3)
+#define DWC3_OCFG_OTGVERSION		BIT(2)
+#define DWC3_OCFG_HNPCAP		BIT(1)
+#define DWC3_OCFG_SRPCAP		BIT(0)
+
+/* OTG CTL Register */
+#define DWC3_OCTL_OTG3GOERR		BIT(7)
+#define DWC3_OCTL_PERIMODE		BIT(6)
+#define DWC3_OCTL_PRTPWRCTL		BIT(5)
+#define DWC3_OCTL_HNPREQ		BIT(4)
+#define DWC3_OCTL_SESREQ		BIT(3)
+#define DWC3_OCTL_TERMSELIDPULSE	BIT(2)
+#define DWC3_OCTL_DEVSETHNPEN		BIT(1)
+#define DWC3_OCTL_HSTSETHNPEN		BIT(0)
+
+/* OTG Event Register */
+#define DWC3_OEVT_DEVICEMODE		BIT(31)
+#define DWC3_OEVT_XHCIRUNSTPSET		BIT(27)
+#define DWC3_OEVT_DEVRUNSTPSET		BIT(26)
+#define DWC3_OEVT_HIBENTRY		BIT(25)
+#define DWC3_OEVT_CONIDSTSCHNG		BIT(24)
+#define DWC3_OEVT_HRRCONFNOTIF		BIT(23)
+#define DWC3_OEVT_HRRINITNOTIF		BIT(22)
+#define DWC3_OEVT_ADEVIDLE		BIT(21)
+#define DWC3_OEVT_ADEVBHOSTEND		BIT(20)
+#define DWC3_OEVT_ADEVHOST		BIT(19)
+#define DWC3_OEVT_ADEVHNPCHNG		BIT(18)
+#define DWC3_OEVT_ADEVSRPDET		BIT(17)
+#define DWC3_OEVT_ADEVSESSENDDET	BIT(16)
+#define DWC3_OEVT_BDEVBHOSTEND		BIT(11)
+#define DWC3_OEVT_BDEVHNPCHNG		BIT(10)
+#define DWC3_OEVT_BDEVSESSVLDDET	BIT(9)
+#define DWC3_OEVT_BDEVVBUSCHNG		BIT(8)
+#define DWC3_OEVT_BSESSVLD		BIT(3)
+#define DWC3_OEVT_HSTNEGSTS		BIT(2)
+#define DWC3_OEVT_SESREQSTS		BIT(1)
+#define DWC3_OEVT_ERROR			BIT(0)
+
+/* OTG Event Enable Register */
+#define DWC3_OEVTEN_XHCIRUNSTPSETEN	BIT(27)
+#define DWC3_OEVTEN_DEVRUNSTPSETEN	BIT(26)
+#define DWC3_OEVTEN_HIBENTRYEN		BIT(25)
+#define DWC3_OEVTEN_CONIDSTSCHNGEN	BIT(24)
+#define DWC3_OEVTEN_HRRCONFNOTIFEN	BIT(23)
+#define DWC3_OEVTEN_HRRINITNOTIFEN	BIT(22)
+#define DWC3_OEVTEN_ADEVIDLEEN		BIT(21)
+#define DWC3_OEVTEN_ADEVBHOSTENDEN	BIT(20)
+#define DWC3_OEVTEN_ADEVHOSTEN		BIT(19)
+#define DWC3_OEVTEN_ADEVHNPCHNGEN	BIT(18)
+#define DWC3_OEVTEN_ADEVSRPDETEN	BIT(17)
+#define DWC3_OEVTEN_ADEVSESSENDDETEN	BIT(16)
+#define DWC3_OEVTEN_BDEVHOSTENDEN	BIT(11)
+#define DWC3_OEVTEN_BDEVHNPCHNGEN	BIT(10)
+#define DWC3_OEVTEN_BDEVSESSVLDDETEN	BIT(9)
+#define DWC3_OEVTEN_BDEVVBUSCHNGE	BIT(8)
+
+/* OTG Status Register */
+#define DWC3_OSTS_DEVRUNSTP		BIT(13)
+#define DWC3_OSTS_XHCIRUNSTP		BIT(12)
+#define DWC3_OSTS_PERIPHERALSTATE	BIT(4)
+#define DWC3_OSTS_XHCIPRTPOWER		BIT(3)
+#define DWC3_OSTS_BSESVLD		BIT(2)
+#define DWC3_OSTS_VBUSVLD		BIT(1)
+#define DWC3_OSTS_CONIDSTS		BIT(0)
+
 /* Structures */
 
 struct dwc3_trb;
-- 
2.5.0

[PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared

[Roger Quadros]

The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED.

Use request_threaded_irq() to ensure that irqflags match for the
shared interrupt handlers. 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.

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

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 22e9606..51ca098 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
 {
 	struct dwc3_omap	*omap = _omap;
 	u32			reg;
+	int			ret = IRQ_NONE;
 
 	reg = dwc3_omap_read_irqmisc_status(omap);
 
+	if (reg)
+		ret = IRQ_HANDLED;
+
 	if (reg & USBOTGSS_IRQMISC_DMADISABLECLR)
 		omap->dma_status = false;
 
 	dwc3_omap_write_irqmisc_status(omap, reg);
 
 	reg = dwc3_omap_read_irq0_status(omap);
+	if (reg)
+		ret = IRQ_HANDLED;
 
 	dwc3_omap_write_irq0_status(omap, reg);
 
-	return IRQ_HANDLED;
+	return ret;
 }
 
 static void dwc3_omap_enable_irqs(struct dwc3_omap *omap)
@@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
 	omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE);
 
-	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,
+					NULL, IRQF_SHARED, "dwc3-omap", omap);
 	if (ret) {
 		dev_err(dev, "failed to request IRQ #%d --> %d\n",
 				omap->irq, ret);
-- 
2.5.0

[PATCH v6 03/10] 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 | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 51ca098..c9b918d 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -233,19 +233,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
-				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
+		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
 				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
 		dwc3_omap_write_utmi_ctrl(omap, val);
@@ -254,14 +249,16 @@ 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
 				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT);
-		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.5.0

[PATCH v6 04/10] usb: dwc3: omap: fix up error path on probe()

[Roger Quadros]

From: Felipe Balbi <balbi@kernel.org>

Even if pm_runtime_get*() fails, we *MUST* call
pm_runtime_put_sync() before disabling PM.

While at it, remove superfluous dwc3_omap_disable_irqs()
in error path.

Signed-off-by: Felipe Balbi <balbi@kernel.org>
[nsekhar@ti.com: patch description updates]
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index c9b918d..3497b25 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -499,7 +499,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0) {
 		dev_err(dev, "get_sync failed with err %d\n", ret);
-		goto err0;
+		goto err1;
 	}
 
 	dwc3_omap_map_offset(omap);
@@ -519,28 +519,24 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 
 	ret = dwc3_omap_extcon_register(omap);
 	if (ret < 0)
-		goto err2;
+		goto err1;
 
 	ret = of_platform_populate(node, NULL, NULL, dev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to create dwc3 core\n");
-		goto err3;
+		goto err2;
 	}
 
 	dwc3_omap_enable_irqs(omap);
 
 	return 0;
 
-err3:
+err2:
 	extcon_unregister_notifier(omap->edev, EXTCON_USB, &omap->vbus_nb);
 	extcon_unregister_notifier(omap->edev, EXTCON_USB_HOST, &omap->id_nb);
-err2:
-	dwc3_omap_disable_irqs(omap);
 
 err1:
 	pm_runtime_put_sync(dev);
-
-err0:
 	pm_runtime_disable(dev);
 
 	return ret;
-- 
2.5.0

[PATCH v6 05/10] 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   | 26 ++++++++++++++++----------
 drivers/usb/dwc3/core.h   |  5 +++++
 drivers/usb/dwc3/gadget.c | 19 ++++++++++++++++++-
 drivers/usb/dwc3/host.c   | 19 +++++++++++++++++++
 4 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 17fd8144..1c754749 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -746,6 +746,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 {
 	struct device *dev = dwc->dev;
 	int ret;
+	struct resource *res;
+	struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
 
 	switch (dwc->dr_mode) {
 	case USB_DR_MODE_PERIPHERAL:
@@ -765,6 +767,20 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 		}
 		break;
 	case USB_DR_MODE_OTG:
+		dwc->otg_irq = platform_get_irq_byname(dwc3_pdev, "otg");
+		if (dwc->otg_irq <= 0) {
+			dwc->otg_irq = platform_get_irq_byname(dwc3_pdev,
+							       "dwc_usb3");
+			if (dwc->otg_irq <= 0) {
+				res = platform_get_resource(dwc3_pdev,
+							    IORESOURCE_IRQ, 0);
+				if (!res) {
+					dev_err(dwc->dev, "missing otg IRQ\n");
+					return -ENODEV;
+				}
+				dwc->otg_irq = res->start;
+			}
+		}
 		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
 		ret = dwc3_host_init(dwc);
 		if (ret) {
@@ -831,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/core.h b/drivers/usb/dwc3/core.h
index 81039f7..79422dd 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -754,6 +754,8 @@ 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
+ * @otg_irq: IRQ number for OTG IRQs
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
@@ -857,6 +859,9 @@ struct dwc3 {
 
 	enum usb_dr_mode	dr_mode;
 
+	int			gadget_irq;
+	int			otg_irq;
+
 	/* used for suspend/resume */
 	u32			dcfg;
 	u32			gctl;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ac170f..90f514c 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1617,7 +1617,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);
 	if (ret) {
@@ -2795,6 +2795,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc)
 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.5.0

[PATCH v6 06/10] usb: dwc3: add dual-role support

[Roger Quadros]

Register with the USB OTG core. Since we don't support
OTG yet we just work as a dual-role device even
if device tree says "otg".

Get ID and VBUS information from the OTG controller
and kick the OTG state machine.

Make sure dual-role functionality works across system
suspend/resume.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/core.c   | 544 ++++++++++++++++++++++++++++++++++++++++++++--
 drivers/usb/dwc3/core.h   |  20 ++
 drivers/usb/dwc3/gadget.c |   6 +-
 drivers/usb/dwc3/host.c   |   2 +
 4 files changed, 550 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1c754749..f24c091 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -57,6 +57,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
 	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
 	reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
 	reg |= DWC3_GCTL_PRTCAPDIR(mode);
+	dwc->current_mode = mode;
 	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
 }
 
@@ -742,13 +743,444 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
 	return 0;
 }
 
-static int dwc3_core_init_mode(struct dwc3 *dwc)
+/* Get OTG events and sync it to OTG fsm */
+static void dwc3_otg_fsm_sync(struct dwc3 *dwc)
+{
+	u32 reg;
+	int id, vbus;
+
+	/*
+	 * calling usb_otg_sync_inputs() during resume breaks host
+	 * if adapter was removed during suspend as xhci driver
+	 * is not prepared to see hcd removal before xhci_resume.
+	 */
+	if (dwc->otg_prevent_sync)
+		return;
+
+	reg = dwc3_readl(dwc->regs, DWC3_OSTS);
+	dwc3_trace(trace_dwc3_core, "otgstatus 0x%x\n", reg);
+
+	id = !!(reg & DWC3_OSTS_CONIDSTS);
+	vbus = !!(reg & DWC3_OSTS_BSESVLD);
+
+	dwc3_trace(trace_dwc3_core, "id %d vbus %d\n", id, vbus);
+	dwc->otg->fsm.id = id;
+	dwc->otg->fsm.b_sess_vld = vbus;
+	usb_otg_sync_inputs(dwc->otg);
+}
+
+static void dwc3_otg_mask_irq(struct dwc3 *dwc)
+{
+	dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+	dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
+}
+
+static void dwc3_otg_unmask_irq(struct dwc3 *dwc)
+{
+	dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
+}
+
+static void dwc3_otg_disable_events(struct dwc3 *dwc, u32 disable_mask)
+{
+	dwc->oevten &= ~(disable_mask);
+	dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
+}
+
+static void dwc3_otg_enable_events(struct dwc3 *dwc, u32 enable_mask)
+{
+	dwc->oevten |= (enable_mask);
+	dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
+}
+
+#define DWC3_OTG_ALL_EVENTS	(DWC3_OEVTEN_XHCIRUNSTPSETEN | \
+		DWC3_OEVTEN_DEVRUNSTPSETEN | DWC3_OEVTEN_HIBENTRYEN | \
+		DWC3_OEVTEN_CONIDSTSCHNGEN | DWC3_OEVTEN_HRRCONFNOTIFEN | \
+		DWC3_OEVTEN_HRRINITNOTIFEN | DWC3_OEVTEN_ADEVIDLEEN | \
+		DWC3_OEVTEN_ADEVBHOSTENDEN | DWC3_OEVTEN_ADEVHOSTEN | \
+		DWC3_OEVTEN_ADEVHNPCHNGEN | DWC3_OEVTEN_ADEVSRPDETEN | \
+		DWC3_OEVTEN_ADEVSESSENDDETEN | DWC3_OEVTEN_BDEVHOSTENDEN | \
+		DWC3_OEVTEN_BDEVHNPCHNGEN | DWC3_OEVTEN_BDEVSESSVLDDETEN | \
+		DWC3_OEVTEN_BDEVVBUSCHNGE)
+
+static int dwc3_drd_start_host(struct usb_otg *otg, int on);
+static int dwc3_drd_start_gadget(struct usb_otg *otg, int on);
+static irqreturn_t dwc3_otg_thread_irq(int irq, void *_dwc)
+{
+	struct dwc3 *dwc = _dwc;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+
+	/*
+	 * this bit is needed for otg-host to work after system suspend/resume
+	 */
+	if ((dwc->otg->state == OTG_STATE_A_HOST) &&
+	    !(dwc->oevt & DWC3_OEVT_DEVICEMODE)) {
+		spin_unlock_irqrestore(&dwc->lock, flags);
+		dwc3_drd_start_host(dwc->otg, true);
+		spin_lock_irqsave(&dwc->lock, flags);
+	}
+
+	dwc3_otg_fsm_sync(dwc);
+	dwc3_otg_unmask_irq(dwc);
+
+	dwc->oevt = 0;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t dwc3_otg_irq(int irq, void *_dwc)
+{
+	struct dwc3 *dwc = _dwc;
+	irqreturn_t ret = IRQ_NONE;
+	u32 reg;
+
+	reg = dwc3_readl(dwc->regs, DWC3_OEVT);
+	if (reg) {
+		dwc->oevt = reg;
+		dwc3_writel(dwc->regs, DWC3_OEVT, reg);
+		dwc3_otg_mask_irq(dwc);
+		ret = IRQ_WAKE_THREAD;
+	}
+
+	return ret;
+}
+
+/* --------------------- Dual-Role management ------------------------------- */
+static void dwc3_otgregs_init(struct dwc3 *dwc)
+{
+	u32 reg;
+
+	/*
+	 * Prevent host/device reset from resetting OTG core.
+	 * If we don't do this then xhci_reset (USBCMD.HCRST) will reset
+	 * the signal outputs sent to the PHY, the OTG FSM logic of the
+	 * core and also the resets to the VBUS filters inside the core.
+	 */
+	reg = dwc3_readl(dwc->regs, DWC3_OCFG);
+	reg |= DWC3_OCFG_SFTRSTMASK;
+	dwc3_writel(dwc->regs, DWC3_OCFG, reg);
+
+	/* Disable hibernation for simplicity */
+	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+	reg &= ~DWC3_GCTL_GBLHIBERNATIONEN;
+	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+
+	/*
+	 * Initialize OTG registers as per
+	 * Figure 11-4 OTG Driver Overall Programming Flow
+	 */
+	/* OCFG.SRPCap = 0, OCFG.HNPCap = 0 */
+	reg = dwc3_readl(dwc->regs, DWC3_OCFG);
+	reg &= ~(DWC3_OCFG_SRPCAP | DWC3_OCFG_HNPCAP);
+	dwc3_writel(dwc->regs, DWC3_OCFG, reg);
+	/* OEVT = FFFF */
+	dwc3_writel(dwc->regs, DWC3_OEVT, ~0);
+	/* OEVTEN = 0 */
+	dwc3_otg_disable_events(dwc, DWC3_OTG_ALL_EVENTS);
+	/* OEVTEN.ConIDStsChngEn = 1. Instead we enable all events */
+	dwc3_otg_enable_events(dwc, DWC3_OTG_ALL_EVENTS);
+	/*
+	 * OCTL.PeriMode = 1, OCTL.DevSetHNPEn = 0, OCTL.HstSetHNPEn = 0,
+	 * OCTL.HNPReq = 0
+	 */
+	reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+	reg |= DWC3_OCTL_PERIMODE;
+	reg &= ~(DWC3_OCTL_DEVSETHNPEN | DWC3_OCTL_HSTSETHNPEN |
+		 DWC3_OCTL_HNPREQ);
+	dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+}
+
+static int dwc3_drd_start_host(struct usb_otg *otg, int on)
+{
+	struct dwc3 *dwc = dev_get_drvdata(otg->dev);
+	u32 reg;
+	unsigned long flags;
+
+	dwc3_trace(trace_dwc3_core, "%s: %d\n", __func__, on);
+
+	/* switch OTG core */
+	if (on) {
+		/* As per Figure 11-10 A-Device Flow Diagram */
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		/* OCFG.HNPCap = 0, OCFG.SRPCap = 0 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCFG);
+		reg &= ~(DWC3_OCFG_SRPCAP | DWC3_OCFG_HNPCAP);
+		dwc3_writel(dwc->regs, DWC3_OCFG, reg);
+
+		/*
+		 * OCTL.PeriMode=0, OCTL.TermSelDLPulse = 0,
+		 * OCTL.DevSetHNPEn = 0, OCTL.HstSetHNPEn = 0
+		 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+		reg &= ~(DWC3_OCTL_PERIMODE | DWC3_OCTL_TERMSELIDPULSE |
+			 DWC3_OCTL_DEVSETHNPEN | DWC3_OCTL_HSTSETHNPEN);
+		dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+
+		/*
+		 * OCFG.DisPrtPwrCutoff = 0/1
+		 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCFG);
+		reg &= ~DWC3_OCFG_DISPWRCUTTOFF;
+		dwc3_writel(dwc->regs, DWC3_OCFG, reg);
+
+		/* start the xHCI host driver */
+		spin_unlock_irqrestore(&dwc->lock, flags);
+		usb_otg_start_host(otg, true);
+		spin_lock_irqsave(&dwc->lock, flags);
+
+		/*
+		 * OCFG.SRPCap = 1, OCFG.HNPCap = GHWPARAMS6.HNP_CAP
+		 * We don't want SRP/HNP for simple dual-role so leave
+		 * these disabled.
+		 */
+
+		/*
+		 * OEVTEN.OTGADevHostEvntEn = 1
+		 * OEVTEN.OTGADevSessEndDetEvntEn = 1
+		 * We don't want HNP/role-swap so leave these disabled.
+		 */
+
+		/* GUSB2PHYCFG.ULPIAutoRes = 1/0, GUSB2PHYCFG.SusPHY = 1 */
+		if (!dwc->dis_u2_susphy_quirk) {
+			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+		}
+
+		/* Set Port Power to enable VBUS: OCTL.PrtPwrCtl = 1 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+		reg |= DWC3_OCTL_PRTPWRCTL;
+		dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+		spin_unlock_irqrestore(&dwc->lock, flags);
+	} else {
+		/*
+		 * Exit from A-device flow as per
+		 * Figure 11-4 OTG Driver Overall Programming Flow
+		 */
+		/* stop the HCD */
+		usb_otg_start_host(otg, false);
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		/*
+		 * OEVTEN.OTGADevBHostEndEvntEn=0, OEVTEN.OTGADevHNPChngEvntEn=0
+		 * OEVTEN.OTGADevSessEndDetEvntEn=0,
+		 * OEVTEN.OTGADevHostEvntEn = 0
+		 * But we don't disable any OTG events
+		 */
+
+		/* OCTL.HstSetHNPEn = 0, OCTL.PrtPwrCtl=0 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+		reg &= ~(DWC3_OCTL_HSTSETHNPEN | DWC3_OCTL_PRTPWRCTL);
+		dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+
+		/* Initialize OTG registers */
+		dwc3_otgregs_init(dwc);
+		spin_unlock_irqrestore(&dwc->lock, flags);
+	}
+
+	return 0;
+}
+
+static int dwc3_drd_start_gadget(struct usb_otg *otg, int on)
+{
+	struct dwc3 *dwc = dev_get_drvdata(otg->dev);
+	u32 reg;
+	unsigned long flags;
+
+	dwc3_trace(trace_dwc3_core, "%s: %d\n", __func__, on);
+	if (on)
+		dwc3_event_buffers_setup(dwc);
+
+	if (on) {
+		/* As per Figure 11-20 B-Device Flow Diagram */
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		/*
+		 * OCFG.HNPCap = GHWPARAMS6.HNP_CAP, OCFG.SRPCap = 1
+		 * but we set them to 0 for simple dual-role operation.
+		 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCFG);
+		reg &= ~(DWC3_OCFG_SRPCAP | DWC3_OCFG_HNPCAP);
+		/* OCFG.OTGSftRstMsk = 0/1 */
+		reg |= DWC3_OCFG_SFTRSTMASK;
+		dwc3_writel(dwc->regs, DWC3_OCFG, reg);
+		/*
+		 * OCTL.PeriMode = 1
+		 * OCTL.TermSelDLPulse = 0/1, OCTL.HNPReq = 0
+		 * OCTL.DevSetHNPEn = 0, OCTL.HstSetHNPEn = 0
+		 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+		reg |= DWC3_OCTL_PERIMODE;
+		reg &= ~(DWC3_OCTL_TERMSELIDPULSE | DWC3_OCTL_HNPREQ |
+			 DWC3_OCTL_DEVSETHNPEN | DWC3_OCTL_HSTSETHNPEN);
+		dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+		/* OEVTEN.OTGBDevSesVldDetEvntEn = 1 */
+		dwc3_otg_enable_events(dwc, DWC3_OEVT_BDEVSESSVLDDET);
+		/* GUSB2PHYCFG.ULPIAutoRes = 0, GUSB2PHYCFG0.SusPHY = 1 */
+		if (!dwc->dis_u2_susphy_quirk) {
+			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+		}
+		/* GCTL.GblHibernationEn = 0 */
+		reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+		reg &= ~DWC3_GCTL_GBLHIBERNATIONEN;
+		dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+
+		spin_unlock_irqrestore(&dwc->lock, flags);
+
+		/* start the Peripheral driver  */
+		usb_otg_start_gadget(otg, true);
+	} else {
+		/*
+		 * Exit from B-device flow as per
+		 * Figure 11-4 OTG Driver Overall Programming Flow
+		 */
+		/* stop the Peripheral driver */
+		usb_otg_start_gadget(otg, false);
+
+		spin_lock_irqsave(&dwc->lock, flags);
+
+		/*
+		 * OEVTEN.OTGBDevHNPChngEvntEn = 0
+		 * OEVTEN.OTGBDevVBusChngEvntEn = 0
+		 * OEVTEN.OTGBDevBHostEndEvntEn = 0
+		 */
+		reg = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+		reg &= ~(DWC3_OEVT_BDEVHNPCHNG | DWC3_OEVT_BDEVVBUSCHNG |
+			 DWC3_OEVT_BDEVBHOSTEND);
+		dwc3_writel(dwc->regs, DWC3_OEVTEN, reg);
+
+		/* OCTL.DevSetHNPEn = 0, OCTL.HNPReq = 0, OCTL.PeriMode=1 */
+		reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+		reg &= ~(DWC3_OCTL_DEVSETHNPEN | DWC3_OCTL_HNPREQ);
+		reg |= DWC3_OCTL_PERIMODE;
+		dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+
+		/* Initialize OTG registers */
+		dwc3_otgregs_init(dwc);
+		spin_unlock_irqrestore(&dwc->lock, flags);
+	}
+
+	return 0;
+}
+
+static struct otg_fsm_ops dwc3_drd_ops = {
+	.start_host = dwc3_drd_start_host,
+	.start_gadget = dwc3_drd_start_gadget,
+};
+
+static int dwc3_drd_register(struct dwc3 *dwc)
+{
+	int ret;
+
+	/* register parent as DRD device with OTG core */
+	dwc->otg = usb_otg_register(dwc->dev, &dwc->otg_config);
+	if (IS_ERR(dwc->otg)) {
+		ret = PTR_ERR(dwc->otg);
+		if (ret == -ENOTSUPP)
+			dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
+		else
+			dev_err(dwc->dev, "Failed to register with OTG core\n");
+
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dwc3_drd_init(struct dwc3 *dwc)
 {
-	struct device *dev = dwc->dev;
 	int ret;
+	struct usb_otg_caps *otgcaps = &dwc->otg_caps;
+	u32 reg;
+	unsigned long flags;
 	struct resource *res;
 	struct platform_device *dwc3_pdev = to_platform_device(dwc->dev);
 
+	dwc->otg_irq = platform_get_irq_byname(dwc3_pdev, "otg");
+	if (dwc->otg_irq <= 0) {
+		dwc->otg_irq = platform_get_irq_byname(dwc3_pdev,
+				"dwc_usb3");
+		if (dwc->otg_irq <= 0) {
+			res = platform_get_resource(dwc3_pdev,
+						    IORESOURCE_IRQ, 0);
+			if (!res) {
+				dev_err(dwc->dev, "missing otg IRQ\n");
+				return -ENODEV;
+			}
+			dwc->otg_irq = res->start;
+		}
+	}
+
+	otgcaps->otg_rev = 0;
+	otgcaps->hnp_support = false;
+	otgcaps->srp_support = false;
+	otgcaps->adp_support = false;
+	dwc->otg_config.fsm_ops = &dwc3_drd_ops;
+	dwc->otg_config.otg_caps = otgcaps;
+
+	ret = dwc3_drd_register(dwc);
+	if (ret)
+		return ret;
+
+	/* disable all otg irqs */
+	dwc3_otg_disable_events(dwc, DWC3_OTG_ALL_EVENTS);
+	/* clear all events */
+	dwc3_writel(dwc->regs, DWC3_OEVT, ~0);
+
+	ret = request_threaded_irq(dwc->otg_irq, dwc3_otg_irq,
+				   dwc3_otg_thread_irq,
+				   IRQF_SHARED, "dwc3-otg", dwc);
+	if (ret) {
+		dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
+			dwc->otg_irq, ret);
+		ret = -ENODEV;
+		goto error;
+	}
+
+	spin_lock_irqsave(&dwc->lock, flags);
+
+	/*
+	 * As per Figure 11-4 OTG Driver Overall Programming Flow,
+	 * block "Initialize GCTL for OTG operation".
+	 */
+	/* GCTL.PrtCapDir=2'b11 */
+	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
+	/* GUSB2PHYCFG0.SusPHY=0 */
+	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+	reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+
+	/* Initialize OTG registers */
+	dwc3_otgregs_init(dwc);
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	dwc3_otg_fsm_sync(dwc);
+
+	return 0;
+
+error:
+	usb_otg_unregister(dwc->dev);
+
+	return ret;
+}
+
+static void dwc3_drd_exit(struct dwc3 *dwc)
+{
+	usb_otg_unregister(dwc->dev);
+}
+
+/* -------------------------------------------------------------------------- */
+
+static int dwc3_core_init_mode(struct dwc3 *dwc)
+{
+	struct device *dev = dwc->dev;
+	int ret;
+
 	switch (dwc->dr_mode) {
 	case USB_DR_MODE_PERIPHERAL:
 		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
@@ -767,30 +1199,32 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 		}
 		break;
 	case USB_DR_MODE_OTG:
-		dwc->otg_irq = platform_get_irq_byname(dwc3_pdev, "otg");
-		if (dwc->otg_irq <= 0) {
-			dwc->otg_irq = platform_get_irq_byname(dwc3_pdev,
-							       "dwc_usb3");
-			if (dwc->otg_irq <= 0) {
-				res = platform_get_resource(dwc3_pdev,
-							    IORESOURCE_IRQ, 0);
-				if (!res) {
-					dev_err(dwc->dev, "missing otg IRQ\n");
-					return -ENODEV;
-				}
-				dwc->otg_irq = res->start;
+		ret = dwc3_drd_init(dwc);
+		if (ret) {
+			dev_err(dev,
+				"limiting to peripheral only as dual-role init failed: %d",
+				ret);
+			dwc->dr_mode = USB_DR_MODE_PERIPHERAL;
+			dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+			ret = dwc3_gadget_init(dwc);
+			if (ret) {
+				dev_err(dev, "failed to initialize gadget\n");
+				return ret;
 			}
 		}
-		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
+
 		ret = dwc3_host_init(dwc);
 		if (ret) {
 			dev_err(dev, "failed to initialize host\n");
+			dwc3_drd_exit(dwc);
 			return ret;
 		}
 
 		ret = dwc3_gadget_init(dwc);
 		if (ret) {
 			dev_err(dev, "failed to initialize gadget\n");
+			dwc3_host_exit(dwc);
+			dwc3_drd_exit(dwc);
 			return ret;
 		}
 		break;
@@ -814,6 +1248,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
 	case USB_DR_MODE_OTG:
 		dwc3_host_exit(dwc);
 		dwc3_gadget_exit(dwc);
+		dwc3_drd_exit(dwc);
 		break;
 	default:
 		/* do nothing */
@@ -1142,6 +1577,30 @@ static int dwc3_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM_SLEEP
+static int dwc3_prepare(struct device *dev)
+{
+	struct dwc3	*dwc = dev_get_drvdata(dev);
+	unsigned long	flags;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	dwc->otg_prevent_sync = true;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	return 0;
+}
+
+static void dwc3_complete(struct device *dev)
+{
+	struct dwc3	*dwc = dev_get_drvdata(dev);
+	unsigned long	flags;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	dwc->otg_prevent_sync = false;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+	if (dwc->dr_mode == USB_DR_MODE_OTG)
+		dwc3_otg_fsm_sync(dwc);
+}
+
 static int dwc3_suspend(struct device *dev)
 {
 	struct dwc3	*dwc = dev_get_drvdata(dev);
@@ -1149,18 +1608,40 @@ static int dwc3_suspend(struct device *dev)
 
 	spin_lock_irqsave(&dwc->lock, flags);
 
+	/* Save OTG state only if we're really using it */
+	if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
+		dwc->ocfg = dwc3_readl(dwc->regs, DWC3_OCFG);
+		dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
+		dwc3_otg_mask_irq(dwc);
+	}
+
+	dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
+
 	switch (dwc->dr_mode) {
 	case USB_DR_MODE_PERIPHERAL:
-	case USB_DR_MODE_OTG:
 		dwc3_gadget_suspend(dwc);
-		/* FALLTHROUGH */
+		break;
+	case USB_DR_MODE_OTG:
+		dwc->otg_protocol = dwc->otg->fsm.protocol;
+
+		switch (dwc->otg->fsm.protocol) {
+		case PROTO_GADGET:
+			dwc3_gadget_suspend(dwc);
+			break;
+		case PROTO_HOST:
+		case PROTO_UNDEF:
+		default:
+			/* nothing */
+			break;
+		}
 	case USB_DR_MODE_HOST:
+	case USB_DR_MODE_UNKNOWN:
 	default:
-		dwc3_event_buffers_cleanup(dwc);
+		/* nothing */
 		break;
 	}
 
-	dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
+	dwc3_event_buffers_cleanup(dwc);
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	usb_phy_shutdown(dwc->usb3_phy);
@@ -1198,15 +1679,34 @@ static int dwc3_resume(struct device *dev)
 
 	switch (dwc->dr_mode) {
 	case USB_DR_MODE_PERIPHERAL:
-	case USB_DR_MODE_OTG:
 		dwc3_gadget_resume(dwc);
-		/* FALLTHROUGH */
+		break;
+	case USB_DR_MODE_OTG:
+		switch (dwc->otg_protocol) {
+		case PROTO_GADGET:
+			dwc3_gadget_resume(dwc);
+			break;
+		case PROTO_HOST:
+			break;
+		case PROTO_UNDEF:
+		default:
+			/* nothing */
+			break;
+		}
 	case USB_DR_MODE_HOST:
+	case USB_DR_MODE_UNKNOWN:
 	default:
 		/* do nothing */
 		break;
 	}
 
+	/* Restore OTG state only if we're really using it */
+	if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
+		dwc3_writel(dwc->regs, DWC3_OCFG, dwc->ocfg);
+		dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
+		dwc3_otg_unmask_irq(dwc);
+	}
+
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	pm_runtime_disable(dev);
@@ -1223,6 +1723,8 @@ err_usb2phy_init:
 
 static const struct dev_pm_ops dwc3_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
+	.prepare = dwc3_prepare,
+	.complete = dwc3_complete,
 };
 
 #define DWC3_PM_OPS	&(dwc3_dev_pm_ops)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 79422dd..c9f83c5 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -31,6 +31,7 @@
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
 #include <linux/ulpi/interface.h>
+#include <linux/usb/otg-fsm.h>
 
 #include <linux/phy/phy.h>
 
@@ -754,6 +755,12 @@ struct dwc3_scratchpad_array {
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
  * @revision: revision register contents
  * @dr_mode: requested mode of operation
+ * @otg: usb otg data structure
+ * @otg_config: otg controller configuration
+ * @otg_prevent_sync: flag to block events to otg fsm
+ * @otg_protocol: saved copy of otg state during suspend
+ * @current_mode: current mode of operation written to PRTCAPDIR
+ * @oevt: cached OEVT register during OTG irq
  * @gadget_irq: IRQ number for Peripheral IRQs
  * @otg_irq: IRQ number for OTG IRQs
  * @usb2_phy: pointer to USB2 PHY
@@ -763,6 +770,9 @@ struct dwc3_scratchpad_array {
  * @ulpi: pointer to ulpi interface
  * @dcfg: saved contents of DCFG register
  * @gctl: saved contents of GCTL register
+ * @ocfg: saved contents of OCFG register
+ * @octl: saved contents of OCTL register
+ * @oevten: saved contents of OEVTEN register
  * @isoch_delay: wValue from Set Isochronous Delay request;
  * @u2sel: parameter from Set SEL request.
  * @u2pel: parameter from Set SEL request.
@@ -858,6 +868,13 @@ struct dwc3 {
 	size_t			regs_size;
 
 	enum usb_dr_mode	dr_mode;
+	struct usb_otg		*otg;
+	struct usb_otg_caps	otg_caps;
+	struct usb_otg_config	otg_config;
+	bool			otg_prevent_sync;
+	int			otg_protocol;
+	u32			current_mode;
+	u32			oevt;
 
 	int			gadget_irq;
 	int			otg_irq;
@@ -865,6 +882,9 @@ struct dwc3 {
 	/* used for suspend/resume */
 	u32			dcfg;
 	u32			gctl;
+	u32			ocfg;
+	u32			octl;
+	u32			oevten;
 
 	u32			nr_scratch;
 	u32			num_event_buffers;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 90f514c..83d5c57 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2894,7 +2894,11 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	if (ret)
 		goto err5;
 
-	ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget);
+	if (dwc->dr_mode == USB_DR_MODE_OTG)
+		ret = usb_otg_add_gadget_udc(dwc->dev, &dwc->gadget, dwc->dev);
+	else
+		ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget);
+
 	if (ret) {
 		dev_err(dwc->dev, "failed to register udc\n");
 		goto err5;
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index f2b60a4..89339d1 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -69,6 +69,8 @@ int dwc3_host_init(struct dwc3 *dwc)
 	memset(&pdata, 0, sizeof(pdata));
 
 	pdata.usb3_lpm_capable = dwc->usb3_lpm_capable;
+	if (dwc->dr_mode == USB_DR_MODE_OTG)
+		pdata.otg_dev = dwc->dev;
 
 	ret = platform_device_add_data(xhci, &pdata, sizeof(pdata));
 	if (ret) {
-- 
2.5.0

[PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode

[Roger Quadros]

Gadget controller might not be always active during suspend/
resume when we are operating in dual-role/otg mode.
Check if we're active and only if we are then perform
necessary actions during suspend/resume.

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

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 83d5c57..1ca5ac0 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2952,6 +2952,9 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
 
 int dwc3_gadget_suspend(struct dwc3 *dwc)
 {
+	if (!dwc->gadget_driver)
+		return 0;
+
 	if (dwc->pullups_connected) {
 		dwc3_gadget_disable_irq(dwc);
 		dwc3_gadget_run_stop(dwc, true, true);
@@ -2970,6 +2973,9 @@ int dwc3_gadget_resume(struct dwc3 *dwc)
 	struct dwc3_ep		*dep;
 	int			ret;
 
+	if (!dwc->gadget_driver)
+		return 0;
+
 	/* Start with SuperSpeed Default */
 	dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 
-- 
2.5.0

[PATCH v6 08/10] usb: dwc3: core: fix PHY handling during suspend

[Roger Quadros]

From: Felipe Balbi <balbi@kernel.org>

we need to power off the PHY during suspend and
power it back on during resume.

Signed-off-by: Felipe Balbi <balbi@kernel.org>
[nsekhar@ti.com: fix call to usb_phy_set_suspend() in dwc3_suspend()]
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/core.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f24c091..60665dd 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1649,6 +1649,11 @@ static int dwc3_suspend(struct device *dev)
 	phy_exit(dwc->usb2_generic_phy);
 	phy_exit(dwc->usb3_generic_phy);
 
+	usb_phy_set_suspend(dwc->usb2_phy, 1);
+	usb_phy_set_suspend(dwc->usb3_phy, 1);
+	WARN_ON(phy_power_off(dwc->usb2_generic_phy) < 0);
+	WARN_ON(phy_power_off(dwc->usb3_generic_phy) < 0);
+
 	pinctrl_pm_select_sleep_state(dev);
 
 	return 0;
@@ -1662,11 +1667,21 @@ static int dwc3_resume(struct device *dev)
 
 	pinctrl_pm_select_default_state(dev);
 
+	usb_phy_set_suspend(dwc->usb2_phy, 0);
+	usb_phy_set_suspend(dwc->usb3_phy, 0);
+	ret = phy_power_on(dwc->usb2_generic_phy);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_power_on(dwc->usb3_generic_phy);
+	if (ret < 0)
+		goto err_usb2phy_power;
+
 	usb_phy_init(dwc->usb3_phy);
 	usb_phy_init(dwc->usb2_phy);
 	ret = phy_init(dwc->usb2_generic_phy);
 	if (ret < 0)
-		return ret;
+		goto err_usb3phy_power;
 
 	ret = phy_init(dwc->usb3_generic_phy);
 	if (ret < 0)
@@ -1718,6 +1733,12 @@ static int dwc3_resume(struct device *dev)
 err_usb2phy_init:
 	phy_exit(dwc->usb2_generic_phy);
 
+err_usb3phy_power:
+	phy_power_off(dwc->usb3_generic_phy);
+
+err_usb2phy_power:
+	phy_power_off(dwc->usb2_generic_phy);
+
 	return ret;
 }
 
-- 
2.5.0

[PATCH v6 09/10] ARM: dts: dra7*-evm: Enable dual-role for usb1

[Roger Quadros]

Now that we have dual-role support working at USB core,
enable dual-role support for usb1 controller.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts  | 2 +-
 arch/arm/boot/dts/dra72-evm.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index d9b8723..61c88f7 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -722,7 +722,7 @@
 };
 
 &usb1 {
-	dr_mode = "peripheral";
+	dr_mode = "otg";
 	pinctrl-names = "default";
 	pinctrl-0 = <&usb1_pins>;
 };
diff --git a/arch/arm/boot/dts/dra72-evm.dts b/arch/arm/boot/dts/dra72-evm.dts
index 6affe2d..f7d345e 100644
--- a/arch/arm/boot/dts/dra72-evm.dts
+++ b/arch/arm/boot/dts/dra72-evm.dts
@@ -592,7 +592,7 @@
 };
 
 &usb1 {
-	dr_mode = "peripheral";
+	dr_mode = "otg";
 	pinctrl-names = "default";
 	pinctrl-0 = <&usb1_pins>;
 };
-- 
2.5.0

[PATCH v6 10/10] ARM: dts: am43xx: Enable dual-role on USB1

[Roger Quadros]

USB1 port is micro-AB type and can function as peripheral
as well as host. Enable dual-role mode for USB1.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/am437x-gp-evm.dts  | 2 +-
 arch/arm/boot/dts/am437x-sk-evm.dts  | 2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index 8889be1..5834448 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -765,7 +765,7 @@
 };
 
 &usb1 {
-	dr_mode = "peripheral";
+	dr_mode = "otg";
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
index d82dd6e..ad772da 100644
--- a/arch/arm/boot/dts/am437x-sk-evm.dts
+++ b/arch/arm/boot/dts/am437x-sk-evm.dts
@@ -571,7 +571,7 @@
 };
 
 &usb1 {
-	dr_mode = "peripheral";
+	dr_mode = "otg";
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&usb1_pins>;
diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 83dfafa..b2b596b 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -675,7 +675,7 @@
 };
 
 &usb1 {
-	dr_mode = "peripheral";
+	dr_mode = "otg";
 	status = "okay";
 };
 
-- 
2.5.0

Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED.
>
> Use request_threaded_irq() to ensure that irqflags match for the
> shared interrupt handlers. 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.

then it seems you need to _first_ fix the OTG IRQ handler. Why does it
defer ? At a minimum, first switch to threaded IRQ handler, then (in
another patch) switch to IRQF_SHARED

> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/usb/dwc3/dwc3-omap.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index 22e9606..51ca098 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
>  {
>  	struct dwc3_omap	*omap = _omap;
>  	u32			reg;
> +	int			ret = IRQ_NONE;
>  
>  	reg = dwc3_omap_read_irqmisc_status(omap);
>  
> +	if (reg)
> +		ret = IRQ_HANDLED;

you can avoid the local variable by returning early here.

	if (!reg)
        	return IRQ_NONE;

>  	if (reg & USBOTGSS_IRQMISC_DMADISABLECLR)
>  		omap->dma_status = false;
>  
>  	dwc3_omap_write_irqmisc_status(omap, reg);
>  
>  	reg = dwc3_omap_read_irq0_status(omap);
> +	if (reg)
> +		ret = IRQ_HANDLED;

and this check is probably unnecessary.

> @@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>  	reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
>  	omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE);
>  
> -	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,

this switch to threaded IRQ is not part of $subject.

-- 
balbi

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

Re: [PATCH v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> Don't make any decisions regarding VBUS session based on ID
> status. That is best left to the OTG core.

what about builds who don't want OTG and/or dual-role ?

> 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 | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index 51ca098..c9b918d 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -233,19 +233,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
> -				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
> +		val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG;

this creates the possibility of having a USB peripheral with VBUS_VALID,
right 

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

I remember discussing this with TI's IP owner back in OMAP5 days. This
code was a result of talking to that guy and was, back then, tested by
Silicon Validation team. I would strongly advise that before changing
these bits you check with whoever's currently handling this IP inside TI
to make sure your changes are still within the expectations of the
wrapper block.

-- 
balbi

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

Re: [PATCH v6 04/10] usb: dwc3: omap: fix up error path on probe()

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> From: Felipe Balbi <balbi@kernel.org>
>
> Even if pm_runtime_get*() fails, we *MUST* call
> pm_runtime_put_sync() before disabling PM.
>
> While at it, remove superfluous dwc3_omap_disable_irqs()
> in error path.
>
> Signed-off-by: Felipe Balbi <balbi@kernel.org>
> [nsekhar@ti.com: patch description updates]
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

this fix is unrelated to DRD/OTG work, right ? Care to rebase only this
patch on v4.6-rc3 so I can apply it during current -rc ?

-- 
balbi

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

Re: [PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> Gadget controller might not be always active during suspend/
> resume when we are operating in dual-role/otg mode.
> Check if we're active and only if we are then perform
> necessary actions during suspend/resume.

I don't get this. If we're operating in OTG, we should have a gadget
driver loaded, no ?

-- 
balbi

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

Re: [PATCH v6 08/10] usb: dwc3: core: fix PHY handling during suspend

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> From: Felipe Balbi <balbi@kernel.org>
>
> we need to power off the PHY during suspend and
> power it back on during resume.
>
> Signed-off-by: Felipe Balbi <balbi@kernel.org>
> [nsekhar@ti.com: fix call to usb_phy_set_suspend() in dwc3_suspend()]
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

is this also a fix which needs to be rebased on v4.6-rc3 and merged
during current -rc cycle ?

-- 
balbi

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

Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared

[Roger Quadros]

On 11/04/16 15:13, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED.
>>
>> Use request_threaded_irq() to ensure that irqflags match for the
>> shared interrupt handlers. 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.
> 
> then it seems you need to _first_ fix the OTG IRQ handler. Why does it

There is no OTG irq handler yet.

> defer ? At a minimum, first switch to threaded IRQ handler, then (in
> another patch) switch to IRQF_SHARED

OK.

> 
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/usb/dwc3/dwc3-omap.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>> index 22e9606..51ca098 100644
>> --- a/drivers/usb/dwc3/dwc3-omap.c
>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
>>  {
>>  	struct dwc3_omap	*omap = _omap;
>>  	u32			reg;
>> +	int			ret = IRQ_NONE;
>>  
>>  	reg = dwc3_omap_read_irqmisc_status(omap);
>>  
>> +	if (reg)
>> +		ret = IRQ_HANDLED;
> 
> you can avoid the local variable by returning early here.

How can we return early? we need to check irq0_status as well right?

> 
> 	if (!reg)
>         	return IRQ_NONE;
> 
>>  	if (reg & USBOTGSS_IRQMISC_DMADISABLECLR)
>>  		omap->dma_status = false;
>>  
>>  	dwc3_omap_write_irqmisc_status(omap, reg);
>>  
>>  	reg = dwc3_omap_read_irq0_status(omap);
>> +	if (reg)
>> +		ret = IRQ_HANDLED;
> 
> and this check is probably unnecessary.
> 
>> @@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>>  	reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
>>  	omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE);
>>  
>> -	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,
> 
> this switch to threaded IRQ is not part of $subject.
> 
OK.

cheers,
-roger

Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
>>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>>> index 22e9606..51ca098 100644
>>> --- a/drivers/usb/dwc3/dwc3-omap.c
>>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>>> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
>>>  {
>>>  	struct dwc3_omap	*omap = _omap;
>>>  	u32			reg;
>>> +	int			ret = IRQ_NONE;
>>>  
>>>  	reg = dwc3_omap_read_irqmisc_status(omap);
>>>  
>>> +	if (reg)
>>> +		ret = IRQ_HANDLED;
>> 
>> you can avoid the local variable by returning early here.
>
> How can we return early? we need to check irq0_status as well right?

Oh, that's true.

There's one thing that I noticed though. dma_status is only written to,
never read, so you should be able to remove it completely (a bit
off-topic, sorry).

-- 
balbi

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

Re: [PATCH v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently

[Roger Quadros]

On 11/04/16 15:18, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> Don't make any decisions regarding VBUS session based on ID
>> status. That is best left to the OTG core.
> 
> what about builds who don't want OTG and/or dual-role ?
> 
>> 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 | 15 ++++++---------
>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>> index 51ca098..c9b918d 100644
>> --- a/drivers/usb/dwc3/dwc3-omap.c
>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>> @@ -233,19 +233,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
>> -				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
>> +		val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG;
> 
> this creates the possibility of having a USB peripheral with VBUS_VALID,
> right 

Sorry, I didn't get what you meant.

> 
>>  		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
>>  				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
> 
> I remember discussing this with TI's IP owner back in OMAP5 days. This
> code was a result of talking to that guy and was, back then, tested by
> Silicon Validation team. I would strongly advise that before changing
> these bits you check with whoever's currently handling this IP inside TI
> to make sure your changes are still within the expectations of the
> wrapper block.
> 
OK, I wasn't aware about this. I will check with the Silicon team.

cheers,
-roger

Re: [PATCH v6 04/10] usb: dwc3: omap: fix up error path on probe()

[Roger Quadros]

On 11/04/16 15:20, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> From: Felipe Balbi <balbi@kernel.org>
>>
>> Even if pm_runtime_get*() fails, we *MUST* call
>> pm_runtime_put_sync() before disabling PM.
>>
>> While at it, remove superfluous dwc3_omap_disable_irqs()
>> in error path.
>>
>> Signed-off-by: Felipe Balbi <balbi@kernel.org>
>> [nsekhar@ti.com: patch description updates]
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> this fix is unrelated to DRD/OTG work, right ? Care to rebase only this
> patch on v4.6-rc3 so I can apply it during current -rc ?
> 
Sure, will do that.

cheers,
-roger

Re: [PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode

[Roger Quadros]

On 11/04/16 15:23, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> Gadget controller might not be always active during suspend/
>> resume when we are operating in dual-role/otg mode.
>> Check if we're active and only if we are then perform
>> necessary actions during suspend/resume.
> 
> I don't get this. If we're operating in OTG, we should have a gadget
> driver loaded, no ?
> 
At boot gadget driver is not automatically loaded. We're still in OTG mode
but OTG state machine hasn't started.
System suspend/resume can still happen.

User might also load/unload the gadget driver prior to system suspend.

cheers,
-roger

Re: [PATCH v6 08/10] usb: dwc3: core: fix PHY handling during suspend

[Roger Quadros]

On 11/04/16 15:24, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> From: Felipe Balbi <balbi@kernel.org>
>>
>> we need to power off the PHY during suspend and
>> power it back on during resume.
>>
>> Signed-off-by: Felipe Balbi <balbi@kernel.org>
>> [nsekhar@ti.com: fix call to usb_phy_set_suspend() in dwc3_suspend()]
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> is this also a fix which needs to be rebased on v4.6-rc3 and merged
> during current -rc cycle ?
> 
Yes, I'll post it separately.

cheers,
-roger

Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared

[Roger Quadros]

On 11/04/16 15:58, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>>>> index 22e9606..51ca098 100644
>>>> --- a/drivers/usb/dwc3/dwc3-omap.c
>>>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>>>> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
>>>>  {
>>>>  	struct dwc3_omap	*omap = _omap;
>>>>  	u32			reg;
>>>> +	int			ret = IRQ_NONE;
>>>>  
>>>>  	reg = dwc3_omap_read_irqmisc_status(omap);
>>>>  
>>>> +	if (reg)
>>>> +		ret = IRQ_HANDLED;
>>>
>>> you can avoid the local variable by returning early here.
>>
>> How can we return early? we need to check irq0_status as well right?
> 
> Oh, that's true.
> 
> There's one thing that I noticed though. dma_status is only written to,
> never read, so you should be able to remove it completely (a bit
> off-topic, sorry).
> 
I'll send a patch for that. Can it go in -rc as well along with the other 2?

cheers,
-roger

Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> On 11/04/16 15:58, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Roger Quadros <rogerq@ti.com> writes:
>>>>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>>>>> index 22e9606..51ca098 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-omap.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>>>>> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
>>>>>  {
>>>>>  	struct dwc3_omap	*omap = _omap;
>>>>>  	u32			reg;
>>>>> +	int			ret = IRQ_NONE;
>>>>>  
>>>>>  	reg = dwc3_omap_read_irqmisc_status(omap);
>>>>>  
>>>>> +	if (reg)
>>>>> +		ret = IRQ_HANDLED;
>>>>
>>>> you can avoid the local variable by returning early here.
>>>
>>> How can we return early? we need to check irq0_status as well right?
>> 
>> Oh, that's true.
>> 
>> There's one thing that I noticed though. dma_status is only written to,
>> never read, so you should be able to remove it completely (a bit
>> off-topic, sorry).
>> 
> I'll send a patch for that. Can it go in -rc as well along with the other 2?

probably not, as it's not fixing any bug ;-)

-- 
balbi

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

Re: [PATCH v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> On 11/04/16 15:18, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Roger Quadros <rogerq@ti.com> writes:
>>> Don't make any decisions regarding VBUS session based on ID
>>> status. That is best left to the OTG core.
>> 
>> what about builds who don't want OTG and/or dual-role ?
>> 
>>> 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 | 15 ++++++---------
>>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>>> index 51ca098..c9b918d 100644
>>> --- a/drivers/usb/dwc3/dwc3-omap.c
>>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>>> @@ -233,19 +233,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
>>> -				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
>>> +		val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG;
>> 
>> this creates the possibility of having a USB peripheral with VBUS_VALID,
>> right 
>
> Sorry, I didn't get what you meant.

if you're touching these bits independently from each other, won't you
have a situation where you notify IDFLOAT but don't notify that VBUS is
off ?

-- 
balbi

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

Re: [PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> On 11/04/16 15:23, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Roger Quadros <rogerq@ti.com> writes:
>>> Gadget controller might not be always active during suspend/
>>> resume when we are operating in dual-role/otg mode.
>>> Check if we're active and only if we are then perform
>>> necessary actions during suspend/resume.
>> 
>> I don't get this. If we're operating in OTG, we should have a gadget
>> driver loaded, no ?
>> 
> At boot gadget driver is not automatically loaded. We're still in OTG mode
> but OTG state machine hasn't started.
> System suspend/resume can still happen.
>
> User might also load/unload the gadget driver prior to system suspend.

good point, this should go in the -rc too.

-- 
balbi

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

Re: [PATCH v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently

[Roger Quadros]

On 11/04/16 16:26, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> On 11/04/16 15:18, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Roger Quadros <rogerq@ti.com> writes:
>>>> Don't make any decisions regarding VBUS session based on ID
>>>> status. That is best left to the OTG core.
>>>
>>> what about builds who don't want OTG and/or dual-role ?
>>>
>>>> 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 | 15 ++++++---------
>>>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>>>> index 51ca098..c9b918d 100644
>>>> --- a/drivers/usb/dwc3/dwc3-omap.c
>>>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>>>> @@ -233,19 +233,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
>>>> -				| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT;
>>>> +		val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG;
>>>
>>> this creates the possibility of having a USB peripheral with VBUS_VALID,
>>> right 
>>
>> Sorry, I didn't get what you meant.
> 
> if you're touching these bits independently from each other, won't you
> have a situation where you notify IDFLOAT but don't notify that VBUS is
> off ?
> 
We're setting the mailbox state for both ID and VBUS in dwc3_omap_extcon_register()
depending on the initial extcon state. After that, ID and VBUS will be signalled
only when they change.

cheers,
-roger

Re: [PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode

[Roger Quadros]

On 11/04/16 16:26, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> On 11/04/16 15:23, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Roger Quadros <rogerq@ti.com> writes:
>>>> Gadget controller might not be always active during suspend/
>>>> resume when we are operating in dual-role/otg mode.
>>>> Check if we're active and only if we are then perform
>>>> necessary actions during suspend/resume.
>>>
>>> I don't get this. If we're operating in OTG, we should have a gadget
>>> driver loaded, no ?
>>>
>> At boot gadget driver is not automatically loaded. We're still in OTG mode
>> but OTG state machine hasn't started.
>> System suspend/resume can still happen.
>>
>> User might also load/unload the gadget driver prior to system suspend.
> 
> good point, this should go in the -rc too.
> 
But there is no dual-role mode currently so it won't fix any bug yet :).

cheers,
-roger

Re: [PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode

[Felipe Balbi]

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
> On 11/04/16 16:26, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Roger Quadros <rogerq@ti.com> writes:
>>> On 11/04/16 15:23, Felipe Balbi wrote:
>>>>
>>>> Hi,
>>>>
>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>> Gadget controller might not be always active during suspend/
>>>>> resume when we are operating in dual-role/otg mode.
>>>>> Check if we're active and only if we are then perform
>>>>> necessary actions during suspend/resume.
>>>>
>>>> I don't get this. If we're operating in OTG, we should have a gadget
>>>> driver loaded, no ?
>>>>
>>> At boot gadget driver is not automatically loaded. We're still in OTG mode
>>> but OTG state machine hasn't started.
>>> System suspend/resume can still happen.
>>>
>>> User might also load/unload the gadget driver prior to system suspend.
>> 
>> good point, this should go in the -rc too.
>> 
> But there is no dual-role mode currently so it won't fix any bug yet :).

this should be a problem even for device-only, right ?

i) boot-up
ii) modprobe dwc3
iii) echo mem > /sys/power/state

-- 
balbi

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

Re: [PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode

[Roger Quadros]

On 12/04/16 11:00, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> On 11/04/16 16:26, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Roger Quadros <rogerq@ti.com> writes:
>>>> On 11/04/16 15:23, Felipe Balbi wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>> Gadget controller might not be always active during suspend/
>>>>>> resume when we are operating in dual-role/otg mode.
>>>>>> Check if we're active and only if we are then perform
>>>>>> necessary actions during suspend/resume.
>>>>>
>>>>> I don't get this. If we're operating in OTG, we should have a gadget
>>>>> driver loaded, no ?
>>>>>
>>>> At boot gadget driver is not automatically loaded. We're still in OTG mode
>>>> but OTG state machine hasn't started.
>>>> System suspend/resume can still happen.
>>>>
>>>> User might also load/unload the gadget driver prior to system suspend.
>>>
>>> good point, this should go in the -rc too.
>>>
>> But there is no dual-role mode currently so it won't fix any bug yet :).
> 
> this should be a problem even for device-only, right ?
> 
> i) boot-up
> ii) modprobe dwc3
> iii) echo mem > /sys/power/state
> 
Indeed. It is applicable for device-only mode as well. I'll send this patch for rc then.

cheers,
-roger