[PATCH 1/5] usb: chipidea: replace ci_role with usb_role

[PATCH 1/5] usb: chipidea: replace ci_role with usb_role

[jun.li]

From: Li Jun <jun.li@nxp.com>

Since there is usb_role which has similar definition like ci_role,
switch to use usb_role, then we can directly compare usb role
with a common definition, this can benifit on usb role switch
class support.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/chipidea/ci.h      | 28 ++++++++++++----------------
 drivers/usb/chipidea/core.c    | 34 +++++++++++++++++-----------------
 drivers/usb/chipidea/debug.c   | 12 ++++++------
 drivers/usb/chipidea/host.c    |  6 +++---
 drivers/usb/chipidea/otg.c     | 14 +++++++-------
 drivers/usb/chipidea/otg.h     |  2 +-
 drivers/usb/chipidea/otg_fsm.c |  4 ++--
 drivers/usb/chipidea/udc.c     |  6 +++---
 8 files changed, 51 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 6a2cc5c..82b86cd 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -16,6 +16,7 @@
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg-fsm.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/role.h>
 #include <linux/ulpi/interface.h>
 
 /******************************************************************************
@@ -102,12 +103,6 @@ struct ci_hw_ep {
 	struct td_node				*pending_td;
 };
 
-enum ci_role {
-	CI_ROLE_HOST = 0,
-	CI_ROLE_GADGET,
-	CI_ROLE_END,
-};
-
 enum ci_revision {
 	CI_REVISION_1X = 10,	/* Revision 1.x */
 	CI_REVISION_20 = 20, /* Revision 2.0 */
@@ -208,8 +203,8 @@ struct ci_hdrc {
 	spinlock_t			lock;
 	struct hw_bank			hw_bank;
 	int				irq;
-	struct ci_role_driver		*roles[CI_ROLE_END];
-	enum ci_role			role;
+	struct ci_role_driver		*roles[USB_ROLE_DEVICE + 1];
+	enum usb_role			role;
 	bool				is_otg;
 	struct usb_otg			otg;
 	struct otg_fsm			fsm;
@@ -258,15 +253,16 @@ struct ci_hdrc {
 
 static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
 {
-	BUG_ON(ci->role >= CI_ROLE_END || !ci->roles[ci->role]);
+	WARN_ON((ci->role != USB_ROLE_HOST && ci->role != USB_ROLE_DEVICE) ||
+	       !ci->roles[ci->role]);
 	return ci->roles[ci->role];
 }
 
-static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role)
+static inline int ci_role_start(struct ci_hdrc *ci, enum usb_role role)
 {
 	int ret;
 
-	if (role >= CI_ROLE_END)
+	if (role != USB_ROLE_HOST && role != USB_ROLE_DEVICE)
 		return -EINVAL;
 
 	if (!ci->roles[role])
@@ -280,12 +276,12 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role)
 
 static inline void ci_role_stop(struct ci_hdrc *ci)
 {
-	enum ci_role role = ci->role;
+	enum usb_role role = ci->role;
 
-	if (role == CI_ROLE_END)
+	if (role == USB_ROLE_NONE)
 		return;
 
-	ci->role = CI_ROLE_END;
+	ci->role = USB_ROLE_NONE;
 
 	ci->roles[role]->stop(ci);
 }
@@ -416,8 +412,8 @@ static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
 #ifdef CONFIG_USB_OTG_FSM
 	struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
 
-	return ci->is_otg && ci->roles[CI_ROLE_HOST] &&
-		ci->roles[CI_ROLE_GADGET] && (otg_caps->srp_support ||
+	return ci->is_otg && ci->roles[USB_ROLE_HOST] &&
+		ci->roles[USB_ROLE_DEVICE] && (otg_caps->srp_support ||
 		otg_caps->hnp_support || otg_caps->adp_support);
 #else
 	return false;
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 26062d6..bc24c5b 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -581,7 +581,7 @@ static irqreturn_t ci_irq(int irq, void *data)
 	}
 
 	/* Handle device/host interrupt */
-	if (ci->role != CI_ROLE_END)
+	if (ci->role != USB_ROLE_NONE)
 		ret = ci_role(ci)->irq(ci);
 
 	return ret;
@@ -835,7 +835,7 @@ 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->roles[CI_ROLE_GADGET])
+	if (ci->is_otg && ci->roles[USB_ROLE_DEVICE])
 		ci_hdrc_otg_destroy(ci);
 }
 
