Add fully tested id switch and vbus connect detect support for Chipidea

The main design flow is the same with msm otg driver, that is the id and vbus interrupt are handled at core driver, others are handled by individual drivers.

- At former design, when switch usb role from device->host, it will call udc_stop, it will remove the gadget driver, so when switch role from host->device, it can't add gadget driver any more.
At new design, when role switch occurs, the gadget just calls usb_gadget_vbus_disconnect/usb_gadget_vbus_connect as well as reset controller, it will not free any device/gadget structure

- Add vbus connect and disconnect to core interrupt handler, it can notify udc driver by calling usb_gadget_vbus_disconnect /usb_gadget_vbus_connect.

Signed-off-by Peter Chen <[email protected]>

---
 drivers/usb/chipidea/bits.h |   10 +++
 drivers/usb/chipidea/ci.h   |    8 ++-
 drivers/usb/chipidea/core.c |  177 ++++++++++++++++++++++++++++++++++++++----
 drivers/usb/chipidea/otg.c  |   28 +++++---
 drivers/usb/chipidea/otg.h  |    3 +
 drivers/usb/chipidea/udc.c  |    2 +
 6 files changed, 200 insertions(+), 28 deletions(-)

drivers/usb/chipidea/bits.h +10 -0

index 050de85..ba9c6ef 100644
@@ -65,11 +65,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)

drivers/usb/chipidea/ci.h +7 -1

index 8702871..325d790 100644
@@ -130,6 +130,7 @@ struct hw_bank {
  * @transceiver: pointer to USB PHY, if any
  * @hcd: pointer to usb_hcd for ehci host driver
  * @otg: for otg support
+ * @events: events for otg, and handled at ci_role_work
  */
 struct ci13xxx {
 	struct device			*dev;

@@ -140,6 +141,7 @@ struct ci13xxx {
 	enum ci_role			role;
 	bool				is_otg;
 	struct work_struct		work;
+	struct delayed_work		dwork;
 	struct workqueue_struct		*wq;
 
 	struct dma_pool			*qh_pool;

@@ -165,7 +167,9 @@ struct ci13xxx {
 	bool				global_phy;
 	struct usb_phy			*transceiver;
 	struct usb_hcd			*hcd;
-	struct usb_otg      otg;
+	struct usb_otg      		otg;
+	bool				id_event;
+	bool				b_sess_valid_event;
 };
 
 static inline struct ci_role_driver *ci_role(struct ci13xxx *ci)

@@ -314,4 +318,6 @@ int hw_port_test_set(struct ci13xxx *ci, u8 mode);
 
 u8 hw_port_test_get(struct ci13xxx *ci);
 
+void ci_handle_vbus_change(struct ci13xxx *ci);
+
 #endif	/* __DRIVERS_USB_CHIPIDEA_CI_H */

drivers/usb/chipidea/core.c +116 -7

index aebf695..f8f8484 100644
@@ -73,6 +73,7 @@
 #include "bits.h"
 #include "host.h"
 #include "debug.h"
+#include "otg.h"
 
 /* Controller register map */
 static uintptr_t ci_regs_nolpm[] = {

@@ -199,6 +200,14 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 	if (ci->hw_ep_max > ENDPT_MAX)
 		return -ENODEV;
 
+	/* Disable all interrupts bits */
+	hw_write(ci, OP_USBINTR, 0xffffffff, 0);
+	ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS);
+
+	/* Clear all interrupts status bits*/
+	hw_write(ci, OP_USBSTS, 0xffffffff, 0xffffffff);
+	ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS);
+
 	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);
 

@@ -265,24 +274,124 @@ static enum ci_role ci_otg_role(struct ci13xxx *ci)
 }
 
 /**
- * ci_role_work - perform role changing based on ID pin
- * @work: work struct
+ * 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
  */
-static void ci_role_work(struct work_struct *work)
+static int hw_wait_reg(struct ci13xxx *ci, enum ci13xxx_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;
+}
+
+#define CI_VBUS_STABLE_TIMEOUT 500
+static void ci_handle_id_switch(struct ci13xxx *ci)
 {
-	struct ci13xxx *ci = container_of(work, struct ci13xxx, 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);
+		/* 1. Finish the current role */
+		if (ci->role == CI_ROLE_GADGET) {
+			usb_gadget_vbus_disconnect(&ci->gadget);
+			/* host doesn't care B_SESSION_VALID event */
+			ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+			ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
+			ci->role = CI_ROLE_END;
+			/* reset controller */
+			hw_device_reset(ci, USBMODE_CM_IDLE);
+		} else if (ci->role == CI_ROLE_HOST) {
+			ci_role_stop(ci);
+			/* reset controller */
+			hw_device_reset(ci, USBMODE_CM_IDLE);
+		}
+
+		/* 2. Turn on/off vbus according to coming role */
+		if (ci_otg_role(ci) == CI_ROLE_GADGET) {
+			otg_set_vbus(&ci->otg, false);
+			/* wait vbus lower than OTGSC_BSV */
+			hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
+					CI_VBUS_STABLE_TIMEOUT);
+		} else if (ci_otg_role(ci) == CI_ROLE_HOST) {
+			otg_set_vbus(&ci->otg, true);
+			/* wait vbus higher than OTGSC_AVV */
+			hw_wait_reg(ci, OP_OTGSC, OTGSC_AVV, OTGSC_AVV,
+					CI_VBUS_STABLE_TIMEOUT);
+		}
+
+		/* 3. Begin the new role */
+		if (ci_otg_role(ci) == CI_ROLE_GADGET) {
+			ci->role = role;
+			ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+			ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+		} else if (ci_otg_role(ci) == CI_ROLE_HOST) {
+			ci_role_start(ci, role);
+		}
 	}
 }
 
