[RESEND PATCH v3 0/4] Cleanup for dwc3-omap

[RESEND PATCH v3 0/4] Cleanup 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()

George Cherian (4):
  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

 drivers/usb/dwc3/dwc3-omap.c | 170 +++++++++++++++++++++----------------------
 1 file changed, 84 insertions(+), 86 deletions(-)

-- 
1.8.3.1

[RESEND PATCH v3 1/4] 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 | 36 ++++--------------------------------
 1 file changed, 4 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 4af4c35..999bdc8 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,10 @@ 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 wrapper revision is same, even
+	 * though there are changes in wrapper register offsets.
+	 * Using dt compatible to differentiate  AM437x.
 	 */
 
 	if (of_device_is_compatible(node, "ti,am437x-dwc3")) {
-- 
1.8.3.1

[RESEND PATCH v3 2/4] 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 | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 999bdc8..09918ac 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -378,6 +378,25 @@ 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 wrapper revision is same  even
+	 * though there are changes in wrapper register offsets.
+	 * Using dt compatible to differentiate  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,19 +461,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 		goto err0;
 	}
 
-	/* Differentiate between OMAP5 and AM437x.
-	 * For OMAP5(ES2.0) and AM437x wrapper revision is same, even
-	 * though there are changes in wrapper register offsets.
-	 * Using dt compatible to differentiate  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

[RESEND PATCH v3 3/4] 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 09918ac..56ec6eb 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -397,6 +397,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;
@@ -410,8 +434,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 	int			ret;
 	int			irq;
 
-	int			utmi_mode = 0;
-
 	u32			reg;
 
 	void __iomem		*base;
@@ -462,23 +484,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

[RESEND PATCH v3 4/4] 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 56ec6eb..7594535 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -421,6 +421,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;
@@ -428,7 +464,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;
@@ -500,31 +535,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

Re: [RESEND PATCH v3 2/4] usb: dwc3: dwc3-omap: Add dwc3_omap_map_offset() function

[Sergei Shtylyov]

Hello.

On 07/16/2014 05:07 PM, George Cherian wrote:

> Move map offset to its own seperate function.

    /seperate/separate/.

> Improve code readability, decrease the dwc3_probe() size.

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

> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index 999bdc8..09918ac 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -378,6 +378,25 @@ 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 wrapper revision is same  even

   I don't think removing comma before "even" was correct.

> +	 * though there are changes in wrapper register offsets.
> +	 * Using dt compatible to differentiate  AM437x.

    s/dt/DT/.

> +	 */
> +
> +	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,19 +461,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>   		goto err0;
>   	}
>
> -	/* Differentiate between OMAP5 and AM437x.
> -	 * For OMAP5(ES2.0) and AM437x wrapper revision is same, even
> -	 * though there are changes in wrapper register offsets.
> -	 * Using dt compatible to differentiate  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);

WBR, Sergei