linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] usb: chipidea: replace ci_role with usb_role
@ 2019-08-07  7:23 jun.li
  2019-08-07  7:23 ` [PATCH v2 2/2] usb: chipidea: add role switch class support jun.li
  0 siblings, 1 reply; 7+ messages in thread
From: jun.li @ 2019-08-07  7:23 UTC (permalink / raw)
  To: Peter.Chen; +Cc: gregkh, jun.li, linux-imx, linux-usb

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>
---
v1 -> v2
No change for this patch.

 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH v2 2/2] usb: chipidea: add role switch class support
  2019-08-07  7:23 [PATCH v2 1/2] usb: chipidea: replace ci_role with usb_role jun.li
@ 2019-08-07  7:23 ` jun.li
  2019-08-07 18:39   ` kbuild test robot
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: jun.li @ 2019-08-07  7:23 UTC (permalink / raw)
  To: Peter.Chen; +Cc: gregkh, jun.li, linux-imx, linux-usb

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

USB role is fully controlled by usb role switch consumer(e.g. typec),
usb port can be at host mode(USB_ROLE_HOST), device mode connected to
host(USB_ROLE_DEVICE), or not connecting any parter(USB_ROLE_NONE).

Signed-off-by: Li Jun <jun.li@nxp.com>
---

Change for v2:
- Support USB_ROLE_NONE, which for usb port not connecting any
  device or host, and will be in low power mode.

 drivers/usb/chipidea/ci.h   |   3 ++
 drivers/usb/chipidea/core.c | 120 +++++++++++++++++++++++++++++++++-----------
 drivers/usb/chipidea/otg.c  |  20 ++++++++
 3 files changed, 114 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 82b86cd..f0aec1d 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -205,6 +205,7 @@ struct ci_hdrc {
 	int				irq;
 	struct ci_role_driver		*roles[USB_ROLE_DEVICE + 1];
 	enum usb_role			role;
+	enum usb_role			target_role;
 	bool				is_otg;
 	struct usb_otg			otg;
 	struct otg_fsm			fsm;
@@ -212,6 +213,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 +246,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..965ce17 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -587,6 +587,42 @@ 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)
+		return 0;
+
+	spin_lock_irqsave(&ci->lock, flags);
+	ci->role_switch_event = true;
+	ci->target_role = role;
+	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 +725,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 +947,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 +1127,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 +1142,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 +1192,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..03675b6 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -214,6 +214,26 @@ static void ci_otg_work(struct work_struct *work)
 		ci_handle_vbus_change(ci);
 	}
 
+	if (ci->role_switch_event) {
+		ci->role_switch_event = false;
+
+		/* Stop current role */
+		if (ci->role == USB_ROLE_DEVICE) {
+			usb_gadget_vbus_disconnect(&ci->gadget);
+			ci->role = USB_ROLE_NONE;
+		} else if (ci->role == USB_ROLE_HOST) {
+			ci_role_stop(ci);
+		}
+
+		/* Start target role */
+		if (ci->target_role == USB_ROLE_DEVICE) {
+			usb_gadget_vbus_connect(&ci->gadget);
+			ci->role = USB_ROLE_DEVICE;
+		} else if (ci->target_role == USB_ROLE_HOST) {
+			ci_role_start(ci, USB_ROLE_HOST);
+		}
+	}
+
 	pm_runtime_put_sync(ci->dev);
 
 	enable_irq(ci->irq);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] usb: chipidea: add role switch class support
  2019-08-07  7:23 ` [PATCH v2 2/2] usb: chipidea: add role switch class support jun.li
@ 2019-08-07 18:39   ` kbuild test robot
  2019-08-08  8:22   ` kbuild test robot
  2019-08-08  9:30   ` Peter Chen
  2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2019-08-07 18:39 UTC (permalink / raw)
  To: jun.li; +Cc: kbuild-all, Peter.Chen, gregkh, jun.li, linux-imx, linux-usb

[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc3 next-20190807]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/jun-li-nxp-com/usb-chipidea-replace-ci_role-with-usb_role/20190807-185922
config: arm-imx_v6_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/chipidea/core.o: In function `ci_hdrc_remove':
>> core.c:(.text+0x900): undefined reference to `usb_role_switch_unregister'
   drivers/usb/chipidea/core.o: In function `ci_hdrc_probe':
