[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Peter Chen]

This patchset adds tested otg id switch function and
vbus connect and disconnect detection for chipidea driver.
And fix kinds of bugs found at chipidea drivers after enabling
id and vbus detection.

This patch is fully tested at imx6 sabresd platform.
My chipidea repo: https://github.com/hzpeterchen/linux-usb.git

Changes for v12:
- Rebased greg's usb-next tree (3.10.0-rc7+)
- Split more small patches for single function and fix.

Changes for v11:
- mark ci_handle_vbus_change as static as it is only used at core.c
[3/9]
- Move the vbus operation for platform code to host code, as vbus
operation is common operation, and host is the only user for vbus.
When it is host mode, we need to open vbus, when it is out of host
mode, we need to close vbus. [6/9] [8/9]
- Delete the delayed work at core.c as it is not needed. [7/9]

Changes for v10:
- Delete [8/9] at v9, ci core's drvdata must be set for further operation.
[8/8]

Changes for v9:
- Some small comments from Alex like: variable comment for otg event
additional newline. [3/9]
- Import function tell show if the controller has otg capable, if
the controller supports both host and device, we think it is otg
capable, and can read OTGSC. [3/9]
- Merge two otg patches [v8 3/8] and [v8 4/8] to one [v9 3/9]. [3/9]
- Add inline to ci_hdrc_gadget_destroy if CONFIG_USB_CHIPIDEA_UDC
is not defined, it can fix one build warning "defined but not used"
[3/9]
- One comment from Felipe about changing calling gadget disconnect
API at chipidea's udc driver. I move calling ci->driver->disconnect
from _gadget_stop_activity to which calls _gadget_stop_activity except
ci13xxx_stop, as udc core will call disconnect when do rmmod gadget. [7/9]
- Add ci core probe's return value to ci's platform_data, we do this
for getting core's probe's result at platform layer, and quit it
if the core's probe fails. [8/9] [9/9]

Changes for v8:
- Add ci_supports_gadget helper to know if the controller
supports gadget, if the controller supports gadget, it
needs to read otgsc to know the vbus value, basically,
if the controller supports gadget, it will support host
as well [3/8]
- At ci_hdrc_probe, it needs to add free memory at error path
[3/8]
- Cosolidate ci->driver = NULL at ci13xxx_stop
[8/8]

Changes for v7:
For Patch 8/8, we only need to set ci->driver to NULL when usb cable
is not connected, for other changes, it will case some runtime pm
unmatch and un-align with udc-core & composite driver problems.

Changes for v6:
- Add Alex comments for init/destroy function [3/8] [4/8]
- Remove memset(&ci->gadget, 0, sizeof(ci->gadget)) at destory function [4/8]
- Add Kishon's comment: Change the format of struct usb_otg otg at drivers/usb/chipidea/ci.h
[1/8]
- Add comments for CI_VBUS_STABLE_TIMEOUT [3/8]
- Change the otg_set_peripheral return value check as the fully
chipidea driver users don't need it. [4/8]
- Fix one bug that the oops when re-plug in usb cable after
rmmod gadget [8/8]

Peter Chen (13):
  usb: chipidea: add vbus regulator control
  usb: chipidea: imx: remove vbus regulator operation
  usb: chipidea: udc: otg_set_peripheral is useless for some chipidea
    users
  usb: chipidea: otg: Add otg file used to access otgsc
  usb: chipidea: Add role init and destory APIs
  usb: chipidea: add otg_cap attribute for otg capable
  usb: chipidea: disable all interrupts and clear all interrupts status
  usb: chipidea: move otg relate things to otg file
  usb: chipidea: add vbus interrupt handler
  usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts
  usb: chipidea: udc: misuse flag CI_HDRC_REGS_SHARED and
    CI_HDRC_PULLUP_ON_VBUS
  usb: chipidea: udc: .pullup is valid when vbus is on at
    CI_HDRC_PULLUP_ON_VBUS
  usb: chipidea: udc: fix the oops when plugs in usb cable after rmmod
    gadget

 drivers/usb/chipidea/Makefile      |    2 +-
 drivers/usb/chipidea/bits.h        |   10 +++
 drivers/usb/chipidea/ci.h          |    8 ++
 drivers/usb/chipidea/ci_hdrc_imx.c |   30 ++-----
 drivers/usb/chipidea/core.c        |  158 +++++++++++++++++++++++------------
 drivers/usb/chipidea/host.c        |   30 +++++++-
 drivers/usb/chipidea/host.h        |    6 ++
 drivers/usb/chipidea/otg.c         |  135 ++++++++++++++++++++++++++++++
 drivers/usb/chipidea/otg.h         |   22 +++++
 drivers/usb/chipidea/udc.c         |   72 +++++++++++++----
 drivers/usb/chipidea/udc.h         |    6 ++
 include/linux/usb/chipidea.h       |   14 +++
 12 files changed, 397 insertions(+), 96 deletions(-)
 create mode 100644 drivers/usb/chipidea/otg.c
 create mode 100644 drivers/usb/chipidea/otg.h

[PATCH v12 01/13] usb: chipidea: add vbus regulator control

[Peter Chen]

For boards which have board level vbus control (eg, through gpio), we
need to vbus operation according to below rules:
- For host, we need open vbus before start hcd, and close it
after remove hcd.
- For otg, the vbus needs to be on/off when usb role switches.
When the host roles begins, it opens vbus; when the host role
finishes, it closes vbus.

We put vbus operation to host as host is the only vbus user,
When we are at host mode, the vbus is on, when we are not at
host mode, vbus should be off.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/host.c  |   23 ++++++++++++++++++++++-
 include/linux/usb/chipidea.h |    1 +
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 40d0fda..e94e52b 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -24,6 +24,7 @@
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
 #include <linux/usb/chipidea.h>
+#include <linux/regulator/consumer.h>
 
 #include "../host/ehci.h"
 
@@ -64,9 +65,19 @@ static int host_start(struct ci_hdrc *ci)
 	ehci->caps = ci->hw_bank.cap;
 	ehci->has_hostpc = ci->hw_bank.lpm;
 
+	if (ci->platdata->reg_vbus) {
+		ret = regulator_enable(ci->platdata->reg_vbus);
+		if (ret) {
+			dev_err(ci->dev,
+				"Failed to enable vbus regulator, ret=%d\n",
+				ret);
+			goto put_hcd;
+		}
+	}
+
 	ret = usb_add_hcd(hcd, 0, 0);
 	if (ret)
-		usb_put_hcd(hcd);
+		goto disable_reg;
 	else
 		ci->hcd = hcd;
 
@@ -74,6 +85,14 @@ static int host_start(struct ci_hdrc *ci)
 		hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
 
 	return ret;
+
+disable_reg:
+	regulator_disable(ci->platdata->reg_vbus);
+
+put_hcd:
+	usb_put_hcd(hcd);
+
+	return ret;
 }
 
 static void host_stop(struct ci_hdrc *ci)
@@ -82,6 +101,8 @@ static void host_stop(struct ci_hdrc *ci)
 
 	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
+	if (ci->platdata->reg_vbus)
+		regulator_disable(ci->platdata->reg_vbus);
 }
 
 int ci_hdrc_host_init(struct ci_hdrc *ci)
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 2562994..118bf66 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -24,6 +24,7 @@ struct ci_hdrc_platform_data {
 #define CI_HDRC_CONTROLLER_RESET_EVENT		0
 #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1
 	void	(*notify_event) (struct ci_hdrc *ci, unsigned event);
+	struct regulator *reg_vbus;
 };
 
 /* Default offset of capability registers */
-- 
1.7.0.4

[PATCH v12 02/13] usb: chipidea: imx: remove vbus regulator operation

[Peter Chen]

Since we have added vbus reguatlor operation at common
host file (chipidea/host.c), the glue layer vbus operation
isn't needed any more.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/ci_hdrc_imx.c |   30 +++++++-----------------------
 1 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 14362c0..d06355e 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -31,7 +31,6 @@ struct ci_hdrc_imx_data {
 	struct usb_phy *phy;
 	struct platform_device *ci_pdev;
 	struct clk *clk;
-	struct regulator *reg_vbus;
 };
 
 static const struct usbmisc_ops *usbmisc_ops;
@@ -141,22 +140,13 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
-	/* we only support host now, so enable vbus here */
-	data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
-	if (!IS_ERR(data->reg_vbus)) {
-		ret = regulator_enable(data->reg_vbus);
-		if (ret) {
-			dev_err(&pdev->dev,
-				"Failed to enable vbus regulator, err=%d\n",
-				ret);
-			goto err_clk;
-		}
-	} else {
-		data->reg_vbus = NULL;
-	}
-
 	pdata.phy = data->phy;
 
+	/* Get the vbus regulator */
+	pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
+	if (IS_ERR(pdata.reg_vbus))
+		pdata.reg_vbus = NULL;
+
 	if (!pdev->dev.dma_mask)
 		pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
 	if (!pdev->dev.coherent_dma_mask)
@@ -167,7 +157,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(&pdev->dev,
 				"usbmisc init failed, ret=%d\n", ret);
-			goto err;
+			goto err_clk;
 		}
 	}
 
@@ -179,7 +169,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev,
 			"Can't register ci_hdrc platform device, err=%d\n",
 			ret);
-		goto err;
+		goto err_clk;
 	}
 
 	if (usbmisc_ops && usbmisc_ops->post) {
@@ -200,9 +190,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 
 disable_device:
 	ci_hdrc_remove_device(data->ci_pdev);
-err:
-	if (data->reg_vbus)
-		regulator_disable(data->reg_vbus);
 err_clk:
 	clk_disable_unprepare(data->clk);
 	return ret;
@@ -215,9 +202,6 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	ci_hdrc_remove_device(data->ci_pdev);
 
-	if (data->reg_vbus)
-		regulator_disable(data->reg_vbus);
-
 	if (data->phy) {
 		usb_phy_shutdown(data->phy);
 		module_put(data->phy->dev->driver->owner);
-- 
1.7.0.4

[PATCH v12 02/13] usb: chipidea: imx: remove vbus regulator operation

[Sascha Hauer]

On Thu, Jul 11, 2013 at 02:27:10PM +0800, Peter Chen wrote:
> Since we have added vbus reguatlor operation at common
> host file (chipidea/host.c), the glue layer vbus operation
> isn't needed any more.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c |   30 +++++++-----------------------
>  1 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 14362c0..d06355e 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -31,7 +31,6 @@ struct ci_hdrc_imx_data {
>  	struct usb_phy *phy;
>  	struct platform_device *ci_pdev;
>  	struct clk *clk;
> -	struct regulator *reg_vbus;
>  };
>  
>  static const struct usbmisc_ops *usbmisc_ops;
> @@ -141,22 +140,13 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  		goto err_clk;
>  	}
>  
> -	/* we only support host now, so enable vbus here */
> -	data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> -	if (!IS_ERR(data->reg_vbus)) {
> -		ret = regulator_enable(data->reg_vbus);
> -		if (ret) {
> -			dev_err(&pdev->dev,
> -				"Failed to enable vbus regulator, err=%d\n",
> -				ret);
> -			goto err_clk;
> -		}
> -	} else {
> -		data->reg_vbus = NULL;
> -	}
> -
>  	pdata.phy = data->phy;
>  
> +	/* Get the vbus regulator */
> +	pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> +	if (IS_ERR(pdata.reg_vbus))
> +		pdata.reg_vbus = NULL;

I think you should bail out at least in the -EPROBE_DEFER case.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v12 02/13] usb: chipidea: imx: remove vbus regulator operation

[Peter Chen]

On Thu, Jul 11, 2013 at 08:37:19AM +0200, Sascha Hauer wrote:
> >  
> > -	/* we only support host now, so enable vbus here */
> > -	data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > -	if (!IS_ERR(data->reg_vbus)) {
> > -		ret = regulator_enable(data->reg_vbus);
> > -		if (ret) {
> > -			dev_err(&pdev->dev,
> > -				"Failed to enable vbus regulator, err=%d\n",
> > -				ret);
> > -			goto err_clk;
> > -		}
> > -	} else {
> > -		data->reg_vbus = NULL;
> > -	}
> > -
> >  	pdata.phy = data->phy;
> >  
> > +	/* Get the vbus regulator */
> > +	pdata.reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> > +	if (IS_ERR(pdata.reg_vbus))
> > +		pdata.reg_vbus = NULL;
> 
> I think you should bail out at least in the -EPROBE_DEFER case.
> 

Oh, correct. I will add another patch behind this for fix.
In fact, I met this problem at FSL mx6-auto board which uses
gpio-expendor chip max7310 as regulator-gpio for enable vbus
output.

-- 

Best Regards,
Peter Chen

[PATCH v12 03/13] usb: chipidea: udc: otg_set_peripheral is useless for some chipidea users

[Peter Chen]

It is useless at below cases:
- If we implement both usb host and device at chipidea driver.
- If we don't need phy->otg.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/udc.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index e475fcd..116c762 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1805,7 +1805,12 @@ static int udc_start(struct ci_hdrc *ci)
 	if (ci->transceiver) {
 		retval = otg_set_peripheral(ci->transceiver->otg,
 						&ci->gadget);
-		if (retval)
+		/*
+		 * If we implement all USB functions using chipidea drivers,
+		 * it doesn't need to call above API, meanwhile, if we only
+		 * use gadget function, calling above API is useless.
+		 */
+		if (retval && retval != -ENOTSUPP)
 			goto put_transceiver;
 	}
 
-- 
1.7.0.4

[PATCH v12 04/13] usb: chipidea: otg: Add otg file used to access otgsc

[Peter Chen]

This file is mainly used to access otgsc currently, it may
add otg related things in the future.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/Makefile |    2 +-
 drivers/usb/chipidea/bits.h   |   10 ++++++++
 drivers/usb/chipidea/core.c   |    3 +-
 drivers/usb/chipidea/otg.c    |   50 +++++++++++++++++++++++++++++++++++++++++
 drivers/usb/chipidea/otg.h    |   19 +++++++++++++++
 5 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 3bbbcba..9d0e288 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -2,7 +2,7 @@ ccflags-$(CONFIG_USB_CHIPIDEA_DEBUG) := -DDEBUG
 
 obj-$(CONFIG_USB_CHIPIDEA)		+= ci_hdrc.o
 
-ci_hdrc-y				:= core.o
+ci_hdrc-y				:= core.o otg.o
 ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC)	+= udc.o
 ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)	+= host.o
 ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG)	+= debug.o
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index aefa026..dd0cf9e 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -79,11 +79,21 @@
 #define OTGSC_ASVIS	      BIT(18)
 #define OTGSC_BSVIS	      BIT(19)
 #define OTGSC_BSEIS	      BIT(20)
+#define OTGSC_1MSIS	      BIT(21)
+#define OTGSC_DPIS	      BIT(22)
 #define OTGSC_IDIE	      BIT(24)
 #define OTGSC_AVVIE	      BIT(25)
 #define OTGSC_ASVIE	      BIT(26)
 #define OTGSC_BSVIE	      BIT(27)
 #define OTGSC_BSEIE	      BIT(28)
+#define OTGSC_1MSIE	      BIT(29)
+#define OTGSC_DPIE	      BIT(30)
+#define OTGSC_INT_EN_BITS	(OTGSC_IDIE | OTGSC_AVVIE | OTGSC_ASVIE \
+				| OTGSC_BSVIE | OTGSC_BSEIE | OTGSC_1MSIE \
+				| OTGSC_DPIE)
+#define OTGSC_INT_STATUS_BITS	(OTGSC_IDIS | OTGSC_AVVIS | OTGSC_ASVIS	\
+				| OTGSC_BSVIS | OTGSC_BSEIS | OTGSC_1MSIS \
+				| OTGSC_DPIS)
 
 /* USBMODE */
 #define USBMODE_CM            (0x03UL <<  0)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index a5df24c..761f7e8 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -71,6 +71,7 @@
 #include "bits.h"
 #include "host.h"
 #include "debug.h"