@@ -860,7 +860,7 @@ static ssize_t role_show(struct device *dev, struct device_attribute *attr,
 {
 	struct ci_hdrc *ci = dev_get_drvdata(dev);
 
-	if (ci->role != CI_ROLE_END)
+	if (ci->role != USB_ROLE_NONE)
 		return sprintf(buf, "%s\n", ci_role(ci)->name);
 
 	return 0;
@@ -870,27 +870,27 @@ static ssize_t role_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t n)
 {
 	struct ci_hdrc *ci = dev_get_drvdata(dev);
-	enum ci_role role;
+	enum usb_role role;
 	int ret;
 
-	if (!(ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET])) {
+	if (!(ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE])) {
 		dev_warn(dev, "Current configuration is not dual-role, quit\n");
 		return -EPERM;
 	}
 
-	for (role = CI_ROLE_HOST; role < CI_ROLE_END; role++)
+	for (role = USB_ROLE_DEVICE; role > USB_ROLE_NONE; role--)
 		if (!strncmp(buf, ci->roles[role]->name,
 			     strlen(ci->roles[role]->name)))
 			break;
 
-	if (role == CI_ROLE_END || role == ci->role)
+	if (role == USB_ROLE_NONE || role == ci->role)
 		return -EINVAL;
 
 	pm_runtime_get_sync(dev);
 	disable_irq(ci->irq);
 	ci_role_stop(ci);
 	ret = ci_role_start(ci, role);
-	if (!ret && ci->role == CI_ROLE_GADGET)
+	if (!ret && ci->role == USB_ROLE_DEVICE)
 		ci_handle_vbus_change(ci);
 	enable_irq(ci->irq);
 	pm_runtime_put_sync(dev);
@@ -1037,13 +1037,13 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
+	if (!ci->roles[USB_ROLE_HOST] && !ci->roles[USB_ROLE_DEVICE]) {
 		dev_err(dev, "no supported roles\n");
 		ret = -ENODEV;
 		goto deinit_gadget;
 	}
 
-	if (ci->is_otg && ci->roles[CI_ROLE_GADGET]) {
+	if (ci->is_otg && ci->roles[USB_ROLE_DEVICE]) {
 		ret = ci_hdrc_otg_init(ci);
 		if (ret) {
 			dev_err(dev, "init otg fails, ret = %d\n", ret);
@@ -1051,7 +1051,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
+	if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
 		if (ci->is_otg) {
 			ci->role = ci_otg_role(ci);
 			/* Enable ID change irq */
@@ -1062,17 +1062,17 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 			 * role switch, the defalt role is gadget, and the
 			 * user can switch it through debugfs.
 			 */
-			ci->role = CI_ROLE_GADGET;
+			ci->role = USB_ROLE_DEVICE;
 		}
 	} else {
-		ci->role = ci->roles[CI_ROLE_HOST]
-			? CI_ROLE_HOST
-			: CI_ROLE_GADGET;
+		ci->role = ci->roles[USB_ROLE_HOST]
+			? USB_ROLE_HOST
+			: USB_ROLE_DEVICE;
 	}
 
 	if (!ci_otg_is_fsm_mode(ci)) {
 		/* only update vbus status for peripheral */
-		if (ci->role == CI_ROLE_GADGET)
+		if (ci->role == USB_ROLE_DEVICE)
 			ci_handle_vbus_change(ci);
 
 		ret = ci_role_start(ci, ci->role);
@@ -1115,7 +1115,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 remove_debug:
 	dbg_remove_files(ci);
 stop:
-	if (ci->is_otg && ci->roles[CI_ROLE_GADGET])
+	if (ci->is_otg && ci->roles[USB_ROLE_DEVICE])
 		ci_hdrc_otg_destroy(ci);
 deinit_gadget:
 	ci_hdrc_gadget_destroy(ci);
diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
index fcc91a3..a3c022e 100644
--- a/drivers/usb/chipidea/debug.c
+++ b/drivers/usb/chipidea/debug.c
@@ -124,7 +124,7 @@ static int ci_qheads_show(struct seq_file *s, void *data)
 	unsigned long flags;
 	unsigned i, j;
 
-	if (ci->role != CI_ROLE_GADGET) {
+	if (ci->role != USB_ROLE_DEVICE) {
 		seq_printf(s, "not in gadget mode\n");
 		return 0;
 	}
@@ -158,7 +158,7 @@ static int ci_requests_show(struct seq_file *s, void *data)
 	struct td_node *node, *tmpnode;
 	unsigned i, j, qsize = sizeof(struct ci_hw_td)/sizeof(u32);
 
-	if (ci->role != CI_ROLE_GADGET) {
+	if (ci->role != USB_ROLE_DEVICE) {
 		seq_printf(s, "not in gadget mode\n");
 		return 0;
 	}
@@ -251,7 +251,7 @@ static int ci_role_show(struct seq_file *s, void *data)
 {
 	struct ci_hdrc *ci = s->private;
 
-	if (ci->role != CI_ROLE_END)
+	if (ci->role != USB_ROLE_NONE)
 		seq_printf(s, "%s\n", ci_role(ci)->name);
 
 	return 0;
@@ -262,20 +262,20 @@ static ssize_t ci_role_write(struct file *file, const char __user *ubuf,
 {
 	struct seq_file *s = file->private_data;
 	struct ci_hdrc *ci = s->private;
-	enum ci_role role;
+	enum usb_role role;
 	char buf[8];
 	int ret;
 
 	if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
 		return -EFAULT;
 
-	for (role = CI_ROLE_HOST; role < CI_ROLE_END; role++)
+	for (role = USB_ROLE_DEVICE; role > USB_ROLE_NONE; role--)
 		if (ci->roles[role] &&
 		    !strncmp(buf, ci->roles[role]->name,
 			     strlen(ci->roles[role]->name)))
 			break;
 
-	if (role == CI_ROLE_END || role == ci->role)
+	if (role == USB_ROLE_NONE || role == ci->role)
 		return -EINVAL;
 
 	pm_runtime_get_sync(ci->dev);
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index b45ceb9..dcce43d 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -198,7 +198,7 @@ static void host_stop(struct ci_hdrc *ci)
 			ci->platdata->notify_event(ci,
 				CI_HDRC_CONTROLLER_STOPPED_EVENT);
 		usb_remove_hcd(hcd);
-		ci->role = CI_ROLE_END;
+		ci->role = USB_ROLE_NONE;
 		synchronize_irq(ci->irq);
 		usb_put_hcd(hcd);
 		if (ci->platdata->reg_vbus && !ci_otg_is_fsm_mode(ci) &&
@@ -216,7 +216,7 @@ static void host_stop(struct ci_hdrc *ci)
 
 void ci_hdrc_host_destroy(struct ci_hdrc *ci)
 {
-	if (ci->role == CI_ROLE_HOST && ci->hcd)
+	if (ci->role == USB_ROLE_HOST && ci->hcd)
 		host_stop(ci);
 }
 
@@ -362,7 +362,7 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
 	rdrv->stop	= host_stop;
 	rdrv->irq	= host_irq;
 	rdrv->name	= "host";
-	ci->roles[CI_ROLE_HOST] = rdrv;
+	ci->roles[USB_ROLE_HOST] = rdrv;
 
 	return 0;
 }
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index f25d482..5bde0b5 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -117,11 +117,11 @@ void hw_write_otgsc(struct ci_hdrc *ci, u32 mask, u32 data)
  * ci_otg_role - pick role based on ID pin state
  * @ci: the controller
  */
-enum ci_role ci_otg_role(struct ci_hdrc *ci)
+enum usb_role ci_otg_role(struct ci_hdrc *ci)
 {
-	enum ci_role role = hw_read_otgsc(ci, OTGSC_ID)
-		? CI_ROLE_GADGET
-		: CI_ROLE_HOST;
+	enum usb_role role = hw_read_otgsc(ci, OTGSC_ID)
+		? USB_ROLE_DEVICE
+		: USB_ROLE_HOST;
 
 	return role;
 }
@@ -164,7 +164,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
 
 static void ci_handle_id_switch(struct ci_hdrc *ci)
 {
-	enum ci_role role = ci_otg_role(ci);
+	enum usb_role role = ci_otg_role(ci);
 
 	if (role != ci->role) {
 		dev_dbg(ci->dev, "switching from %s to %s\n",
@@ -172,7 +172,7 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
 
 		ci_role_stop(ci);
 
-		if (role == CI_ROLE_GADGET &&
+		if (role == USB_ROLE_DEVICE &&
 				IS_ERR(ci->platdata->vbus_extcon.edev))
 			/*
 			 * Wait vbus lower than OTGSC_BSV before connecting
@@ -185,7 +185,7 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
 
 		ci_role_start(ci, role);
 		/* vbus change may have already occurred */
-		if (role == CI_ROLE_GADGET)
+		if (role == USB_ROLE_DEVICE)
 			ci_handle_vbus_change(ci);
 	}
 }
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 4f8b817..f64bfc6 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -12,7 +12,7 @@ u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask);
 void hw_write_otgsc(struct ci_hdrc *ci, u32 mask, u32 data);
 int ci_hdrc_otg_init(struct ci_hdrc *ci);
 void ci_hdrc_otg_destroy(struct ci_hdrc *ci);
-enum ci_role ci_otg_role(struct ci_hdrc *ci);
+enum usb_role ci_otg_role(struct ci_hdrc *ci);
 void ci_handle_vbus_change(struct ci_hdrc *ci);
 static inline void ci_otg_queue_work(struct ci_hdrc *ci)
 {
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 6ed4b00..78c96a1 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -547,10 +547,10 @@ static int ci_otg_start_host(struct otg_fsm *fsm, int on)
 
 	if (on) {
 		ci_role_stop(ci);
-		ci_role_start(ci, CI_ROLE_HOST);
+		ci_role_start(ci, USB_ROLE_HOST);
 	} else {
 		ci_role_stop(ci);
-		ci_role_start(ci, CI_ROLE_GADGET);
+		ci_role_start(ci, USB_ROLE_DEVICE);
 	}
 	return 0;
 }
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 6a5ee8e..e1d745f7 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1606,7 +1606,7 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
 	 * Data+ pullup controlled by OTG state machine in OTG fsm mode;
 	 * and don't touch Data+ in host mode for dual role config.
 	 */
-	if (ci_otg_is_fsm_mode(ci) || ci->role == CI_ROLE_HOST)
+	if (ci_otg_is_fsm_mode(ci) || ci->role == USB_ROLE_DEVICE)
 		return 0;
 
 	pm_runtime_get_sync(&ci->gadget.dev);
@@ -1973,7 +1973,7 @@ static int udc_start(struct ci_hdrc *ci)
  */
 void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
 {
-	if (!ci->roles[CI_ROLE_GADGET])
+	if (!ci->roles[USB_ROLE_DEVICE])
 		return;
 
 	usb_del_gadget_udc(&ci->gadget);
@@ -2039,7 +2039,7 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
 
 	ret = udc_start(ci);
 	if (!ret)
-		ci->roles[CI_ROLE_GADGET] = rdrv;
+		ci->roles[USB_ROLE_DEVICE] = rdrv;
 
 	return ret;
 }
-- 
2.7.4

[PATCH 2/5] usb: chipidea: add role switch class support

[jun.li]

From: Li Jun <jun.li@nxp.com>

USB role is fully controlled by usb role switch consumer(e.g. typec),
usb port either at host mode, or at device connected mode, will not
stay at USB_ROLE_NONE mode.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/chipidea/ci.h   |   2 +
 drivers/usb/chipidea/core.c | 125 ++++++++++++++++++++++++++++++++++----------
 drivers/usb/chipidea/otg.c  |  13 +++++
 3 files changed, 111 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 82b86cd..5e2f0bc 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -212,6 +212,7 @@ struct ci_hdrc {
 	ktime_t				hr_timeouts[NUM_OTG_FSM_TIMERS];
 	unsigned			enabled_otg_timer_bits;
 	enum otg_fsm_timer		next_otg_timer;
+	struct usb_role_switch		*role_switch;
 	struct work_struct		work;
 	struct workqueue_struct		*wq;
 
@@ -244,6 +245,7 @@ struct ci_hdrc {
 	struct dentry			*debugfs;
 	bool				id_event;
 	bool				b_sess_valid_event;
+	bool				role_switch_event;
 	bool				imx28_write_fix;
 	bool				supports_runtime_pm;
 	bool				in_lpm;
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index bc24c5b..1bcf6f6 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
 	return ret;
 }
 
+static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
+{
+	struct ci_hdrc *ci = dev_get_drvdata(dev);
+	unsigned long flags;
+
+	if (ci->role == role || role == USB_ROLE_NONE)
+		return 0;
+
+	spin_lock_irqsave(&ci->lock, flags);
+	ci->role_switch_event = true;
+	if (ci->role == USB_ROLE_NONE) {
+		if (role == USB_ROLE_DEVICE)
+			ci->role = USB_ROLE_HOST;
+		else
+			ci->role = USB_ROLE_DEVICE;
+	}
+	spin_unlock_irqrestore(&ci->lock, flags);
+
+	ci_otg_queue_work(ci);
+
+	return 0;
+}
+
+static enum usb_role ci_usb_role_switch_get(struct device *dev)
+{
+	struct ci_hdrc *ci = dev_get_drvdata(dev);
+	unsigned long flags;
+	enum usb_role role;
+
+	spin_lock_irqsave(&ci->lock, flags);
+	role = ci->role;
+	spin_unlock_irqrestore(&ci->lock, flags);
+
+	return role;
+}
+
+static struct usb_role_switch_desc ci_role_switch = {
+	.set = ci_usb_role_switch_set,
+	.get = ci_usb_role_switch_get,
+};
+
 static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
 			     void *ptr)
 {
@@ -689,6 +730,9 @@ static int ci_get_platdata(struct device *dev,
 	if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
 		platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
 
+	if (device_property_read_bool(dev, "usb-role-switch"))
+		ci_role_switch.fwnode = dev->fwnode;
+
 	ext_id = ERR_PTR(-ENODEV);
 	ext_vbus = ERR_PTR(-ENODEV);
 	if (of_property_read_bool(dev->of_node, "extcon")) {
@@ -908,6 +952,43 @@ static const struct attribute_group ci_attr_group = {
 	.attrs = ci_attrs,
 };
 
+static int ci_start_initial_role(struct ci_hdrc *ci)
+{
+	int ret = 0;
+
+	if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
+		if (ci->is_otg) {
+			ci->role = ci_otg_role(ci);
+			/* Enable ID change irq */
+			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
+		} 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.
+			 */
+			ci->role = USB_ROLE_DEVICE;
+		}
+	} else {
+		ci->role = ci->roles[USB_ROLE_HOST]
+			? USB_ROLE_HOST
+			: USB_ROLE_DEVICE;
+	}
+
+	if (!ci_otg_is_fsm_mode(ci)) {
+		/* only update vbus status for peripheral */
+		if (ci->role == USB_ROLE_DEVICE)
+			ci_handle_vbus_change(ci);
+
+		ret = ci_role_start(ci, ci->role);
+		if (ret)
+			dev_err(ci->dev, "can't start %s role\n",
+						ci_role(ci)->name);
+	}
+
+	return ret;
+}
+
 static int ci_hdrc_probe(struct platform_device *pdev)
 {
 	struct device	*dev = &pdev->dev;
@@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
-		if (ci->is_otg) {
-			ci->role = ci_otg_role(ci);
-			/* Enable ID change irq */
-			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
-		} 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.
-			 */
-			ci->role = USB_ROLE_DEVICE;
-		}
-	} else {
-		ci->role = ci->roles[USB_ROLE_HOST]
-			? USB_ROLE_HOST
-			: USB_ROLE_DEVICE;
-	}
-
-	if (!ci_otg_is_fsm_mode(ci)) {
-		/* only update vbus status for peripheral */
-		if (ci->role == USB_ROLE_DEVICE)
-			ci_handle_vbus_change(ci);
-
-		ret = ci_role_start(ci, ci->role);
-		if (ret) {
-			dev_err(dev, "can't start %s role\n",
-						ci_role(ci)->name);
+	if (!ci_role_switch.fwnode) {
+		ret = ci_start_initial_role(ci);
+		if (ret)
 			goto stop;
-		}
 	}
 
 	ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
@@ -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret)
 		goto stop;
 
+	if (ci_role_switch.fwnode) {
+		ci->role_switch = usb_role_switch_register(dev,
+					&ci_role_switch);
+		if (IS_ERR(ci->role_switch)) {
+			ret = PTR_ERR(ci->role_switch);
+			goto stop;
+		}
+	}
+
 	if (ci->supports_runtime_pm) {
 		pm_runtime_set_active(&pdev->dev);
 		pm_runtime_enable(&pdev->dev);
@@ -1133,6 +1197,9 @@ static int ci_hdrc_remove(struct platform_device *pdev)
 {
 	struct ci_hdrc *ci = platform_get_drvdata(pdev);
 
+	if (ci->role_switch)
+		usb_role_switch_unregister(ci->role_switch);
+
 	if (ci->supports_runtime_pm) {
 		pm_runtime_get_sync(&pdev->dev);
 		pm_runtime_disable(&pdev->dev);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 5bde0b5..0a22855 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work)
 		ci_handle_vbus_change(ci);
 	}
 
+	if (ci->role_switch_event) {
+		ci->role_switch_event = false;
+
+		if (ci->role == USB_ROLE_DEVICE) {
+			usb_gadget_vbus_disconnect(&ci->gadget);
+			ci_role_start(ci, USB_ROLE_HOST);
+		} else if (ci->role == USB_ROLE_HOST) {
+			ci_role_stop(ci);
+			usb_gadget_vbus_connect(&ci->gadget);
+			ci->role = USB_ROLE_DEVICE;
+		}
+	}
+
 	pm_runtime_put_sync(ci->dev);
 
 	enable_irq(ci->irq);
-- 
2.7.4

Re: [PATCH 2/5] usb: chipidea: add role switch class support

[Peter Chen]

> USB role is fully controlled by usb role switch consumer(e.g. typec),
> usb port either at host mode, or at device connected mode, will not
> stay at USB_ROLE_NONE mode.
>

Then, if the Type-C cable is disconnected from PC host, the controller
driver can't be notified?If that, how controller enters low power mode at this
situation?

Peter

> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/usb/chipidea/ci.h   |   2 +
>  drivers/usb/chipidea/core.c | 125 ++++++++++++++++++++++++++++++++++----------
>  drivers/usb/chipidea/otg.c  |  13 +++++
>  3 files changed, 111 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 82b86cd..5e2f0bc 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -212,6 +212,7 @@ struct ci_hdrc {
>         ktime_t                         hr_timeouts[NUM_OTG_FSM_TIMERS];
>         unsigned                        enabled_otg_timer_bits;
>         enum otg_fsm_timer              next_otg_timer;
> +       struct usb_role_switch          *role_switch;
>         struct work_struct              work;
>         struct workqueue_struct         *wq;
>
> @@ -244,6 +245,7 @@ struct ci_hdrc {
>         struct dentry                   *debugfs;
>         bool                            id_event;
>         bool                            b_sess_valid_event;
> +       bool                            role_switch_event;
>         bool                            imx28_write_fix;
>         bool                            supports_runtime_pm;
>         bool                            in_lpm;
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index bc24c5b..1bcf6f6 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
>         return ret;
>  }
>
> +static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
> +{
> +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> +       unsigned long flags;
> +
> +       if (ci->role == role || role == USB_ROLE_NONE)
> +               return 0;
> +
> +       spin_lock_irqsave(&ci->lock, flags);
> +       ci->role_switch_event = true;
> +       if (ci->role == USB_ROLE_NONE) {
> +               if (role == USB_ROLE_DEVICE)
> +                       ci->role = USB_ROLE_HOST;
> +               else
> +                       ci->role = USB_ROLE_DEVICE;
> +       }
> +       spin_unlock_irqrestore(&ci->lock, flags);
> +
> +       ci_otg_queue_work(ci);
> +
> +       return 0;
> +}
> +
> +static enum usb_role ci_usb_role_switch_get(struct device *dev)
> +{
> +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> +       unsigned long flags;
> +       enum usb_role role;
> +
> +       spin_lock_irqsave(&ci->lock, flags);
> +       role = ci->role;
> +       spin_unlock_irqrestore(&ci->lock, flags);
> +
> +       return role;
> +}
> +
> +static struct usb_role_switch_desc ci_role_switch = {
> +       .set = ci_usb_role_switch_set,
> +       .get = ci_usb_role_switch_get,
> +};
> +
>  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
>                              void *ptr)
>  {
> @@ -689,6 +730,9 @@ static int ci_get_platdata(struct device *dev,
>         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
>                 platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
>
> +       if (device_property_read_bool(dev, "usb-role-switch"))
> +               ci_role_switch.fwnode = dev->fwnode;
> +
>         ext_id = ERR_PTR(-ENODEV);
>         ext_vbus = ERR_PTR(-ENODEV);
>         if (of_property_read_bool(dev->of_node, "extcon")) {
> @@ -908,6 +952,43 @@ static const struct attribute_group ci_attr_group = {
>         .attrs = ci_attrs,
>  };
>
> +static int ci_start_initial_role(struct ci_hdrc *ci)
> +{
> +       int ret = 0;
> +
> +       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> +               if (ci->is_otg) {
> +                       ci->role = ci_otg_role(ci);
> +                       /* Enable ID change irq */
> +                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> +               } 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.
> +                        */
> +                       ci->role = USB_ROLE_DEVICE;
> +               }
> +       } else {
> +               ci->role = ci->roles[USB_ROLE_HOST]
> +                       ? USB_ROLE_HOST
> +                       : USB_ROLE_DEVICE;
> +       }
> +
> +       if (!ci_otg_is_fsm_mode(ci)) {
> +               /* only update vbus status for peripheral */
> +               if (ci->role == USB_ROLE_DEVICE)
> +                       ci_handle_vbus_change(ci);
> +
> +               ret = ci_role_start(ci, ci->role);
> +               if (ret)
> +                       dev_err(ci->dev, "can't start %s role\n",
> +                                               ci_role(ci)->name);
> +       }
> +
> +       return ret;
> +}
> +
>  static int ci_hdrc_probe(struct platform_device *pdev)
>  {
>         struct device   *dev = &pdev->dev;
> @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>                 }
>         }
>
> -       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> -               if (ci->is_otg) {
> -                       ci->role = ci_otg_role(ci);
> -                       /* Enable ID change irq */
> -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> -               } 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.
> -                        */
> -                       ci->role = USB_ROLE_DEVICE;
> -               }
> -       } else {
> -               ci->role = ci->roles[USB_ROLE_HOST]
> -                       ? USB_ROLE_HOST
> -                       : USB_ROLE_DEVICE;
> -       }
> -
> -       if (!ci_otg_is_fsm_mode(ci)) {
> -               /* only update vbus status for peripheral */
> -               if (ci->role == USB_ROLE_DEVICE)
> -                       ci_handle_vbus_change(ci);
> -
> -               ret = ci_role_start(ci, ci->role);
> -               if (ret) {
> -                       dev_err(dev, "can't start %s role\n",
> -                                               ci_role(ci)->name);
> +       if (!ci_role_switch.fwnode) {
> +               ret = ci_start_initial_role(ci);
> +               if (ret)
>                         goto stop;
> -               }
>         }
>
>         ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
> @@ -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>         if (ret)
>                 goto stop;
>
> +       if (ci_role_switch.fwnode) {
> +               ci->role_switch = usb_role_switch_register(dev,
> +                                       &ci_role_switch);
> +               if (IS_ERR(ci->role_switch)) {
> +                       ret = PTR_ERR(ci->role_switch);
> +                       goto stop;
> +               }
> +       }
> +
>         if (ci->supports_runtime_pm) {
>                 pm_runtime_set_active(&pdev->dev);
>                 pm_runtime_enable(&pdev->dev);
> @@ -1133,6 +1197,9 @@ static int ci_hdrc_remove(struct platform_device *pdev)
>  {
>         struct ci_hdrc *ci = platform_get_drvdata(pdev);
>
> +       if (ci->role_switch)
> +               usb_role_switch_unregister(ci->role_switch);
> +
>         if (ci->supports_runtime_pm) {
>                 pm_runtime_get_sync(&pdev->dev);
>                 pm_runtime_disable(&pdev->dev);
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 5bde0b5..0a22855 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work)
>                 ci_handle_vbus_change(ci);
>         }
>
> +       if (ci->role_switch_event) {
> +               ci->role_switch_event = false;
> +
> +               if (ci->role == USB_ROLE_DEVICE) {
> +                       usb_gadget_vbus_disconnect(&ci->gadget);
> +                       ci_role_start(ci, USB_ROLE_HOST);
> +               } else if (ci->role == USB_ROLE_HOST) {
> +                       ci_role_stop(ci);
> +                       usb_gadget_vbus_connect(&ci->gadget);
> +                       ci->role = USB_ROLE_DEVICE;
> +               }
> +       }
> +
>         pm_runtime_put_sync(ci->dev);
>
>         enable_irq(ci->irq);
> --
> 2.7.4
>

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Jun Li]

> -----Original Message-----
> From: Peter Chen <hzpeterchen@gmail.com>
> Sent: 2019年8月2日 17:41
> To: Jun Li <jun.li@nxp.com>
> Cc: Peter Chen <peter.chen@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; dl-linux-imx <linux-imx@nxp.com>; USB list
> <linux-usb@vger.kernel.org>
> Subject: Re: [PATCH 2/5] usb: chipidea: add role switch class support
> 
> > USB role is fully controlled by usb role switch consumer(e.g. typec),
> > usb port either at host mode, or at device connected mode, will not
> > stay at USB_ROLE_NONE mode.
> >
> 
> Then, if the Type-C cable is disconnected from PC host, the controller driver can't be
> notified?If that, how controller enters low power mode at this situation?

The controller driver can be notified, but there are only role(host or device)
Information, so in you mentioned case, controller driver will get the same
input before and after disconnect from host, can't know detachment and
enter low power mode accordingly, this is more like something internal
handling under device role.

Li Jun

> 
> Peter
> 
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  drivers/usb/chipidea/ci.h   |   2 +
> >  drivers/usb/chipidea/core.c | 125
> > ++++++++++++++++++++++++++++++++++----------
> >  drivers/usb/chipidea/otg.c  |  13 +++++
> >  3 files changed, 111 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index 82b86cd..5e2f0bc 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -212,6 +212,7 @@ struct ci_hdrc {
> >         ktime_t                         hr_timeouts[NUM_OTG_FSM_TIMERS];
> >         unsigned                        enabled_otg_timer_bits;
> >         enum otg_fsm_timer              next_otg_timer;
> > +       struct usb_role_switch          *role_switch;
> >         struct work_struct              work;
> >         struct workqueue_struct         *wq;
> >
> > @@ -244,6 +245,7 @@ struct ci_hdrc {
> >         struct dentry                   *debugfs;
> >         bool                            id_event;
> >         bool                            b_sess_valid_event;
> > +       bool                            role_switch_event;
> >         bool                            imx28_write_fix;
> >         bool                            supports_runtime_pm;
> >         bool                            in_lpm;
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index bc24c5b..1bcf6f6 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
> >         return ret;
> >  }
> >
> > +static int ci_usb_role_switch_set(struct device *dev, enum usb_role
> > +role) {
> > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > +       unsigned long flags;
> > +
> > +       if (ci->role == role || role == USB_ROLE_NONE)
> > +               return 0;
> > +
> > +       spin_lock_irqsave(&ci->lock, flags);
> > +       ci->role_switch_event = true;
> > +       if (ci->role == USB_ROLE_NONE) {
> > +               if (role == USB_ROLE_DEVICE)
> > +                       ci->role = USB_ROLE_HOST;
> > +               else
> > +                       ci->role = USB_ROLE_DEVICE;
> > +       }
> > +       spin_unlock_irqrestore(&ci->lock, flags);
> > +
> > +       ci_otg_queue_work(ci);
> > +
> > +       return 0;
> > +}
> > +
> > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > +       unsigned long flags;
> > +       enum usb_role role;
> > +
> > +       spin_lock_irqsave(&ci->lock, flags);
> > +       role = ci->role;
> > +       spin_unlock_irqrestore(&ci->lock, flags);
> > +
> > +       return role;
> > +}
> > +
> > +static struct usb_role_switch_desc ci_role_switch = {
> > +       .set = ci_usb_role_switch_set,
> > +       .get = ci_usb_role_switch_get, };
> > +
> >  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> >                              void *ptr)  { @@ -689,6 +730,9 @@ static
> > int ci_get_platdata(struct device *dev,
> >         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> >                 platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
> >
> > +       if (device_property_read_bool(dev, "usb-role-switch"))
> > +               ci_role_switch.fwnode = dev->fwnode;
> > +
> >         ext_id = ERR_PTR(-ENODEV);
> >         ext_vbus = ERR_PTR(-ENODEV);
> >         if (of_property_read_bool(dev->of_node, "extcon")) { @@ -908,6
> > +952,43 @@ static const struct attribute_group ci_attr_group = {
> >         .attrs = ci_attrs,
> >  };
> >
> > +static int ci_start_initial_role(struct ci_hdrc *ci) {
> > +       int ret = 0;
> > +
> > +       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > +               if (ci->is_otg) {
> > +                       ci->role = ci_otg_role(ci);
> > +                       /* Enable ID change irq */
> > +                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > +               } 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.
> > +                        */
> > +                       ci->role = USB_ROLE_DEVICE;
> > +               }
> > +       } else {
> > +               ci->role = ci->roles[USB_ROLE_HOST]
> > +                       ? USB_ROLE_HOST
> > +                       : USB_ROLE_DEVICE;
> > +       }
> > +
> > +       if (!ci_otg_is_fsm_mode(ci)) {
> > +               /* only update vbus status for peripheral */
> > +               if (ci->role == USB_ROLE_DEVICE)
> > +                       ci_handle_vbus_change(ci);
> > +
> > +               ret = ci_role_start(ci, ci->role);
> > +               if (ret)
> > +                       dev_err(ci->dev, "can't start %s role\n",
> > +                                               ci_role(ci)->name);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> >  static int ci_hdrc_probe(struct platform_device *pdev)  {
> >         struct device   *dev = &pdev->dev;
> > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> >                 }
> >         }
> >
> > -       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > -               if (ci->is_otg) {
> > -                       ci->role = ci_otg_role(ci);
> > -                       /* Enable ID change irq */
> > -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > -               } 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.
> > -                        */
> > -                       ci->role = USB_ROLE_DEVICE;
> > -               }
> > -       } else {
> > -               ci->role = ci->roles[USB_ROLE_HOST]
> > -                       ? USB_ROLE_HOST
> > -                       : USB_ROLE_DEVICE;
> > -       }
> > -
> > -       if (!ci_otg_is_fsm_mode(ci)) {
> > -               /* only update vbus status for peripheral */
> > -               if (ci->role == USB_ROLE_DEVICE)
> > -                       ci_handle_vbus_change(ci);
> > -
> > -               ret = ci_role_start(ci, ci->role);
> > -               if (ret) {
> > -                       dev_err(dev, "can't start %s role\n",
> > -                                               ci_role(ci)->name);
> > +       if (!ci_role_switch.fwnode) {
> > +               ret = ci_start_initial_role(ci);
> > +               if (ret)
> >                         goto stop;
> > -               }
> >         }
> >
> >         ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED, @@
> > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto stop;
> >
> > +       if (ci_role_switch.fwnode) {
> > +               ci->role_switch = usb_role_switch_register(dev,
> > +                                       &ci_role_switch);
> > +               if (IS_ERR(ci->role_switch)) {
> > +                       ret = PTR_ERR(ci->role_switch);
> > +                       goto stop;
> > +               }
> > +       }
> > +
> >         if (ci->supports_runtime_pm) {
> >                 pm_runtime_set_active(&pdev->dev);
> >                 pm_runtime_enable(&pdev->dev); @@ -1133,6 +1197,9 @@
> > static int ci_hdrc_remove(struct platform_device *pdev)  {
> >         struct ci_hdrc *ci = platform_get_drvdata(pdev);
> >
> > +       if (ci->role_switch)
> > +               usb_role_switch_unregister(ci->role_switch);
> > +
> >         if (ci->supports_runtime_pm) {
> >                 pm_runtime_get_sync(&pdev->dev);
> >                 pm_runtime_disable(&pdev->dev); diff --git
> > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> > 5bde0b5..0a22855 100644
> > --- a/drivers/usb/chipidea/otg.c
> > +++ b/drivers/usb/chipidea/otg.c
> > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work)
> >                 ci_handle_vbus_change(ci);
> >         }
> >
> > +       if (ci->role_switch_event) {
> > +               ci->role_switch_event = false;
> > +
> > +               if (ci->role == USB_ROLE_DEVICE) {
> > +                       usb_gadget_vbus_disconnect(&ci->gadget);
> > +                       ci_role_start(ci, USB_ROLE_HOST);
> > +               } else if (ci->role == USB_ROLE_HOST) {
> > +                       ci_role_stop(ci);
> > +                       usb_gadget_vbus_connect(&ci->gadget);
> > +                       ci->role = USB_ROLE_DEVICE;
> > +               }
> > +       }
> > +
> >         pm_runtime_put_sync(ci->dev);
> >
> >         enable_irq(ci->irq);
> > --
> > 2.7.4
> >

Re: [PATCH 2/5] usb: chipidea: add role switch class support

[Peter Chen]

> >
> > > USB role is fully controlled by usb role switch consumer(e.g. typec),
> > > usb port either at host mode, or at device connected mode, will not
> > > stay at USB_ROLE_NONE mode.
> > >
> >
> > Then, if the Type-C cable is disconnected from PC host, the controller driver can't be
> > notified?If that, how controller enters low power mode at this situation?
>
> The controller driver can be notified, but there are only role(host or device)
> Information, so in you mentioned case, controller driver will get the same
> input before and after disconnect from host, can't know detachment and
> enter low power mode accordingly, this is more like something internal
> handling under device role.
>

If the Type-C can't pass "gadget disconnect" event, you may handle it
at UDC driver itself, eg at ci_usb_role_switch_set. Otherwise, the gadget
class driver can't be notified disconnection, and the controller itself can't
enter LPM as well.

Peter

> Li Jun
>
> >
> > Peter
> >
> > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > ---
> > >  drivers/usb/chipidea/ci.h   |   2 +
> > >  drivers/usb/chipidea/core.c | 125
> > > ++++++++++++++++++++++++++++++++++----------
> > >  drivers/usb/chipidea/otg.c  |  13 +++++
> > >  3 files changed, 111 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > index 82b86cd..5e2f0bc 100644
> > > --- a/drivers/usb/chipidea/ci.h
> > > +++ b/drivers/usb/chipidea/ci.h
> > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > >         ktime_t                         hr_timeouts[NUM_OTG_FSM_TIMERS];
> > >         unsigned                        enabled_otg_timer_bits;
> > >         enum otg_fsm_timer              next_otg_timer;
> > > +       struct usb_role_switch          *role_switch;
> > >         struct work_struct              work;
> > >         struct workqueue_struct         *wq;
> > >
> > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > >         struct dentry                   *debugfs;
> > >         bool                            id_event;
> > >         bool                            b_sess_valid_event;
> > > +       bool                            role_switch_event;
> > >         bool                            imx28_write_fix;
> > >         bool                            supports_runtime_pm;
> > >         bool                            in_lpm;
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index bc24c5b..1bcf6f6 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
> > >         return ret;
> > >  }
> > >
> > > +static int ci_usb_role_switch_set(struct device *dev, enum usb_role
> > > +role) {
> > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > +       unsigned long flags;
> > > +
> > > +       if (ci->role == role || role == USB_ROLE_NONE)
> > > +               return 0;
> > > +
> > > +       spin_lock_irqsave(&ci->lock, flags);
> > > +       ci->role_switch_event = true;
> > > +       if (ci->role == USB_ROLE_NONE) {
> > > +               if (role == USB_ROLE_DEVICE)
> > > +                       ci->role = USB_ROLE_HOST;
> > > +               else
> > > +                       ci->role = USB_ROLE_DEVICE;
> > > +       }
> > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > +
> > > +       ci_otg_queue_work(ci);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > +       unsigned long flags;
> > > +       enum usb_role role;
> > > +
> > > +       spin_lock_irqsave(&ci->lock, flags);
> > > +       role = ci->role;
> > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > +
> > > +       return role;
> > > +}
> > > +
> > > +static struct usb_role_switch_desc ci_role_switch = {
> > > +       .set = ci_usb_role_switch_set,
> > > +       .get = ci_usb_role_switch_get, };
> > > +
> > >  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> > >                              void *ptr)  { @@ -689,6 +730,9 @@ static
> > > int ci_get_platdata(struct device *dev,
> > >         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> > >                 platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
> > >
> > > +       if (device_property_read_bool(dev, "usb-role-switch"))
> > > +               ci_role_switch.fwnode = dev->fwnode;
> > > +
> > >         ext_id = ERR_PTR(-ENODEV);
> > >         ext_vbus = ERR_PTR(-ENODEV);
> > >         if (of_property_read_bool(dev->of_node, "extcon")) { @@ -908,6
> > > +952,43 @@ static const struct attribute_group ci_attr_group = {
> > >         .attrs = ci_attrs,
> > >  };
> > >
> > > +static int ci_start_initial_role(struct ci_hdrc *ci) {
> > > +       int ret = 0;
> > > +
> > > +       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > > +               if (ci->is_otg) {
> > > +                       ci->role = ci_otg_role(ci);
> > > +                       /* Enable ID change irq */
> > > +                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > +               } 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.
> > > +                        */
> > > +                       ci->role = USB_ROLE_DEVICE;
> > > +               }
> > > +       } else {
> > > +               ci->role = ci->roles[USB_ROLE_HOST]
> > > +                       ? USB_ROLE_HOST
> > > +                       : USB_ROLE_DEVICE;
> > > +       }
> > > +
> > > +       if (!ci_otg_is_fsm_mode(ci)) {
> > > +               /* only update vbus status for peripheral */
> > > +               if (ci->role == USB_ROLE_DEVICE)
> > > +                       ci_handle_vbus_change(ci);
> > > +
> > > +               ret = ci_role_start(ci, ci->role);
> > > +               if (ret)
> > > +                       dev_err(ci->dev, "can't start %s role\n",
> > > +                                               ci_role(ci)->name);
> > > +       }
> > > +
> > > +       return ret;
> > > +}
> > > +
> > >  static int ci_hdrc_probe(struct platform_device *pdev)  {
> > >         struct device   *dev = &pdev->dev;
> > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > >                 }
> > >         }
> > >
> > > -       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > > -               if (ci->is_otg) {
> > > -                       ci->role = ci_otg_role(ci);
> > > -                       /* Enable ID change irq */
> > > -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > -               } 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.
> > > -                        */
> > > -                       ci->role = USB_ROLE_DEVICE;
> > > -               }
> > > -       } else {
> > > -               ci->role = ci->roles[USB_ROLE_HOST]
> > > -                       ? USB_ROLE_HOST
> > > -                       : USB_ROLE_DEVICE;
> > > -       }
> > > -
> > > -       if (!ci_otg_is_fsm_mode(ci)) {
> > > -               /* only update vbus status for peripheral */
> > > -               if (ci->role == USB_ROLE_DEVICE)
> > > -                       ci_handle_vbus_change(ci);
> > > -
> > > -               ret = ci_role_start(ci, ci->role);
> > > -               if (ret) {
> > > -                       dev_err(dev, "can't start %s role\n",
> > > -                                               ci_role(ci)->name);
> > > +       if (!ci_role_switch.fwnode) {
> > > +               ret = ci_start_initial_role(ci);
> > > +               if (ret)
> > >                         goto stop;
> > > -               }
> > >         }
> > >
> > >         ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED, @@
> > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto stop;
> > >
> > > +       if (ci_role_switch.fwnode) {
> > > +               ci->role_switch = usb_role_switch_register(dev,
> > > +                                       &ci_role_switch);
> > > +               if (IS_ERR(ci->role_switch)) {
> > > +                       ret = PTR_ERR(ci->role_switch);
> > > +                       goto stop;
> > > +               }
> > > +       }
> > > +
> > >         if (ci->supports_runtime_pm) {
> > >                 pm_runtime_set_active(&pdev->dev);
> > >                 pm_runtime_enable(&pdev->dev); @@ -1133,6 +1197,9 @@
> > > static int ci_hdrc_remove(struct platform_device *pdev)  {
> > >         struct ci_hdrc *ci = platform_get_drvdata(pdev);
> > >
> > > +       if (ci->role_switch)
> > > +               usb_role_switch_unregister(ci->role_switch);
> > > +
> > >         if (ci->supports_runtime_pm) {
> > >                 pm_runtime_get_sync(&pdev->dev);
> > >                 pm_runtime_disable(&pdev->dev); diff --git
> > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> > > 5bde0b5..0a22855 100644
> > > --- a/drivers/usb/chipidea/otg.c
> > > +++ b/drivers/usb/chipidea/otg.c
> > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work)
> > >                 ci_handle_vbus_change(ci);
> > >         }
> > >
> > > +       if (ci->role_switch_event) {
> > > +               ci->role_switch_event = false;
> > > +
> > > +               if (ci->role == USB_ROLE_DEVICE) {
> > > +                       usb_gadget_vbus_disconnect(&ci->gadget);
> > > +                       ci_role_start(ci, USB_ROLE_HOST);
> > > +               } else if (ci->role == USB_ROLE_HOST) {
> > > +                       ci_role_stop(ci);
> > > +                       usb_gadget_vbus_connect(&ci->gadget);
> > > +                       ci->role = USB_ROLE_DEVICE;
> > > +               }
> > > +       }
> > > +
> > >         pm_runtime_put_sync(ci->dev);
> > >
> > >         enable_irq(ci->irq);
> > > --
> > > 2.7.4
> > >

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Jun Li]

> -----Original Message-----
> From: Peter Chen <hzpeterchen@gmail.com>
> Sent: 2019年8月5日 11:15
> To: Jun Li <jun.li@nxp.com>
> Cc: Peter Chen <peter.chen@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; dl-linux-imx <linux-imx@nxp.com>; USB list
> <linux-usb@vger.kernel.org>
> Subject: Re: [PATCH 2/5] usb: chipidea: add role switch class support
> 
> > >
> > > > USB role is fully controlled by usb role switch consumer(e.g.
> > > > typec), usb port either at host mode, or at device connected mode,
> > > > will not stay at USB_ROLE_NONE mode.
> > > >
> > >
> > > Then, if the Type-C cable is disconnected from PC host, the
> > > controller driver can't be notified?If that, how controller enters low power mode at this
> situation?
> >
> > The controller driver can be notified, but there are only role(host or
> > device) Information, so in you mentioned case, controller driver will
> > get the same input before and after disconnect from host, can't know
> > detachment and enter low power mode accordingly, this is more like
> > something internal handling under device role.
> >
> 
> If the Type-C can't pass "gadget disconnect" event, you may handle it at UDC driver itself,
> eg at ci_usb_role_switch_set. Otherwise, the gadget class driver can't be notified
> disconnection, and the controller itself can't enter LPM as well.

OK, I will enable VBUS irq to handle this in v2.

Li Jun
> 
> Peter
> 
> > Li Jun
> >
> > >
> > > Peter
> > >
> > > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > > ---
> > > >  drivers/usb/chipidea/ci.h   |   2 +
> > > >  drivers/usb/chipidea/core.c | 125
> > > > ++++++++++++++++++++++++++++++++++----------
> > > >  drivers/usb/chipidea/otg.c  |  13 +++++
> > > >  3 files changed, 111 insertions(+), 29 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > > index 82b86cd..5e2f0bc 100644
> > > > --- a/drivers/usb/chipidea/ci.h
> > > > +++ b/drivers/usb/chipidea/ci.h
> > > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > > >         ktime_t
> hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > >         unsigned                        enabled_otg_timer_bits;
> > > >         enum otg_fsm_timer              next_otg_timer;
> > > > +       struct usb_role_switch          *role_switch;
> > > >         struct work_struct              work;
> > > >         struct workqueue_struct         *wq;
> > > >
> > > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > > >         struct dentry                   *debugfs;
> > > >         bool                            id_event;
> > > >         bool                            b_sess_valid_event;
> > > > +       bool                            role_switch_event;
> > > >         bool                            imx28_write_fix;
> > > >         bool                            supports_runtime_pm;
> > > >         bool                            in_lpm;
> > > > diff --git a/drivers/usb/chipidea/core.c
> > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644
> > > > --- a/drivers/usb/chipidea/core.c
> > > > +++ b/drivers/usb/chipidea/core.c
> > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
> > > >         return ret;
> > > >  }
> > > >
> > > > +static int ci_usb_role_switch_set(struct device *dev, enum
> > > > +usb_role
> > > > +role) {
> > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > +       unsigned long flags;
> > > > +
> > > > +       if (ci->role == role || role == USB_ROLE_NONE)
> > > > +               return 0;
> > > > +
> > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > +       ci->role_switch_event = true;
> > > > +       if (ci->role == USB_ROLE_NONE) {
> > > > +               if (role == USB_ROLE_DEVICE)
> > > > +                       ci->role = USB_ROLE_HOST;
> > > > +               else
> > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > +       }
> > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > +
> > > > +       ci_otg_queue_work(ci);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > +       unsigned long flags;
> > > > +       enum usb_role role;
> > > > +
> > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > +       role = ci->role;
> > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > +
> > > > +       return role;
> > > > +}
> > > > +
> > > > +static struct usb_role_switch_desc ci_role_switch = {
> > > > +       .set = ci_usb_role_switch_set,
> > > > +       .get = ci_usb_role_switch_get, };
> > > > +
> > > >  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> > > >                              void *ptr)  { @@ -689,6 +730,9 @@
> > > > static int ci_get_platdata(struct device *dev,
> > > >         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> > > >                 platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
> > > >
> > > > +       if (device_property_read_bool(dev, "usb-role-switch"))
> > > > +               ci_role_switch.fwnode = dev->fwnode;
> > > > +
> > > >         ext_id = ERR_PTR(-ENODEV);
> > > >         ext_vbus = ERR_PTR(-ENODEV);
> > > >         if (of_property_read_bool(dev->of_node, "extcon")) { @@
> > > > -908,6
> > > > +952,43 @@ static const struct attribute_group ci_attr_group = {
> > > >         .attrs = ci_attrs,
> > > >  };
> > > >
> > > > +static int ci_start_initial_role(struct ci_hdrc *ci) {
> > > > +       int ret = 0;
> > > > +
> > > > +       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > > > +               if (ci->is_otg) {
> > > > +                       ci->role = ci_otg_role(ci);
> > > > +                       /* Enable ID change irq */
> > > > +                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > +               } 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.
> > > > +                        */
> > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > +               }
> > > > +       } else {
> > > > +               ci->role = ci->roles[USB_ROLE_HOST]
> > > > +                       ? USB_ROLE_HOST
> > > > +                       : USB_ROLE_DEVICE;
> > > > +       }
> > > > +
> > > > +       if (!ci_otg_is_fsm_mode(ci)) {
> > > > +               /* only update vbus status for peripheral */
> > > > +               if (ci->role == USB_ROLE_DEVICE)
> > > > +                       ci_handle_vbus_change(ci);
> > > > +
> > > > +               ret = ci_role_start(ci, ci->role);
> > > > +               if (ret)
> > > > +                       dev_err(ci->dev, "can't start %s role\n",
> > > > +                                               ci_role(ci)->name);
> > > > +       }
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > >  static int ci_hdrc_probe(struct platform_device *pdev)  {
> > > >         struct device   *dev = &pdev->dev;
> > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > >                 }
> > > >         }
> > > >
> > > > -       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > > > -               if (ci->is_otg) {
> > > > -                       ci->role = ci_otg_role(ci);
> > > > -                       /* Enable ID change irq */
> > > > -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > -               } 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.
> > > > -                        */
> > > > -                       ci->role = USB_ROLE_DEVICE;
> > > > -               }
> > > > -       } else {
> > > > -               ci->role = ci->roles[USB_ROLE_HOST]
> > > > -                       ? USB_ROLE_HOST
> > > > -                       : USB_ROLE_DEVICE;
> > > > -       }
> > > > -
> > > > -       if (!ci_otg_is_fsm_mode(ci)) {
> > > > -               /* only update vbus status for peripheral */
> > > > -               if (ci->role == USB_ROLE_DEVICE)
> > > > -                       ci_handle_vbus_change(ci);
> > > > -
> > > > -               ret = ci_role_start(ci, ci->role);
> > > > -               if (ret) {
> > > > -                       dev_err(dev, "can't start %s role\n",
> > > > -                                               ci_role(ci)->name);
> > > > +       if (!ci_role_switch.fwnode) {
> > > > +               ret = ci_start_initial_role(ci);
> > > > +               if (ret)
> > > >                         goto stop;
> > > > -               }
> > > >         }
> > > >
> > > >         ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
> > > > @@
> > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto stop;
> > > >
> > > > +       if (ci_role_switch.fwnode) {
> > > > +               ci->role_switch = usb_role_switch_register(dev,
> > > > +                                       &ci_role_switch);
> > > > +               if (IS_ERR(ci->role_switch)) {
> > > > +                       ret = PTR_ERR(ci->role_switch);
> > > > +                       goto stop;
> > > > +               }
> > > > +       }
> > > > +
> > > >         if (ci->supports_runtime_pm) {
> > > >                 pm_runtime_set_active(&pdev->dev);
> > > >                 pm_runtime_enable(&pdev->dev); @@ -1133,6 +1197,9
> > > > @@ static int ci_hdrc_remove(struct platform_device *pdev)  {
> > > >         struct ci_hdrc *ci = platform_get_drvdata(pdev);
> > > >
> > > > +       if (ci->role_switch)
> > > > +               usb_role_switch_unregister(ci->role_switch);
> > > > +
> > > >         if (ci->supports_runtime_pm) {
> > > >                 pm_runtime_get_sync(&pdev->dev);
> > > >                 pm_runtime_disable(&pdev->dev); diff --git
> > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> > > > 5bde0b5..0a22855 100644
> > > > --- a/drivers/usb/chipidea/otg.c
> > > > +++ b/drivers/usb/chipidea/otg.c
> > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work)
> > > >                 ci_handle_vbus_change(ci);
> > > >         }
> > > >
> > > > +       if (ci->role_switch_event) {
> > > > +               ci->role_switch_event = false;
> > > > +
> > > > +               if (ci->role == USB_ROLE_DEVICE) {
> > > > +                       usb_gadget_vbus_disconnect(&ci->gadget);
> > > > +                       ci_role_start(ci, USB_ROLE_HOST);
> > > > +               } else if (ci->role == USB_ROLE_HOST) {
> > > > +                       ci_role_stop(ci);
> > > > +                       usb_gadget_vbus_connect(&ci->gadget);
> > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > +               }
> > > > +       }
> > > > +
> > > >         pm_runtime_put_sync(ci->dev);
> > > >
> > > >         enable_irq(ci->irq);
> > > > --
> > > > 2.7.4
> > > >

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Peter Chen]

 
> > > > > USB role is fully controlled by usb role switch consumer(e.g.
> > > > > typec), usb port either at host mode, or at device connected
> > > > > mode, will not stay at USB_ROLE_NONE mode.
> > > > >
> > > >
> > > > Then, if the Type-C cable is disconnected from PC host, the
> > > > controller driver can't be notified?If that, how controller enters
> > > > low power mode at this
> > situation?
> > >
> > > The controller driver can be notified, but there are only role(host
> > > or
> > > device) Information, so in you mentioned case, controller driver
> > > will get the same input before and after disconnect from host, can't
> > > know detachment and enter low power mode accordingly, this is more
> > > like something internal handling under device role.
> > >
> >
> > If the Type-C can't pass "gadget disconnect" event, you may handle it
> > at UDC driver itself, eg at ci_usb_role_switch_set. Otherwise, the
> > gadget class driver can't be notified disconnection, and the controller itself can't
> enter LPM as well.
> 
> OK, I will enable VBUS irq to handle this in v2.
> 

You may use connect bit at portsc since VBUS may not connect to SoC.

Peter

> Li Jun
> >
> > Peter
> >
> > > Li Jun
> > >
> > > >
> > > > Peter
> > > >
> > > > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > > > ---
> > > > >  drivers/usb/chipidea/ci.h   |   2 +
> > > > >  drivers/usb/chipidea/core.c | 125
> > > > > ++++++++++++++++++++++++++++++++++----------
> > > > >  drivers/usb/chipidea/otg.c  |  13 +++++
> > > > >  3 files changed, 111 insertions(+), 29 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/chipidea/ci.h
> > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644
> > > > > --- a/drivers/usb/chipidea/ci.h
> > > > > +++ b/drivers/usb/chipidea/ci.h
> > > > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > > > >         ktime_t
> > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > > >         unsigned                        enabled_otg_timer_bits;
> > > > >         enum otg_fsm_timer              next_otg_timer;
> > > > > +       struct usb_role_switch          *role_switch;
> > > > >         struct work_struct              work;
> > > > >         struct workqueue_struct         *wq;
> > > > >
> > > > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > > > >         struct dentry                   *debugfs;
> > > > >         bool                            id_event;
> > > > >         bool                            b_sess_valid_event;
> > > > > +       bool                            role_switch_event;
> > > > >         bool                            imx28_write_fix;
> > > > >         bool                            supports_runtime_pm;
> > > > >         bool                            in_lpm;
> > > > > diff --git a/drivers/usb/chipidea/core.c
> > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644
> > > > > --- a/drivers/usb/chipidea/core.c
> > > > > +++ b/drivers/usb/chipidea/core.c
> > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
> > > > >         return ret;
> > > > >  }
> > > > >
> > > > > +static int ci_usb_role_switch_set(struct device *dev, enum
> > > > > +usb_role
> > > > > +role) {
> > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > +       unsigned long flags;
> > > > > +
> > > > > +       if (ci->role == role || role == USB_ROLE_NONE)
> > > > > +               return 0;
> > > > > +
> > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > +       ci->role_switch_event = true;
> > > > > +       if (ci->role == USB_ROLE_NONE) {
> > > > > +               if (role == USB_ROLE_DEVICE)
> > > > > +                       ci->role = USB_ROLE_HOST;
> > > > > +               else
> > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > +       }
> > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > +
> > > > > +       ci_otg_queue_work(ci);
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > > > +
> > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > +       unsigned long flags;
> > > > > +       enum usb_role role;
> > > > > +
> > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > +       role = ci->role;
> > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > +
> > > > > +       return role;
> > > > > +}
> > > > > +
> > > > > +static struct usb_role_switch_desc ci_role_switch = {
> > > > > +       .set = ci_usb_role_switch_set,
> > > > > +       .get = ci_usb_role_switch_get, };
> > > > > +
> > > > >  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> > > > >                              void *ptr)  { @@ -689,6 +730,9 @@
> > > > > static int ci_get_platdata(struct device *dev,
> > > > >         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> > > > >                 platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
> > > > >
> > > > > +       if (device_property_read_bool(dev, "usb-role-switch"))
> > > > > +               ci_role_switch.fwnode = dev->fwnode;
> > > > > +
> > > > >         ext_id = ERR_PTR(-ENODEV);
> > > > >         ext_vbus = ERR_PTR(-ENODEV);
> > > > >         if (of_property_read_bool(dev->of_node, "extcon")) { @@
> > > > > -908,6
> > > > > +952,43 @@ static const struct attribute_group ci_attr_group = {
> > > > >         .attrs = ci_attrs,
> > > > >  };
> > > > >
> > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) {
> > > > > +       int ret = 0;
> > > > > +
> > > > > +       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > > > > +               if (ci->is_otg) {
> > > > > +                       ci->role = ci_otg_role(ci);
> > > > > +                       /* Enable ID change irq */
> > > > > +                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > > +               } 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.
> > > > > +                        */
> > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > +               }
> > > > > +       } else {
> > > > > +               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > +                       ? USB_ROLE_HOST
> > > > > +                       : USB_ROLE_DEVICE;
> > > > > +       }
> > > > > +
> > > > > +       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > +               /* only update vbus status for peripheral */
> > > > > +               if (ci->role == USB_ROLE_DEVICE)
> > > > > +                       ci_handle_vbus_change(ci);
> > > > > +
> > > > > +               ret = ci_role_start(ci, ci->role);
> > > > > +               if (ret)
> > > > > +                       dev_err(ci->dev, "can't start %s role\n",
> > > > > +                                               ci_role(ci)->name);
> > > > > +       }
> > > > > +
> > > > > +       return ret;
> > > > > +}
> > > > > +
> > > > >  static int ci_hdrc_probe(struct platform_device *pdev)  {
> > > > >         struct device   *dev = &pdev->dev;
> > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct
> platform_device *pdev)
> > > > >                 }
> > > > >         }
> > > > >
> > > > > -       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > > > > -               if (ci->is_otg) {
> > > > > -                       ci->role = ci_otg_role(ci);
> > > > > -                       /* Enable ID change irq */
> > > > > -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > > -               } 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.
> > > > > -                        */
> > > > > -                       ci->role = USB_ROLE_DEVICE;
> > > > > -               }
> > > > > -       } else {
> > > > > -               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > -                       ? USB_ROLE_HOST
> > > > > -                       : USB_ROLE_DEVICE;
> > > > > -       }
> > > > > -
> > > > > -       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > -               /* only update vbus status for peripheral */
> > > > > -               if (ci->role == USB_ROLE_DEVICE)
> > > > > -                       ci_handle_vbus_change(ci);
> > > > > -
> > > > > -               ret = ci_role_start(ci, ci->role);
> > > > > -               if (ret) {
> > > > > -                       dev_err(dev, "can't start %s role\n",
> > > > > -                                               ci_role(ci)->name);
> > > > > +       if (!ci_role_switch.fwnode) {
> > > > > +               ret = ci_start_initial_role(ci);
> > > > > +               if (ret)
> > > > >                         goto stop;
> > > > > -               }
> > > > >         }
> > > > >
> > > > >         ret = devm_request_irq(dev, ci->irq, ci_irq,
> > > > > IRQF_SHARED, @@
> > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct platform_device
> *pdev)
> > > > >         if (ret)
> > > > >                 goto stop;
> > > > >
> > > > > +       if (ci_role_switch.fwnode) {
> > > > > +               ci->role_switch = usb_role_switch_register(dev,
> > > > > +                                       &ci_role_switch);
> > > > > +               if (IS_ERR(ci->role_switch)) {
> > > > > +                       ret = PTR_ERR(ci->role_switch);
> > > > > +                       goto stop;
> > > > > +               }
> > > > > +       }
> > > > > +
> > > > >         if (ci->supports_runtime_pm) {
> > > > >                 pm_runtime_set_active(&pdev->dev);
> > > > >                 pm_runtime_enable(&pdev->dev); @@ -1133,6
> > > > > +1197,9 @@ static int ci_hdrc_remove(struct platform_device *pdev)  {
> > > > >         struct ci_hdrc *ci = platform_get_drvdata(pdev);
> > > > >
> > > > > +       if (ci->role_switch)
> > > > > +               usb_role_switch_unregister(ci->role_switch);
> > > > > +
> > > > >         if (ci->supports_runtime_pm) {
> > > > >                 pm_runtime_get_sync(&pdev->dev);
> > > > >                 pm_runtime_disable(&pdev->dev); diff --git
> > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> > > > > 5bde0b5..0a22855 100644
> > > > > --- a/drivers/usb/chipidea/otg.c
> > > > > +++ b/drivers/usb/chipidea/otg.c
> > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work)
> > > > >                 ci_handle_vbus_change(ci);
> > > > >         }
> > > > >
> > > > > +       if (ci->role_switch_event) {
> > > > > +               ci->role_switch_event = false;
> > > > > +
> > > > > +               if (ci->role == USB_ROLE_DEVICE) {
> > > > > +                       usb_gadget_vbus_disconnect(&ci->gadget);
> > > > > +                       ci_role_start(ci, USB_ROLE_HOST);
> > > > > +               } else if (ci->role == USB_ROLE_HOST) {
> > > > > +                       ci_role_stop(ci);
> > > > > +                       usb_gadget_vbus_connect(&ci->gadget);
> > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > +               }
> > > > > +       }
> > > > > +
> > > > >         pm_runtime_put_sync(ci->dev);
> > > > >
> > > > >         enable_irq(ci->irq);
> > > > > --
> > > > > 2.7.4
> > > > >

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Jun Li]