>> core.c:(.text+0x152c): undefined reference to `usb_role_switch_register'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36386 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] usb: chipidea: add role switch class support
  2019-08-07  7:23 ` [PATCH v2 2/2] usb: chipidea: add role switch class support jun.li
  2019-08-07 18:39   ` kbuild test robot
@ 2019-08-08  8:22   ` kbuild test robot
  2019-08-08  9:30   ` Peter Chen
  2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2019-08-08  8:22 UTC (permalink / raw)
  To: jun.li; +Cc: kbuild-all, Peter.Chen, gregkh, jun.li, linux-imx, linux-usb

[-- Attachment #1: Type: text/plain, Size: 7762 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc3 next-20190807]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/jun-li-nxp-com/usb-chipidea-replace-ci_role-with-usb_role/20190807-185922
config: x86_64-randconfig-c002-201931 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/usb/chipidea/core.o: in function `ci_hdrc_remove':
>> drivers/usb/chipidea/core.c:1196: undefined reference to `usb_role_switch_unregister'
   ld: drivers/usb/chipidea/core.o: in function `ci_hdrc_probe':
>> drivers/usb/chipidea/core.c:1146: undefined reference to `usb_role_switch_register'

vim +1196 drivers/usb/chipidea/core.c

   986	
   987	static int ci_hdrc_probe(struct platform_device *pdev)
   988	{
   989		struct device	*dev = &pdev->dev;
   990		struct ci_hdrc	*ci;
   991		struct resource	*res;
   992		void __iomem	*base;
   993		int		ret;
   994		enum usb_dr_mode dr_mode;
   995	
   996		if (!dev_get_platdata(dev)) {
   997			dev_err(dev, "platform data missing\n");
   998			return -ENODEV;
   999		}
  1000	
  1001		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1002		base = devm_ioremap_resource(dev, res);
  1003		if (IS_ERR(base))
  1004			return PTR_ERR(base);
  1005	
  1006		ci = devm_kzalloc(dev, sizeof(*ci), GFP_KERNEL);
  1007		if (!ci)
  1008			return -ENOMEM;
  1009	
  1010		spin_lock_init(&ci->lock);
  1011		ci->dev = dev;
  1012		ci->platdata = dev_get_platdata(dev);
  1013		ci->imx28_write_fix = !!(ci->platdata->flags &
  1014			CI_HDRC_IMX28_WRITE_FIX);
  1015		ci->supports_runtime_pm = !!(ci->platdata->flags &
  1016			CI_HDRC_SUPPORTS_RUNTIME_PM);
  1017		platform_set_drvdata(pdev, ci);
  1018	
  1019		ret = hw_device_init(ci, base);
  1020		if (ret < 0) {
  1021			dev_err(dev, "can't initialize hardware\n");
  1022			return -ENODEV;
  1023		}
  1024	
  1025		ret = ci_ulpi_init(ci);
  1026		if (ret)
  1027			return ret;
  1028	
  1029		if (ci->platdata->phy) {
  1030			ci->phy = ci->platdata->phy;
  1031		} else if (ci->platdata->usb_phy) {
  1032			ci->usb_phy = ci->platdata->usb_phy;
  1033		} else {
  1034			/* Look for a generic PHY first */
  1035			ci->phy = devm_phy_get(dev->parent, "usb-phy");
  1036	
  1037			if (PTR_ERR(ci->phy) == -EPROBE_DEFER) {
  1038				ret = -EPROBE_DEFER;
  1039				goto ulpi_exit;
  1040			} else if (IS_ERR(ci->phy)) {
  1041				ci->phy = NULL;
  1042			}
  1043	
  1044			/* Look for a legacy USB PHY from device-tree next */
  1045			if (!ci->phy) {
  1046				ci->usb_phy = devm_usb_get_phy_by_phandle(dev->parent,
  1047									  "phys", 0);
  1048	
  1049				if (PTR_ERR(ci->usb_phy) == -EPROBE_DEFER) {
  1050					ret = -EPROBE_DEFER;
  1051					goto ulpi_exit;
  1052				} else if (IS_ERR(ci->usb_phy)) {
  1053					ci->usb_phy = NULL;
  1054				}
  1055			}
  1056	
  1057			/* Look for any registered legacy USB PHY as last resort */
  1058			if (!ci->phy && !ci->usb_phy) {
  1059				ci->usb_phy = devm_usb_get_phy(dev->parent,
  1060							       USB_PHY_TYPE_USB2);
  1061	
  1062				if (PTR_ERR(ci->usb_phy) == -EPROBE_DEFER) {
  1063					ret = -EPROBE_DEFER;
  1064					goto ulpi_exit;
  1065				} else if (IS_ERR(ci->usb_phy)) {
  1066					ci->usb_phy = NULL;
  1067				}
  1068			}
  1069	
  1070			/* No USB PHY was found in the end */
  1071			if (!ci->phy && !ci->usb_phy) {
  1072				ret = -ENXIO;
  1073				goto ulpi_exit;
  1074			}
  1075		}
  1076	
  1077		ret = ci_usb_phy_init(ci);
  1078		if (ret) {
  1079			dev_err(dev, "unable to init phy: %d\n", ret);
  1080			return ret;
  1081		}
  1082	
  1083		ci->hw_bank.phys = res->start;
  1084	
  1085		ci->irq = platform_get_irq(pdev, 0);
  1086		if (ci->irq < 0) {
  1087			dev_err(dev, "missing IRQ\n");
  1088			ret = ci->irq;
  1089			goto deinit_phy;
  1090		}
  1091	
  1092		ci_get_otg_capable(ci);
  1093	
  1094		dr_mode = ci->platdata->dr_mode;
  1095		/* initialize role(s) before the interrupt is requested */
  1096		if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
  1097			ret = ci_hdrc_host_init(ci);
  1098			if (ret) {
  1099				if (ret == -ENXIO)
  1100					dev_info(dev, "doesn't support host\n");
  1101				else
  1102					goto deinit_phy;
  1103			}
  1104		}
  1105	
  1106		if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
  1107			ret = ci_hdrc_gadget_init(ci);
  1108			if (ret) {
  1109				if (ret == -ENXIO)
  1110					dev_info(dev, "doesn't support gadget\n");
  1111				else
  1112					goto deinit_host;
  1113			}
  1114		}
  1115	
  1116		if (!ci->roles[USB_ROLE_HOST] && !ci->roles[USB_ROLE_DEVICE]) {
  1117			dev_err(dev, "no supported roles\n");
  1118			ret = -ENODEV;
  1119			goto deinit_gadget;
  1120		}
  1121	
  1122		if (ci->is_otg && ci->roles[USB_ROLE_DEVICE]) {
  1123			ret = ci_hdrc_otg_init(ci);
  1124			if (ret) {
  1125				dev_err(dev, "init otg fails, ret = %d\n", ret);
  1126				goto deinit_gadget;
  1127			}
  1128		}
  1129	
  1130		if (!ci_role_switch.fwnode) {
  1131			ret = ci_start_initial_role(ci);
  1132			if (ret)
  1133				goto stop;
  1134		}
  1135	
  1136		ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
  1137				ci->platdata->name, ci);
  1138		if (ret)
  1139			goto stop;
  1140	
  1141		ret = ci_extcon_register(ci);
  1142		if (ret)
  1143			goto stop;
  1144	
  1145		if (ci_role_switch.fwnode) {
> 1146			ci->role_switch = usb_role_switch_register(dev,
  1147						&ci_role_switch);
  1148			if (IS_ERR(ci->role_switch)) {
  1149				ret = PTR_ERR(ci->role_switch);
  1150				goto stop;
  1151			}
  1152		}
  1153	
  1154		if (ci->supports_runtime_pm) {
  1155			pm_runtime_set_active(&pdev->dev);
  1156			pm_runtime_enable(&pdev->dev);
  1157			pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
  1158			pm_runtime_mark_last_busy(ci->dev);
  1159			pm_runtime_use_autosuspend(&pdev->dev);
  1160		}
  1161	
  1162		if (ci_otg_is_fsm_mode(ci))
  1163			ci_hdrc_otg_fsm_start(ci);
  1164	
  1165		device_set_wakeup_capable(&pdev->dev, true);
  1166		dbg_create_files(ci);
  1167	
  1168		ret = sysfs_create_group(&dev->kobj, &ci_attr_group);
  1169		if (ret)
  1170			goto remove_debug;
  1171	
  1172		return 0;
  1173	
  1174	remove_debug:
  1175		dbg_remove_files(ci);
  1176	stop:
  1177		if (ci->is_otg && ci->roles[USB_ROLE_DEVICE])
  1178			ci_hdrc_otg_destroy(ci);
  1179	deinit_gadget:
  1180		ci_hdrc_gadget_destroy(ci);
  1181	deinit_host:
  1182		ci_hdrc_host_destroy(ci);
  1183	deinit_phy:
  1184		ci_usb_phy_exit(ci);
  1185	ulpi_exit:
  1186		ci_ulpi_exit(ci);
  1187	
  1188		return ret;
  1189	}
  1190	
  1191	static int ci_hdrc_remove(struct platform_device *pdev)
  1192	{
  1193		struct ci_hdrc *ci = platform_get_drvdata(pdev);
  1194	
  1195		if (ci->role_switch)
> 1196			usb_role_switch_unregister(ci->role_switch);
  1197	
  1198		if (ci->supports_runtime_pm) {
  1199			pm_runtime_get_sync(&pdev->dev);
  1200			pm_runtime_disable(&pdev->dev);
  1201			pm_runtime_put_noidle(&pdev->dev);
  1202		}
  1203	
  1204		dbg_remove_files(ci);
  1205		sysfs_remove_group(&ci->dev->kobj, &ci_attr_group);
  1206		ci_role_destroy(ci);
  1207		ci_hdrc_enter_lpm(ci, true);
  1208		ci_usb_phy_exit(ci);
  1209		ci_ulpi_exit(ci);
  1210	
  1211		return 0;
  1212	}
  1213	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36907 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread
* RE: [PATCH v2 2/2] usb: chipidea: add role switch class support
  2019-08-07  7:23 ` [PATCH v2 2/2] usb: chipidea: add role switch class support jun.li
  2019-08-07 18:39   ` kbuild test robot
  2019-08-08  8:22   ` kbuild test robot