+void ci_handle_vbus_change(struct ci13xxx *ci)
+{
+	u32 otgsc = hw_read(ci, OP_OTGSC, ~0);
+
+	if (otgsc & OTGSC_BSV)
+		usb_gadget_vbus_connect(&ci->gadget);
+	else
+		usb_gadget_vbus_disconnect(&ci->gadget);
+}
+
+/**
+ * ci_otg_work - perform otg (vbus/id) event handle
+ * @work: work struct
+ */
+static void ci_otg_work(struct work_struct *work)
+{
+	struct ci13xxx *ci = container_of(work, struct ci13xxx, 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);
+}
+
+static void ci_delayed_work(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct ci13xxx *ci = container_of(dwork, struct ci13xxx, dwork);
+
+	otg_set_vbus(&ci->otg, true);
+
+}
+
 static ssize_t show_role(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {

drivers/usb/chipidea/core.c +29 -6

-	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 ((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;
 }
 

@@ -398,6 +524,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	struct resource	*res;
 	void __iomem	*base;
 	int		ret;
+	u32		otgsc;
 
 	if (!dev->platform_data) {
 		dev_err(dev, "platform data missing\n");

@@ -443,7 +570,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	INIT_WORK(&ci->work, ci_role_work);
+	INIT_WORK(&ci->work, ci_otg_work);
+	INIT_DELAYED_WORK(&ci->dwork, ci_delayed_work);
 	ci->wq = create_singlethread_workqueue("ci_otg");
 	if (!ci->wq) {
 		dev_err(dev, "can't create workqueue\n");

@@ -466,7 +594,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	}
 
 	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
+		dev_dbg(dev, "support otg\n");
 		ci->is_otg = true;
+		/* if otg is supported, create struct usb_otg */
+		ci_hdrc_otg_init(ci);
 		/* ID pin needs 1ms debouce time, we delay 2ms for safe */
 		mdelay(2);
 		ci->role = ci_otg_role(ci);

@@ -483,6 +614,17 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		goto rm_wq;

drivers/usb/chipidea/core.c +5 -2

+	}
+
 	platform_set_drvdata(pdev, ci);
 	ret = request_irq(ci->irq, ci_irq, IRQF_SHARED, ci->platdata->name,
 			  ci);

@@ -493,8 +635,9 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret)
 		goto rm_attr;
 
-	if (ci->is_otg)
-		hw_write(ci, OP_OTGSC, OTGSC_IDIE, OTGSC_IDIE);
+	/* Handle the situation that usb device at the MicroB to A cable */
+	if (ci->is_otg && !(otgsc & OTGSC_ID))
+		queue_delayed_work(ci->wq, &ci->dwork, msecs_to_jiffies(500));
 
 	return ret;
 

drivers/usb/chipidea/otg.c +18 -10

index 7dea3b3..2986d91 100644
@@ -10,21 +10,28 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/platform_device.h>
-#include <linux/module.h>
-#include <linux/io.h>
-#include <linux/irq.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/gadget.h>
 #include <linux/usb/chipidea.h>
 
 #include "ci.h"
-#include "udc.h"
 #include "bits.h"
-#include "host.h"
-#include "debug.h"
+
+void ci_clear_otg_interrupt(struct ci13xxx *ci, u32 bits)
+{
+	/* Only clear request bits */
+	hw_write(ci, OP_OTGSC, OTGSC_INT_STATUS_BITS, bits);
+}
+
+void ci_enable_otg_interrupt(struct ci13xxx *ci, u32 bits)
+{
+	hw_write(ci, OP_OTGSC, bits, bits);
+}
+
+void ci_disable_otg_interrupt(struct ci13xxx *ci, u32 bits)
+{
+	hw_write(ci, OP_OTGSC, bits, 0);
+}
 
 static int ci_otg_set_peripheral(struct usb_otg *otg,
 		struct usb_gadget *periph)

@@ -55,6 +62,7 @@ int ci_hdrc_otg_init(struct ci13xxx *ci)
 	ci->otg.set_host = ci_otg_set_host;
 	if (!IS_ERR_OR_NULL(ci->transceiver))
 		ci->transceiver->otg = &ci->otg;
+	ci_enable_otg_interrupt(ci, OTGSC_IDIE);
 
 	return 0;
 }

drivers/usb/chipidea/otg.h +3 -0

index b4c6b3e..fa30428 100644
@@ -2,5 +2,8 @@
 #define __DRIVERS_USB_CHIPIDEA_OTG_H
 
 int ci_hdrc_otg_init(struct ci13xxx *ci);
+void ci_clear_otg_interrupt(struct ci13xxx *ci, u32 bits);
+void ci_enable_otg_interrupt(struct ci13xxx *ci, u32 bits);
+void ci_disable_otg_interrupt(struct ci13xxx *ci, u32 bits);
 
 #endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */

drivers/usb/chipidea/udc.c +2 -0

index d214448..83e54bb 100644
@@ -1371,6 +1371,7 @@ static int ci13xxx_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)