+#include "otg.h"
 
 /* Controller register map */
 static uintptr_t ci_regs_nolpm[] = {
@@ -508,7 +509,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		goto stop;
 
 	if (ci->is_otg)
-		hw_write(ci, OP_OTGSC, OTGSC_IDIE, OTGSC_IDIE);
+		ci_hdrc_otg_init(ci);
 
 	ret = dbg_create_files(ci);
 	if (!ret)
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
new file mode 100644
index 0000000..abefb4d
--- /dev/null
+++ b/drivers/usb/chipidea/otg.c
@@ -0,0 +1,50 @@
+/*
+ * otg.c - ChipIdea USB IP core OTG driver
+ *
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * Author: Peter Chen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * This file mainly handles otgsc register, it may include OTG operation
+ * in the future.
+ */
+
+#include <linux/usb/otg.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/chipidea.h>
+
+#include "ci.h"
+#include "bits.h"
+
+void ci_clear_otg_interrupt(struct ci_hdrc *ci, u32 bits)
+{
+	/* Only clear request bits */
+	hw_write(ci, OP_OTGSC, OTGSC_INT_STATUS_BITS, bits);
+}
+
+void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
+{
+	hw_write(ci, OP_OTGSC, bits, bits);
+}
+
+void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
+{
+	hw_write(ci, OP_OTGSC, bits, 0);
+}
+
+/**
+ * ci_hdrc_otg_init - initialize otgsc bits
+ * ci: the controller
+ */
+int ci_hdrc_otg_init(struct ci_hdrc *ci)
+{
+	ci_enable_otg_interrupt(ci, OTGSC_IDIE);
+
+	return 0;
+}
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
new file mode 100644
index 0000000..f24ec37
--- /dev/null
+++ b/drivers/usb/chipidea/otg.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * Author: Peter Chen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __DRIVERS_USB_CHIPIDEA_OTG_H
+#define __DRIVERS_USB_CHIPIDEA_OTG_H
+
+int ci_hdrc_otg_init(struct ci_hdrc *ci);
+void ci_clear_otg_interrupt(struct ci_hdrc *ci, u32 bits);
+void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
+void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
+
+#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
-- 
1.7.0.4

[PATCH v12 05/13] usb: chipidea: Add role init and destory APIs

[Peter Chen]

- The role's init will be called at probe procedure.
- The role's destory will be called at fail patch
at probe and driver's removal.
- The role's start/stop will be called when specific
role has started.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/core.c |   10 ++++++++--
 drivers/usb/chipidea/host.c |    7 +++++++
 drivers/usb/chipidea/host.h |    6 ++++++
 drivers/usb/chipidea/udc.c  |   36 +++++++++++++++++++++++++++---------
 drivers/usb/chipidea/udc.h  |    6 ++++++
 5 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 761f7e8..93961ff 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -399,6 +399,12 @@ void ci_hdrc_remove_device(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(ci_hdrc_remove_device);
 
+static inline void ci_role_destroy(struct ci_hdrc *ci)
+{
+	ci_hdrc_gadget_destroy(ci);
+	ci_hdrc_host_destroy(ci);
+}
+
 static int ci_hdrc_probe(struct platform_device *pdev)
 {
 	struct device	*dev = &pdev->dev;
@@ -517,7 +523,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 	free_irq(ci->irq, ci);
 stop:
-	ci_role_stop(ci);
+	ci_role_destroy(ci);
 rm_wq:
 	flush_workqueue(ci->wq);
 	destroy_workqueue(ci->wq);
@@ -533,7 +539,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
 	flush_workqueue(ci->wq);
 	destroy_workqueue(ci->wq);
 	free_irq(ci->irq, ci);
-	ci_role_stop(ci);
+	ci_role_destroy(ci);
 
 	return 0;
 }
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index e94e52b..382be5b 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -105,6 +105,13 @@ static void host_stop(struct ci_hdrc *ci)
 		regulator_disable(ci->platdata->reg_vbus);
 }
 
+
+void ci_hdrc_host_destroy(struct ci_hdrc *ci)
+{
+	if (ci->role == CI_ROLE_HOST)
+		host_stop(ci);
+}
+
 int ci_hdrc_host_init(struct ci_hdrc *ci)
 {
 	struct ci_role_driver *rdrv;
diff --git a/drivers/usb/chipidea/host.h b/drivers/usb/chipidea/host.h
index 058875c..5707bf3 100644
--- a/drivers/usb/chipidea/host.h
+++ b/drivers/usb/chipidea/host.h
@@ -4,6 +4,7 @@
 #ifdef CONFIG_USB_CHIPIDEA_HOST
 
 int ci_hdrc_host_init(struct ci_hdrc *ci);
+void ci_hdrc_host_destroy(struct ci_hdrc *ci);
 
 #else
 
@@ -12,6 +13,11 @@ static inline int ci_hdrc_host_init(struct ci_hdrc *ci)
 	return -ENXIO;
 }
 
+static inline void ci_hdrc_host_destroy(struct ci_hdrc *ci)
+{
+
+}
+
 #endif
 
 #endif /* __DRIVERS_USB_CHIPIDEA_HOST_H */
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 116c762..24a100d 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -27,6 +27,7 @@
 #include "udc.h"
 #include "bits.h"
 #include "debug.h"
+#include "otg.h"
 
 /* control endpoint description */
 static const struct usb_endpoint_descriptor
@@ -1844,13 +1845,13 @@ free_qh_pool:
 }
 
 /**
- * udc_remove: parent remove must call this to remove UDC
+ * ci_hdrc_gadget_destroy: parent remove must call this to remove UDC
  *
  * No interrupts active, the IRQ has been released
  */
-static void udc_stop(struct ci_hdrc *ci)
+void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
 {
-	if (ci == NULL)
+	if (!ci->roles[CI_ROLE_GADGET])
 		return;
 
 	usb_del_gadget_udc(&ci->gadget);
@@ -1865,15 +1866,32 @@ static void udc_stop(struct ci_hdrc *ci)
 		if (ci->global_phy)
 			usb_put_phy(ci->transceiver);
 	}
-	/* my kobject is dynamic, I swear! */
-	memset(&ci->gadget, 0, sizeof(ci->gadget));
+}
+
+static int udc_id_switch_for_device(struct ci_hdrc *ci)
+{
+	if (ci->is_otg) {
+		ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+		ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+	}
+
+	return 0;
+}
+
+static void udc_id_switch_for_host(struct ci_hdrc *ci)
+{
+	if (ci->is_otg) {
+		/* host doesn't care B_SESSION_VALID event */
+		ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+		ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
+	}
 }
 
 /**
  * ci_hdrc_gadget_init - initialize device related bits
  * ci: the controller
  *
- * This function enables the gadget role, if the device is "device capable".
+ * This function initializes the gadget, if the device is "device capable".
  */
 int ci_hdrc_gadget_init(struct ci_hdrc *ci)
 {
@@ -1886,11 +1904,11 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
 	if (!rdrv)
 		return -ENOMEM;
 
-	rdrv->start	= udc_start;
-	rdrv->stop	= udc_stop;
+	rdrv->start	= udc_id_switch_for_device;
+	rdrv->stop	= udc_id_switch_for_host;
 	rdrv->irq	= udc_irq;
 	rdrv->name	= "gadget";
 	ci->roles[CI_ROLE_GADGET] = rdrv;
 
-	return 0;
+	return udc_start(ci);
 }
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index 455ac21..e66df00 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -84,6 +84,7 @@ struct ci_hw_req {
 #ifdef CONFIG_USB_CHIPIDEA_UDC
 
 int ci_hdrc_gadget_init(struct ci_hdrc *ci);
+void ci_hdrc_gadget_destroy(struct ci_hdrc *ci);
 
 #else
 
@@ -92,6 +93,11 @@ static inline int ci_hdrc_gadget_init(struct ci_hdrc *ci)
 	return -ENXIO;
 }
 
+static inline void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
+{
+
+}
+
 #endif
 
 #endif /* __DRIVERS_USB_CHIPIDEA_UDC_H */
-- 
1.7.0.4

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Peter Chen]

Since we need otgsc to know vbus's status at some chipidea
controllers even it is peripheral-only mode. Besides, some
SoCs (eg, AR9331 SoC) don't have otgsc register even
the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
We inroduce otg_cap attribute to indicate if the controller
is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
is exception, the platform can override it by device tree or platform data.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/core.c  |   35 ++++++++++++++++++++++++++++-------
 include/linux/usb/chipidea.h |   13 +++++++++++++
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 93961ff..e8ceb04 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -405,6 +405,18 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
 	ci_hdrc_host_destroy(ci);
 }
 
+static void ci_get_otg_capable(struct ci_hdrc *ci)
+{
+	if (ci->platdata->otg_cap != OTG_CAP_ATTR_IS_NOT_EXISTED)
+		ci->is_otg = (ci->platdata->otg_cap == OTG_CAP_ATTR_IS_TRUE);
+	else
+		ci->is_otg = (hw_read(ci, CAP_DCCPARAMS,
+				DCCPARAMS_DC | DCCPARAMS_HC)
+					== (DCCPARAMS_DC | DCCPARAMS_HC));
+	if (ci->is_otg)
+		dev_dbg(ci->dev, "It is OTG capable controller\n");
+}
+
 static int ci_hdrc_probe(struct platform_device *pdev)
 {
 	struct device	*dev = &pdev->dev;
@@ -461,6 +473,9 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	/* To know if controller is OTG capable or not */
+	ci_get_otg_capable(ci);
+
 	if (!ci->platdata->phy_mode)
 		ci->platdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
 
@@ -491,10 +506,19 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	}
 
 	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
-		ci->is_otg = true;
-		/* ID pin needs 1ms debouce time, we delay 2ms for safe */
-		mdelay(2);
-		ci->role = ci_otg_role(ci);
+		if (ci->is_otg) {
+			/* ID pin needs 1ms debouce time, we delay 2ms for safe */
+			mdelay(2);
+			ci->role = ci_otg_role(ci);
+			ci_hdrc_otg_init(ci);
+		} else {
+			/*
+			 * If the controller is not OTG capable, but support
+			 * role switch, the defalt role is gadget, and the
+			 * user can switch it through debugfs (proc in future?)
+			 */
+			ci->role = CI_ROLE_GADGET;
+		}
 	} else {
 		ci->role = ci->roles[CI_ROLE_HOST]
 			? CI_ROLE_HOST
@@ -514,9 +538,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret)
 		goto stop;
 
-	if (ci->is_otg)
-		ci_hdrc_otg_init(ci);
-
 	ret = dbg_create_files(ci);
 	if (!ret)
 		return 0;
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 118bf66..0a906b4 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -7,6 +7,12 @@
 
 #include <linux/usb/otg.h>
 
+enum usb_otg_cap {
+	OTG_CAP_ATTR_IS_NOT_EXISTED = 0,
+	OTG_CAP_ATTR_IS_TRUE,
+	OTG_CAP_ATTR_IS_FALSE,
+};
+
 struct ci_hdrc;
 struct ci_hdrc_platform_data {
 	const char	*name;
@@ -25,6 +31,13 @@ struct ci_hdrc_platform_data {
 #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1
 	void	(*notify_event) (struct ci_hdrc *ci, unsigned event);
 	struct regulator *reg_vbus;
+	/*
+	 * Please only set otg_cap when the otg capability can't be
+	 * judged by CAP_DCCPARAMS, eg, the DCCPARAMS_DC and DCCPARAMS_HC
+	 * are both 1 at CAP_DCCPARAMS, but the controller doesn't have
+	 * OTGSC register (eg, AR9331 SoC).
+	 */
+	enum usb_otg_cap otg_cap;
 };
 
 /* Default offset of capability registers */
-- 
1.7.0.4

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Marek Vasut]

Dear Peter Chen,

> Since we need otgsc to know vbus's status at some chipidea
> controllers even it is peripheral-only mode. Besides, some
> SoCs (eg, AR9331 SoC) don't have otgsc register even
> the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> We inroduce otg_cap attribute to indicate if the controller
> is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
> is exception, the platform can override it by device tree or platform data.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  drivers/usb/chipidea/core.c  |   35 ++++++++++++++++++++++++++++-------
>  include/linux/usb/chipidea.h |   13 +++++++++++++
>  2 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 93961ff..e8ceb04 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -405,6 +405,18 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
>  	ci_hdrc_host_destroy(ci);
>  }
> 
> +static void ci_get_otg_capable(struct ci_hdrc *ci)
> +{
> +	if (ci->platdata->otg_cap != OTG_CAP_ATTR_IS_NOT_EXISTED)

IS_NONEXISTENT , no?

[..]

Best regards,
Marek Vasut

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Peter Chen]

On Thu, Jul 11, 2013 at 05:36:10PM +0200, Marek Vasut wrote:
> Dear Peter Chen,
> 
> > Since we need otgsc to know vbus's status at some chipidea
> > controllers even it is peripheral-only mode. Besides, some
> > SoCs (eg, AR9331 SoC) don't have otgsc register even
> > the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> > We inroduce otg_cap attribute to indicate if the controller
> > is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> > and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
> > is exception, the platform can override it by device tree or platform data.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> >  drivers/usb/chipidea/core.c  |   35 ++++++++++++++++++++++++++++-------
> >  include/linux/usb/chipidea.h |   13 +++++++++++++
> >  2 files changed, 41 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 93961ff..e8ceb04 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -405,6 +405,18 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
> >  	ci_hdrc_host_destroy(ci);
> >  }
> > 
> > +static void ci_get_otg_capable(struct ci_hdrc *ci)
> > +{
> > +	if (ci->platdata->otg_cap != OTG_CAP_ATTR_IS_NOT_EXISTED)
> 
> IS_NONEXISTENT , no?
> 

You mean the English for this string? ok, I can change.
It means if the "otg_cap" is not existed at platform data,
then, the "otg_cap" can be determined by DCCPARAMS

-- 

Best Regards,
Peter Chen

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Marek Vasut]

Dear Peter Chen,

> On Thu, Jul 11, 2013 at 05:36:10PM +0200, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > Since we need otgsc to know vbus's status at some chipidea
> > > controllers even it is peripheral-only mode. Besides, some
> > > SoCs (eg, AR9331 SoC) don't have otgsc register even
> > > the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> > > We inroduce otg_cap attribute to indicate if the controller
> > > is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> > > and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if
> > > there is exception, the platform can override it by device tree or
> > > platform data.
> > > 
> > > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > > ---
> > > 
> > >  drivers/usb/chipidea/core.c  |   35
> > >  ++++++++++++++++++++++++++++------- include/linux/usb/chipidea.h |  
> > >  13 +++++++++++++
> > >  2 files changed, 41 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index 93961ff..e8ceb04 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -405,6 +405,18 @@ static inline void ci_role_destroy(struct ci_hdrc
> > > *ci)
> > > 
> > >  	ci_hdrc_host_destroy(ci);
> > >  
> > >  }
> > > 
> > > +static void ci_get_otg_capable(struct ci_hdrc *ci)
> > > +{
> > > +	if (ci->platdata->otg_cap != OTG_CAP_ATTR_IS_NOT_EXISTED)
> > 
> > IS_NONEXISTENT , no?
> 
> You mean the English for this string? ok, I can change.

Yes, IS_NOT_EXISTED doesn't make much sense. Sorry, don't mean to nitpick.

> It means if the "otg_cap" is not existed at platform data,
> then, the "otg_cap" can be determined by DCCPARAMS

Then maybe just call it OTG_CAP_ATTR_NOT_SET or something .

Best regards,
Marek Vasut

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Alexander Shishkin]

Peter Chen <peter.chen@freescale.com> writes:

> Since we need otgsc to know vbus's status at some chipidea
> controllers even it is peripheral-only mode. Besides, some
> SoCs (eg, AR9331 SoC) don't have otgsc register even
> the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> We inroduce otg_cap attribute to indicate if the controller
> is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
> is exception, the platform can override it by device tree or platform data.
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---