@ 2019-08-08  9:30   ` Peter Chen
  2019-08-09  7:29     ` Jun Li
  2 siblings, 1 reply; 7+ messages in thread
From: Peter Chen @ 2019-08-08  9:30 UTC (permalink / raw)
  To: Jun Li; +Cc: gregkh, Jun Li, dl-linux-imx, linux-usb

 
> USB role is fully controlled by usb role switch consumer(e.g. typec), usb port can be
> at host mode(USB_ROLE_HOST), device mode connected to
> host(USB_ROLE_DEVICE), or not connecting any parter(USB_ROLE_NONE).
> 

%s/parter/partner ?

Are there any ways you could get external cable status from usb-switch or type-c like external
connector? If there are, you could update otgsc value for otgsc_read, or change cable status,
and avoid changing common handling, like ci_hdrc_probe and  ci_otg_work. And it could
benefit for other use cases, like booting with cable connected and switch role through /sys.

Peter

> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
> 
> Change for v2:
> - Support USB_ROLE_NONE, which for usb port not connecting any
>   device or host, and will be in low power mode.
> 
>  drivers/usb/chipidea/ci.h   |   3 ++
>  drivers/usb/chipidea/core.c | 120 +++++++++++++++++++++++++++++++++-------
> ----
>  drivers/usb/chipidea/otg.c  |  20 ++++++++
>  3 files changed, 114 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index
> 82b86cd..f0aec1d 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -205,6 +205,7 @@ struct ci_hdrc {
>  	int				irq;
>  	struct ci_role_driver		*roles[USB_ROLE_DEVICE + 1];
>  	enum usb_role			role;
> +	enum usb_role			target_role;
>  	bool				is_otg;
>  	struct usb_otg			otg;
>  	struct otg_fsm			fsm;
> @@ -212,6 +213,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 +246,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..965ce17 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -587,6 +587,42 @@ 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)
> +		return 0;
> +
> +	spin_lock_irqsave(&ci->lock, flags);
> +	ci->role_switch_event = true;
> +	ci->target_role = role;
> +	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 +725,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 +947,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 +1127,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
> +1142,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 +1192,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..03675b6 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -214,6 +214,26 @@ static void ci_otg_work(struct work_struct *work)
>  		ci_handle_vbus_change(ci);
>  	}
> 
> +	if (ci->role_switch_event) {
> +		ci->role_switch_event = false;
> +
> +		/* Stop current role */
> +		if (ci->role == USB_ROLE_DEVICE) {
> +			usb_gadget_vbus_disconnect(&ci->gadget);
> +			ci->role = USB_ROLE_NONE;
> +		} else if (ci->role == USB_ROLE_HOST) {
> +			ci_role_stop(ci);
> +		}
> +
> +		/* Start target role */
> +		if (ci->target_role == USB_ROLE_DEVICE) {
> +			usb_gadget_vbus_connect(&ci->gadget);
> +			ci->role = USB_ROLE_DEVICE;
> +		} else if (ci->target_role == USB_ROLE_HOST) {
> +			ci_role_start(ci, USB_ROLE_HOST);
> +		}
> +	}
> +
>  	pm_runtime_put_sync(ci->dev);
> 
>  	enable_irq(ci->irq);
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread
* RE: [PATCH v2 2/2] usb: chipidea: add role switch class support
  2019-08-08  9:30   ` Peter Chen
@ 2019-08-09  7:29     ` Jun Li
  2019-08-09 10:11       ` Peter Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Jun Li @ 2019-08-09  7:29 UTC (permalink / raw)
  To: Peter Chen; +Cc: gregkh, dl-linux-imx, linux-usb


> -----Original Message-----
> From: Peter Chen
> Sent: 2019年8月8日 17:31
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; Jun Li <jun.li@nxp.com>; dl-linux-imx
> <linux-imx@nxp.com>; linux-usb@vger.kernel.org
> Subject: RE: [PATCH v2 2/2] usb: chipidea: add role switch class support
> 
> 
> > USB role is fully controlled by usb role switch consumer(e.g. typec),
> > usb port can be at host mode(USB_ROLE_HOST), device mode connected to
> > host(USB_ROLE_DEVICE), or not connecting any parter(USB_ROLE_NONE).
> >
> 
> %s/parter/partner ?

Yes, I will update.

> 
> Are there any ways you could get external cable status from usb-switch or type-c like
> external connector? If there are, you could update otgsc value for otgsc_read, or change
> cable status, and avoid changing common handling, like ci_hdrc_probe and  ci_otg_work.
> And it could benefit for other use cases, like booting with cable connected and switch role
> through /sys.

The new role switch class has nothing to do with extcon, it's using graph bindings
to link the connection, so there is no extcon_dev, change for ci_hdrc_probe() is
required as property usb-role-switch has to be specified, there may be some code
reuse if still touch the external cable even no extcon dev, but that will make existing
external cable complicated and not clean.

On the other hand, a new GPIO based role switch driver is on review:
https://patchwork.kernel.org/patch/11056379/
Seems this is the direction to move out from extcon.

Li Jun
> 
> Peter
> 
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >
> > Change for v2:
> > - Support USB_ROLE_NONE, which for usb port not connecting any
> >   device or host, and will be in low power mode.
> >
> >  drivers/usb/chipidea/ci.h   |   3 ++
> >  drivers/usb/chipidea/core.c | 120
> > +++++++++++++++++++++++++++++++++-------
> > ----
> >  drivers/usb/chipidea/otg.c  |  20 ++++++++
> >  3 files changed, 114 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index 82b86cd..f0aec1d 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -205,6 +205,7 @@ struct ci_hdrc {
> >  	int				irq;
> >  	struct ci_role_driver		*roles[USB_ROLE_DEVICE + 1];
> >  	enum usb_role			role;
> > +	enum usb_role			target_role;
> >  	bool				is_otg;
> >  	struct usb_otg			otg;
> >  	struct otg_fsm			fsm;
> > @@ -212,6 +213,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 +246,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..965ce17 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -587,6 +587,42 @@ 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)
> > +		return 0;
> > +
> > +	spin_lock_irqsave(&ci->lock, flags);
> > +	ci->role_switch_event = true;
> > +	ci->target_role = role;
> > +	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 +725,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
> > +947,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 +1127,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
> > +1142,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 +1192,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..03675b6 100644
> > --- a/drivers/usb/chipidea/otg.c
> > +++ b/drivers/usb/chipidea/otg.c
> > @@ -214,6 +214,26 @@ static void ci_otg_work(struct work_struct *work)
> >  		ci_handle_vbus_change(ci);
> >  	}
> >
> > +	if (ci->role_switch_event) {
> > +		ci->role_switch_event = false;
> > +
> > +		/* Stop current role */
> > +		if (ci->role == USB_ROLE_DEVICE) {
> > +			usb_gadget_vbus_disconnect(&ci->gadget);
> > +			ci->role = USB_ROLE_NONE;
> > +		} else if (ci->role == USB_ROLE_HOST) {
> > +			ci_role_stop(ci);
> > +		}
> > +
> > +		/* Start target role */
> > +		if (ci->target_role == USB_ROLE_DEVICE) {
> > +			usb_gadget_vbus_connect(&ci->gadget);
> > +			ci->role = USB_ROLE_DEVICE;
> > +		} else if (ci->target_role == USB_ROLE_HOST) {
> > +			ci_role_start(ci, USB_ROLE_HOST);
> > +		}
> > +	}
> > +
> >  	pm_runtime_put_sync(ci->dev);
> >
> >  	enable_irq(ci->irq);
> > --
> > 2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread
* RE: [PATCH v2 2/2] usb: chipidea: add role switch class support
  2019-08-09  7:29     ` Jun Li