@@ -1378,6 +1379,7 @@ static int ci13xxx_vbus_session(struct usb_gadget *_gadget, int is_active)
 				CI13XXX_CONTROLLER_STOPPED_EVENT);
 			_gadget_stop_activity(&ci->gadget);
 			pm_runtime_put_sync(&_gadget->dev);
+			dev_dbg(ci->dev, "Disconnected from host\n");
 		}
 	}
 

Signature · 1 line

1.7.0.4

Quoted 7 lines · Peter Chen

static ssize_t show_role(struct device *dev, struct device_attribute *attr,

drivers/usb/chipidea/core.c +0 -0

 			 char *buf)
 {

@@ -321,19 +430,36 @@ static irqreturn_t ci_irq(int irq, void *data)
 	irqreturn_t ret = IRQ_NONE;
 	u32 otgsc = 0;
 

drivers/usb/chipidea/core.c static irqreturn_t ci_irq(int irq, void *data) · line 433

Quoted 3 lines

drivers/usb/chipidea/core.c +1 -2

-	if (ci->is_otg)
-		otgsc = hw_read(ci, OP_OTGSC, ~0);
+	otgsc = hw_read(ci, OP_OTGSC, ~0);

My spec says that in non-otg implementations OTGSC is reserved. We probably shouldn't try any of this for such devices.

drivers/usb/chipidea/core.c static int ci_hdrc_probe(struct platform_device *pdev) · line 624

Quoted 9 lines

drivers/usb/chipidea/core.c +8 -0

+	otgsc = hw_read(ci, OP_OTGSC, ~0);
+	/*
+	 * if it is device mode:
+	 * - Enable vbus detect
+	 * - If it has already connected to host, notify udc
+	 */
+	if (ci->role == CI_ROLE_GADGET) {
+		ci_enable_otg_interrupt(ci, OTGSC_BSVIE);

We should only do this for otg capable devices, otherwise OTGSC is reserved.

Review static irqreturn_t ci_irq(int irq, void *data) · line 442

Quoted 2 lines

(quoted diff) +1 -0

+		ci_handle_vbus_change(ci);

Shouldn't this be part of the gadget role start like I suggested a couple of months back? Maybe ci_enable_otg_interrupt() can go there too.

The main design flow is the same with msm otg driver, that is the id and vbus interrupt are handled at core driver, others are handled by individual drivers.

- At former design, when switch usb role from device->host, it will call udc_stop, it will remove the gadget driver, so when switch role from host->device, it can't add gadget driver any more.
At new design, when role switch occurs, the gadget just calls usb_gadget_vbus_disconnect/usb_gadget_vbus_connect as well as reset controller, it will not free any device/gadget structure

- Add vbus connect and disconnect to core interrupt handler, it can notify udc driver by calling usb_gadget_vbus_disconnect /usb_gadget_vbus_connect.

Signed-off-by Peter Chen <[email protected]>

drivers/usb/chipidea/core.c +1 -0

@@ -483,6 +614,17 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		goto rm_wq;
 	}
 
+	otgsc = hw_read(ci, OP_OTGSC, ~0);

Quoted 16 lines · Peter Chen

The main design flow is the same with msm otg driver, that is the id and vbus interrupt are handled at core driver, others are handled by individual drivers.

- At former design, when switch usb role from device->host, it will call udc_stop, it will remove the gadget driver, so when switch role from host->device, it can't add gadget driver any more.
At new design, when role switch occurs, the gadget just calls usb_gadget_vbus_disconnect/usb_gadget_vbus_connect as well as reset controller, it will not free any device/gadget structure

- Add vbus connect and disconnect to core interrupt handler, it can notify udc driver by calling usb_gadget_vbus_disconnect /usb_gadget_vbus_connect.

Signed-off-by Peter Chen <[email protected]>

[snip]

Quoted 4 lines

drivers/usb/chipidea/core.c +0 -0

@@ -483,6 +614,17 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		goto rm_wq;
 	}
 

drivers/usb/chipidea/core.c static int ci_hdrc_probe(struct platform_device *pdev) · line 627

Quoted 11 lines

drivers/usb/chipidea/core.c +11 -0

+	otgsc = hw_read(ci, OP_OTGSC, ~0);
+	/*
+	 * if it is device mode:
+	 * - Enable vbus detect
+	 * - If it has already connected to host, notify udc
+	 */
+	if (ci->role == CI_ROLE_GADGET) {
+		ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+		ci_handle_vbus_change(ci);
+	}
+

