[PATCH v2 0/6] Cleanup and fixes for dwc3-omap

[PATCH v2 0/6] Cleanup and fixes for dwc3-omap

[George Cherian]

The series does some refactoring on dwc3_probe()

Patch 1 - Now that we use driver compatible for revision check, remove the unnecessary logic.
Patch 2-4 - reduce the size of dwc3_probe()
Patch 5 - Fix the crash on dwc3_omap removal
Patch 6 - Addresses the issue of  xhci hang while resuming from system sleep.


George Cherian (6):
  usb: dwc3: dwc3-omap: Remove x_major calculation from revision
    register
  usb: dwc3: dwc3-omap: Add dwc3_omap_map_offset() function
  usb: dwc3: dwc3-omap: Add dwc3_omap_set_utmi_mode() function
  usb: dwc3: dwc3-omap: Add dwc3_omap_extcon_register function
  usb: dwc3: dwc3-omap: Fix the crash on module removal
  usb: dwc3: dwc3-omap: Disable/Enable only wrapper interrupts in
    prepare/complete

 drivers/usb/dwc3/dwc3-omap.c | 186 ++++++++++++++++++++++---------------------
 1 file changed, 97 insertions(+), 89 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/6] usb: dwc3: dwc3-omap: Remove x_major calculation from revision register

[George Cherian]

Remove the x_major calculation logic from the wrapper revision register
to differentiate between OMAP5 and AM437x. This was done to find the
register offsets of wrapper register. Now that We do it using dt
compatible, remove the whole logic.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 35 +++--------------------------------
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 1160ff4..53f6490 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -77,10 +77,6 @@
 #define USBOTGSS_DEV_EBC_EN			0x0110
 #define USBOTGSS_DEBUG_OFFSET			0x0600
 
-/* REVISION REGISTER */
-#define USBOTGSS_REVISION_XMAJOR(reg)		((reg >> 8) & 0x7)
-#define USBOTGSS_REVISION_XMAJOR1		1
-#define USBOTGSS_REVISION_XMAJOR2		2
 /* SYSCONFIG REGISTER */
 #define USBOTGSS_SYSCONFIG_DMADISABLE		(1 << 16)
 
@@ -129,7 +125,6 @@ struct dwc3_omap {
 	u32			irq_eoi_offset;
 	u32			debug_offset;
 	u32			irq0_offset;
-	u32			revision;
 
 	u32			dma_status:1;
 
@@ -397,7 +392,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	int			irq;
 
 	int			utmi_mode = 0;
-	int			x_major;
 
 	u32			reg;
 
@@ -448,32 +442,9 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 		goto err0;
 	}
 