Hi
> -----Original Message-----
> From: Peter Chen
> Sent: 2019年8月5日 12:57
> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; dl-linux-imx
> <linux-imx@nxp.com>; USB list <linux-usb@vger.kernel.org>
> Subject: RE: [PATCH 2/5] usb: chipidea: add role switch class support
> 
> 
> > > > > > USB role is fully controlled by usb role switch consumer(e.g.
> > > > > > typec), usb port either at host mode, or at device connected
> > > > > > mode, will not stay at USB_ROLE_NONE mode.
> > > > > >
> > > > >
> > > > > Then, if the Type-C cable is disconnected from PC host, the
> > > > > controller driver can't be notified?If that, how controller
> > > > > enters low power mode at this
> > > situation?
> > > >
> > > > The controller driver can be notified, but there are only
> > > > role(host or
> > > > device) Information, so in you mentioned case, controller driver
> > > > will get the same input before and after disconnect from host,
> > > > can't know detachment and enter low power mode accordingly, this
> > > > is more like something internal handling under device role.
> > > >
> > >
> > > If the Type-C can't pass "gadget disconnect" event, you may handle
> > > it at UDC driver itself, eg at ci_usb_role_switch_set. Otherwise,
> > > the gadget class driver can't be notified disconnection, and the
> > > controller itself can't
> > enter LPM as well.
> >
> > OK, I will enable VBUS irq to handle this in v2.
> >
> 
> You may use connect bit at portsc since VBUS may not connect to SoC.