Actually, this doesn't work, neither here, nor in udc_start(), where the next patch moves it. If you start in host role with A plug, unplug it, plug B and load a gadget module,

Quoted 25 lines · Alexander Shishkin

Quoted 16 lines · Peter Chen

The main design flow is the same with msm otg driver, that is the id and vbus interrupt are handled at core driver, others are handled by individual drivers.

- At former design, when switch usb role from device->host, it will call udc_stop, it will remove the gadget driver, so when switch role from host->device, it can't add gadget driver any more.
At new design, when role switch occurs, the gadget just calls usb_gadget_vbus_disconnect/usb_gadget_vbus_connect as well as reset controller, it will not free any device/gadget structure

- Add vbus connect and disconnect to core interrupt handler, it can notify udc driver by calling usb_gadget_vbus_disconnect /usb_gadget_vbus_connect.

Signed-off-by Peter Chen <[email protected]>

[snip]

Quoted 4 lines

(quoted diff) +0 -0

@@ -483,6 +614,17 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		goto rm_wq;
 	}
 

Quoted 15 lines

Review static int ci_hdrc_probe(struct platform_device *pdev) · line 627

Quoted 11 lines

(quoted diff) +11 -0

+	otgsc = hw_read(ci, OP_OTGSC, ~0);
+	/*
+	 * if it is device mode:
+	 * - Enable vbus detect
+	 * - If it has already connected to host, notify udc
+	 */
+	if (ci->role == CI_ROLE_GADGET) {
+		ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+		ci_handle_vbus_change(ci);
+	}
+

Actually, this doesn't work, neither here, nor in udc_start(), where the next patch moves it. If you start in host role with A plug, unplug it, plug B and load a gadget module,

When we unplug A, device's role start will be called, that is udc_id_switch_for_device in my patch, it will enable vbus interrupt. Then, when we plug B, there will be an vbus interrupt, then the ci->vbus_active will be set, then when we load a gadget module, the enumeration will begin like I said at last email.

Ok, I say "fully tested" means I tested cases for my development, which
includes: host/device is plugged during boots up, device/host switch, host only controller, load gadget before/next cable plugs in. If you think it is not enough, I will skip "fully" when sends next version patch.

Quoted 1 line · Peter Chen

On Thu, Jan 24, 2013 at 05:25:17PM +0200, Alexander Shishkin wrote:

What doesn't happen is the reset into device mode (unless you have _REGS_SHARED set, which by the way seems a bit misleading) and we're still doing nothing till the vbus interrupt comes. So the REGS_SHARED is the real problem in this case.

The main design flow is the same with msm otg driver, that is the id and vbus interrupt are handled at core driver, others are handled by individual drivers.

- At former design, when switch usb role from device->host, it will call udc_stop, it will remove the gadget driver, so when switch role from host->device, it can't add gadget driver any more.
At new design, when role switch occurs, the gadget just calls usb_gadget_vbus_disconnect/usb_gadget_vbus_connect as well as reset controller, it will not free any device/gadget structure

- Add vbus connect and disconnect to core interrupt handler, it can notify udc driver by calling usb_gadget_vbus_disconnect /usb_gadget_vbus_connect.

Signed-off-by Peter Chen <[email protected]>

---
  drivers/usb/chipidea/bits.h |   10 +++
  drivers/usb/chipidea/ci.h   |    8 ++-
  drivers/usb/chipidea/core.c |  177 ++++++++++++++++++++++++++++++++++++++----
  drivers/usb/chipidea/otg.c  |   28 +++++---
  drivers/usb/chipidea/otg.h  |    3 +
  drivers/usb/chipidea/udc.c  |    2 +
  6 files changed, 200 insertions(+), 28 deletions(-)

drivers/usb/chipidea/bits.h +10 -0

index 050de85..ba9c6ef 100644
@@ -65,11 +65,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)

drivers/usb/chipidea/ci.h +2 -0

index 8702871..325d790 100644
@@ -130,6 +130,7 @@ struct hw_bank {
   * @transceiver: pointer to USB PHY, if any
   * @hcd: pointer to usb_hcd for ehci host driver
   * @otg: for otg support
+ * @events: events for otg, and handled at ci_role_work
   */
  struct ci13xxx {
  	struct device			*dev;

@@ -140,6 +141,7 @@ struct ci13xxx {
  	enum ci_role			role;
  	bool				is_otg;
  	struct work_struct		work;
+	struct delayed_work		dwork;
  	struct workqueue_struct		*wq;
 
  	struct dma_pool			*qh_pool;

drivers/usb/chipidea/ci.h +4 -0

+	bool				id_event;
+	bool				b_sess_valid_event;
  };
 
  static inline struct ci_role_driver *ci_role(struct ci13xxx *ci)

@@ -314,4 +318,6 @@ int hw_port_test_set(struct ci13xxx *ci, u8 mode);
 
  u8 hw_port_test_get(struct ci13xxx *ci);
 
+void ci_handle_vbus_change(struct ci13xxx *ci);
+
  #endif	/* __DRIVERS_USB_CHIPIDEA_CI_H */

drivers/usb/chipidea/core.c +30 -3

index aebf695..f8f8484 100644
@@ -73,6 +73,7 @@
  #include "bits.h"
  #include "host.h"
  #include "debug.h"
+#include "otg.h"
 
  /* Controller register map */
  static uintptr_t ci_regs_nolpm[] = {

@@ -199,6 +200,14 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
  	if (ci->hw_ep_max > ENDPT_MAX)
  		return -ENODEV;
 
+	/* Disable all interrupts bits */
+	hw_write(ci, OP_USBINTR, 0xffffffff, 0);
+	ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS);
+
+	/* Clear all interrupts status bits*/
+	hw_write(ci, OP_USBSTS, 0xffffffff, 0xffffffff);
+	ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS);
+
  	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);
 