-	reg = dwc3_omap_readl(omap->base, USBOTGSS_REVISION);
-	omap->revision = reg;
-	x_major = USBOTGSS_REVISION_XMAJOR(reg);
-
-	/* Differentiate between OMAP5 and AM437x */
-	switch (x_major) {
-	case USBOTGSS_REVISION_XMAJOR1:
-	case USBOTGSS_REVISION_XMAJOR2:
-		omap->irq_eoi_offset = 0;
-		omap->irq0_offset = 0;
-		omap->irqmisc_offset = 0;
-		omap->utmi_otg_offset = 0;
-		omap->debug_offset = 0;
-		break;
-	default:
-		/* Default to the latest revision */
-		omap->irq_eoi_offset = USBOTGSS_EOI_OFFSET;
-		omap->irq0_offset = USBOTGSS_IRQ0_OFFSET;
-		omap->irqmisc_offset = USBOTGSS_IRQMISC_OFFSET;
-		omap->utmi_otg_offset = USBOTGSS_UTMI_OTG_OFFSET;
-		omap->debug_offset = USBOTGSS_DEBUG_OFFSET;
-		break;
-	}
-
-	/* For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
-	 * changes in wrapper registers, Using dt compatible for aegis
+	/* Differentiate between OMAP5 and AM437x
+	 * For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
+	 * changes in wrapper registers, Using dt compatible for AM437x
 	 */
 
 	if (of_device_is_compatible(node, "ti,am437x-dwc3")) {
-- 
1.8.3.1

Re: [PATCH v2 1/6] usb: dwc3: dwc3-omap: Remove x_major calculation from revision register

[Sergei Shtylyov]

Hello.

On 19-05-2014 12:32, George Cherian wrote:

> Remove the x_major calculation logic from the wrapper revision register
> to differentiate between OMAP5 and AM437x. This was done to find the
> register offsets of wrapper register. Now that We do it using dt
> compatible, remove the whole logic.

> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>   drivers/usb/dwc3/dwc3-omap.c | 35 +++--------------------------------
>   1 file changed, 3 insertions(+), 32 deletions(-)

> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index 1160ff4..53f6490 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
[...]
> @@ -448,32 +442,9 @@ static int dwc3_omap_probe(struct platform_device *pdev)
[...]
> -	/* For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
> -	 * changes in wrapper registers, Using dt compatible for aegis
> +	/* Differentiate between OMAP5 and AM437x
> +	 * For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
> +	 * changes in wrapper registers, Using dt compatible for AM437x

    This sentence doesn't look complete...

>   	 */
>
>   	if (of_device_is_compatible(node, "ti,am437x-dwc3")) {

WBR, Sergei

[PATCH v2 2/6] usb: dwc3: dwc3-omap: Add dwc3_omap_map_offset() function

[George Cherian]

Move map offset to its own seperate function.
Improve code readability, decrease the dwc3_probe() size.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 53f6490..0df8adf 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -378,6 +378,24 @@ static int dwc3_omap_vbus_notifier(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+static void dwc3_omap_map_offset(struct dwc3_omap *omap)
+{
+	struct device_node	*node = omap->dev->of_node;
+
+	/* Differentiate between OMAP5 and AM437x
+	 * For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
+	 * changes in wrapper registers, Using dt compatible for AM437x
+	 */
+
+	if (of_device_is_compatible(node, "ti,am437x-dwc3")) {
+		omap->irq_eoi_offset = USBOTGSS_EOI_OFFSET;
+		omap->irq0_offset = USBOTGSS_IRQ0_OFFSET;
+		omap->irqmisc_offset = USBOTGSS_IRQMISC_OFFSET;
+		omap->utmi_otg_offset = USBOTGSS_UTMI_OTG_OFFSET;
+		omap->debug_offset = USBOTGSS_DEBUG_OFFSET;
+	}
+}
+
 static int dwc3_omap_probe(struct platform_device *pdev)
 {
 	struct device_node	*node = pdev->dev.of_node;
@@ -442,18 +460,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 		goto err0;
 	}
 
-	/* Differentiate between OMAP5 and AM437x
-	 * For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
-	 * changes in wrapper registers, Using dt compatible for AM437x
-	 */
-
-	if (of_device_is_compatible(node, "ti,am437x-dwc3")) {
-		omap->irq_eoi_offset = USBOTGSS_EOI_OFFSET;
-		omap->irq0_offset = USBOTGSS_IRQ0_OFFSET;
-		omap->irqmisc_offset = USBOTGSS_IRQMISC_OFFSET;
-		omap->utmi_otg_offset = USBOTGSS_UTMI_OTG_OFFSET;
-		omap->debug_offset = USBOTGSS_DEBUG_OFFSET;
-	}
+	dwc3_omap_map_offset(omap);
 
 	reg = dwc3_omap_read_utmi_status(omap);
 
-- 
1.8.3.1

[PATCH v2 3/6] usb: dwc3: dwc3-omap: Add dwc3_omap_set_utmi_mode() function

[George Cherian]

Move find and set the utmi mode to its own seperate function.
Improve code readability, decrease the dwc3_probe() size.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 0df8adf..2223ab8 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -396,6 +396,30 @@ static void dwc3_omap_map_offset(struct dwc3_omap *omap)
 	}
 }
 
+static void dwc3_omap_set_utmi_mode(struct dwc3_omap *omap)
+{
+	u32			reg;
+	struct device_node	*node = omap->dev->of_node;
+	int			utmi_mode = 0;
+
+	reg = dwc3_omap_read_utmi_status(omap);
+
+	of_property_read_u32(node, "utmi-mode", &utmi_mode);
+
+	switch (utmi_mode) {
+	case DWC3_OMAP_UTMI_MODE_SW:
+		reg |= USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
+		break;
+	case DWC3_OMAP_UTMI_MODE_HW:
+		reg &= ~USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
+		break;
+	default:
+		dev_dbg(omap->dev, "UNKNOWN utmi mode %d\n", utmi_mode);
+	}
+
+	dwc3_omap_write_utmi_status(omap, reg);
+}
+
 static int dwc3_omap_probe(struct platform_device *pdev)
 {
 	struct device_node	*node = pdev->dev.of_node;
@@ -409,8 +433,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	int			ret = -ENOMEM;
 	int			irq;
 
-	int			utmi_mode = 0;
-
 	u32			reg;
 
 	void __iomem		*base;
@@ -461,23 +483,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	}
 
 	dwc3_omap_map_offset(omap);
-
-	reg = dwc3_omap_read_utmi_status(omap);
-
-	of_property_read_u32(node, "utmi-mode", &utmi_mode);
-
-	switch (utmi_mode) {
-	case DWC3_OMAP_UTMI_MODE_SW:
-		reg |= USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
-		break;
-	case DWC3_OMAP_UTMI_MODE_HW:
-		reg &= ~USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
-		break;
-	default:
-		dev_dbg(dev, "UNKNOWN utmi mode %d\n", utmi_mode);
-	}
-
-	dwc3_omap_write_utmi_status(omap, reg);
+	dwc3_omap_set_utmi_mode(omap);
 
 	/* check the DMA Status */
 	reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
-- 
1.8.3.1

[PATCH v2 4/6] usb: dwc3: dwc3-omap: Add dwc3_omap_extcon_register function

[George Cherian]

Move the extcon related code to its own function.
Improve code readability, decrease the dwc3_probe() size.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 65 ++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 2223ab8..131d75a 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -420,6 +420,42 @@ static void dwc3_omap_set_utmi_mode(struct dwc3_omap *omap)
 	dwc3_omap_write_utmi_status(omap, reg);
 }
 