By this way, disconnect can be decided but connect is a problem in
current driver, as controller was stopped in low power mode so can't
detect connection from host, unless we also update this behavior too.

Li Jun
> 
> Peter
> 
> > Li Jun
> > >
> > > Peter
> > >
> > > > Li Jun
> > > >
> > > > >
> > > > > Peter
> > > > >
> > > > > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > > > > ---
> > > > > >  drivers/usb/chipidea/ci.h   |   2 +
> > > > > >  drivers/usb/chipidea/core.c | 125
> > > > > > ++++++++++++++++++++++++++++++++++----------
> > > > > >  drivers/usb/chipidea/otg.c  |  13 +++++
> > > > > >  3 files changed, 111 insertions(+), 29 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/usb/chipidea/ci.h
> > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644
> > > > > > --- a/drivers/usb/chipidea/ci.h
> > > > > > +++ b/drivers/usb/chipidea/ci.h
> > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > > > > >         ktime_t
> > > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > > > >         unsigned                        enabled_otg_timer_bits;
> > > > > >         enum otg_fsm_timer              next_otg_timer;
> > > > > > +       struct usb_role_switch          *role_switch;
> > > > > >         struct work_struct              work;
> > > > > >         struct workqueue_struct         *wq;
> > > > > >
> > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > > > > >         struct dentry                   *debugfs;
> > > > > >         bool                            id_event;
> > > > > >         bool                            b_sess_valid_event;
> > > > > > +       bool                            role_switch_event;
> > > > > >         bool                            imx28_write_fix;
> > > > > >         bool                            supports_runtime_pm;
> > > > > >         bool                            in_lpm;
> > > > > > diff --git a/drivers/usb/chipidea/core.c
> > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644
> > > > > > --- a/drivers/usb/chipidea/core.c
> > > > > > +++ b/drivers/usb/chipidea/core.c
> > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
> > > > > >         return ret;
> > > > > >  }
> > > > > >
> > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum
> > > > > > +usb_role
> > > > > > +role) {
> > > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > +       unsigned long flags;
> > > > > > +
> > > > > > +       if (ci->role == role || role == USB_ROLE_NONE)
> > > > > > +               return 0;
> > > > > > +
> > > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > > +       ci->role_switch_event = true;
> > > > > > +       if (ci->role == USB_ROLE_NONE) {
> > > > > > +               if (role == USB_ROLE_DEVICE)
> > > > > > +                       ci->role = USB_ROLE_HOST;
> > > > > > +               else
> > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > +       }
> > > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > +
> > > > > > +       ci_otg_queue_work(ci);
> > > > > > +
> > > > > > +       return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > +       unsigned long flags;
> > > > > > +       enum usb_role role;
> > > > > > +
> > > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > > +       role = ci->role;
> > > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > +
> > > > > > +       return role;
> > > > > > +}
> > > > > > +
> > > > > > +static struct usb_role_switch_desc ci_role_switch = {
> > > > > > +       .set = ci_usb_role_switch_set,
> > > > > > +       .get = ci_usb_role_switch_get, };
> > > > > > +
> > > > > >  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> > > > > >                              void *ptr)  { @@ -689,6 +730,9 @@
> > > > > > static int ci_get_platdata(struct device *dev,
> > > > > >         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> > > > > >                 platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
> > > > > >
> > > > > > +       if (device_property_read_bool(dev, "usb-role-switch"))
> > > > > > +               ci_role_switch.fwnode = dev->fwnode;
> > > > > > +
> > > > > >         ext_id = ERR_PTR(-ENODEV);
> > > > > >         ext_vbus = ERR_PTR(-ENODEV);
> > > > > >         if (of_property_read_bool(dev->of_node, "extcon")) {
> > > > > > @@
> > > > > > -908,6
> > > > > > +952,43 @@ static const struct attribute_group ci_attr_group =
> > > > > > +{
> > > > > >         .attrs = ci_attrs,
> > > > > >  };
> > > > > >
> > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) {
> > > > > > +       int ret = 0;
> > > > > > +
> > > > > > +       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE])
> {
> > > > > > +               if (ci->is_otg) {
> > > > > > +                       ci->role = ci_otg_role(ci);
> > > > > > +                       /* Enable ID change irq */
> > > > > > +                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > > > +               } 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.
> > > > > > +                        */
> > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > +               }
> > > > > > +       } else {
> > > > > > +               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > > +                       ? USB_ROLE_HOST
> > > > > > +                       : USB_ROLE_DEVICE;
> > > > > > +       }
> > > > > > +
> > > > > > +       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > > +               /* only update vbus status for peripheral */
> > > > > > +               if (ci->role == USB_ROLE_DEVICE)
> > > > > > +                       ci_handle_vbus_change(ci);
> > > > > > +
> > > > > > +               ret = ci_role_start(ci, ci->role);
> > > > > > +               if (ret)
> > > > > > +                       dev_err(ci->dev, "can't start %s role\n",
> > > > > > +                                               ci_role(ci)->name);
> > > > > > +       }
> > > > > > +
> > > > > > +       return ret;
> > > > > > +}
> > > > > > +
> > > > > >  static int ci_hdrc_probe(struct platform_device *pdev)  {
> > > > > >         struct device   *dev = &pdev->dev;
> > > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct
> > platform_device *pdev)
> > > > > >                 }
> > > > > >         }
> > > > > >
> > > > > > -       if (ci->roles[USB_ROLE_HOST] && ci->roles[USB_ROLE_DEVICE]) {
> > > > > > -               if (ci->is_otg) {
> > > > > > -                       ci->role = ci_otg_role(ci);
> > > > > > -                       /* Enable ID change irq */
> > > > > > -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > > > -               } 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.
> > > > > > -                        */
> > > > > > -                       ci->role = USB_ROLE_DEVICE;
> > > > > > -               }
> > > > > > -       } else {
> > > > > > -               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > > -                       ? USB_ROLE_HOST
> > > > > > -                       : USB_ROLE_DEVICE;
> > > > > > -       }
> > > > > > -
> > > > > > -       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > > -               /* only update vbus status for peripheral */
> > > > > > -               if (ci->role == USB_ROLE_DEVICE)
> > > > > > -                       ci_handle_vbus_change(ci);
> > > > > > -
> > > > > > -               ret = ci_role_start(ci, ci->role);
> > > > > > -               if (ret) {
> > > > > > -                       dev_err(dev, "can't start %s role\n",
> > > > > > -                                               ci_role(ci)->name);
> > > > > > +       if (!ci_role_switch.fwnode) {
> > > > > > +               ret = ci_start_initial_role(ci);
> > > > > > +               if (ret)
> > > > > >                         goto stop;
> > > > > > -               }
> > > > > >         }
> > > > > >
> > > > > >         ret = devm_request_irq(dev, ci->irq, ci_irq,
> > > > > > IRQF_SHARED, @@
> > > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct
> > > > > > platform_device
> > *pdev)
> > > > > >         if (ret)
> > > > > >                 goto stop;
> > > > > >
> > > > > > +       if (ci_role_switch.fwnode) {
> > > > > > +               ci->role_switch = usb_role_switch_register(dev,
> > > > > > +                                       &ci_role_switch);
> > > > > > +               if (IS_ERR(ci->role_switch)) {
> > > > > > +                       ret = PTR_ERR(ci->role_switch);
> > > > > > +                       goto stop;
> > > > > > +               }
> > > > > > +       }
> > > > > > +
> > > > > >         if (ci->supports_runtime_pm) {
> > > > > >                 pm_runtime_set_active(&pdev->dev);
> > > > > >                 pm_runtime_enable(&pdev->dev); @@ -1133,6
> > > > > > +1197,9 @@ static int ci_hdrc_remove(struct platform_device
> > > > > > +*pdev)  {
> > > > > >         struct ci_hdrc *ci = platform_get_drvdata(pdev);
> > > > > >
> > > > > > +       if (ci->role_switch)
> > > > > > +               usb_role_switch_unregister(ci->role_switch);
> > > > > > +
> > > > > >         if (ci->supports_runtime_pm) {
> > > > > >                 pm_runtime_get_sync(&pdev->dev);
> > > > > >                 pm_runtime_disable(&pdev->dev); diff --git
> > > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> > > > > > index
> > > > > > 5bde0b5..0a22855 100644
> > > > > > --- a/drivers/usb/chipidea/otg.c
> > > > > > +++ b/drivers/usb/chipidea/otg.c
> > > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct *work)
> > > > > >                 ci_handle_vbus_change(ci);
> > > > > >         }
> > > > > >
> > > > > > +       if (ci->role_switch_event) {
> > > > > > +               ci->role_switch_event = false;
> > > > > > +
> > > > > > +               if (ci->role == USB_ROLE_DEVICE) {
> > > > > > +                       usb_gadget_vbus_disconnect(&ci->gadget);
> > > > > > +                       ci_role_start(ci, USB_ROLE_HOST);
> > > > > > +               } else if (ci->role == USB_ROLE_HOST) {
> > > > > > +                       ci_role_stop(ci);
> > > > > > +                       usb_gadget_vbus_connect(&ci->gadget);
> > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > +               }
> > > > > > +       }
> > > > > > +
> > > > > >         pm_runtime_put_sync(ci->dev);
> > > > > >
> > > > > >         enable_irq(ci->irq);
> > > > > > --
> > > > > > 2.7.4
> > > > > >

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Peter Chen]

 
> >
> > You may use connect bit at portsc since VBUS may not connect to SoC.
> 
> By this way, disconnect can be decided but connect is a problem in current driver,
> as controller was stopped in low power mode so can't detect connection from host,
> unless we also update this behavior too.
> 