@@ -265,24 +274,124 @@ static enum ci_role ci_otg_role(struct ci13xxx *ci)
  }
 
  /**
- * ci_role_work - perform role changing based on ID pin
- * @work: work struct
+ * 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
   */
-static void ci_role_work(struct work_struct *work)
+static int hw_wait_reg(struct ci13xxx *ci, enum ci13xxx_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",

Hi,

drivers/usb/chipidea/ci.h struct ci13xxx { · line 170

Quoted 5 lines

drivers/usb/chipidea/ci.h +1 -1

 	bool				global_phy;
 	struct usb_phy			*transceiver;
 	struct usb_hcd			*hcd;
-	struct usb_otg      otg;
+	struct usb_otg      		otg;

You have added *otg* in previous patch and added a tab for *otg* in this patch.

Review

Quoted 3 lines

(quoted diff) +2 -0

+
+#define CI_VBUS_STABLE_TIMEOUT 500

Just curious.. how was this timeout value obtained?

Thanks
Kishon

Quoted 9 lines · kishon

Hi,

drivers/usb/chipidea/ci.h struct ci13xxx { · line 170

Quoted 5 lines

drivers/usb/chipidea/ci.h +1 -1

 	bool				global_phy;
 	struct usb_phy			*transceiver;
 	struct usb_hcd			*hcd;
-	struct usb_otg      otg;
+	struct usb_otg      		otg;

You have added *otg* in previous patch and added a tab for *otg* in this patch.

thanks, I will change at previous patch

Quoted 1 line

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+#define CI_VBUS_STABLE_TIMEOUT 500

Just curious.. how was this timeout value obtained?

Just a timeout value, if the vbus goes to required value, it will quit. Besides, 5s for vbus stable should be enough for an well behaviour hardware.

Quoted 17 lines · Peter Chen

Quoted 9 lines · kishon

Hi,

drivers/usb/chipidea/ci.h struct ci13xxx { · line 170

Quoted 5 lines

drivers/usb/chipidea/ci.h +1 -1

 	bool				global_phy;
 	struct usb_phy			*transceiver;
 	struct usb_hcd			*hcd;
-	struct usb_otg      otg;
+	struct usb_otg      		otg;

You have added *otg* in previous patch and added a tab for *otg* in this patch.

thanks, I will change at previous patch

Quoted 5 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

+
+#define CI_VBUS_STABLE_TIMEOUT 500

Just curious.. how was this timeout value obtained?

Quoted 2 lines

Just a timeout value, if the vbus goes to required value, it will quit. Besides, 5s for vbus stable should be enough for an well behaviour hardware.

Can you also mention this in the patch, since it raises questions?

Thanks,
--
Alex

- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
- Create the gadget at ci_hdrc_probe if the gadget is supported at that port, the main purpose for this is to avoid gadget module load fail at init.rc

Signed-off-by Peter Chen <[email protected]>

---
 drivers/usb/chipidea/ci.h   |   19 +++++++++++-
 drivers/usb/chipidea/core.c |   65 ++++++++++++++++++------------------------
 drivers/usb/chipidea/host.c |    2 +
 drivers/usb/chipidea/udc.c  |   33 ++++++++++++++++++++-
 4 files changed, 78 insertions(+), 41 deletions(-)

drivers/usb/chipidea/ci.h +7 -2

index 325d790..00939e6 100644
@@ -67,14 +67,18 @@ enum ci_role {
 
 /**
  * struct ci_role_driver - host/gadget role driver
- * start: start this role
- * stop: stop this role
+ * init: init this role (used at module probe)
+ * start: start this role (used at id switch)
+ * stop: stop this role (used at id switch)
+ * destroy: destroy this role (used at module remove)
  * irq: irq handler for this role
  * name: role name string (host/gadget)
  */
 struct ci_role_driver {
+	int		(*init)(struct ci13xxx *);
 	int		(*start)(struct ci13xxx *);
 	void		(*stop)(struct ci13xxx *);
+	void		(*destroy)(struct ci13xxx *);
 	irqreturn_t	(*irq)(struct ci13xxx *);
 	const char	*name;
 };

@@ -206,6 +210,17 @@ static inline void ci_role_stop(struct ci13xxx *ci)
 	ci->roles[role]->stop(ci);
 }
 
+static inline void ci_role_destroy(struct ci13xxx *ci)

drivers/usb/chipidea/ci.h +0 -0

 /******************************************************************************
  * REGISTERS
  *****************************************************************************/

drivers/usb/chipidea/core.c +28 -37

index f8f8484..a5adf1a 100644
@@ -315,27 +315,16 @@ static void ci_handle_id_switch(struct ci13xxx *ci)
 			ci_role(ci)->name, ci->roles[role]->name);
 
 		/* 1. Finish the current role */
-		if (ci->role == CI_ROLE_GADGET) {
-			usb_gadget_vbus_disconnect(&ci->gadget);
-			/* host doesn't care B_SESSION_VALID event */
-			ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
-			ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
-			ci->role = CI_ROLE_END;
-			/* reset controller */
-			hw_device_reset(ci, USBMODE_CM_IDLE);
-		} else if (ci->role == CI_ROLE_HOST) {
-			ci_role_stop(ci);
-			/* reset controller */
-			hw_device_reset(ci, USBMODE_CM_IDLE);
-		}
+		ci_role_stop(ci);
+		hw_device_reset(ci, USBMODE_CM_IDLE);
 
 		/* 2. Turn on/off vbus according to coming role */
-		if (ci_otg_role(ci) == CI_ROLE_GADGET) {
+		if (role == CI_ROLE_GADGET) {
 			otg_set_vbus(&ci->otg, false);
 			/* wait vbus lower than OTGSC_BSV */
 			hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
 					CI_VBUS_STABLE_TIMEOUT);
-		} else if (ci_otg_role(ci) == CI_ROLE_HOST) {
+		} else if (role == CI_ROLE_HOST) {
 			otg_set_vbus(&ci->otg, true);
 			/* wait vbus higher than OTGSC_AVV */
 			hw_wait_reg(ci, OP_OTGSC, OTGSC_AVV, OTGSC_AVV,

@@ -343,13 +332,7 @@ static void ci_handle_id_switch(struct ci13xxx *ci)
 		}
 
 		/* 3. Begin the new role */
-		if (ci_otg_role(ci) == CI_ROLE_GADGET) {
-			ci->role = role;
-			ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
-			ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
-		} else if (ci_otg_role(ci) == CI_ROLE_HOST) {
-			ci_role_start(ci, role);
-		}
+		ci_role_start(ci, role);
 	}
 }
 

@@ -585,7 +568,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 	ret = ci_hdrc_gadget_init(ci);
 	if (ret)
-		dev_info(dev, "doesn't support gadget\n");
+		dev_info(dev, "doesn't support gadget, ret=%d\n", ret);
 
 	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
 		dev_err(dev, "no supported roles\n");

@@ -607,22 +590,30 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 			: CI_ROLE_GADGET;
 	}
 
-	ret = ci_role_start(ci, ci->role);
-	if (ret) {
-		dev_err(dev, "can't start %s role\n", ci_role(ci)->name);
-		ret = -ENODEV;
-		goto rm_wq;
-	}
-
 	otgsc = hw_read(ci, OP_OTGSC, ~0);
+
 	/*
-	 * if it is device mode:
-	 * - Enable vbus detect
-	 * - If it has already connected to host, notify udc
+	 * If the gadget is supported, call its init unconditionally,
+	 * We need to support load gadget module at init.rc.
 	 */
-	if (ci->role == CI_ROLE_GADGET) {
-		ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
-		ci_handle_vbus_change(ci);
+	if (ci->roles[CI_ROLE_GADGET]) {
+		ret = ci->roles[CI_ROLE_GADGET]->init(ci);
+		if (ret) {
+			dev_err(dev, "can't init %s role, ret=%d\n",
+					ci_role(ci)->name, ret);
+			ret = -ENODEV;
+			goto rm_wq;
+		}
+	}
+
+	if (ci->role == CI_ROLE_HOST) {
+		ret = ci->roles[CI_ROLE_HOST]->init(ci);
+		if (ret) {
+			dev_err(dev, "can't init %s role, ret=%d\n",
+					ci_role(ci)->name, ret);
+			ret = -ENODEV;
+			goto rm_wq;
+		}
 	}
 
 	platform_set_drvdata(pdev, ci);

@@ -660,7 +651,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
 	destroy_workqueue(ci->wq);
 	device_remove_file(ci->dev, &dev_attr_role);
 	free_irq(ci->irq, ci);
-	ci_role_stop(ci);
+	ci_role_destroy(ci);
 
 	return 0;
 }

drivers/usb/chipidea/host.c +0 -0

index caecad9..6024a4f 100644

drivers/usb/chipidea/host.c +0 -0

 	rdrv->irq	= host_irq;
 	rdrv->name	= "host";
 	ci->roles[CI_ROLE_HOST] = rdrv;

drivers/usb/chipidea/udc.c +31 -2

index 83e54bb..e6a4ec0 100644
@@ -31,6 +31,7 @@
 
 #include "ci.h"
 #include "udc.h"
+#include "otg.h"
 #include "bits.h"
 #include "debug.h"
 

@@ -1754,6 +1755,16 @@ static int udc_start(struct ci13xxx *ci)
 	pm_runtime_no_callbacks(&ci->gadget.dev);
 	pm_runtime_enable(&ci->gadget.dev);
 
+	/*
+	 * if it is device mode:
+	 * - Enable vbus detect
+	 * - If it has already connected to host, notify udc
+	 */
+	if (ci->role == CI_ROLE_GADGET) {
+		ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+		ci_handle_vbus_change(ci);
+	}
+
 	return retval;
 
 remove_trans:

@@ -1780,6 +1791,22 @@ free_qh_pool:
 	return retval;
 }
 