+static int dwc3_omap_extcon_register(struct dwc3_omap *omap)
+{
+	u32			ret;
+	struct device_node	*node = omap->dev->of_node;
+	struct extcon_dev	*edev;
+
+	if (of_property_read_bool(node, "extcon")) {
+		edev = extcon_get_edev_by_phandle(omap->dev, 0);
+		if (IS_ERR(edev)) {
+			dev_vdbg(omap->dev, "couldn't get extcon device\n");
+			return -EPROBE_DEFER;
+		}
+
+		omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
+		ret = extcon_register_interest(&omap->extcon_vbus_dev,
+					       edev->name, "USB",
+					       &omap->vbus_nb);
+		if (ret < 0)
+			dev_vdbg(omap->dev, "failed to register notifier for USB\n");
+
+		omap->id_nb.notifier_call = dwc3_omap_id_notifier;
+		ret = extcon_register_interest(&omap->extcon_id_dev,
+					       edev->name, "USB-HOST",
+					       &omap->id_nb);
+		if (ret < 0)
+			dev_vdbg(omap->dev, "failed to register notifier for USB-HOST\n");
+
+		if (extcon_get_cable_state(edev, "USB") == true)
+			dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+		if (extcon_get_cable_state(edev, "USB-HOST") == true)
+			dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+	}
+
+	return 0;
+}
+
 static int dwc3_omap_probe(struct platform_device *pdev)
 {
 	struct device_node	*node = pdev->dev.of_node;
@@ -427,7 +463,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	struct dwc3_omap	*omap;
 	struct resource		*res;
 	struct device		*dev = &pdev->dev;
-	struct extcon_dev	*edev;
 	struct regulator	*vbus_reg = NULL;
 
 	int			ret = -ENOMEM;
@@ -499,31 +534,9 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 
 	dwc3_omap_enable_irqs(omap);
 
-	if (of_property_read_bool(node, "extcon")) {
-		edev = extcon_get_edev_by_phandle(dev, 0);
-		if (IS_ERR(edev)) {
-			dev_vdbg(dev, "couldn't get extcon device\n");
-			ret = -EPROBE_DEFER;
-			goto err2;
-		}
-
-		omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
-		ret = extcon_register_interest(&omap->extcon_vbus_dev,
-			edev->name, "USB", &omap->vbus_nb);
-		if (ret < 0)
-			dev_vdbg(dev, "failed to register notifier for USB\n");
-		omap->id_nb.notifier_call = dwc3_omap_id_notifier;
-		ret = extcon_register_interest(&omap->extcon_id_dev, edev->name,
-					 "USB-HOST", &omap->id_nb);
-		if (ret < 0)
-			dev_vdbg(dev,
-				"failed to register notifier for USB-HOST\n");
-
-		if (extcon_get_cable_state(edev, "USB") == true)
-			dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
-		if (extcon_get_cable_state(edev, "USB-HOST") == true)
-			dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
-	}
+	ret = dwc3_omap_extcon_register(omap);
+	if (ret < 0)
+		goto err2;
 
 	ret = of_platform_populate(node, NULL, NULL, dev);
 	if (ret) {
-- 
1.8.3.1

[PATCH v2 5/6] usb: dwc3: dwc3-omap: Fix the crash on module removal

[George Cherian]

Following crash is seen on dwc3_omap removal
Unable to handle kernel NULL pointer dereference at virtual address 00000018
pgd = ec098000
[00000018] *pgd=ad1f9831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] SMP ARM
Modules linked in: usb_f_ss_lb g_zero usb_f_acm u_serial usb_f_ecm u_ether libcomposite configfs snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_hwdep snd_soc_omap snd_pcm_dmaengine snd_soc_core snd_compress snd_pcm snd_tim]
CPU: 0 PID: 1296 Comm: rmmod Tainted: G        W     3.15.0-rc4-02716-g95c4e18-dirty #10
task: ed05a080 ti: ec368000 task.ti: ec368000
PC is at release_resource+0x14/0x7c
LR is at release_resource+0x10/0x7c
pc : [<c0044724>]    lr : [<c0044720>]    psr: 60000013
sp : ec369ec0  ip : 60000013  fp : 00021008
r10: 00000000  r9 : ec368000  r8 : c000e7a4
r7 : 00000081  r6 : bf0062c0  r5 : ed7cd000  r4 : ed7d85c0
r3 : 00000000  r2 : 00000000  r1 : 00000011  r0 : c086d08c
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c5387d  Table: ac098059  DAC: 00000015
Process rmmod (pid: 1296, stack limit = 0xec368248)
Stack: (0xec369ec0 to 0xec36a000)
9ec0: 00000000 00000001 ed7cd000 c034de94 ed7cd010 ed7cd000 00000000 c034e194
9ee0: 00000000 bf0062cc ed7cd010 c03490b0 ed154cc0 ed4c2570 ed2b8410 ed156810
ed156810 bf006d24 c034db9c c034db84 c034c518
9f20: bf006d24 ed156810 bf006d24 c034cd2c bf006d24 bf006d68 00000800 c034c340
9f40: 00000000 c00a9e5c 00000020 00000000 bf006d68 00000800 ec369f4c 33637764
9f60: 616d6f5f 00000070 00000001 ec368000 ed05a080 c000e670 00000001 c0084010
9f80: 00021088 00000800 00021088 00000081 80000010 0000e6f4 00021088 00000800
9fa0: 00021088 c000e5e0 00021088 00000800 000210b8 00000800 e04f6d00 e04f6d00
9fc0: 00021088 00000800 00021088 00000081 00000001 00000000 be91de08 00021008
9fe0: 4d768880 be91dbb4 b6fc5984 4d76888c 80000010 000210b8 00000000 00000000
[<c0044724>] (release_resource) from [<c034de94>] (platform_device_del+0x6c/0x9c)
[<c034de94>] (platform_device_del) from [<c034e194>] (platform_device_unregister+0xc/0x18)
[<c034e194>] (platform_device_unregister) from [<bf0062cc>] (dwc3_omap_remove_core+0xc/0x14 [dwc3_omap])
[<bf0062cc>] (dwc3_omap_remove_core [dwc3_omap]) from [<c03490b0>] (device_for_each_child+0x34/0x74)
[<c03490b0>] (device_for_each_child) from [<bf0062b4>] (dwc3_omap_remove+0x6c/0x78 [dwc3_omap])
[<bf0062b4>] (dwc3_omap_remove [dwc3_omap]) from [<c034db9c>] (platform_drv_remove+0x18/0x1c)
[<c034db9c>] (platform_drv_remove) from [<c034c518>] (__device_release_driver+0x70/0xc8)
[<c034c518>] (__device_release_driver) from [<c034cd2c>] (driver_detach+0xb4/0xb8)
[<c034cd2c>] (driver_detach) from [<c034c340>] (bus_remove_driver+0x4c/0x90)
[<c034c340>] (bus_remove_driver) from [<c00a9e5c>] (SyS_delete_module+0x10c/0x198)
[<c00a9e5c>] (SyS_delete_module) from [<c000e5e0>] (ret_fast_syscall+0x0/0x48)
Code: e1a04000 e59f0068 eb14505e e5943010 (e5932018)
---[ end trace 7e2a8746ff4fc811 ]---
Segmentation fault