@ 2019-08-09 10:11       ` Peter Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Chen @ 2019-08-09 10:11 UTC (permalink / raw)
  To: Jun Li; +Cc: gregkh, dl-linux-imx, linux-usb

 
> > > USB role is fully controlled by usb role switch consumer(e.g.
> > > typec), usb port can be at host mode(USB_ROLE_HOST), device mode
> > > connected to host(USB_ROLE_DEVICE), or not connecting any
> parter(USB_ROLE_NONE).
> > >
> >
> > %s/parter/partner ?
> 
> Yes, I will update.
> 
> >
> > Are there any ways you could get external cable status from usb-switch
> > or type-c like external connector? If there are, you could update
> > otgsc value for otgsc_read, or change cable status, and avoid changing common
> handling, like ci_hdrc_probe and  ci_otg_work.
> > And it could benefit for other use cases, like booting with cable
> > connected and switch role through /sys.
> 
> The new role switch class has nothing to do with extcon, it's using graph bindings to
> link the connection, so there is no extcon_dev, change for ci_hdrc_probe() is
> required as property usb-role-switch has to be specified, there may be some code
> reuse if still touch the external cable even no extcon dev, but that will make existing
> external cable complicated and not clean.
> 

I don't mean using extcon directly.

At ci_usb_role_switch_set, you could know current "id" and "vbus" status.
as well as if "id" and "vbus" has changed. So, you could update ci->id_event
or ci->vbus_event. And at otgsc_read, you could return correct vbus and id value.

So, you may not need to change main function for ci_otg_work, for ci_hdrc_probe,
you could only need to register usb-switch class.

Surely, you may need to introduce some structures or variables for usb switch, but
it could keep main structure not changing, and just differentiate the way to get
id and vbus.


> On the other hand, a new GPIO based role switch driver is on review:
> https://patchwork.kernel.org/patch/11056379/
> Seems this is the direction to move out from extcon.
> 

If external connector is given up, we need to use that.

When you update the v3, please fix the build error reported by kbuild test robot.

Peter	

> Li Jun
> >
> > Peter
> >
> > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > ---
> > >
> > > Change for v2:
> > > - Support USB_ROLE_NONE, which for usb port not connecting any
> > >   device or host, and will be in low power mode.
> > >
> > >  drivers/usb/chipidea/ci.h   |   3 ++
> > >  drivers/usb/chipidea/core.c | 120
> > > +++++++++++++++++++++++++++++++++-------
> > > ----
> > >  drivers/usb/chipidea/otg.c  |  20 ++++++++
> > >  3 files changed, 114 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > index 82b86cd..f0aec1d 100644
> > > --- a/drivers/usb/chipidea/ci.h
> > > +++ b/drivers/usb/chipidea/ci.h
> > > @@ -205,6 +205,7 @@ struct ci_hdrc {
> > >  	int				irq;
> > >  	struct ci_role_driver		*roles[USB_ROLE_DEVICE + 1];
> > >  	enum usb_role			role;
> > > +	enum usb_role			target_role;
> > >  	bool				is_otg;
> > >  	struct usb_otg			otg;
> > >  	struct otg_fsm			fsm;
> > > @@ -212,6 +213,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 +246,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..965ce17 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -587,6 +587,42 @@ 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)
> > > +		return 0;
> > > +
> > > +	spin_lock_irqsave(&ci->lock, flags);
> > > +	ci->role_switch_event = true;
> > > +	ci->target_role = role;
> > > +	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 +725,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
> > > +947,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 +1127,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
> > > +1142,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 +1192,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..03675b6 100644
> > > --- a/drivers/usb/chipidea/otg.c
> > > +++ b/drivers/usb/chipidea/otg.c
> > > @@ -214,6 +214,26 @@ static void ci_otg_work(struct work_struct *work)
> > >  		ci_handle_vbus_change(ci);
> > >  	}
> > >
> > > +	if (ci->role_switch_event) {
> > > +		ci->role_switch_event = false;
> > > +
> > > +		/* Stop current role */
> > > +		if (ci->role == USB_ROLE_DEVICE) {
> > > +			usb_gadget_vbus_disconnect(&ci->gadget);
> > > +			ci->role = USB_ROLE_NONE;
> > > +		} else if (ci->role == USB_ROLE_HOST) {
> > > +			ci_role_stop(ci);
> > > +		}
> > > +
> > > +		/* Start target role */
> > > +		if (ci->target_role == USB_ROLE_DEVICE) {
> > > +			usb_gadget_vbus_connect(&ci->gadget);
> > > +			ci->role = USB_ROLE_DEVICE;
> > > +		} else if (ci->target_role == USB_ROLE_HOST) {
> > > +			ci_role_start(ci, USB_ROLE_HOST);
> > > +		}
> > > +	}
> > > +
> > >  	pm_runtime_put_sync(ci->dev);
> > >
> > >  	enable_irq(ci->irq);
> > > --
> > > 2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-09 10:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  7:23 [PATCH v2 1/2] usb: chipidea: replace ci_role with usb_role jun.li
2019-08-07  7:23 ` [PATCH v2 2/2] usb: chipidea: add role switch class support jun.li
2019-08-07 18:39   ` kbuild test robot
2019-08-08  8:22   ` kbuild test robot
2019-08-08  9:30   ` Peter Chen
2019-08-09  7:29     ` Jun Li
2019-08-09 10:11       ` Peter Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).