[...]

> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> index 118bf66..0a906b4 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -7,6 +7,12 @@
>  
>  #include <linux/usb/otg.h>
>  
> +enum usb_otg_cap {
> +	OTG_CAP_ATTR_IS_NOT_EXISTED = 0,
> +	OTG_CAP_ATTR_IS_TRUE,
> +	OTG_CAP_ATTR_IS_FALSE,
> +};

We don't really need all three, do we? We only need to know if the
controller *can't* do otg. The rest we can infer from dr_mode and
DCCPARAMS. So it can be another bit in flags rather than a separate
field in platdata.

Regards,
--
Alex

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Peter Chen]

On Fri, Jul 12, 2013 at 11:12:01AM +0300, Alexander Shishkin wrote:
> Peter Chen <peter.chen@freescale.com> writes:
> 
> > Since we need otgsc to know vbus's status at some chipidea
> > controllers even it is peripheral-only mode. Besides, some
> > SoCs (eg, AR9331 SoC) don't have otgsc register even
> > the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> > We inroduce otg_cap attribute to indicate if the controller
> > is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> > and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
> > is exception, the platform can override it by device tree or platform data.
> >
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> 
> [...]
> 
> > diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> > index 118bf66..0a906b4 100644
> > --- a/include/linux/usb/chipidea.h
> > +++ b/include/linux/usb/chipidea.h
> > @@ -7,6 +7,12 @@
> >  
> >  #include <linux/usb/otg.h>
> >  
> > +enum usb_otg_cap {
> > +	OTG_CAP_ATTR_IS_NOT_EXISTED = 0,
> > +	OTG_CAP_ATTR_IS_TRUE,
> > +	OTG_CAP_ATTR_IS_FALSE,
> > +};
> 
> We don't really need all three, do we? We only need to know if the
> controller *can't* do otg. The rest we can infer from dr_mode and
> DCCPARAMS. So it can be another bit in flags rather than a separate
> field in platdata.
> 

In order to cover below two cases, I have no other idea.

1. For mips Soc (ARxxx), if dr_mode = otg and it is 1 for both
mode at DCCPARAMS,  It can switch role through proc or whatever,
but NO otgsc register, it is NOT otg-capable.

2. For peripheral-only case at most chipidea controllers,
the dr_mode = peripheral and it is 1 for both modes at DCCPARAMS,
the otgsc can be visited, it is otg-capable.

-- 

Best Regards,
Peter Chen

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Alexander Shishkin]

Peter Chen <peter.chen@freescale.com> writes:

> On Fri, Jul 12, 2013 at 11:12:01AM +0300, Alexander Shishkin wrote:
>> Peter Chen <peter.chen@freescale.com> writes:
>> 
>> > Since we need otgsc to know vbus's status at some chipidea
>> > controllers even it is peripheral-only mode. Besides, some
>> > SoCs (eg, AR9331 SoC) don't have otgsc register even
>> > the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
>> > We inroduce otg_cap attribute to indicate if the controller
>> > is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
>> > and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
>> > is exception, the platform can override it by device tree or platform data.
>> >
>> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
>> > ---
>> 
>> [...]
>> 
>> > diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
>> > index 118bf66..0a906b4 100644
>> > --- a/include/linux/usb/chipidea.h
>> > +++ b/include/linux/usb/chipidea.h
>> > @@ -7,6 +7,12 @@
>> >  
>> >  #include <linux/usb/otg.h>
>> >  
>> > +enum usb_otg_cap {
>> > +	OTG_CAP_ATTR_IS_NOT_EXISTED = 0,
>> > +	OTG_CAP_ATTR_IS_TRUE,
>> > +	OTG_CAP_ATTR_IS_FALSE,
>> > +};
>> 
>> We don't really need all three, do we? We only need to know if the
>> controller *can't* do otg. The rest we can infer from dr_mode and
>> DCCPARAMS. So it can be another bit in flags rather than a separate
>> field in platdata.
>> 
>
> In order to cover below two cases, I have no other idea.
>
> 1. For mips Soc (ARxxx), if dr_mode = otg and it is 1 for both
> mode at DCCPARAMS,  It can switch role through proc or whatever,
> but NO otgsc register, it is NOT otg-capable.

For this case, platform should set

platdata->flags &= CI_HDRC_NO_OTG

, which will set ci->is_otg = false. No ci->is_otg => no touching
OTGSC.

> 2. For peripheral-only case at most chipidea controllers,
> the dr_mode = peripheral and it is 1 for both modes at DCCPARAMS,
> the otgsc can be visited, it is otg-capable.

Then platform *doesn't* set CI_HDRC_NO_OTG and ci->is_otg is set to
true, because DCCPARAMS.DC==1 and DCCPARAMS.HC==1, but we only have one
role initialized, because of dr_mode==peripheral, so no role switching
happens, but OTGSC accesses are fine.

Makes sense?

Regards,
--
Alex

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Peter Chen]

On Fri, Jul 12, 2013 at 12:42:10PM +0300, Alexander Shishkin wrote:
> Peter Chen <peter.chen@freescale.com> writes:
> 
> > On Fri, Jul 12, 2013 at 11:12:01AM +0300, Alexander Shishkin wrote:
> >> Peter Chen <peter.chen@freescale.com> writes:
> >> 
> >> > Since we need otgsc to know vbus's status at some chipidea
> >> > controllers even it is peripheral-only mode. Besides, some
> >> > SoCs (eg, AR9331 SoC) don't have otgsc register even
> >> > the DCCPARAMS_DC and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS.
> >> > We inroduce otg_cap attribute to indicate if the controller
> >> > is otg capable, defaultly, we follow the rule that if DCCPARAMS_DC
> >> > and DCCPARAMS_HC are both 1 at CAP_DCCPARAMS are otg capable, but if there
> >> > is exception, the platform can override it by device tree or platform data.
> >> >
> >> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> >> > ---
> >> 
> >> [...]
> >> 
> >> > diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> >> > index 118bf66..0a906b4 100644
> >> > --- a/include/linux/usb/chipidea.h
> >> > +++ b/include/linux/usb/chipidea.h
> >> > @@ -7,6 +7,12 @@
> >> >  
> >> >  #include <linux/usb/otg.h>
> >> >  
> >> > +enum usb_otg_cap {
> >> > +	OTG_CAP_ATTR_IS_NOT_EXISTED = 0,
> >> > +	OTG_CAP_ATTR_IS_TRUE,
> >> > +	OTG_CAP_ATTR_IS_FALSE,
> >> > +};
> >> 
> >> We don't really need all three, do we? We only need to know if the
> >> controller *can't* do otg. The rest we can infer from dr_mode and
> >> DCCPARAMS. So it can be another bit in flags rather than a separate
> >> field in platdata.
> >> 
> >
> > In order to cover below two cases, I have no other idea.
> >
> > 1. For mips Soc (ARxxx), if dr_mode = otg and it is 1 for both
> > mode at DCCPARAMS,  It can switch role through proc or whatever,
> > but NO otgsc register, it is NOT otg-capable.
> 
> For this case, platform should set
> 
> platdata->flags &= CI_HDRC_NO_OTG
> 
> , which will set ci->is_otg = false. No ci->is_otg => no touching
> OTGSC.
> 
> > 2. For peripheral-only case at most chipidea controllers,
> > the dr_mode = peripheral and it is 1 for both modes at DCCPARAMS,
> > the otgsc can be visited, it is otg-capable.
> 
> Then platform *doesn't* set CI_HDRC_NO_OTG and ci->is_otg is set to
> true, because DCCPARAMS.DC==1 and DCCPARAMS.HC==1, but we only have one
> role initialized, because of dr_mode==peripheral, so no role switching
> happens, but OTGSC accesses are fine.
> 
> Makes sense?
> 

Agree, I will add the define of CI_HDRC_NO_OTG to
struct ci_hdrc_platform_data, and add below comments

/* Only set it when DCCPARAMS.DC==1 and DCCPARAMS.HC==1,
 * but otg is not supported (no register otgsc).
 */

-- 

Best Regards,
Peter Chen

[PATCH v12 06/13] usb: chipidea: add otg_cap attribute for otg capable

[Peter Chen]

On Fri, Jul 12, 2013 at 12:42:10PM +0300, Alexander Shishkin wrote:
> Peter Chen <peter.chen@freescale.com> writes:
> 

Hi Alex, do you have any other comments for this patchset?
If you haven't, I will send the v13 patchset with current
comments.

-- 

Best Regards,
Peter Chen

[PATCH v12 07/13] usb: chipidea: disable all interrupts and clear all interrupts status

[Peter Chen]

During the initialization, it needs to disable all interrupts
enable bit as well as clear all interrupts status bits to avoid
exceptional interrupt.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/core.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index e8ceb04..f701e3b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -198,6 +198,12 @@ static int hw_device_init(struct ci_hdrc *ci, void __iomem *base)
 	if (ci->hw_ep_max > ENDPT_MAX)
 		return -ENODEV;
 
+	/* Disable all interrupts bits */
+	hw_write(ci, OP_USBINTR, 0xffffffff, 0);
+
+	/* Clear all interrupts status bits*/
+	hw_write(ci, OP_USBSTS, 0xffffffff, 0xffffffff);
+
 	dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",
 		ci->hw_bank.lpm, ci->hw_bank.cap, ci->hw_bank.op);
 
@@ -413,8 +419,11 @@ static void ci_get_otg_capable(struct ci_hdrc *ci)
 		ci->is_otg = (hw_read(ci, CAP_DCCPARAMS,
 				DCCPARAMS_DC | DCCPARAMS_HC)
 					== (DCCPARAMS_DC | DCCPARAMS_HC));
-	if (ci->is_otg)
+	if (ci->is_otg) {
 		dev_dbg(ci->dev, "It is OTG capable controller\n");
+		ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS);
+		ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS);
+	}
 }
 
 static int ci_hdrc_probe(struct platform_device *pdev)
-- 
1.7.0.4

[PATCH v12 08/13] usb: chipidea: move otg relate things to otg file

[Peter Chen]

Move otg relate things to otg file.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/core.c |   63 +++++++++----------------------------------
 drivers/usb/chipidea/otg.c  |   57 +++++++++++++++++++++++++++++++++++++-
 drivers/usb/chipidea/otg.h  |    2 +
 3 files changed, 70 insertions(+), 52 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index f701e3b..0197da2 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -295,40 +295,6 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
 	return 0;
 }
 
-/**
- * ci_otg_role - pick role based on ID pin state
- * @ci: the controller
- */
-static enum ci_role ci_otg_role(struct ci_hdrc *ci)
-{
-	u32 sts = hw_read(ci, OP_OTGSC, ~0);
-	enum ci_role role = sts & OTGSC_ID
-		? CI_ROLE_GADGET
-		: CI_ROLE_HOST;
-
-	return role;
-}
-
-/**
- * ci_role_work - perform role changing based on ID pin
- * @work: work struct
- */
-static void ci_role_work(struct work_struct *work)
-{
-	struct ci_hdrc *ci = container_of(work, struct ci_hdrc, work);
-	enum ci_role role = ci_otg_role(ci);
-
-	if (role != ci->role) {
-		dev_dbg(ci->dev, "switching from %s to %s\n",
-			ci_role(ci)->name, ci->roles[role]->name);
-
-		ci_role_stop(ci);
-		ci_role_start(ci, role);
-	}
-
-	enable_irq(ci->irq);
-}
-
 static irqreturn_t ci_irq(int irq, void *data)
 {
 	struct ci_hdrc *ci = data;
@@ -409,6 +375,8 @@ static inline void ci_role_destroy(struct ci_hdrc *ci)
 {
 	ci_hdrc_gadget_destroy(ci);
 	ci_hdrc_host_destroy(ci);
+	if (ci->is_otg)
+		ci_hdrc_otg_destory(ci);
 }
 
 static void ci_get_otg_capable(struct ci_hdrc *ci)
@@ -475,13 +443,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	INIT_WORK(&ci->work, ci_role_work);
-	ci->wq = create_singlethread_workqueue("ci_otg");
-	if (!ci->wq) {
-		dev_err(dev, "can't create workqueue\n");
-		return -ENODEV;
-	}
-
 	/* To know if controller is OTG capable or not */
 	ci_get_otg_capable(ci);
 
@@ -510,8 +471,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
 		dev_err(dev, "no supported roles\n");
-		ret = -ENODEV;
-		goto rm_wq;
+		return -ENODEV;
+	}
+
+	if (ci->is_otg) {
+		ret = ci_hdrc_otg_init(ci);
+		if (ret) {
+			dev_err(dev, "init otg fails, ret = %d\n", ret);
+			goto stop;
+		}
 	}
 
 	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
@@ -519,7 +487,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 			/* ID pin needs 1ms debouce time, we delay 2ms for safe */
 			mdelay(2);
 			ci->role = ci_otg_role(ci);
-			ci_hdrc_otg_init(ci);
+			ci_enable_otg_interrupt(ci, OTGSC_IDIE);
 		} else {
 			/*
 			 * If the controller is not OTG capable, but support
@@ -538,7 +506,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(dev, "can't start %s role\n", ci_role(ci)->name);
 		ret = -ENODEV;
-		goto rm_wq;
+		goto stop;
 	}
 
 	platform_set_drvdata(pdev, ci);
@@ -554,9 +522,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	free_irq(ci->irq, ci);
 stop:
 	ci_role_destroy(ci);
-rm_wq:
-	flush_workqueue(ci->wq);
-	destroy_workqueue(ci->wq);
 
 	return ret;
 }
@@ -566,8 +531,6 @@ static int ci_hdrc_remove(struct platform_device *pdev)
 	struct ci_hdrc *ci = platform_get_drvdata(pdev);
 
 	dbg_remove_files(ci);
-	flush_workqueue(ci->wq);
-	destroy_workqueue(ci->wq);
 	free_irq(ci->irq, ci);
 	ci_role_destroy(ci);
 
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index abefb4d..68f2faf 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -39,12 +39,65 @@ void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
 }
 
 /**
- * ci_hdrc_otg_init - initialize otgsc bits
+ * ci_otg_role - pick role based on ID pin state
+ * @ci: the controller
+ */
+enum ci_role ci_otg_role(struct ci_hdrc *ci)
+{
+	u32 sts = hw_read(ci, OP_OTGSC, ~0);
+	enum ci_role role = sts & OTGSC_ID
+		? CI_ROLE_GADGET
+		: CI_ROLE_HOST;
+
+	return role;
+}
+
+/**
+ * ci_role_work - perform role changing based on ID pin
+ * @work: work struct
+ */
+static void ci_role_work(struct work_struct *work)
+{
+	struct ci_hdrc *ci = container_of(work, struct ci_hdrc, work);
+	enum ci_role role = ci_otg_role(ci);
+
+	if (role != ci->role) {
+		dev_dbg(ci->dev, "switching from %s to %s\n",
+			ci_role(ci)->name, ci->roles[role]->name);
+
+		ci_role_stop(ci);
+		ci_role_start(ci, role);
+	}
+
+	enable_irq(ci->irq);
+}
+
+/**
+ * ci_hdrc_otg_init - initialize otg struct
  * ci: the controller
  */
 int ci_hdrc_otg_init(struct ci_hdrc *ci)
 {
-	ci_enable_otg_interrupt(ci, OTGSC_IDIE);
+	INIT_WORK(&ci->work, ci_role_work);
+	ci->wq = create_singlethread_workqueue("ci_otg");
+	if (!ci->wq) {
+		dev_err(ci->dev, "can't create workqueue\n");
+		return -ENODEV;
+	}
 
 	return 0;
 }
+
+/**
+ * ci_hdrc_otg_destroy - destory otg struct
+ * ci: the controller
+ */
+void ci_hdrc_otg_destory(struct ci_hdrc *ci)
+{
+	if (ci->wq) {
+		flush_workqueue(ci->wq);
+		destroy_workqueue(ci->wq);
+	}
+	ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS);
+	ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS);
+}
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index f24ec37..8913062 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -15,5 +15,7 @@ int ci_hdrc_otg_init(struct ci_hdrc *ci);
 void ci_clear_otg_interrupt(struct ci_hdrc *ci, u32 bits);
 void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
 void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