For connection, if current role is USB_ROLE_NONE, you may start device mode.

Peter

> Li Jun
> >
> > Peter
> >
> > > Li Jun
> > > >
> > > > Peter
> > > >
> > > > > Li Jun
> > > > >
> > > > > >
> > > > > > Peter
> > > > > >
> > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > > > > > ---
> > > > > > >  drivers/usb/chipidea/ci.h   |   2 +
> > > > > > >  drivers/usb/chipidea/core.c | 125
> > > > > > > ++++++++++++++++++++++++++++++++++----------
> > > > > > >  drivers/usb/chipidea/otg.c  |  13 +++++
> > > > > > >  3 files changed, 111 insertions(+), 29 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/usb/chipidea/ci.h
> > > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644
> > > > > > > --- a/drivers/usb/chipidea/ci.h
> > > > > > > +++ b/drivers/usb/chipidea/ci.h
> > > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > > > > > >         ktime_t
> > > > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > > > > >         unsigned                        enabled_otg_timer_bits;
> > > > > > >         enum otg_fsm_timer              next_otg_timer;
> > > > > > > +       struct usb_role_switch          *role_switch;
> > > > > > >         struct work_struct              work;
> > > > > > >         struct workqueue_struct         *wq;
> > > > > > >
> > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > > > > > >         struct dentry                   *debugfs;
> > > > > > >         bool                            id_event;
> > > > > > >         bool                            b_sess_valid_event;
> > > > > > > +       bool                            role_switch_event;
> > > > > > >         bool                            imx28_write_fix;
> > > > > > >         bool                            supports_runtime_pm;
> > > > > > >         bool                            in_lpm;
> > > > > > > diff --git a/drivers/usb/chipidea/core.c
> > > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6 100644
> > > > > > > --- a/drivers/usb/chipidea/core.c
> > > > > > > +++ b/drivers/usb/chipidea/core.c
> > > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
> > > > > > >         return ret;
> > > > > > >  }
> > > > > > >
> > > > > > > +static int ci_usb_role_switch_set(struct device *dev, enum
> > > > > > > +usb_role
> > > > > > > +role) {
> > > > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > +       unsigned long flags;
> > > > > > > +
> > > > > > > +       if (ci->role == role || role == USB_ROLE_NONE)
> > > > > > > +               return 0;
> > > > > > > +
> > > > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > > > +       ci->role_switch_event = true;
> > > > > > > +       if (ci->role == USB_ROLE_NONE) {
> > > > > > > +               if (role == USB_ROLE_DEVICE)
> > > > > > > +                       ci->role = USB_ROLE_HOST;
> > > > > > > +               else
> > > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > > +       }
> > > > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > +
> > > > > > > +       ci_otg_queue_work(ci);
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > +       unsigned long flags;
> > > > > > > +       enum usb_role role;
> > > > > > > +
> > > > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > > > +       role = ci->role;
> > > > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > +
> > > > > > > +       return role;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static struct usb_role_switch_desc ci_role_switch = {
> > > > > > > +       .set = ci_usb_role_switch_set,
> > > > > > > +       .get = ci_usb_role_switch_get, };
> > > > > > > +
> > > > > > >  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> > > > > > >                              void *ptr)  { @@ -689,6 +730,9
> > > > > > > @@ static int ci_get_platdata(struct device *dev,
> > > > > > >         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> > > > > > >                 platdata->flags |=
> > > > > > > CI_HDRC_SET_NON_ZERO_TTHA;
> > > > > > >
> > > > > > > +       if (device_property_read_bool(dev, "usb-role-switch"))
> > > > > > > +               ci_role_switch.fwnode = dev->fwnode;
> > > > > > > +
> > > > > > >         ext_id = ERR_PTR(-ENODEV);
> > > > > > >         ext_vbus = ERR_PTR(-ENODEV);
> > > > > > >         if (of_property_read_bool(dev->of_node, "extcon")) {
> > > > > > > @@
> > > > > > > -908,6
> > > > > > > +952,43 @@ static const struct attribute_group ci_attr_group
> > > > > > > += {
> > > > > > >         .attrs = ci_attrs,
> > > > > > >  };
> > > > > > >
> > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) {
> > > > > > > +       int ret = 0;
> > > > > > > +
> > > > > > > +       if (ci->roles[USB_ROLE_HOST] &&
> > > > > > > + ci->roles[USB_ROLE_DEVICE])
> > {
> > > > > > > +               if (ci->is_otg) {
> > > > > > > +                       ci->role = ci_otg_role(ci);
> > > > > > > +                       /* Enable ID change irq */
> > > > > > > +                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > > > > +               } 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.
> > > > > > > +                        */
> > > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > > +               }
> > > > > > > +       } else {
> > > > > > > +               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > > > +                       ? USB_ROLE_HOST
> > > > > > > +                       : USB_ROLE_DEVICE;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > > > +               /* only update vbus status for peripheral */
> > > > > > > +               if (ci->role == USB_ROLE_DEVICE)
> > > > > > > +                       ci_handle_vbus_change(ci);
> > > > > > > +
> > > > > > > +               ret = ci_role_start(ci, ci->role);
> > > > > > > +               if (ret)
> > > > > > > +                       dev_err(ci->dev, "can't start %s role\n",
> > > > > > > +                                               ci_role(ci)->name);
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       return ret;
> > > > > > > +}
> > > > > > > +
> > > > > > >  static int ci_hdrc_probe(struct platform_device *pdev)  {
> > > > > > >         struct device   *dev = &pdev->dev;
> > > > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct
> > > platform_device *pdev)
> > > > > > >                 }
> > > > > > >         }
> > > > > > >
> > > > > > > -       if (ci->roles[USB_ROLE_HOST] && ci-
> >roles[USB_ROLE_DEVICE]) {
> > > > > > > -               if (ci->is_otg) {
> > > > > > > -                       ci->role = ci_otg_role(ci);
> > > > > > > -                       /* Enable ID change irq */
> > > > > > > -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > > > > -               } 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.
> > > > > > > -                        */
> > > > > > > -                       ci->role = USB_ROLE_DEVICE;
> > > > > > > -               }
> > > > > > > -       } else {
> > > > > > > -               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > > > -                       ? USB_ROLE_HOST
> > > > > > > -                       : USB_ROLE_DEVICE;
> > > > > > > -       }
> > > > > > > -
> > > > > > > -       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > > > -               /* only update vbus status for peripheral */
> > > > > > > -               if (ci->role == USB_ROLE_DEVICE)
> > > > > > > -                       ci_handle_vbus_change(ci);
> > > > > > > -
> > > > > > > -               ret = ci_role_start(ci, ci->role);
> > > > > > > -               if (ret) {
> > > > > > > -                       dev_err(dev, "can't start %s role\n",
> > > > > > > -                                               ci_role(ci)->name);
> > > > > > > +       if (!ci_role_switch.fwnode) {
> > > > > > > +               ret = ci_start_initial_role(ci);
> > > > > > > +               if (ret)
> > > > > > >                         goto stop;
> > > > > > > -               }
> > > > > > >         }
> > > > > > >
> > > > > > >         ret = devm_request_irq(dev, ci->irq, ci_irq,
> > > > > > > IRQF_SHARED, @@
> > > > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct
> > > > > > > platform_device
> > > *pdev)
> > > > > > >         if (ret)
> > > > > > >                 goto stop;
> > > > > > >
> > > > > > > +       if (ci_role_switch.fwnode) {
> > > > > > > +               ci->role_switch = usb_role_switch_register(dev,
> > > > > > > +                                       &ci_role_switch);
> > > > > > > +               if (IS_ERR(ci->role_switch)) {
> > > > > > > +                       ret = PTR_ERR(ci->role_switch);
> > > > > > > +                       goto stop;
> > > > > > > +               }
> > > > > > > +       }
> > > > > > > +
> > > > > > >         if (ci->supports_runtime_pm) {
> > > > > > >                 pm_runtime_set_active(&pdev->dev);
> > > > > > >                 pm_runtime_enable(&pdev->dev); @@ -1133,6
> > > > > > > +1197,9 @@ static int ci_hdrc_remove(struct platform_device
> > > > > > > +*pdev)  {
> > > > > > >         struct ci_hdrc *ci = platform_get_drvdata(pdev);
> > > > > > >
> > > > > > > +       if (ci->role_switch)
> > > > > > > +               usb_role_switch_unregister(ci->role_switch);
> > > > > > > +
> > > > > > >         if (ci->supports_runtime_pm) {
> > > > > > >                 pm_runtime_get_sync(&pdev->dev);
> > > > > > >                 pm_runtime_disable(&pdev->dev); diff --git
> > > > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> > > > > > > index
> > > > > > > 5bde0b5..0a22855 100644
> > > > > > > --- a/drivers/usb/chipidea/otg.c
> > > > > > > +++ b/drivers/usb/chipidea/otg.c
> > > > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct work_struct
> *work)
> > > > > > >                 ci_handle_vbus_change(ci);
> > > > > > >         }
> > > > > > >
> > > > > > > +       if (ci->role_switch_event) {
> > > > > > > +               ci->role_switch_event = false;
> > > > > > > +
> > > > > > > +               if (ci->role == USB_ROLE_DEVICE) {
> > > > > > > +                       usb_gadget_vbus_disconnect(&ci->gadget);
> > > > > > > +                       ci_role_start(ci, USB_ROLE_HOST);
> > > > > > > +               } else if (ci->role == USB_ROLE_HOST) {
> > > > > > > +                       ci_role_stop(ci);
> > > > > > > +                       usb_gadget_vbus_connect(&ci->gadget);
> > > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > > +               }
> > > > > > > +       }
> > > > > > > +
> > > > > > >         pm_runtime_put_sync(ci->dev);
> > > > > > >
> > > > > > >         enable_irq(ci->irq);
> > > > > > > --
> > > > > > > 2.7.4
> > > > > > >

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Jun Li]