+static int udc_id_switch_for_device(struct ci13xxx *ci)
+{
+	ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+	ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+
+	return 0;
+}
+
+static void udc_id_switch_for_host(struct ci13xxx *ci)
+{
+	usb_gadget_vbus_disconnect(&ci->gadget);
+	/* host doesn't care B_SESSION_VALID event */
+	ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+	ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
+}
+
 /**
  * udc_remove: parent remove must call this to remove UDC
  *

@@ -1825,8 +1852,10 @@ int ci_hdrc_gadget_init(struct ci13xxx *ci)
 	if (!rdrv)
 		return -ENOMEM;
 
-	rdrv->start	= udc_start;
-	rdrv->stop	= udc_stop;
+	rdrv->init	= udc_start;
+	rdrv->start	= udc_id_switch_for_device;
+	rdrv->stop	= udc_id_switch_for_host;
+	rdrv->destroy	= udc_stop;
 	rdrv->irq	= udc_irq;
 	rdrv->name	= "gadget";
 	ci->roles[CI_ROLE_GADGET] = rdrv;

Signature · 1 line

1.7.0.4

Quoted 5 lines · Peter Chen

- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
- Create the gadget at ci_hdrc_probe if the gadget is supported at that port, the main purpose for this is to avoid gadget module load fail at init.rc

I don't think it's necessary to mention android-specific init scripts here in our patches.

drivers/usb/chipidea/ci.h static inline void ci_role_stop(struct ci13xxx *ci) · line 223

Quoted 13 lines

drivers/usb/chipidea/ci.h +11 -0

+static inline void ci_role_destroy(struct ci13xxx *ci)
+{
+	enum ci_role role = ci->role;
+
+	if (role == CI_ROLE_END)
+		return;
+
+	ci->role = CI_ROLE_END;
+
+	ci->roles[role]->destroy(ci);
+}

What does this do? It should take role an a parameter, at least. Or it can be called ci_roles_destroy() and, well, destroy all the roles. Now we're in a situation where we destroy one.

The idea is that, with this api, we can (and should) have both roles allocated all the time, but only one role start()'ed.

What we can do here is move the udc's .init() code to ci_hdrc_gadget_init() and add a complementary ci_hdrc_gadget_destroy(), which we call in ci_hdrc_remove() and probe's error path. And we can probably do something similar for the host, or just leave it as it is now. Seems simpler to me.

Quoted 9 lines

}

drivers/usb/chipidea/host.c +0 -0

index caecad9..6024a4f 100644
@@ -92,8 +92,10 @@ int ci_hdrc_host_init(struct ci13xxx *ci)
 	if (!rdrv)
 		return -ENOMEM;
 

drivers/usb/chipidea/host.c int ci_hdrc_host_init(struct ci13xxx *ci) · line 98

Quoted 4 lines

drivers/usb/chipidea/host.c +2 -0

+	rdrv->init	= host_start;
 	rdrv->start	= host_start;
 	rdrv->stop	= host_stop;
+	rdrv->destroy	= host_stop;

Will this allocate the hcd twice if we start in host role? Looks like that.

Quoted 10 lines · Alexander Shishkin

Quoted 5 lines · Peter Chen

- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
- Create the gadget at ci_hdrc_probe if the gadget is supported at that port, the main purpose for this is to avoid gadget module load fail at init.rc

I don't think it's necessary to mention android-specific init scripts here in our patches.

Ok, will just mention init script.

Quoted 17 lines

Review

Quoted 13 lines

(quoted diff) +11 -0

+static inline void ci_role_destroy(struct ci13xxx *ci)
+{
+	enum ci_role role = ci->role;
+
+	if (role == CI_ROLE_END)
+		return;
+
+	ci->role = CI_ROLE_END;
+
+	ci->roles[role]->destroy(ci);
+}

What does this do? It should take role an a parameter, at least. Or it can be called ci_roles_destroy() and, well, destroy all the roles. Now we're in a situation where we destroy one.

Yes, this function has some problems, I suggest just call two lines at ci_role_destory, do you think so?

	ci->roles[role]->destroy(ci);
	ci->role = CI_ROLE_END;

Quoted 9 lines

The idea is that, with this api, we can (and should) have both roles allocated all the time, but only one role start()'ed.

What we can do here is move the udc's .init() code to ci_hdrc_gadget_init() and add a complementary ci_hdrc_gadget_destroy(), which we call in ci_hdrc_remove() and probe's error path. And we can probably do something similar for the host, or just leave it as it is now. Seems simpler to me.

Yes, current code has bug that it should call ci_role_destroy at probe's error path.
For your comments, it still needs to add destory function for udc which will be called at core.c. I still prefer current way due to below reasons:

- It can keep ci_hdrc_gadget_init() clean
- .init and .remove are different logical function with .start and .stop. The .init and .remove are used for create and destroy, .start and .stop are used at id role switch.

Quoted 9 lines · Peter Chen

Quoted 8 lines · Alexander Shishkin

Quoted 5 lines · Peter Chen

- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
- Create the gadget at ci_hdrc_probe if the gadget is supported at that port, the main purpose for this is to avoid gadget module load fail at init.rc

Which is shorter (32 insertions(+), 53 deletions(-)) and makes the main probe easier on the eyes. What do you think?

Quoted 15 lines · Alexander Shishkin

Quoted 9 lines · Peter Chen

Quoted 8 lines · Alexander Shishkin

Quoted 5 lines · Peter Chen

- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
- Create the gadget at ci_hdrc_probe if the gadget is supported at that port, the main purpose for this is to avoid gadget module load fail at init.rc

drivers/usb/chipidea/core.c +0 -0

@@ -402,6 +402,12 @@ static ssize_t store_role(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 

drivers/usb/chipidea/core.c static ssize_t store_role(struct device *dev, struct device_attribute *attr, · line 410

Quoted 6 lines

drivers/usb/chipidea/core.c +6 -0

+	/*
+	 * there won't be an interrupt in case of manual switching,
+	 * so we need to check vbus session manually
+	 */
+	ci_handle_vbus_change(ci);
+