+void ci_hdrc_otg_destory(struct ci_hdrc *ci);
+enum ci_role ci_otg_role(struct ci_hdrc *ci);
 
 #endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
-- 
1.7.0.4

[PATCH v12 09/13] usb: chipidea: add vbus interrupt handler

[Peter Chen]

We add vbus interrupt handler at ci_otg_work, it uses OTGSC_BSV(at otgsc)
to know it is connect or disconnet event.
Meanwhile, we introduce two flags id_event and b_sess_valid_event to
indicate it is an id interrupt or a vbus interrupt.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/ci.h   |    5 +++++
 drivers/usb/chipidea/core.c |   28 +++++++++++++++++++++++-----
 drivers/usb/chipidea/otg.c  |   42 +++++++++++++++++++++++++++++++++++-------
 drivers/usb/chipidea/otg.h  |    1 +
 drivers/usb/chipidea/udc.c  |    7 +++++++
 5 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 33cb29f..3160363 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -132,6 +132,9 @@ struct hw_bank {
  * @transceiver: pointer to USB PHY, if any
  * @hcd: pointer to usb_hcd for ehci host driver
  * @debugfs: root dentry for this controller in debugfs
+ * @id_event: indicates there is an id event, and handled at ci_otg_work
+ * @b_sess_valid_event: indicates there is a vbus event, and handled
+ * at ci_otg_work
  */
 struct ci_hdrc {
 	struct device			*dev;
@@ -168,6 +171,8 @@ struct ci_hdrc {
 	struct usb_phy			*transceiver;
 	struct usb_hcd			*hcd;
 	struct dentry			*debugfs;
+	bool				id_event;
+	bool				b_sess_valid_event;
 };
 
 static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 0197da2..7a07300 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -304,16 +304,34 @@ static irqreturn_t ci_irq(int irq, void *data)
 	if (ci->is_otg)
 		otgsc = hw_read(ci, OP_OTGSC, ~0);
 
-	if (ci->role != CI_ROLE_END)
-		ret = ci_role(ci)->irq(ci);
+	/*
+	 * Handle id change interrupt, it indicates device/host function
+	 * switch.
+	 */
+	if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
+		ci->id_event = true;
+		ci_clear_otg_interrupt(ci, OTGSC_IDIS);
+		disable_irq_nosync(ci->irq);
+		queue_work(ci->wq, &ci->work);
+		return IRQ_HANDLED;
+	}
 
-	if (ci->is_otg && (otgsc & OTGSC_IDIS)) {
-		hw_write(ci, OP_OTGSC, OTGSC_IDIS, OTGSC_IDIS);
+	/*
+	 * Handle vbus change interrupt, it indicates device connection
+	 * and disconnection events.
+	 */
+	if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
+		ci->b_sess_valid_event = true;
+		ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
 		disable_irq_nosync(ci->irq);
 		queue_work(ci->wq, &ci->work);
-		ret = IRQ_HANDLED;
+		return IRQ_HANDLED;
 	}
 
+	/* Handle device/host interrupt */
+	if (ci->role != CI_ROLE_END)
+		ret = ci_role(ci)->irq(ci);
+
 	return ret;
 }
 
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 68f2faf..c74194d 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -52,13 +52,23 @@ enum ci_role ci_otg_role(struct ci_hdrc *ci)
 	return role;
 }
 
-/**
- * ci_role_work - perform role changing based on ID pin
- * @work: work struct
- */
-static void ci_role_work(struct work_struct *work)
+void ci_handle_vbus_change(struct ci_hdrc *ci)
+{
+	u32 otgsc;
+
+	if (!ci->is_otg)
+		return;
+
+	otgsc = hw_read(ci, OP_OTGSC, ~0);
+
+	if (otgsc & OTGSC_BSV)
+		usb_gadget_vbus_connect(&ci->gadget);
+	else
+		usb_gadget_vbus_disconnect(&ci->gadget);
+}
+
+static void ci_handle_id_switch(struct ci_hdrc *ci)
 {
-	struct ci_hdrc *ci = container_of(work, struct ci_hdrc, work);
 	enum ci_role role = ci_otg_role(ci);
 
 	if (role != ci->role) {
@@ -68,17 +78,35 @@ static void ci_role_work(struct work_struct *work)
 		ci_role_stop(ci);
 		ci_role_start(ci, role);
 	}
+}
+/**
+ * ci_otg_work - perform otg (vbus/id) event handle
+ * @work: work struct
+ */
+static void ci_otg_work(struct work_struct *work)
+{
+	struct ci_hdrc *ci = container_of(work, struct ci_hdrc, work);
+
+	if (ci->id_event) {
+		ci->id_event = false;
+		ci_handle_id_switch(ci);
+	} else if (ci->b_sess_valid_event) {
+		ci->b_sess_valid_event = false;
+		ci_handle_vbus_change(ci);
+	} else
+		dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
 
 	enable_irq(ci->irq);
 }
 
+
 /**
  * ci_hdrc_otg_init - initialize otg struct
  * ci: the controller
  */
 int ci_hdrc_otg_init(struct ci_hdrc *ci)
 {
-	INIT_WORK(&ci->work, ci_role_work);
+	INIT_WORK(&ci->work, ci_otg_work);
 	ci->wq = create_singlethread_workqueue("ci_otg");
 	if (!ci->wq) {
 		dev_err(ci->dev, "can't create workqueue\n");
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 8913062..b8d5c05 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -17,5 +17,6 @@ void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
 void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits);
 void ci_hdrc_otg_destory(struct ci_hdrc *ci);
 enum ci_role ci_otg_role(struct ci_hdrc *ci);
+void ci_handle_vbus_change(struct ci_hdrc *ci);
 
 #endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 24a100d..c70ce38 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -85,8 +85,10 @@ static int hw_device_state(struct ci_hdrc *ci, u32 dma)
 		/* interrupt, error, port change, reset, sleep/suspend */
 		hw_write(ci, OP_USBINTR, ~0,
 			     USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
+		hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
 	} else {
 		hw_write(ci, OP_USBINTR, ~0, 0);
+		hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
 	}
 	return 0;
 }
@@ -1460,6 +1462,7 @@ static int ci_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
 			pm_runtime_get_sync(&_gadget->dev);
 			hw_device_reset(ci, USBMODE_CM_DC);
 			hw_device_state(ci, ci->ep0out->qh.dma);
+			dev_dbg(ci->dev, "Connected to host\n");
 		} else {
 			hw_device_state(ci, 0);
 			if (ci->platdata->notify_event)
@@ -1467,6 +1470,7 @@ static int ci_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
 				CI_HDRC_CONTROLLER_STOPPED_EVENT);
 			_gadget_stop_activity(&ci->gadget);
 			pm_runtime_put_sync(&_gadget->dev);
+			dev_dbg(ci->dev, "Disconnected from host\n");
 		}
 	}
 
@@ -1822,6 +1826,9 @@ static int udc_start(struct ci_hdrc *ci)
 	pm_runtime_no_callbacks(&ci->gadget.dev);
 	pm_runtime_enable(&ci->gadget.dev);
 
+	/* Update ci->vbus_active */
+	ci_handle_vbus_change(ci);
+
 	return retval;
 
 remove_trans:
-- 
1.7.0.4

[PATCH v12 10/13] usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts

[Peter Chen]

When the gadget role starts, we need to make sure the vbus is lower
than OTGSC_BSV, or there will be an vbus interrupt since we use
B_SESSION_VALID as vbus interrupt to indicate connect and disconnect.
When the host role starts, it may not be useful to wait vbus to lower
than OTGSC_BSV, but it can indicate some hardware problems like the
vbus is still higher than OTGSC_BSV after we disconnect to host some
time later (500 jiffies currently), which is obvious not correct.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/ci.h   |    3 +++
 drivers/usb/chipidea/core.c |   32 ++++++++++++++++++++++++++++++++
 drivers/usb/chipidea/otg.c  |    4 ++++
 3 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 3160363..df27696 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -308,4 +308,7 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
 
 u8 hw_port_test_get(struct ci_hdrc *ci);
 
+int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
+				u32 value, unsigned long timeout);
+
 #endif	/* __DRIVERS_USB_CHIPIDEA_CI_H */
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7a07300..1c39541 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -295,6 +295,38 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
 	return 0;
 }
 
+/**
+ * hw_wait_reg: wait the register value
+ *
+ * Sometimes, it needs to wait register value before going on.
+ * Eg, when switch to device mode, the vbus value should be lower
+ * than OTGSC_BSV before connects to host.
+ *
+ * @ci: the controller
+ * @reg: register index
+ * @mask: mast bit
+ * @value: the bit value to wait
+ * @timeout: timeout to indicate an error
+ *
+ * This function returns an error code if timeout
+ */
+int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
+				u32 value, unsigned long timeout)
+{
+	unsigned long elapse = jiffies + timeout;
+
+	while (hw_read(ci, reg, mask) != value) {
+		if (time_after(jiffies, elapse)) {
+			dev_err(ci->dev, "timeout waiting for %08x in %d\n",
+					mask, reg);
+			return -ETIMEDOUT;
+		}
+		msleep(20);
+	}
+
+	return 0;
+}
+
 static irqreturn_t ci_irq(int irq, void *data)
 {
 	struct ci_hdrc *ci = data;
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index c74194d..8aa0241 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -67,6 +67,7 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
 		usb_gadget_vbus_disconnect(&ci->gadget);
 }
 
+#define CI_VBUS_STABLE_TIMEOUT 500
 static void ci_handle_id_switch(struct ci_hdrc *ci)
 {
 	enum ci_role role = ci_otg_role(ci);
@@ -76,6 +77,9 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
 			ci_role(ci)->name, ci->roles[role]->name);
 
 		ci_role_stop(ci);
+		/* wait vbus lower than OTGSC_BSV */
+		hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
+				CI_VBUS_STABLE_TIMEOUT);
 		ci_role_start(ci, role);
 	}
 }
-- 
1.7.0.4

[PATCH v12 10/13] usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts

[Marc Kleine-Budde]

On 07/11/2013 08:27 AM, Peter Chen wrote:
> When the gadget role starts, we need to make sure the vbus is lower
> than OTGSC_BSV, or there will be an vbus interrupt since we use
> B_SESSION_VALID as vbus interrupt to indicate connect and disconnect.
> When the host role starts, it may not be useful to wait vbus to lower
> than OTGSC_BSV, but it can indicate some hardware problems like the
> vbus is still higher than OTGSC_BSV after we disconnect to host some
> time later (500 jiffies currently), which is obvious not correct.

Jiffies ist not constant. I think you have to specify the timeout in
fractions of HZ. The system ticks with HZ jiffies per second. So a
timeout of 500ms is "HZ / 2".

Marc

> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  drivers/usb/chipidea/ci.h   |    3 +++
>  drivers/usb/chipidea/core.c |   32 ++++++++++++++++++++++++++++++++
>  drivers/usb/chipidea/otg.c  |    4 ++++
>  3 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 3160363..df27696 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -308,4 +308,7 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
>  u8 hw_port_test_get(struct ci_hdrc *ci);
>  
> +int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> +				u32 value, unsigned long timeout);
> +
>  #endif	/* __DRIVERS_USB_CHIPIDEA_CI_H */
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 7a07300..1c39541 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -295,6 +295,38 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
>  	return 0;
>  }
>  
> +/**
> + * hw_wait_reg: wait the register value
> + *
> + * Sometimes, it needs to wait register value before going on.
> + * Eg, when switch to device mode, the vbus value should be lower
> + * than OTGSC_BSV before connects to host.
> + *
> + * @ci: the controller
> + * @reg: register index
> + * @mask: mast bit
> + * @value: the bit value to wait
> + * @timeout: timeout to indicate an error
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Better: timeout in jiffies

> + *
> + * This function returns an error code if timeout
> + */
> +int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> +				u32 value, unsigned long timeout)
> +{
> +	unsigned long elapse = jiffies + timeout;
> +
> +	while (hw_read(ci, reg, mask) != value) {
> +		if (time_after(jiffies, elapse)) {
> +			dev_err(ci->dev, "timeout waiting for %08x in %d\n",
> +					mask, reg);
> +			return -ETIMEDOUT;
> +		}
> +		msleep(20);
> +	}
> +
> +	return 0;
> +}
> +
>  static irqreturn_t ci_irq(int irq, void *data)
>  {
>  	struct ci_hdrc *ci = data;
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index c74194d..8aa0241 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -67,6 +67,7 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
>  		usb_gadget_vbus_disconnect(&ci->gadget);
>  }
>  
> +#define CI_VBUS_STABLE_TIMEOUT 500
>  static void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
>  	enum ci_role role = ci_otg_role(ci);
> @@ -76,6 +77,9 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
>  			ci_role(ci)->name, ci->roles[role]->name);
>  
>  		ci_role_stop(ci);
> +		/* wait vbus lower than OTGSC_BSV */
> +		hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
> +				CI_VBUS_STABLE_TIMEOUT);
>  		ci_role_start(ci, role);
>  	}
>  }
> 


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130711/cb139f86/attachment.sig>

[PATCH v12 10/13] usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts

[Peter Chen]

On Thu, Jul 11, 2013 at 09:24:56AM +0200, Marc Kleine-Budde wrote:
> On 07/11/2013 08:27 AM, Peter Chen wrote:
> > When the gadget role starts, we need to make sure the vbus is lower
> > than OTGSC_BSV, or there will be an vbus interrupt since we use
> > B_SESSION_VALID as vbus interrupt to indicate connect and disconnect.
> > When the host role starts, it may not be useful to wait vbus to lower
> > than OTGSC_BSV, but it can indicate some hardware problems like the
> > vbus is still higher than OTGSC_BSV after we disconnect to host some
> > time later (500 jiffies currently), which is obvious not correct.
> 
> Jiffies ist not constant. I think you have to specify the timeout in
> fractions of HZ. The system ticks with HZ jiffies per second. So a
> timeout of 500ms is "HZ / 2".

Thanks, I will use msecs_to_jiffies.