> -----Original Message-----
> From: Peter Chen
> Sent: 2019年8月6日 15:52
> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; dl-linux-imx
> <linux-imx@nxp.com>; USB list <linux-usb@vger.kernel.org>
> Subject: RE: [PATCH 2/5] usb: chipidea: add role switch class support
> 
> 
> > >
> > > You may use connect bit at portsc since VBUS may not connect to SoC.
> >
> > By this way, disconnect can be decided but connect is a problem in
> > current driver, as controller was stopped in low power mode so can't
> > detect connection from host, unless we also update this behavior too.
> >
> 
> For connection, if current role is USB_ROLE_NONE, you may start device mode.

This is assuming set role only can be called one time between disconnect
and connect to host, this may not always true, but this can be resolved,
I think we need handle the case device mode is started but connection does
not happen, so the gadget need be stopped and enter low power mode again,
if this approach is OK to you, I will go in this direction for my v2.

Li Jun 

> 
> Peter
> 
> > Li Jun
> > >
> > > Peter
> > >
> > > > Li Jun
> > > > >
> > > > > Peter
> > > > >
> > > > > > Li Jun
> > > > > >
> > > > > > >
> > > > > > > Peter
> > > > > > >
> > > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > > > > > > ---
> > > > > > > >  drivers/usb/chipidea/ci.h   |   2 +
> > > > > > > >  drivers/usb/chipidea/core.c | 125
> > > > > > > > ++++++++++++++++++++++++++++++++++----------
> > > > > > > >  drivers/usb/chipidea/otg.c  |  13 +++++
> > > > > > > >  3 files changed, 111 insertions(+), 29 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/usb/chipidea/ci.h
> > > > > > > > b/drivers/usb/chipidea/ci.h index 82b86cd..5e2f0bc 100644
> > > > > > > > --- a/drivers/usb/chipidea/ci.h
> > > > > > > > +++ b/drivers/usb/chipidea/ci.h
> > > > > > > > @@ -212,6 +212,7 @@ struct ci_hdrc {
> > > > > > > >         ktime_t
> > > > > hr_timeouts[NUM_OTG_FSM_TIMERS];
> > > > > > > >         unsigned                        enabled_otg_timer_bits;
> > > > > > > >         enum otg_fsm_timer              next_otg_timer;
> > > > > > > > +       struct usb_role_switch          *role_switch;
> > > > > > > >         struct work_struct              work;
> > > > > > > >         struct workqueue_struct         *wq;
> > > > > > > >
> > > > > > > > @@ -244,6 +245,7 @@ struct ci_hdrc {
> > > > > > > >         struct dentry                   *debugfs;
> > > > > > > >         bool                            id_event;
> > > > > > > >         bool                            b_sess_valid_event;
> > > > > > > > +       bool                            role_switch_event;
> > > > > > > >         bool                            imx28_write_fix;
> > > > > > > >         bool                            supports_runtime_pm;
> > > > > > > >         bool                            in_lpm;
> > > > > > > > diff --git a/drivers/usb/chipidea/core.c
> > > > > > > > b/drivers/usb/chipidea/core.c index bc24c5b..1bcf6f6
> > > > > > > > 100644
> > > > > > > > --- a/drivers/usb/chipidea/core.c
> > > > > > > > +++ b/drivers/usb/chipidea/core.c
> > > > > > > > @@ -587,6 +587,47 @@ static irqreturn_t ci_irq(int irq, void *data)
> > > > > > > >         return ret;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +static int ci_usb_role_switch_set(struct device *dev,
> > > > > > > > +enum usb_role
> > > > > > > > +role) {
> > > > > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > > +       unsigned long flags;
> > > > > > > > +
> > > > > > > > +       if (ci->role == role || role == USB_ROLE_NONE)
> > > > > > > > +               return 0;
> > > > > > > > +
> > > > > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > > > > +       ci->role_switch_event = true;
> > > > > > > > +       if (ci->role == USB_ROLE_NONE) {
> > > > > > > > +               if (role == USB_ROLE_DEVICE)
> > > > > > > > +                       ci->role = USB_ROLE_HOST;
> > > > > > > > +               else
> > > > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > > > +       }
> > > > > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > > +
> > > > > > > > +       ci_otg_queue_work(ci);
> > > > > > > > +
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static enum usb_role ci_usb_role_switch_get(struct device *dev) {
> > > > > > > > +       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > > > > > > > +       unsigned long flags;
> > > > > > > > +       enum usb_role role;
> > > > > > > > +
> > > > > > > > +       spin_lock_irqsave(&ci->lock, flags);
> > > > > > > > +       role = ci->role;
> > > > > > > > +       spin_unlock_irqrestore(&ci->lock, flags);
> > > > > > > > +
> > > > > > > > +       return role;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static struct usb_role_switch_desc ci_role_switch = {
> > > > > > > > +       .set = ci_usb_role_switch_set,
> > > > > > > > +       .get = ci_usb_role_switch_get, };
> > > > > > > > +
> > > > > > > >  static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> > > > > > > >                              void *ptr)  { @@ -689,6
> > > > > > > > +730,9 @@ static int ci_get_platdata(struct device *dev,
> > > > > > > >         if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
> > > > > > > >                 platdata->flags |=
> > > > > > > > CI_HDRC_SET_NON_ZERO_TTHA;
> > > > > > > >
> > > > > > > > +       if (device_property_read_bool(dev, "usb-role-switch"))
> > > > > > > > +               ci_role_switch.fwnode = dev->fwnode;
> > > > > > > > +
> > > > > > > >         ext_id = ERR_PTR(-ENODEV);
> > > > > > > >         ext_vbus = ERR_PTR(-ENODEV);
> > > > > > > >         if (of_property_read_bool(dev->of_node, "extcon"))
> > > > > > > > { @@
> > > > > > > > -908,6
> > > > > > > > +952,43 @@ static const struct attribute_group
> > > > > > > > +ci_attr_group = {
> > > > > > > >         .attrs = ci_attrs,  };
> > > > > > > >
> > > > > > > > +static int ci_start_initial_role(struct ci_hdrc *ci) {
> > > > > > > > +       int ret = 0;
> > > > > > > > +
> > > > > > > > +       if (ci->roles[USB_ROLE_HOST] &&
> > > > > > > > + ci->roles[USB_ROLE_DEVICE])
> > > {
> > > > > > > > +               if (ci->is_otg) {
> > > > > > > > +                       ci->role = ci_otg_role(ci);
> > > > > > > > +                       /* Enable ID change irq */
> > > > > > > > +                       hw_write_otgsc(ci, OTGSC_IDIE,
> OTGSC_IDIE);
> > > > > > > > +               } 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.
> > > > > > > > +                        */
> > > > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > > > +               }
> > > > > > > > +       } else {
> > > > > > > > +               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > > > > +                       ? USB_ROLE_HOST
> > > > > > > > +                       : USB_ROLE_DEVICE;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > > > > +               /* only update vbus status for peripheral */
> > > > > > > > +               if (ci->role == USB_ROLE_DEVICE)
> > > > > > > > +                       ci_handle_vbus_change(ci);
> > > > > > > > +
> > > > > > > > +               ret = ci_role_start(ci, ci->role);
> > > > > > > > +               if (ret)
> > > > > > > > +                       dev_err(ci->dev, "can't start %s role\n",
> > > > > > > > +                                               ci_role(ci)->name);
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       return ret;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  static int ci_hdrc_probe(struct platform_device *pdev)  {
> > > > > > > >         struct device   *dev = &pdev->dev;
> > > > > > > > @@ -1051,36 +1132,10 @@ static int ci_hdrc_probe(struct
> > > > platform_device *pdev)
> > > > > > > >                 }
> > > > > > > >         }
> > > > > > > >
> > > > > > > > -       if (ci->roles[USB_ROLE_HOST] && ci-
> > >roles[USB_ROLE_DEVICE]) {
> > > > > > > > -               if (ci->is_otg) {
> > > > > > > > -                       ci->role = ci_otg_role(ci);
> > > > > > > > -                       /* Enable ID change irq */
> > > > > > > > -                       hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > > > > > > > -               } 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.
> > > > > > > > -                        */
> > > > > > > > -                       ci->role = USB_ROLE_DEVICE;
> > > > > > > > -               }
> > > > > > > > -       } else {
> > > > > > > > -               ci->role = ci->roles[USB_ROLE_HOST]
> > > > > > > > -                       ? USB_ROLE_HOST
> > > > > > > > -                       : USB_ROLE_DEVICE;
> > > > > > > > -       }
> > > > > > > > -
> > > > > > > > -       if (!ci_otg_is_fsm_mode(ci)) {
> > > > > > > > -               /* only update vbus status for peripheral */
> > > > > > > > -               if (ci->role == USB_ROLE_DEVICE)
> > > > > > > > -                       ci_handle_vbus_change(ci);
> > > > > > > > -
> > > > > > > > -               ret = ci_role_start(ci, ci->role);
> > > > > > > > -               if (ret) {
> > > > > > > > -                       dev_err(dev, "can't start %s role\n",
> > > > > > > > -                                               ci_role(ci)->name);
> > > > > > > > +       if (!ci_role_switch.fwnode) {
> > > > > > > > +               ret = ci_start_initial_role(ci);
> > > > > > > > +               if (ret)
> > > > > > > >                         goto stop;
> > > > > > > > -               }
> > > > > > > >         }
> > > > > > > >
> > > > > > > >         ret = devm_request_irq(dev, ci->irq, ci_irq,
> > > > > > > > IRQF_SHARED, @@
> > > > > > > > -1092,6 +1147,15 @@ static int ci_hdrc_probe(struct
> > > > > > > > platform_device
> > > > *pdev)
> > > > > > > >         if (ret)
> > > > > > > >                 goto stop;
> > > > > > > >
> > > > > > > > +       if (ci_role_switch.fwnode) {
> > > > > > > > +               ci->role_switch = usb_role_switch_register(dev,
> > > > > > > > +                                       &ci_role_switch);
> > > > > > > > +               if (IS_ERR(ci->role_switch)) {
> > > > > > > > +                       ret = PTR_ERR(ci->role_switch);
> > > > > > > > +                       goto stop;
> > > > > > > > +               }
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > >         if (ci->supports_runtime_pm) {
> > > > > > > >                 pm_runtime_set_active(&pdev->dev);
> > > > > > > >                 pm_runtime_enable(&pdev->dev); @@ -1133,6
> > > > > > > > +1197,9 @@ static int ci_hdrc_remove(struct
> > > > > > > > +platform_device
> > > > > > > > +*pdev)  {
> > > > > > > >         struct ci_hdrc *ci = platform_get_drvdata(pdev);
> > > > > > > >
> > > > > > > > +       if (ci->role_switch)
> > > > > > > > +
> > > > > > > > + usb_role_switch_unregister(ci->role_switch);
> > > > > > > > +
> > > > > > > >         if (ci->supports_runtime_pm) {
> > > > > > > >                 pm_runtime_get_sync(&pdev->dev);
> > > > > > > >                 pm_runtime_disable(&pdev->dev); diff --git
> > > > > > > > a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> > > > > > > > index
> > > > > > > > 5bde0b5..0a22855 100644
> > > > > > > > --- a/drivers/usb/chipidea/otg.c
> > > > > > > > +++ b/drivers/usb/chipidea/otg.c
> > > > > > > > @@ -214,6 +214,19 @@ static void ci_otg_work(struct
> > > > > > > > work_struct
> > *work)
> > > > > > > >                 ci_handle_vbus_change(ci);
> > > > > > > >         }
> > > > > > > >
> > > > > > > > +       if (ci->role_switch_event) {
> > > > > > > > +               ci->role_switch_event = false;
> > > > > > > > +
> > > > > > > > +               if (ci->role == USB_ROLE_DEVICE) {
> > > > > > > > +                       usb_gadget_vbus_disconnect(&ci->gadget);
> > > > > > > > +                       ci_role_start(ci, USB_ROLE_HOST);
> > > > > > > > +               } else if (ci->role == USB_ROLE_HOST) {
> > > > > > > > +                       ci_role_stop(ci);
> > > > > > > > +                       usb_gadget_vbus_connect(&ci->gadget);
> > > > > > > > +                       ci->role = USB_ROLE_DEVICE;
> > > > > > > > +               }
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > >         pm_runtime_put_sync(ci->dev);
> > > > > > > >
> > > > > > > >         enable_irq(ci->irq);
> > > > > > > > --
> > > > > > > > 2.7.4
> > > > > > > >

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Peter Chen]

 
> >
> >
> > > >
> > > > You may use connect bit at portsc since VBUS may not connect to SoC.
> > >
> > > By this way, disconnect can be decided but connect is a problem in
> > > current driver, as controller was stopped in low power mode so can't
> > > detect connection from host, unless we also update this behavior too.
> > >
> >
> > For connection, if current role is USB_ROLE_NONE, you may start device mode.
> 
> This is assuming set role only can be called one time between disconnect and
> connect to host, this may not always true, but this can be resolved, I think we need
> handle the case device mode is started but connection does not happen, so the
> gadget need be stopped and enter low power mode again, if this approach is OK to
> you, I will go in this direction for my v2.
> 
 
After thinking more, I think Type-C tcpm code should set usb_role correctly, it
knows physical connection status well,  and there are already USB_ROLE_NONE
references at tcpm now. Depending on USB device driver workaround to know USB_ROLE_NONE
is not a good solution. Look at another USB role driver, intel-xhci-usb-role-switch.c,
it could also get USB_ROLE_NONE state.

Peter

RE: [PATCH 2/5] usb: chipidea: add role switch class support

[Jun Li]

> -----Original Message-----
> From: Peter Chen
> Sent: 2019年8月7日 10:41
> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; dl-linux-imx
> <linux-imx@nxp.com>; USB list <linux-usb@vger.kernel.org>
> Subject: RE: [PATCH 2/5] usb: chipidea: add role switch class support
> 
> 
> 
> > >
> > >
> > > > >
> > > > > You may use connect bit at portsc since VBUS may not connect to SoC.
> > > >
> > > > By this way, disconnect can be decided but connect is a problem in
> > > > current driver, as controller was stopped in low power mode so
> > > > can't detect connection from host, unless we also update this behavior too.
> > > >
> > >
> > > For connection, if current role is USB_ROLE_NONE, you may start device mode.
> >
> > This is assuming set role only can be called one time between
> > disconnect and connect to host, this may not always true, but this can
> > be resolved, I think we need handle the case device mode is started
> > but connection does not happen, so the gadget need be stopped and
> > enter low power mode again, if this approach is OK to you, I will go in this direction for my
> v2.
> >
> 
> After thinking more, I think Type-C tcpm code should set usb_role correctly, it knows
> physical connection status well,  and there are already USB_ROLE_NONE references at
> tcpm now. Depending on USB device driver workaround to know USB_ROLE_NONE is
> not a good solution. Look at another USB role driver, intel-xhci-usb-role-switch.c, it could
> also get USB_ROLE_NONE state.

Sorry, I re-checked the latest tcpm code found there is already
USB_ROLE_NONE setting for not disconnect, so we are fine to
handle it in usb controller driver, will post v2 for this soon.

Li Jun
> 
> Peter