It may not be used as there will be a vbus interrupt.

drivers/usb/chipidea/udc.c static void udc_stop(struct ci13xxx *ci) · line 1825

Quoted 8 lines

drivers/usb/chipidea/core.c +0 -0

 	return count;
 }
 

drivers/usb/chipidea/udc.c +0 -2

 	}
 	dbg_remove_files(&ci->gadget.dev);
 	device_unregister(&ci->gadget.dev);
-	/* my kobject is dynamic, I swear! */
-	memset(&ci->gadget, 0, sizeof(ci->gadget));

Any reason to delete it, another fix?

Quoted 7 lines

}

/**

drivers/usb/chipidea/host.c +0 -0

@@ -1842,13 +1839,11 @@ int ci_hdrc_gadget_init(struct ci13xxx *ci)
 	if (!rdrv)
 		return -ENOMEM;
 

drivers/usb/chipidea/udc.c int ci_hdrc_gadget_init(struct ci13xxx *ci) · line 1848

Quoted 4 lines

drivers/usb/chipidea/udc.c +0 -2

-	rdrv->init	= udc_start;
 	rdrv->start	= udc_id_switch_for_device;
 	rdrv->stop	= udc_id_switch_for_host;
-	rdrv->destroy	= udc_stop;

Where we call udc_start and udc_stop? And the udc_start should only be called one time.

Quoted 3 lines

Which is shorter (32 insertions(+), 53 deletions(-)) and makes the main probe easier on the eyes. What do you think?

OK, not matter how to change it, it needs to cover below things:
(Covers last email)
- Gadget needs to be only initialized one time.

- Host/device function should be OK when the device on it or the usb cable connects to host during the boots up.

- When the OTG port is at host mode, it should not call any register writing operations at gadget's API.

Quoted 16 lines · Peter Chen

Quoted 15 lines · Alexander Shishkin

Quoted 9 lines · Peter Chen

Quoted 8 lines · Alexander Shishkin

Quoted 5 lines · Peter Chen

- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
- Create the gadget at ci_hdrc_probe if the gadget is supported at that port, the main purpose for this is to avoid gadget module load fail at init.rc

drivers/usb/chipidea/core.c +0 -0

@@ -402,6 +402,12 @@ static ssize_t store_role(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 

Quoted 7 lines

drivers/usb/chipidea/core.c static ssize_t store_role(struct device *dev, struct device_attribute *attr, · line 410

Quoted 6 lines

drivers/usb/chipidea/core.c +6 -0

+	/*
+	 * there won't be an interrupt in case of manual switching,
+	 * so we need to check vbus session manually
+	 */
+	ci_handle_vbus_change(ci);
+

It may not be used as there will be a vbus interrupt.

Not if you write gadget to "role" file.

Quoted 8 lines

drivers/usb/chipidea/udc.c int ci_hdrc_gadget_init(struct ci13xxx *ci) · line 1848

Quoted 6 lines

drivers/usb/chipidea/udc.c +0 -2

 		return -ENOMEM;
 
-	rdrv->init	= udc_start;
 	rdrv->start	= udc_id_switch_for_device;
 	rdrv->stop	= udc_id_switch_for_host;
-	rdrv->destroy	= udc_stop;

Where we call udc_start and udc_stop? And the udc_start should only be called one time.

ci_hdrc_gadget_init() and ci_hdrc_gadget_destroy(). Look closer at the patch.

Quoted 2 lines

- When the OTG port is at host mode, it should not call any register writing operations at gadget's API.

Furthermore, there shouldn't be any calls to the gadget api.