> > +/**
> > + * hw_wait_reg: wait the register value
> > + *
> > + * Sometimes, it needs to wait register value before going on.
> > + * Eg, when switch to device mode, the vbus value should be lower
> > + * than OTGSC_BSV before connects to host.
> > + *
> > + * @ci: the controller
> > + * @reg: register index
> > + * @mask: mast bit
> > + * @value: the bit value to wait
> > + * @timeout: timeout to indicate an error
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Better: timeout in jiffies
> 

As I will use msecs_to_jiffies, @timeout: timeout in millisecond.

Thanks.

-- 

Best Regards,
Peter Chen

[PATCH v12 10/13] usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts

[Marc Kleine-Budde]

On 07/11/2013 11:25 AM, Peter Chen wrote:
> On Thu, Jul 11, 2013 at 09:24:56AM +0200, Marc Kleine-Budde wrote:
>> On 07/11/2013 08:27 AM, Peter Chen wrote:
>>> When the gadget role starts, we need to make sure the vbus is lower
>>> than OTGSC_BSV, or there will be an vbus interrupt since we use
>>> B_SESSION_VALID as vbus interrupt to indicate connect and disconnect.
>>> When the host role starts, it may not be useful to wait vbus to lower
>>> than OTGSC_BSV, but it can indicate some hardware problems like the
>>> vbus is still higher than OTGSC_BSV after we disconnect to host some
>>> time later (500 jiffies currently), which is obvious not correct.
>>
>> Jiffies ist not constant. I think you have to specify the timeout in
>> fractions of HZ. The system ticks with HZ jiffies per second. So a
>> timeout of 500ms is "HZ / 2".
> 
> Thanks, I will use msecs_to_jiffies.

even better.

> 
>>> +/**
>>> + * hw_wait_reg: wait the register value
>>> + *
>>> + * Sometimes, it needs to wait register value before going on.
>>> + * Eg, when switch to device mode, the vbus value should be lower
>>> + * than OTGSC_BSV before connects to host.
>>> + *
>>> + * @ci: the controller
>>> + * @reg: register index
>>> + * @mask: mast bit
>>> + * @value: the bit value to wait
>>> + * @timeout: timeout to indicate an error
>>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> Better: timeout in jiffies
>>
> 
> As I will use msecs_to_jiffies, @timeout: timeout in millisecond.

Good idea - you might want to use timeout_ms.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130711/a1363c68/attachment.sig>

[PATCH v12 11/13] usb: chipidea: udc: misuse flag CI_HDRC_REGS_SHARED and CI_HDRC_PULLUP_ON_VBUS

[Peter Chen]

CI_HDRC_REGS_SHARED stands for the controller registers is shared
with other USB drivers, if all USB drivers are at chipidea/, it doesn't
needed to set.
CI_HDRC_PULLUP_ON_VBUS stands for pullup dp when the vbus is on. This
flag doesn't need to set if the vbus is always on for gadget
since dp has always pulled up after the gadget has initialized.

So, the current code seems to misuse this two flags.
- When the gadget initializes, the controller doesn't need to run if
it depends on vbus (CI_HDRC_PULLUP_ON_VBUS), it does not
relate to shared register.
- When the gadget starts (load one gadget module), the controller
can run if vbus is on (CI_HDRC_PULLUP_ON_VBUS), it also does not
relate to shared register.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/udc.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index c70ce38..45abf4d 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1637,8 +1637,7 @@ static int ci_udc_start(struct usb_gadget *gadget,
 	pm_runtime_get_sync(&ci->gadget.dev);
 	if (ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS) {
 		if (ci->vbus_active) {
-			if (ci->platdata->flags & CI_HDRC_REGS_SHARED)
-				hw_device_reset(ci, USBMODE_CM_DC);
+			hw_device_reset(ci, USBMODE_CM_DC);
 		} else {
 			pm_runtime_put_sync(&ci->gadget.dev);
 			goto done;
@@ -1801,7 +1800,7 @@ static int udc_start(struct ci_hdrc *ci)
 		}
 	}
 
-	if (!(ci->platdata->flags & CI_HDRC_REGS_SHARED)) {
+	if (!(ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS)) {
 		retval = hw_device_reset(ci, USBMODE_CM_DC);
 		if (retval)
 			goto put_transceiver;
-- 
1.7.0.4

[PATCH v12 12/13] usb: chipidea: udc: .pullup is valid when vbus is on at CI_HDRC_PULLUP_ON_VBUS

[Peter Chen]

When the flag CI_HDRC_PULLUP_ON_VBUS is set, .pullup should only be
called when the vbus is active. When the CI_HDRC_PULLUP_ON_VBUS
is set, the controller only begins to run when the vbus is on,
So, it is only meaningful software set pullup/pulldown after
the controller begins to run.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/udc.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 45abf4d..b7ead5f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1514,6 +1514,10 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
 {
 	struct ci_hdrc *ci = container_of(_gadget, struct ci_hdrc, gadget);
 
+	if ((ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS)
+			&& !ci->vbus_active)
+		return -EOPNOTSUPP;
+
 	if (is_on)
 		hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
 	else
-- 
1.7.0.4

[PATCH v12 13/13] usb: chipidea: udc: fix the oops when plugs in usb cable after rmmod gadget

[Peter Chen]

When we rmmod gadget, the ci->driver needs to be cleared.
Otherwise, we plug in usb cable again, the driver will
consider gadget is there, in fact, it was removed.

Besides, consolidate the calling of ci->driver->disconnect, when
we do rmmod gadget, the gadget's disconnect should be called from
udc core.

Below is the oops this patch fixes.

root at freescale ~$ ci_hdrc ci_hdrc.0: Connected to host
Unable to handle kernel paging request at virtual address 7f01171c
pgd = 80004000
[7f01171c] *pgd=4fa1e811, *pte=00000000, *ppte=00000000
Internal error: Oops: 7 [#1] SMP ARM
Modules linked in: f_acm libcomposite u_serial [last unloaded: g_serial]
CPU: 0    Not tainted  (3.8.0-rc5+ #221)
PC is at _gadget_stop_activity+0xbc/0x128
LR is at ep_fifo_flush+0x68/0xa0
pc : [<803634cc>]    lr : [<80363938>]    psr: 20000193
sp : 806c7dc8  ip : 00000000  fp : 806c7de4
r10: 00000000  r9 : 80710ea4  r8 : 00000000
r7 : bf834094  r6 : bf834098  r5 : bf834010  r4 : bf8340a0
r3 : 7f011708  r2 : 01940194  r1 : a0000193  r0 : bf834014
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c53c7d  Table: 4f06404a  DAC: 00000017
Process swapper/0 (pid: 0, stack limit = 0x806c6238)
Stack: (0x806c7dc8 to 0x806c8000)
7dc0:                   bf834010 bf809950 bf834010 0000004b 806c7e44 806c7de8
7de0: 80365400 8036341c 0000ec1c 00000000 0000ec1c 00000040 fff5ede0 bf834014
7e00: 000a1220 00000000 0000002e ffffd958 806c7e3c 806c7e20 8004e870 bf834010
7e20: bf809950 0000004b 0000004b 00000000 80710ea4 00000000 806c7e5c 806c7e48
7e40: 80362888 80365154 bfb35180 bf809950 806c7e9c 806c7e60 8008111c 803627f4
7e60: 00989680 00000000 bf809900 bf809964 812df8c0 bf809900 bf809950 806c3f2c
7e80: 0000004b 00000000 412fc09a 00000000 806c7eb4 806c7ea0 80081368 800810b8
7ea0: bf809900 bf809950 806c7ecc 806c7eb8 800843c8 8008130c 0000004b 806cf748
7ec0: 806c7ee4 806c7ed0 80081088 8008432c 806c6000 806cf748 806c7f0c 806c7ee8
7ee0: 8000fa44 8008105c f400010c 806ce974 806c7f30 f4000110 806d29e8 412fc09a
7f00: 806c7f2c 806c7f10 80008584 8000f9f4 8000fbd0 60000013 ffffffff 806c7f64
7f20: 806c7f84 806c7f30 8000eac0 80008554 00000000 00000000 0000000f 8001aea0
7f40: 806c6000 80712a48 804f0040 00000000 806d29e8 412fc09a 00000000 806c7f84
7f60: 806c7f88 806c7f78 8000fbcc 8000fbd0 60000013 ffffffff 806c7fac 806c7f88
7f80: 8001015c 8000fb9c 806cf5b0 80712980 806a4528 8fffffff 1000406a 412fc09a
7fa0: 806c7fbc 806c7fb0 804e5334 800100ac 806c7ff4 806c7fc0 80668940 804e52d4
7fc0: ffffffff ffffffff 806684bc 00000000 00000000 806a4528 10c53c7d 806ce94c
7fe0: 806a4524 806d29dc 00000000 806c7ff8 10008078 806686b0 00000000 00000000
Backtrace:
[<80363410>] (_gadget_stop_activity+0x0/0x128) from [<80365400>] (udc_irq+0x2b8/0xf38)
 r7:0000004b r6:bf834010 r5:bf809950 r4:bf834010
 [<80365148>] (udc_irq+0x0/0xf38) from [<80362888>] (ci_irq+0xa0/0xf4)
[<803627e8>] (ci_irq+0x0/0xf4) from [<8008111c>] (handle_irq_event_percpu+0x70/0x254)
 r5:bf809950 r4:bfb35180
 [<800810ac>] (handle_irq_event_percpu+0x0/0x254) from [<80081368>] (handle_irq_event+0x68/0x88)
[<80081300>] (handle_irq_event+0x0/0x88) from [<800843c8>] (handle_fasteoi_irq+0xa8/0x178)
 r5:bf809950 r4:bf809900
 [<80084320>] (handle_fasteoi_irq+0x0/0x178) from [<80081088>] (generic_handle_irq+0x38/0x40)
 r5:806cf748 r4:0000004b
 [<80081050>] (generic_handle_irq+0x0/0x40) from [<8000fa44>] (handle_IRQ+0x5c/0xbc)
 r5:806cf748 r4:806c6000
 [<8000f9e8>] (handle_IRQ+0x0/0xbc) from [<80008584>] (gic_handle_irq+0x3c/0x70)
 r9:412fc09a r8:806d29e8 r7:f4000110 r6:806c7f30 r5:806ce974
 r4:f400010c
 [<80008548>] (gic_handle_irq+0x0/0x70) from [<8000eac0>] (__irq_svc+0x40/0x54)
Exception stack(0x806c7f30 to 0x806c7f78)
7f20:                                     00000000 00000000 0000000f 8001aea0
7f40: 806c6000 80712a48 804f0040 00000000 806d29e8 412fc09a 00000000 806c7f84
7f60: 806c7f88 806c7f78 8000fbcc 8000fbd0 60000013 ffffffff
 r7:806c7f64 r6:ffffffff r5:60000013 r4:8000fbd0
 [<8000fb90>] (default_idle+0x0/0x4c) from [<8001015c>] (cpu_idle+0xbc/0xf8)
[<800100a0>] (cpu_idle+0x0/0xf8) from [<804e5334>] (rest_init+0x6c/0x84)
 r9:412fc09a r8:1000406a r7:8fffffff r6:806a4528 r5:80712980
 r4:806cf5b0
 [<804e52c8>] (rest_init+0x0/0x84) from [<80668940>] (start_kernel+0x29c/0x2ec)
[<806686a4>] (start_kernel+0x0/0x2ec) from [<10008078>] (0x10008078)
Code: e12fff33 e5953200 e3530000 0a000002 (e5933014)
---[ end trace aade28ad434062bd ]---
Kernel panic - not syncing: 0xbf8bdfa8)
df60: 00000000 00000000 0000000f 8001aea0 bf8bc000 80712a48 804f0040 00000000
df80: 806d29e8 412fc09a 00000000 bf8bdfb4 bf8bdfb8 bf8bdfa8 8000fbcc 8000fbd0
dfa0: 60000013 ffffffff
 r7:bf8bdf94 r6:ffffffff r5:60000013 r4:8000fbd0
 [<8000fb90>] (default_idle+0x0/0x4c) from [<8001015c>] (cpu_idle+0xbc/0xf8)
[<800100a0>] (cpu_idle+0x0/0xf8) from [<804e7930>] (secondary_start_kernel+0x120/0x148)
 r9:412fc09a r8:1000406a r7:80712d20 r6:10c03c7d r5:00000002
 r4:806e2008
 [<804e7810>] (secondary_start_kernel+0x0/0x148) from [<104e7148>] (0x104e7148)
 r5:0000001f r4:4f8a806a
 CPU3: stopping
 Backtrace:
 [<800132e8>] (dump_backtrace+0x0/0x114) from [<804ea684>] (dump_stack+0x20/0x24)
 r7:00000000 r6:806c3f2c r5:806cf748 r4:80712d18
 [<804ea664>] (dump_stack+0x0/0x24) from [<80014adc>] (handle_IPI+0x140/0x18c)
[<8001499c>] (handle_IPI+0x0/0x18c) from [<800085b0>] (gic_handle_irq+0x68/0x70)
 r9:412fc09a r8:806d29e8 r7:f4000110 r6:bf8bff60 r5:806ce974
 r4:f400010c
 [<80008548>] (gic_handle_irq+0x0/0x70) from [<8000eac0>] (__irq_svc+0x40/0x54)
Exception stack(0xbf8bff60 to 0xbf8bffa8)
ff60: 00000000 00000000 0000000f 8001aea0 bf8be000 80712a48 804f0040 00000000
ff80: 806d29e8 412fc09a 00000000 bf8bffb4 bf8bffb8 bf8bffa8 8000fbcc 8000fbd0
ffa0: 60000013 ffffffff
 r7:bf8bff94 r6:ffffffff r5:60000013 r4:8000fbd0
 [<8000fb90>] (default_idle+0x0/0x4c) from [<8001015c>] (cpu_idle+0xbc/0xf8)
[<800100a0>] (cpu_idle+0x0/0xf8) from [<804e7930>] (secondary_start_kernel+0x120/0x148)
 r9:412fc09a r8:1000406a r7:80712d20 r6:10c03c7d r5:00000003
 r4:806e2008
 [<804e7810>] (secondary_start_kernel+0x0/0x148) from [<104e7148>] (0x104e7148)
 r5:0000001f r4:4f8a806a
 CPU1: stopping
 Backtrace:
 [<800132e8>] (dump_backtrace+0x0/0x114) from [<804ea684>] (dump_stack+0x20/0x24)
 r7:00000000 r6:806c3f2c r5:806cf748 r4:80712d18
 [<804ea664>] (dump_stack+0x0/0x24) from [<80014adc>] (handle_IPI+0x140/0x18c)
[<8001499c>] (handle_IPI+0x0/0x18c) from [<800085b0>] (gic_handle_irq+0x68/0x70)
 r9:412fc09a r8:806d29e8 r7:f4000110 r6:bf8bbf60 r5:806ce974
 r4:f400010c
 [<80008548>] (gic_handle_irq+0x0/0x70) from [<8000eac0>] (__irq_svc+0x40/0x54)
Exception stack(0xbf8bbf60 to 0xbf8bbfa8)
bf60: 00000000 00000000 0000000f 8001aea0 bf8ba000 80712a48 804f0040 00000000
bf80: 806d29e8 412fc09a 00000000 bf8bbfb4 bf8bbfb8 bf8bbfa8 8000fbcc 8000fbd0
bfa0: 60000013 ffffffff
 r7:bf8bbf94 r6:ffffffff r5:60000013 r4:8000fbd0
 [<8000fb90>] (default_idle+0x0/0x4c) from [<8001015c>] (cpu_idle+0xbc/0xf8)
[<800100a0>] (cpu_idle+0x0/0xf8) from [<804e7930>] (secondary_start_kernel+0x120/0x148)
 r9:412fc09a r8:1000406a r7:80712d20 r6:10c03c7d r5:00000001
 r4:806e2008

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/udc.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b7ead5f..aeabdcf 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -686,9 +686,6 @@ static int _gadget_stop_activity(struct usb_gadget *gadget)
 	usb_ep_fifo_flush(&ci->ep0out->ep);
 	usb_ep_fifo_flush(&ci->ep0in->ep);
 
-	if (ci->driver)
-		ci->driver->disconnect(gadget);
-
 	/* make sure to disable all endpoints */
 	gadget_for_each_ep(ep, gadget) {
 		usb_ep_disable(ep);
@@ -717,6 +714,11 @@ __acquires(ci->lock)
 {
 	int retval;
 
+	if (ci->gadget.speed != USB_SPEED_UNKNOWN) {
+		if (ci->driver)
+			ci->driver->disconnect(&ci->gadget);
+	}
+
 	spin_unlock(&ci->lock);
 	retval = _gadget_stop_activity(&ci->gadget);
 	if (retval)
@@ -1464,6 +1466,8 @@ static int ci_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
 			hw_device_state(ci, ci->ep0out->qh.dma);
 			dev_dbg(ci->dev, "Connected to host\n");
 		} else {
+			if (ci->driver)
+				ci->driver->disconnect(&ci->gadget);
 			hw_device_state(ci, 0);
 			if (ci->platdata->notify_event)
 				ci->platdata->notify_event(ci,
@@ -1674,13 +1678,14 @@ static int ci_udc_stop(struct usb_gadget *gadget,
 		if (ci->platdata->notify_event)
 			ci->platdata->notify_event(ci,
 			CI_HDRC_CONTROLLER_STOPPED_EVENT);
-		ci->driver = NULL;
 		spin_unlock_irqrestore(&ci->lock, flags);
 		_gadget_stop_activity(&ci->gadget);
 		spin_lock_irqsave(&ci->lock, flags);
 		pm_runtime_put(&ci->gadget.dev);
 	}
 
+	ci->driver = NULL;
+
 	spin_unlock_irqrestore(&ci->lock, flags);
 
 	return 0;
-- 
1.7.0.4

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> This patchset adds tested otg id switch function and
> vbus connect and disconnect detection for chipidea driver.
> And fix kinds of bugs found at chipidea drivers after enabling
> id and vbus detection.
> 
> This patch is fully tested at imx6 sabresd platform.
> My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> 
> Changes for v12:
> - Rebased greg's usb-next tree (3.10.0-rc7+)
> - Split more small patches for single function and fix.

I tested the patchset. Here are the results:

- VBUS switching

I'm no longer getting any ID interrupts at all when I apply the patch below. The 
board stays in HOST mode all the time. If I configure it as peripheral, it works 
as peripheral. Note with [1], I was able to switch from Peripheral->Host , not 
the other way around.

--- a/arch/arm/boot/dts/imx28-m28evk.dts
+++ b/arch/arm/boot/dts/imx28-m28evk.dts
@@ -240,6 +240,8 @@
 
        ahb at 80080000 {
                usb0: usb at 80080000 {
+                       dr_mode = "otg";
+                       phy_mode = "utmi";
                        vbus-supply = <&reg_usb0_vbus>;
                        pinctrl-names = "default";
                        pinctrl-0 = <&usbphy0_pins_a>;

---------------------------

- MX23 UDC issue

I found a workaround. Now running 'dmesg' via telnet through USB CDC link no 
longer hangs the USB driver, but works as expected. I applied this small patch 
that enables the streaming mode. Works on MX23EVK. It's surprising this issue 
doesn't manifest on MX28, maybe MX28 contains a new revision of the controller. 
I remember there was some discussion about the streaming mode on MXS some time 
ago.

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index d06355e..bba8e25 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -92,8 +92,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
                .name           = "ci_hdrc_imx",
                .capoffset      = DEF_CAPOFFSET,
                .flags          = CI_HDRC_REQUIRE_TRANSCEIVER |
-                                 CI_HDRC_PULLUP_ON_VBUS |
-                                 CI_HDRC_DISABLE_STREAMING,
+                                 CI_HDRC_PULLUP_ON_VBUS,
        };
        struct resource *res;
        int ret;

[1] 
http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/v3.10/topic/usb-
peterchen

Best regards,
Marek Vasut

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Peter Chen]

On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> Hi Peter,
> 
> > This patchset adds tested otg id switch function and
> > vbus connect and disconnect detection for chipidea driver.
> > And fix kinds of bugs found at chipidea drivers after enabling
> > id and vbus detection.
> > 
> > This patch is fully tested at imx6 sabresd platform.
> > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > 
> > Changes for v12:
> > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > - Split more small patches for single function and fix.
> 
> I tested the patchset. Here are the results:
> 
> - VBUS switching
> 
> I'm no longer getting any ID interrupts at all when I apply the patch below. The 
> board stays in HOST mode all the time. If I configure it as peripheral, it works 
> as peripheral. Note with [1], I was able to switch from Peripheral->Host , not 
> the other way around.

Thanks for your testing. But first, can you have me check
if your ID wakeup is enabled?

I can have a test at mx28evk.
Is it current upstream kernel can boot mx28evk run?
I have a RevC board, I would like to know if
any patches needed.
> 
> --- a/arch/arm/boot/dts/imx28-m28evk.dts
> +++ b/arch/arm/boot/dts/imx28-m28evk.dts
> @@ -240,6 +240,8 @@
>  
>         ahb at 80080000 {
>                 usb0: usb at 80080000 {
> +                       dr_mode = "otg";
> +                       phy_mode = "utmi";
>                         vbus-supply = <&reg_usb0_vbus>;
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&usbphy0_pins_a>;
> 
> ---------------------------
> 
> - MX23 UDC issue
> 
> I found a workaround. Now running 'dmesg' via telnet through USB CDC link no 
> longer hangs the USB driver, but works as expected. I applied this small patch 
> that enables the streaming mode. Works on MX23EVK. It's surprising this issue 
> doesn't manifest on MX28, maybe MX28 contains a new revision of the controller. 
> I remember there was some discussion about the streaming mode on MXS some time 
> ago.

It seems not reasonable, the same question for mx23evk,
it can run well with current kernel? I have no board on hand,
let me see if I can find one.
  

-- 

Best Regards,
Peter Chen

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > This patchset adds tested otg id switch function and
> > > vbus connect and disconnect detection for chipidea driver.
> > > And fix kinds of bugs found at chipidea drivers after enabling
> > > id and vbus detection.
> > > 
> > > This patch is fully tested at imx6 sabresd platform.
> > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > 
> > > Changes for v12:
> > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > - Split more small patches for single function and fix.
> > 
> > I tested the patchset. Here are the results:
> > 
> > - VBUS switching
> > 
> > I'm no longer getting any ID interrupts at all when I apply the patch
> > below. The board stays in HOST mode all the time. If I configure it as
> > peripheral, it works as peripheral. Note with [1], I was able to switch
> > from Peripheral->Host , not the other way around.
> 
> Thanks for your testing. But first, can you have me check
> if your ID wakeup is enabled?

ID wakeup? How do I check?

> I can have a test at mx28evk.
> Is it current upstream kernel can boot mx28evk run?

Yes, I use next-20130711 and it works fine.

> I have a RevC board, I would like to know if
> any patches needed.

Apply just this patchset and you should be all set.

> > --- a/arch/arm/boot/dts/imx28-m28evk.dts
> > +++ b/arch/arm/boot/dts/imx28-m28evk.dts
> > @@ -240,6 +240,8 @@
> > 
> >         ahb at 80080000 {
> >         
> >                 usb0: usb at 80080000 {
> > 
> > +                       dr_mode = "otg";
> > +                       phy_mode = "utmi";
> > 
> >                         vbus-supply = <&reg_usb0_vbus>;
> >                         pinctrl-names = "default";
> >                         pinctrl-0 = <&usbphy0_pins_a>;
> > 
> > ---------------------------
> > 
> > - MX23 UDC issue
> > 
> > I found a workaround. Now running 'dmesg' via telnet through USB CDC link
> > no longer hangs the USB driver, but works as expected. I applied this
> > small patch that enables the streaming mode. Works on MX23EVK. It's
> > surprising this issue doesn't manifest on MX28, maybe MX28 contains a
> > new revision of the controller. I remember there was some discussion
> > about the streaming mode on MXS some time ago.
> 
> It seems not reasonable

I talked to Fabio and he said he met similar issue on MX6, where disabling the 
streaming mode fixed the problem. Could it possibly be so that the non-streaming 
mode is broken on mx23?

btw. I also tested this on STMP3780-based device, works the same way as MX23 
does, streaming mode has to be enabled.

> the same question for mx23evk,
> it can run well with current kernel? I have no board on hand,
> let me see if I can find one.

Yes, MX23EVK rev. B2 works just fine with next-20130711 . You need to add the DT 
USB sections for MX23EVK, but they can be copied from imx28.dtsi + imx28-evk.dts 
.

Best regards,
Marek Vasut

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Peter Chen]

On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> Hi Peter,
> 
> > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > Hi Peter,
> > > 
> > > > This patchset adds tested otg id switch function and
> > > > vbus connect and disconnect detection for chipidea driver.
> > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > id and vbus detection.
> > > > 
> > > > This patch is fully tested at imx6 sabresd platform.
> > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > 
> > > > Changes for v12:
> > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > - Split more small patches for single function and fix.
> > > 
> > > I tested the patchset. Here are the results:
> > > 
> > > - VBUS switching
> > > 
> > > I'm no longer getting any ID interrupts at all when I apply the patch
> > > below. The board stays in HOST mode all the time. If I configure it as
> > > peripheral, it works as peripheral. Note with [1], I was able to switch
> > > from Peripheral->Host , not the other way around.
> > 
> > Thanks for your testing. But first, can you have me check
> > if your ID wakeup is enabled?
> 
> ID wakeup? How do I check?
> 

See otgsc at controller register, the ID wakeup enable is bit 24.

-- 

Best Regards,
Peter Chen

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > This patchset adds tested otg id switch function and
> > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > > id and vbus detection.
> > > > > 
> > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > > 
> > > > > Changes for v12:
> > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > - Split more small patches for single function and fix.
> > > > 
> > > > I tested the patchset. Here are the results:
> > > > 
> > > > - VBUS switching
> > > > 
> > > > I'm no longer getting any ID interrupts at all when I apply the patch
> > > > below. The board stays in HOST mode all the time. If I configure it
> > > > as peripheral, it works as peripheral. Note with [1], I was able to
> > > > switch from Peripheral->Host , not the other way around.
> > > 
> > > Thanks for your testing. But first, can you have me check
> > > if your ID wakeup is enabled?
> > 
> > ID wakeup? How do I check?
> 
> See otgsc at controller register, the ID wakeup enable is bit 24.

Yes, ID interrupt (IDIE) is set.

I noticed this backtrace in the kernel bootlog, but this only happens if the 
dr_mode="otg" , it comes from the host-mode irq handler :

[    2.757563] irq 238: nobody cared (try booting with the "irqpoll" option)
[    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
next-20130711-00013-g011c4b3-dirty #703
[    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>] 
(show_stack+0x10/0x14)
[    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>] 
(__report_bad_irq.isra.6+0x20/0xe0)
[    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from 
[<80065c98>] (note_interrupt+0x16c/0x230)
[    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from [<80064000>] 
(handle_irq_event_percpu+0x10c/0x1a4)
[    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from 
[<800640e8>] (handle_irq_event+0x50/0x78)
[    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from [<8006652c>] 
(handle_level_irq+0x88/0x10c)
[    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from [<800638d0>] 
(generic_handle_irq+0x28/0x3c)
[    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from [<8000f84c>] 
(handle_IRQ+0x30/0x84)
[    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>] 
(__irq_svc+0x40/0x6c)
[    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>] 
(__do_softirq+0x90/0x1d8)
[    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>] 
(irq_exit+0x98/0xd4)
[    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>] 
(handle_IRQ+0x34/0x84)
[    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>] 
(__irq_svc+0x40/0x6c)
[    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>] 
(vprintk_emit+0x1bc/0x524)
[    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>] 
(printk+0x30/0x40)
[    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>] 
(mousedev_init+0x4c/0x60)
[    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>] 
(do_one_initcall+0x94/0x14c)
[    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from [<80617b20>] 
(kernel_init_freeable+0x16c/0x22c)
[    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from [<804d8cbc>] 
(kernel_init+0x8/0x150)
[    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>] 
(ret_from_fork+0x14/0x24)
[    2.949932] handlers:
[    2.952227] [<8033fc58>] ci_irq
[    2.955388] Disabling IRQ #238

btw. do you have any kind of other CI13xxx documentation than what's in the CPU 
datasheets ?

Best regards,
Marek Vasut

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Chen Peter-B29397]

 
> 
> Yes, ID interrupt (IDIE) is set.
> 
> I noticed this backtrace in the kernel bootlog, but this only happens if
> the
> dr_mode="otg" , it comes from the host-mode irq handler :
> 
> [    2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> option)
> [    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
> next-20130711-00013-g011c4b3-dirty #703
> [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>]
> (show_stack+0x10/0x14)
> [    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> (__report_bad_irq.isra.6+0x20/0xe0)
> [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> [<80065c98>] (note_interrupt+0x16c/0x230)
> [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from [<80064000>]
> (handle_irq_event_percpu+0x10c/0x1a4)
> [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from
> [<800640e8>] (handle_irq_event+0x50/0x78)
> [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from [<8006652c>]
> (handle_level_irq+0x88/0x10c)
> [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> [<800638d0>]
> (generic_handle_irq+0x28/0x3c)
> [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> [<8000f84c>]
> (handle_IRQ+0x30/0x84)
> [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> (__irq_svc+0x40/0x6c)
> [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> (__do_softirq+0x90/0x1d8)
> [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>]
> (irq_exit+0x98/0xd4)
> [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>]
> (handle_IRQ+0x34/0x84)
> [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>]
> (__irq_svc+0x40/0x6c)
> [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>]
> (vprintk_emit+0x1bc/0x524)
> [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>]
> (printk+0x30/0x40)
> [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> (mousedev_init+0x4c/0x60)
> [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>]
> (do_one_initcall+0x94/0x14c)
> [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from [<80617b20>]
> (kernel_init_freeable+0x16c/0x22c)
> [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from
> [<804d8cbc>]
> (kernel_init+0x8/0x150)
> [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>]
> (ret_from_fork+0x14/0x24)
> [    2.949932] handlers:
> [    2.952227] [<8033fc58>] ci_irq
> [    2.955388] Disabling IRQ #238
> 

See this " Disabling IRQ #238", that is the reason you can't get
ID interrupt.

I have no other idea before I try, you can try below things:

1. please switch VDD 5V SOURCE SELECT to USB 5V
2. try not plug in mouse during the boots up

> btw. do you have any kind of other CI13xxx documentation than what's in
> the CPU
> datasheets ?
> 

Mx28 RM does not include many controller detail, please try to 
read it from mx6/mx5 RM.

Best regards,
Peter
 

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Peter Chen]

On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> Hi Peter,
> 
> > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > Hi Peter,
> > > 
> > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > Hi Peter,
> > > > > 
> > > > > > This patchset adds tested otg id switch function and
> > > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > > > id and vbus detection.
> > > > > > 
> > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > > > 
> > > > > > Changes for v12:
> > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > - Split more small patches for single function and fix.
> > > > > 
> > > > > I tested the patchset. Here are the results:
> > > > > 
> > > > > - VBUS switching
> > > > > 
> > > > > I'm no longer getting any ID interrupts at all when I apply the patch
> > > > > below. The board stays in HOST mode all the time. If I configure it
> > > > > as peripheral, it works as peripheral. Note with [1], I was able to
> > > > > switch from Peripheral->Host , not the other way around.
> > > > 
> > > > Thanks for your testing. But first, can you have me check
> > > > if your ID wakeup is enabled?
> > > 
> > > ID wakeup? How do I check?
> > 
> > See otgsc at controller register, the ID wakeup enable is bit 24.
> 
> Yes, ID interrupt (IDIE) is set.
> 
> I noticed this backtrace in the kernel bootlog, but this only happens if the 
> dr_mode="otg" , it comes from the host-mode irq handler :
> 
> [    2.757563] irq 238: nobody cared (try booting with the "irqpoll" option)
> [    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
> next-20130711-00013-g011c4b3-dirty #703
> [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>] 
> (show_stack+0x10/0x14)
> [    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>] 
> (__report_bad_irq.isra.6+0x20/0xe0)
> [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from 
> [<80065c98>] (note_interrupt+0x16c/0x230)
> [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from [<80064000>] 
> (handle_irq_event_percpu+0x10c/0x1a4)
> [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from 
> [<800640e8>] (handle_irq_event+0x50/0x78)
> [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from [<8006652c>] 
> (handle_level_irq+0x88/0x10c)
> [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from [<800638d0>] 
> (generic_handle_irq+0x28/0x3c)
> [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from [<8000f84c>] 
> (handle_IRQ+0x30/0x84)
> [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>] 
> (__irq_svc+0x40/0x6c)
> [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>] 
> (__do_softirq+0x90/0x1d8)
> [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>] 
> (irq_exit+0x98/0xd4)
> [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>] 
> (handle_IRQ+0x34/0x84)
> [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>] 
> (__irq_svc+0x40/0x6c)
> [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>] 
> (vprintk_emit+0x1bc/0x524)
> [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>] 
> (printk+0x30/0x40)
> [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>] 
> (mousedev_init+0x4c/0x60)
> [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>] 
> (do_one_initcall+0x94/0x14c)
> [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from [<80617b20>] 
> (kernel_init_freeable+0x16c/0x22c)
> [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from [<804d8cbc>] 
> (kernel_init+0x8/0x150)
> [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>] 
> (ret_from_fork+0x14/0x24)
> [    2.949932] handlers:
> [    2.952227] [<8033fc58>] ci_irq
> [    2.955388] Disabling IRQ #238
> 

Marek, I have a test at imx28evk (Freescale imx28evk) for this patchset,
it works well for dual-role switch after adding below dts change.
There is a kernel dump when works at udc mode(due to enable CONFIG_LOCKUP_DETECTOR),
at the first connect, but it does not affect function, it should be a common
chipidea problem, I will check later.

diff --git a/arch/arm/boot/dts/imx28-evk.dts b/arch/arm/boot/dts/imx28-evk.dts
index 3637bf3..e8e2f09 100644
--- a/arch/arm/boot/dts/imx28-evk.dts
+++ b/arch/arm/boot/dts/imx28-evk.dts
@@ -240,6 +240,8 @@
 	ahb at 80080000 {
 		usb0: usb at 80080000 {
 			vbus-supply = <&reg_usb0_vbus>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&usbphy0_pins_c>;
 			status = "okay";
 		};
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 600f7cb..a11e015 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -657,6 +657,16 @@
 					fsl,pull-up = <0>;
 				};
 
+				usbphy0_pins_c: usbphy0 at 2 {
+					reg = <2>;
+					fsl,pinmux-ids = <
+						0x3071 /* MX28_PAD_AUART1_RTS__USB0_ID */
+					>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
 				usbphy1_pins_a: usbphy1 at 0 {
 					reg = <0>;
 					fsl,pinmux-ids = <

-- 

Best Regards,
Peter Chen

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > Hi Peter,
> > > > > > 
> > > > > > > This patchset adds tested otg id switch function and
> > > > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > > > > id and vbus detection.
> > > > > > > 
> > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > > > > 
> > > > > > > Changes for v12:
> > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > - Split more small patches for single function and fix.
> > > > > > 
> > > > > > I tested the patchset. Here are the results:
> > > > > > 
> > > > > > - VBUS switching
> > > > > > 
> > > > > > I'm no longer getting any ID interrupts at all when I apply the
> > > > > > patch below. The board stays in HOST mode all the time. If I
> > > > > > configure it as peripheral, it works as peripheral. Note with
> > > > > > [1], I was able to switch from Peripheral->Host , not the other
> > > > > > way around.
> > > > > 
> > > > > Thanks for your testing. But first, can you have me check
> > > > > if your ID wakeup is enabled?
> > > > 
> > > > ID wakeup? How do I check?
> > > 
> > > See otgsc at controller register, the ID wakeup enable is bit 24.
> > 
> > Yes, ID interrupt (IDIE) is set.
> > 
> > I noticed this backtrace in the kernel bootlog, but this only happens if
> > the dr_mode="otg" , it comes from the host-mode irq handler :
> > 
> > [    2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> > option) [    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
> > next-20130711-00013-g011c4b3-dirty #703
> > [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>]
> > (show_stack+0x10/0x14)
> > [    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> > (__report_bad_irq.isra.6+0x20/0xe0)
> > [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> > [<80065c98>] (note_interrupt+0x16c/0x230)
> > [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from
> > [<800640e8>] (handle_irq_event+0x50/0x78)
> > [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > [<8006652c>] (handle_level_irq+0x88/0x10c)
> > [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> > [<800638d0>] (generic_handle_irq+0x28/0x3c)
> > [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> > [<8000f84c>] (handle_IRQ+0x30/0x84)
> > [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> > (__irq_svc+0x40/0x6c)
> > [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> > (__do_softirq+0x90/0x1d8)
> > [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>]
> > (irq_exit+0x98/0xd4)
> > [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>]
> > (handle_IRQ+0x34/0x84)
> > [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>]
> > (__irq_svc+0x40/0x6c)
> > [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>]
> > (vprintk_emit+0x1bc/0x524)
> > [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>]
> > (printk+0x30/0x40)
> > [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> > (mousedev_init+0x4c/0x60)
> > [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>]
> > (do_one_initcall+0x94/0x14c)
> > [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from
> > [<80617b20>] (kernel_init_freeable+0x16c/0x22c)
> > [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from
> > [<804d8cbc>] (kernel_init+0x8/0x150)
> > [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>]
> > (ret_from_fork+0x14/0x24)
> > [    2.949932] handlers:
> > [    2.952227] [<8033fc58>] ci_irq
> > [    2.955388] Disabling IRQ #238
> 
> Marek, I have a test at imx28evk (Freescale imx28evk) for this patchset,
> it works well for dual-role switch after adding below dts change.
> There is a kernel dump when works at udc mode(due to enable
> CONFIG_LOCKUP_DETECTOR), at the first connect, but it does not affect
> function, it should be a common chipidea problem, I will check later.

Sorry for the long delay. I finally got further with this, at least now I can 
switch Periph->Host again, but if I do so the other way around (unplug host 
adapter and plug computer cable) and dump the controller registers, I still see 
the controller in "host" (according to USBMODE register) mode. The controller 
stays in HOST mode even if I force it to gadget mode by writing "gadget" into 
debugfs "role" entry. I will keep poking.

In the meantime, I guess if it works on the EVK, it might be a hardware error so 
I better stop stalling you. I'll drop you an email if I find what this problem 
is.

Best regards,
Marek Vasut

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Peter Chen]

On Mon, Jul 22, 2013 at 03:15:28AM +0200, Marek Vasut wrote:
> Hi Peter,
> 
> > On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > > Hi Peter,
> > > 
> > > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > > Hi Peter,
> > > > > 
> > > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > > Hi Peter,
> > > > > > > 
> > > > > > > > This patchset adds tested otg id switch function and
> > > > > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > > > > And fix kinds of bugs found at chipidea drivers after enabling
> > > > > > > > id and vbus detection.
> > > > > > > > 
> > > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > > My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
> > > > > > > > 
> > > > > > > > Changes for v12:
> > > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > > - Split more small patches for single function and fix.
> > > > > > > 
> > > > > > > I tested the patchset. Here are the results:
> > > > > > > 
> > > > > > > - VBUS switching
> > > > > > > 
> > > > > > > I'm no longer getting any ID interrupts at all when I apply the
> > > > > > > patch below. The board stays in HOST mode all the time. If I
> > > > > > > configure it as peripheral, it works as peripheral. Note with
> > > > > > > [1], I was able to switch from Peripheral->Host , not the other
> > > > > > > way around.
> > > > > > 
> > > > > > Thanks for your testing. But first, can you have me check
> > > > > > if your ID wakeup is enabled?
> > > > > 
> > > > > ID wakeup? How do I check?
> > > > 
> > > > See otgsc at controller register, the ID wakeup enable is bit 24.
> > > 
> > > Yes, ID interrupt (IDIE) is set.
> > > 
> > > I noticed this backtrace in the kernel bootlog, but this only happens if
> > > the dr_mode="otg" , it comes from the host-mode irq handler :
> > > 
> > > [    2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> > > option) [    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.0-
> > > next-20130711-00013-g011c4b3-dirty #703
> > > [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from [<80011644>]
> > > (show_stack+0x10/0x14)
> > > [    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> > > (__report_bad_irq.isra.6+0x20/0xe0)
> > > [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> > > [<80065c98>] (note_interrupt+0x16c/0x230)
> > > [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4) from
> > > [<800640e8>] (handle_irq_event+0x50/0x78)
> > > [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > > [<8006652c>] (handle_level_irq+0x88/0x10c)
> > > [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> > > [<800638d0>] (generic_handle_irq+0x28/0x3c)
> > > [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> > > [<8000f84c>] (handle_IRQ+0x30/0x84)
> > > [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> > > (__irq_svc+0x40/0x6c)
> > > [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> > > (__do_softirq+0x90/0x1d8)
> > > [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from [<80022edc>]
> > > (irq_exit+0x98/0xd4)
> > > [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>]
> > > (handle_IRQ+0x34/0x84)
> > > [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>]
> > > (__irq_svc+0x40/0x6c)
> > > [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>]
> > > (vprintk_emit+0x1bc/0x524)
> > > [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from [<804da5a4>]
> > > (printk+0x30/0x40)
> > > [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> > > (mousedev_init+0x4c/0x60)
> > > [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from [<806178fc>]
> > > (do_one_initcall+0x94/0x14c)
> > > [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from
> > > [<80617b20>] (kernel_init_freeable+0x16c/0x22c)
> > > [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from
> > > [<804d8cbc>] (kernel_init+0x8/0x150)
> > > [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>]
> > > (ret_from_fork+0x14/0x24)
> > > [    2.949932] handlers:
> > > [    2.952227] [<8033fc58>] ci_irq
> > > [    2.955388] Disabling IRQ #238
> > 
> > Marek, I have a test at imx28evk (Freescale imx28evk) for this patchset,
> > it works well for dual-role switch after adding below dts change.
> > There is a kernel dump when works at udc mode(due to enable
> > CONFIG_LOCKUP_DETECTOR), at the first connect, but it does not affect
> > function, it should be a common chipidea problem, I will check later.
> 
> Sorry for the long delay. I finally got further with this, at least now I can 
> switch Periph->Host again, but if I do so the other way around (unplug host 
> adapter and plug computer cable) and dump the controller registers, I still see 
> the controller in "host" (according to USBMODE register) mode. The controller 
> stays in HOST mode even if I force it to gadget mode by writing "gadget" into 
> debugfs "role" entry. I will keep poking.
> 
> In the meantime, I guess if it works on the EVK, it might be a hardware error so 
> I better stop stalling you. I'll drop you an email if I find what this problem 
> is.
> 

No, the problem is you did not config the USB OTG ID pin.
Do the same thing I have done at imx28evk, you probably will work
if the ID pin is the same at your board.

If it works, please give a tested-by :).

-- 

Best Regards,
Peter Chen

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Dear Peter Chen,

> On Mon, Jul 22, 2013 at 03:15:28AM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > > > Hi Peter,
> > > > > > 
> > > > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > > > Hi Peter,
> > > > > > > > 
> > > > > > > > > This patchset adds tested otg id switch function and
> > > > > > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > > > > > And fix kinds of bugs found at chipidea drivers after
> > > > > > > > > enabling id and vbus detection.
> > > > > > > > > 
> > > > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > > > My chipidea repo:
> > > > > > > > > https://github.com/hzpeterchen/linux-usb.git
> > > > > > > > > 
> > > > > > > > > Changes for v12:
> > > > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > > > - Split more small patches for single function and fix.
> > > > > > > > 
> > > > > > > > I tested the patchset. Here are the results:
> > > > > > > > 
> > > > > > > > - VBUS switching
> > > > > > > > 
> > > > > > > > I'm no longer getting any ID interrupts at all when I apply
> > > > > > > > the patch below. The board stays in HOST mode all the time.
> > > > > > > > If I configure it as peripheral, it works as peripheral.
> > > > > > > > Note with [1], I was able to switch from Peripheral->Host ,
> > > > > > > > not the other way around.
> > > > > > > 
> > > > > > > Thanks for your testing. But first, can you have me check
> > > > > > > if your ID wakeup is enabled?
> > > > > > 
> > > > > > ID wakeup? How do I check?
> > > > > 
> > > > > See otgsc at controller register, the ID wakeup enable is bit 24.
> > > > 
> > > > Yes, ID interrupt (IDIE) is set.
> > > > 
> > > > I noticed this backtrace in the kernel bootlog, but this only happens
> > > > if the dr_mode="otg" , it comes from the host-mode irq handler :
> > > > 
> > > > [    2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> > > > option) [    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted
> > > > 3.10.0- next-20130711-00013-g011c4b3-dirty #703
> > > > [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from
> > > > [<80011644>] (show_stack+0x10/0x14)
> > > > [    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> > > > (__report_bad_irq.isra.6+0x20/0xe0)
> > > > [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> > > > [<80065c98>] (note_interrupt+0x16c/0x230)
> > > > [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > > > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > from [<800640e8>] (handle_irq_event+0x50/0x78)
> > > > [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > > > [<8006652c>] (handle_level_irq+0x88/0x10c)
> > > > [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> > > > [<800638d0>] (generic_handle_irq+0x28/0x3c)
> > > > [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> > > > [<8000f84c>] (handle_IRQ+0x30/0x84)
> > > > [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> > > > (__irq_svc+0x40/0x6c)
> > > > [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> > > > (__do_softirq+0x90/0x1d8)
> > > > [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from
> > > > [<80022edc>] (irq_exit+0x98/0xd4)
> > > > [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>]
> > > > (handle_IRQ+0x34/0x84)
> > > > [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>]
> > > > (__irq_svc+0x40/0x6c)
> > > > [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>]
> > > > (vprintk_emit+0x1bc/0x524)
> > > > [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from
> > > > [<804da5a4>] (printk+0x30/0x40)
> > > > [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> > > > (mousedev_init+0x4c/0x60)
> > > > [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from
> > > > [<806178fc>] (do_one_initcall+0x94/0x14c)
> > > > [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from
> > > > [<80617b20>] (kernel_init_freeable+0x16c/0x22c)
> > > > [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from
> > > > [<804d8cbc>] (kernel_init+0x8/0x150)
> > > > [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>]
> > > > (ret_from_fork+0x14/0x24)
> > > > [    2.949932] handlers:
> > > > [    2.952227] [<8033fc58>] ci_irq
> > > > [    2.955388] Disabling IRQ #238
> > > 
> > > Marek, I have a test at imx28evk (Freescale imx28evk) for this
> > > patchset, it works well for dual-role switch after adding below dts
> > > change. There is a kernel dump when works at udc mode(due to enable
> > > CONFIG_LOCKUP_DETECTOR), at the first connect, but it does not affect
> > > function, it should be a common chipidea problem, I will check later.
> > 
> > Sorry for the long delay. I finally got further with this, at least now I
> > can switch Periph->Host again, but if I do so the other way around
> > (unplug host adapter and plug computer cable) and dump the controller
> > registers, I still see the controller in "host" (according to USBMODE
> > register) mode. The controller stays in HOST mode even if I force it to
> > gadget mode by writing "gadget" into debugfs "role" entry. I will keep
> > poking.
> > 
> > In the meantime, I guess if it works on the EVK, it might be a hardware
> > error so I better stop stalling you. I'll drop you an email if I find
> > what this problem is.
> 
> No, the problem is you did not config the USB OTG ID pin.
> Do the same thing I have done at imx28evk, you probably will work
> if the ID pin is the same at your board.

That one I have configured, exactly the same way you did (except that mine is 
MX28_PAD_PWM2__USB0_ID).

If it was the OTG ID pin issue, would the manual switching not work ?

> If it works, please give a tested-by :).

Best regards,
Marek Vasut

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Peter Chen]

On Mon, Jul 22, 2013 at 03:40:32AM +0200, Marek Vasut wrote:
> Dear Peter Chen,
> 
> > On Mon, Jul 22, 2013 at 03:15:28AM +0200, Marek Vasut wrote:
> > > Hi Peter,
> > > 
> > > > On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > > > > Hi Peter,
> > > > > 
> > > > > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > > > > Hi Peter,
> > > > > > > 
> > > > > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > > > > Hi Peter,
> > > > > > > > > 
> > > > > > > > > > This patchset adds tested otg id switch function and
> > > > > > > > > > vbus connect and disconnect detection for chipidea driver.
> > > > > > > > > > And fix kinds of bugs found at chipidea drivers after
> > > > > > > > > > enabling id and vbus detection.
> > > > > > > > > > 
> > > > > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > > > > My chipidea repo:
> > > > > > > > > > https://github.com/hzpeterchen/linux-usb.git
> > > > > > > > > > 
> > > > > > > > > > Changes for v12:
> > > > > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > > > > - Split more small patches for single function and fix.
> > > > > > > > > 
> > > > > > > > > I tested the patchset. Here are the results:
> > > > > > > > > 
> > > > > > > > > - VBUS switching
> > > > > > > > > 
> > > > > > > > > I'm no longer getting any ID interrupts at all when I apply
> > > > > > > > > the patch below. The board stays in HOST mode all the time.
> > > > > > > > > If I configure it as peripheral, it works as peripheral.
> > > > > > > > > Note with [1], I was able to switch from Peripheral->Host ,
> > > > > > > > > not the other way around.
> > > > > > > > 
> > > > > > > > Thanks for your testing. But first, can you have me check
> > > > > > > > if your ID wakeup is enabled?
> > > > > > > 
> > > > > > > ID wakeup? How do I check?
> > > > > > 
> > > > > > See otgsc at controller register, the ID wakeup enable is bit 24.
> > > > > 
> > > > > Yes, ID interrupt (IDIE) is set.
> > > > > 
> > > > > I noticed this backtrace in the kernel bootlog, but this only happens
> > > > > if the dr_mode="otg" , it comes from the host-mode irq handler :
> > > > > 
> > > > > [    2.757563] irq 238: nobody cared (try booting with the "irqpoll"
> > > > > option) [    2.764398] CPU: 0 PID: 1 Comm: swapper Not tainted
> > > > > 3.10.0- next-20130711-00013-g011c4b3-dirty #703
> > > > > [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from
> > > > > [<80011644>] (show_stack+0x10/0x14)
> > > > > [    2.782027] [<80011644>] (show_stack+0x10/0x14) from [<800659f4>]
> > > > > (__report_bad_irq.isra.6+0x20/0xe0)
> > > > > [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0) from
> > > > > [<80065c98>] (note_interrupt+0x16c/0x230)
> > > > > [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > > > > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > > [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > > from [<800640e8>] (handle_irq_event+0x50/0x78)
> > > > > [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > > > > [<8006652c>] (handle_level_irq+0x88/0x10c)
> > > > > [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> > > > > [<800638d0>] (generic_handle_irq+0x28/0x3c)
> > > > > [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> > > > > [<8000f84c>] (handle_IRQ+0x30/0x84)
> > > > > [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from [<80012160>]
> > > > > (__irq_svc+0x40/0x6c)
> > > > > [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from [<80022a44>]
> > > > > (__do_softirq+0x90/0x1d8)
> > > > > [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from
> > > > > [<80022edc>] (irq_exit+0x98/0xd4)
> > > > > [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from [<8000f850>]
> > > > > (handle_IRQ+0x34/0x84)
> > > > > [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from [<80012160>]
> > > > > (__irq_svc+0x40/0x6c)
> > > > > [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from [<8001d724>]
> > > > > (vprintk_emit+0x1bc/0x524)
> > > > > [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from
> > > > > [<804da5a4>] (printk+0x30/0x40)
> > > > > [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> > > > > (mousedev_init+0x4c/0x60)
> > > > > [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from
> > > > > [<806178fc>] (do_one_initcall+0x94/0x14c)
> > > > > [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from
> > > > > [<80617b20>] (kernel_init_freeable+0x16c/0x22c)
> > > > > [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c) from
> > > > > [<804d8cbc>] (kernel_init+0x8/0x150)
> > > > > [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from [<8000ea70>]
> > > > > (ret_from_fork+0x14/0x24)
> > > > > [    2.949932] handlers:
> > > > > [    2.952227] [<8033fc58>] ci_irq
> > > > > [    2.955388] Disabling IRQ #238
> > > > 
> > > > Marek, I have a test at imx28evk (Freescale imx28evk) for this
> > > > patchset, it works well for dual-role switch after adding below dts
> > > > change. There is a kernel dump when works at udc mode(due to enable
> > > > CONFIG_LOCKUP_DETECTOR), at the first connect, but it does not affect
> > > > function, it should be a common chipidea problem, I will check later.
> > > 
> > > Sorry for the long delay. I finally got further with this, at least now I
> > > can switch Periph->Host again, but if I do so the other way around
> > > (unplug host adapter and plug computer cable) and dump the controller
> > > registers, I still see the controller in "host" (according to USBMODE
> > > register) mode. The controller stays in HOST mode even if I force it to
> > > gadget mode by writing "gadget" into debugfs "role" entry. I will keep
> > > poking.
> > > 
> > > In the meantime, I guess if it works on the EVK, it might be a hardware
> > > error so I better stop stalling you. I'll drop you an email if I find
> > > what this problem is.
> > 
> > No, the problem is you did not config the USB OTG ID pin.
> > Do the same thing I have done at imx28evk, you probably will work
> > if the ID pin is the same at your board.
> 
> That one I have configured, exactly the same way you did (except that mine is 
> MX28_PAD_PWM2__USB0_ID).
> 
> If it was the OTG ID pin issue, would the manual switching not work ?
> 

I can't see at your imx28-m28evk.dts, besides, make sure this pin can be
configed as USB OTG ID pin, and fsl,pull-up = <1>. Since you are still
at host mode, I suspect it is the ID problem.
You can check if there is ID INT when unplug out B-to-A cable.

I have not tried the manual switching, but first, you need to close your
vbus supply.

-- 

Best Regards,
Peter Chen

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Hi Peter,

> On Mon, Jul 22, 2013 at 03:40:32AM +0200, Marek Vasut wrote:
> > Dear Peter Chen,
> > 
> > > On Mon, Jul 22, 2013 at 03:15:28AM +0200, Marek Vasut wrote:
> > > > Hi Peter,
> > > > 
> > > > > On Fri, Jul 12, 2013 at 03:18:31PM +0200, Marek Vasut wrote:
> > > > > > Hi Peter,
> > > > > > 
> > > > > > > On Fri, Jul 12, 2013 at 06:04:43AM +0200, Marek Vasut wrote:
> > > > > > > > Hi Peter,
> > > > > > > > 
> > > > > > > > > On Thu, Jul 11, 2013 at 07:57:19PM +0200, Marek Vasut wrote:
> > > > > > > > > > Hi Peter,
> > > > > > > > > > 
> > > > > > > > > > > This patchset adds tested otg id switch function and
> > > > > > > > > > > vbus connect and disconnect detection for chipidea
> > > > > > > > > > > driver. And fix kinds of bugs found at chipidea
> > > > > > > > > > > drivers after enabling id and vbus detection.
> > > > > > > > > > > 
> > > > > > > > > > > This patch is fully tested at imx6 sabresd platform.
> > > > > > > > > > > My chipidea repo:
> > > > > > > > > > > https://github.com/hzpeterchen/linux-usb.git
> > > > > > > > > > > 
> > > > > > > > > > > Changes for v12:
> > > > > > > > > > > - Rebased greg's usb-next tree (3.10.0-rc7+)
> > > > > > > > > > > - Split more small patches for single function and fix.
> > > > > > > > > > 
> > > > > > > > > > I tested the patchset. Here are the results:
> > > > > > > > > > 
> > > > > > > > > > - VBUS switching
> > > > > > > > > > 
> > > > > > > > > > I'm no longer getting any ID interrupts at all when I
> > > > > > > > > > apply the patch below. The board stays in HOST mode all
> > > > > > > > > > the time. If I configure it as peripheral, it works as
> > > > > > > > > > peripheral. Note with [1], I was able to switch from
> > > > > > > > > > Peripheral->Host , not the other way around.
> > > > > > > > > 
> > > > > > > > > Thanks for your testing. But first, can you have me check
> > > > > > > > > if your ID wakeup is enabled?
> > > > > > > > 
> > > > > > > > ID wakeup? How do I check?
> > > > > > > 
> > > > > > > See otgsc at controller register, the ID wakeup enable is bit
> > > > > > > 24.
> > > > > > 
> > > > > > Yes, ID interrupt (IDIE) is set.
> > > > > > 
> > > > > > I noticed this backtrace in the kernel bootlog, but this only
> > > > > > happens if the dr_mode="otg" , it comes from the host-mode irq
> > > > > > handler :
> > > > > > 
> > > > > > [    2.757563] irq 238: nobody cared (try booting with the
> > > > > > "irqpoll" option) [    2.764398] CPU: 0 PID: 1 Comm: swapper Not
> > > > > > tainted 3.10.0- next-20130711-00013-g011c4b3-dirty #703
> > > > > > [    2.773445] [<80013878>] (unwind_backtrace+0x0/0xe8) from
> > > > > > [<80011644>] (show_stack+0x10/0x14)
> > > > > > [    2.782027] [<80011644>] (show_stack+0x10/0x14) from
> > > > > > [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0)
> > > > > > [    2.791286] [<800659f4>] (__report_bad_irq.isra.6+0x20/0xe0)
> > > > > > from [<80065c98>] (note_interrupt+0x16c/0x230)
> > > > > > [    2.801063] [<80065c98>] (note_interrupt+0x16c/0x230) from
> > > > > > [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > > > [    2.811010] [<80064000>] (handle_irq_event_percpu+0x10c/0x1a4)
> > > > > > from [<800640e8>] (handle_irq_event+0x50/0x78)
> > > > > > [    2.820958] [<800640e8>] (handle_irq_event+0x50/0x78) from
> > > > > > [<8006652c>] (handle_level_irq+0x88/0x10c)
> > > > > > [    2.830210] [<8006652c>] (handle_level_irq+0x88/0x10c) from
> > > > > > [<800638d0>] (generic_handle_irq+0x28/0x3c)
> > > > > > [    2.839637] [<800638d0>] (generic_handle_irq+0x28/0x3c) from
> > > > > > [<8000f84c>] (handle_IRQ+0x30/0x84)
> > > > > > [    2.848461] [<8000f84c>] (handle_IRQ+0x30/0x84) from
> > > > > > [<80012160>] (__irq_svc+0x40/0x6c)
> > > > > > [    2.856510] [<80012160>] (__irq_svc+0x40/0x6c) from
> > > > > > [<80022a44>] (__do_softirq+0x90/0x1d8)
> > > > > > [    2.864812] [<80022a44>] (__do_softirq+0x90/0x1d8) from
> > > > > > [<80022edc>] (irq_exit+0x98/0xd4)
> > > > > > [    2.873025] [<80022edc>] (irq_exit+0x98/0xd4) from
> > > > > > [<8000f850>] (handle_IRQ+0x34/0x84)
> > > > > > [    2.880980] [<8000f850>] (handle_IRQ+0x34/0x84) from
> > > > > > [<80012160>] (__irq_svc+0x40/0x6c)
> > > > > > [    2.889020] [<80012160>] (__irq_svc+0x40/0x6c) from
> > > > > > [<8001d724>] (vprintk_emit+0x1bc/0x524)
> > > > > > [    2.897411] [<8001d724>] (vprintk_emit+0x1bc/0x524) from
> > > > > > [<804da5a4>] (printk+0x30/0x40)
> > > > > > [    2.905551] [<804da5a4>] (printk+0x30/0x40) from [<80630138>]
> > > > > > (mousedev_init+0x4c/0x60)
> > > > > > [    2.913617] [<80630138>] (mousedev_init+0x4c/0x60) from
> > > > > > [<806178fc>] (do_one_initcall+0x94/0x14c)
> > > > > > [    2.922537] [<806178fc>] (do_one_initcall+0x94/0x14c) from
> > > > > > [<80617b20>] (kernel_init_freeable+0x16c/0x22c)
> > > > > > [    2.932230] [<80617b20>] (kernel_init_freeable+0x16c/0x22c)
> > > > > > from [<804d8cbc>] (kernel_init+0x8/0x150)
> > > > > > [    2.941486] [<804d8cbc>] (kernel_init+0x8/0x150) from
> > > > > > [<8000ea70>] (ret_from_fork+0x14/0x24)
> > > > > > [    2.949932] handlers:
> > > > > > [    2.952227] [<8033fc58>] ci_irq
> > > > > > [    2.955388] Disabling IRQ #238
> > > > > 
> > > > > Marek, I have a test at imx28evk (Freescale imx28evk) for this
> > > > > patchset, it works well for dual-role switch after adding below dts
> > > > > change. There is a kernel dump when works at udc mode(due to enable
> > > > > CONFIG_LOCKUP_DETECTOR), at the first connect, but it does not
> > > > > affect function, it should be a common chipidea problem, I will
> > > > > check later.
> > > > 
> > > > Sorry for the long delay. I finally got further with this, at least
> > > > now I can switch Periph->Host again, but if I do so the other way
> > > > around (unplug host adapter and plug computer cable) and dump the
> > > > controller registers, I still see the controller in "host"
> > > > (according to USBMODE register) mode. The controller stays in HOST
> > > > mode even if I force it to gadget mode by writing "gadget" into
> > > > debugfs "role" entry. I will keep poking.
> > > > 
> > > > In the meantime, I guess if it works on the EVK, it might be a
> > > > hardware error so I better stop stalling you. I'll drop you an email
> > > > if I find what this problem is.
> > > 
> > > No, the problem is you did not config the USB OTG ID pin.
> > > Do the same thing I have done at imx28evk, you probably will work
> > > if the ID pin is the same at your board.
> > 
> > That one I have configured, exactly the same way you did (except that
> > mine is MX28_PAD_PWM2__USB0_ID).
> > 
> > If it was the OTG ID pin issue, would the manual switching not work ?
> 
> I can't see at your imx28-m28evk.dts, besides, make sure this pin can be
> configed as USB OTG ID pin, and fsl,pull-up = <1>. Since you are still
> at host mode, I suspect it is the ID problem.
> You can check if there is ID INT when unplug out B-to-A cable.
> 
> I have not tried the manual switching, but first, you need to close your
> vbus supply.

I think we can close this issue, I will now be also getting MX28EVK. Thanks for 
all your help!

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Peter Chen]

On Thu, Jul 25, 2013 at 07:55:23AM +0200, Marek Vasut wrote:
> Hi Peter,
> 
> > 
> > I have not tried the manual switching, but first, you need to close your
> > vbus supply.
> 
> I think we can close this issue, I will now be also getting MX28EVK. Thanks for 
> all your help!
> 
> 

Great. What's the problem? Any changes for this patchset?

-- 

Best Regards,
Peter Chen

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Dear Peter Chen,

> On Thu, Jul 25, 2013 at 07:55:23AM +0200, Marek Vasut wrote:
> > Hi Peter,
> > 
> > > I have not tried the manual switching, but first, you need to close
> > > your vbus supply.
> > 
> > I think we can close this issue, I will now be also getting MX28EVK.
> > Thanks for all your help!
> 
> Great. What's the problem? Any changes for this patchset?

No changes needed, I had VDD5V constantly tied to 5V so I didn't get the VBUS 
interrupt.

Best regards,
Marek Vasut

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Fabio Estevam]

Hi Alexander,

On Thu, Jul 11, 2013 at 3:27 AM, Peter Chen <peter.chen@freescale.com> wrote:
> This patchset adds tested otg id switch function and
> vbus connect and disconnect detection for chipidea driver.
> And fix kinds of bugs found at chipidea drivers after enabling
> id and vbus detection.
>
> This patch is fully tested at imx6 sabresd platform.
> My chipidea repo: https://github.com/hzpeterchen/linux-usb.git
>
> Changes for v12:
> - Rebased greg's usb-next tree (3.10.0-rc7+)
> - Split more small patches for single function and fix.

Any comments about this series?

[PATCH v12 00/13] Add tested id switch and vbus connect detect support for Chipidea

[Marek Vasut]

Dear Peter Chen,

> This patchset adds tested otg id switch function and
> vbus connect and disconnect detection for chipidea driver.
> And fix kinds of bugs found at chipidea drivers after enabling
> id and vbus detection.
> 
> This patch is fully tested at imx6 sabresd platform.
> My chipidea repo: https://github.com/hzpeterchen/linux-usb.git

Patchset:

Tested-by: Marek Vasut <marex@denx.de>

on two STMP3780-based boards (not yet mainline) and two MX28-based boards.

Best regards,
Marek Vasut