Signed-off-by: George Cherian <george.cherian@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 131d75a..e4f681a 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -317,7 +317,7 @@ static int dwc3_omap_remove_core(struct device *dev, void *c)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 
-	platform_device_unregister(pdev);
+	of_device_unregister(pdev);
 
 	return 0;
 }
-- 
1.8.3.1

[PATCH v2 6/6] usb: dwc3: dwc3-omap: Disable/Enable only wrapper interrupts in prepare/complete

[George Cherian]

The dwc3 wrapper driver should not be fiddling with the core interrupts.
Disabling the core interrupts in prepare stops xhci from proper operation.
So remove disable/enable of core interrupts from prepare/complete.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/dwc3/dwc3-omap.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index e4f681a..3f86f29 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -596,7 +596,7 @@ static int dwc3_omap_prepare(struct device *dev)
 {
 	struct dwc3_omap	*omap = dev_get_drvdata(dev);
 
-	dwc3_omap_disable_irqs(omap);
+	dwc3_omap_write_irqmisc_set(omap, 0x00);
 
 	return 0;
 }
@@ -604,8 +604,19 @@ static int dwc3_omap_prepare(struct device *dev)
 static void dwc3_omap_complete(struct device *dev)
 {
 	struct dwc3_omap	*omap = dev_get_drvdata(dev);
+	u32			reg;
 
-	dwc3_omap_enable_irqs(omap);
+	reg = (USBOTGSS_IRQMISC_OEVT |
+			USBOTGSS_IRQMISC_DRVVBUS_RISE |
+			USBOTGSS_IRQMISC_CHRGVBUS_RISE |
+			USBOTGSS_IRQMISC_DISCHRGVBUS_RISE |
+			USBOTGSS_IRQMISC_IDPULLUP_RISE |
+			USBOTGSS_IRQMISC_DRVVBUS_FALL |
+			USBOTGSS_IRQMISC_CHRGVBUS_FALL |
+			USBOTGSS_IRQMISC_DISCHRGVBUS_FALL |
+			USBOTGSS_IRQMISC_IDPULLUP_FALL);
+
+	dwc3_omap_write_irqmisc_set(omap, reg);
 }
 
 static int dwc3_omap_suspend(struct device *dev)
-- 
1.8.3.1