usb: add support for Intel dual role port mux

Add support to retrieve fixed voltage configure information through ACPI interface. This is needed for Intel Bay Trail devices, where a GPIO is used to control the USB vbus.

Signed-off-by Lu Baolu <[email protected]>

---
 drivers/regulator/fixed.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

Quoted 3 lines · Lu Baolu

Add support to retrieve fixed voltage configure information through ACPI interface. This is needed for Intel Bay Trail devices, where a GPIO is used to control the USB vbus.

Can't do anything with this until I get an ack from the "owners" of this file.

Quoted 5 lines · Greg Kroah-Hartman

Quoted 3 lines · Lu Baolu

Add support to retrieve fixed voltage configure information through ACPI interface. This is needed for Intel Bay Trail devices, where a GPIO is used to control the USB vbus.

Quoted 2 lines

Can't do anything with this until I get an ack from the "owners" of this file.

Please allow a reasonable time for review, it's been under a week since this was posted. I would *not* expect this to be applied to another tree, please don't do that.

Quoted 8 lines · Lu Baolu

Add support to retrieve fixed voltage configure information through ACPI interface. This is needed for Intel Bay Trail devices, where a GPIO is used to control the USB vbus.

Signed-off-by Lu Baolu <[email protected]>

---
 drivers/regulator/fixed.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

Can't do anything with this until I get an ack from the "owners" of this file.

And what happened to the acks from other Intel developers for this whole patch series, I don't see that here :(

Several Intel platforms implement USB dual role by having completely separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share a single USB port. There is another external port mux which controls where the data lines should go. While the USB controllers are part of the silicon, the port mux design are platform specific.

This patch adds the generic code to handle such multiple roles of a usb port. It exports the necessary interfaces for other components to register or unregister a usb mux device, and to control its role. It registers the mux device with sysfs as well, so that users are able to control the port role from user space.

Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to swap usb roles as well. This code could also be leveraged for those archs.

Signed-off-by Lu Baolu <[email protected]>

Reviewed-by Heikki Krogerus <[email protected]>

Reviewed-by Felipe Balbi <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-platform |  18 +++
 drivers/usb/Kconfig                          |   2 +
 drivers/usb/Makefile                         |   1 +
 drivers/usb/mux/Kconfig                      |   8 ++
 drivers/usb/mux/Makefile                     |   4 +
 drivers/usb/mux/portmux-core.c               | 202 +++++++++++++++++++++++++++
 include/linux/usb/portmux.h                  |  90 ++++++++++++
 7 files changed, 325 insertions(+)
 create mode 100644 drivers/usb/mux/Kconfig
 create mode 100644 drivers/usb/mux/Makefile
 create mode 100644 drivers/usb/mux/portmux-core.c
 create mode 100644 include/linux/usb/portmux.h

Documentation/ABI/testing/sysfs-bus-platform +18 -0

index 5172a61..b994e4e 100644
@@ -18,3 +18,21 @@ Description:
 		devices to opt-out of driver binding using a driver_override
 		name such as "none".  Only a single driver may be specified in
 		the override, there is no support for parsing delimiters.
+
+What:		/sys/bus/platform/devices/.../portmux.N/name
+		/sys/bus/platform/devices/.../portmux.N/state
+Date:		April 2016
+Contact:	Lu Baolu <[email protected]>
+Description:
+		In some platforms, a single USB port is shared between a USB host
+		controller and a device controller. A USB mux driver is needed to
+		handle the port mux. Read-only attribute "name" shows the name of
+		the port mux device. "state" attribute shows and stores the mux
+		state.
+		For read:
+		'unknown'    - the mux hasn't been set yet;
+		'peripheral' - mux has been switched to PERIPHERAL controller;
+		'host'       - mux has been switched to HOST controller.
+		For write:
+		'peripheral' - mux will be switched to PERIPHERAL controller;
+		'host'       - mux will be switched to HOST controller.

drivers/usb/Kconfig +2 -0

index 8689dcb..328916e 100644
@@ -148,6 +148,8 @@ endif # USB
 
 source "drivers/usb/phy/Kconfig"
 
+source "drivers/usb/mux/Kconfig"
+
 source "drivers/usb/gadget/Kconfig"
 
 config USB_LED_TRIG

drivers/usb/Makefile +1 -0

index dca7856..9a92338 100644
@@ -6,6 +6,7 @@
 
 obj-$(CONFIG_USB)		+= core/
 obj-$(CONFIG_USB_SUPPORT)	+= phy/
+obj-$(CONFIG_USB_SUPPORT)	+= mux/
 
 obj-$(CONFIG_USB_DWC3)		+= dwc3/
 obj-$(CONFIG_USB_DWC2)		+= dwc2/

drivers/usb/mux/Kconfig new file +8 -0

new file mode 100644
index 0000000..ba778f2
@@ -0,0 +1,8 @@
+#
+# USB port mux driver configuration
+#
+
+menuconfig USB_PORTMUX
+	bool "USB dual role port MUX support"
+	help
+	  Generic USB dual role port mux support.

drivers/usb/mux/Makefile new file +4 -0

new file mode 100644
index 0000000..f85df92
@@ -0,0 +1,4 @@
+#
+# Makefile for USB port mux drivers
+#
+obj-$(CONFIG_USB_PORTMUX)		+= portmux-core.o

drivers/usb/mux/portmux-core.c new file +202 -0

new file mode 100644
index 0000000..75fbb45
@@ -0,0 +1,202 @@
+/**
+ * portmux-core.c - USB Port Mux support
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/usb/portmux.h>
+
+static int usb_mux_change_state(struct portmux_dev *pdev,
+				enum portmux_role role)
+{
+	struct device *dev = &pdev->dev;
+	int ret = -EINVAL;
+
+	dev_WARN_ONCE(dev,
+		      !mutex_is_locked(&pdev->mux_mutex),
+		      "mutex is unlocked\n");
+
+	switch (role) {
+	case PORTMUX_HOST:
+		if (pdev->desc->ops->set_host_cb)
+			ret = pdev->desc->ops->set_host_cb(pdev->dev.parent);
+		break;
+	case PORTMUX_DEVICE:
+		if (pdev->desc->ops->set_device_cb)
+			ret = pdev->desc->ops->set_device_cb(pdev->dev.parent);
+		break;
+	default:
+		break;
+	}
+
+	if (!ret)
+		pdev->mux_state = role;
+
+	return ret;
+}
+
+static const char * const role_name[] = {
+	"unknown",	/* PORTMUX_UNKNOWN */
+	"host",		/* PORTMUX_HOST */
+	"peripheral"	/* PORTMUX_DEVICE */
+};
+
+static ssize_t state_show(struct device *dev,
+			  struct device_attribute *attr,
+			  char *buf)
+{
+	struct portmux_dev *pdev = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", role_name[pdev->mux_state]);
+}
+
+static ssize_t state_store(struct device *dev,
+			   struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct portmux_dev *pdev = dev_get_drvdata(dev);
+	enum portmux_role role;
+
+	if (sysfs_streq(buf, "peripheral"))
+		role = PORTMUX_DEVICE;
+	else if (sysfs_streq(buf, "host"))
+		role = PORTMUX_HOST;
+	else
+		return -EINVAL;
+
+	mutex_lock(&pdev->mux_mutex);
+	usb_mux_change_state(pdev, role);
+	mutex_unlock(&pdev->mux_mutex);
+
+	return count;
+}
+static DEVICE_ATTR_RW(state);
+
+static ssize_t name_show(struct device *dev,
+			 struct device_attribute *attr,
+			 char *buf)
+{
+	struct portmux_dev *pdev = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", pdev->desc->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static struct attribute *portmux_attrs[] = {
+	&dev_attr_state.attr,
+	&dev_attr_name.attr,
+	NULL,
+};
+
+static struct attribute_group portmux_attr_grp = {
+	.attrs = portmux_attrs,
+};
+
+static const struct attribute_group *portmux_group[] = {
+	&portmux_attr_grp,
+	NULL,
+};
+
+static void portmux_dev_release(struct device *dev)
+{
+	dev_vdbg(dev, "%s\n", __func__);
+}
+
+/**
+ * portmux_register - register a port mux
+ * @dev: device the mux belongs to
+ * @desc: the descriptor of this port mux
+ *
+ * Called by port mux drivers to register a mux. Returns a valid
+ * pointer to struct portmux_dev on success or an ERR_PTR() on
+ * error.
+ */
+struct portmux_dev *portmux_register(struct portmux_desc *desc)
+{
+	static atomic_t portmux_no = ATOMIC_INIT(-1);
+	struct portmux_dev *pdev;
+	int ret;
+
+	/* parameter sanity check */
+	if (!desc || !desc->name || !desc->ops || !desc->dev)
+		return ERR_PTR(-EINVAL);
+
+	pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
+	if (!pdev)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_init(&pdev->mux_mutex);
+	pdev->desc = desc;
+	pdev->dev.parent = desc->dev;
+	pdev->dev.release = portmux_dev_release;
+	dev_set_name(&pdev->dev, "portmux.%lu",
+		     (unsigned long)atomic_inc_return(&portmux_no));
+	pdev->dev.groups = portmux_group;
+	ret = device_register(&pdev->dev);
+	if (ret) {
+		kfree(pdev);
+		return ERR_PTR(ret);
+	}
+
+	dev_set_drvdata(&pdev->dev, pdev);
+
+	return pdev;
+}
+EXPORT_SYMBOL_GPL(portmux_register);
+
+/**
+ * portmux_unregister - unregister a port mux
+ * @pdev: the port mux device
+ *
+ * Called by port mux drivers to release a mux.
+ */
+void portmux_unregister(struct portmux_dev *pdev)
+{
+	device_unregister(&pdev->dev);
+	kfree(pdev);
+}
+EXPORT_SYMBOL_GPL(portmux_unregister);
+
+/**
+ * portmux_switch - switch the port role
+ * @pdev: the port mux device
+ * @role: the target role
+ *
+ * Called by other components to switch the port role.
+ */
+int portmux_switch(struct portmux_dev *pdev, enum portmux_role role)
+{
+	int ret;
+
+	mutex_lock(&pdev->mux_mutex);
+	ret = usb_mux_change_state(pdev, role);
+	mutex_unlock(&pdev->mux_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(portmux_switch);
+
+#ifdef CONFIG_PM_SLEEP
+/**
+ * portmux_complete - refresh port state during system resumes back
+ * @pdev: the port mux device
+ *
+ * Called by port mux drivers to refresh port state during system
+ * resumes back.
+ */
+void portmux_complete(struct portmux_dev *pdev)
+{
+	mutex_lock(&pdev->mux_mutex);
+	usb_mux_change_state(pdev, pdev->mux_state);
+	mutex_unlock(&pdev->mux_mutex);
+}
+EXPORT_SYMBOL_GPL(portmux_complete);
+#endif

include/linux/usb/portmux.h new file +90 -0

new file mode 100644
index 0000000..093620a
@@ -0,0 +1,90 @@
+/**
+ * portmux.h - USB Port Mux definitions
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX_USB_PORTMUX_H
+#define __LINUX_USB_PORTMUX_H
+
+#include <linux/device.h>
+
+/**
+ * struct portmux_ops - ops two switch the port
+ *
+ * @set_host_cb: callback for switching port to host
+ * @set_device_cb: callback for switching port to device
+ */
+struct portmux_ops {
+	int (*set_host_cb)(struct device *dev);
+	int (*set_device_cb)(struct device *dev);
+};
+
+/**
+ * struct portmux_desc - port mux device descriptor
+ *
+ * @name: the name of the mux device
+ * @dev: the parent of the mux device
+ * @ops: ops to switch the port role
+ */
+struct portmux_desc {
+	const char *name;
+	struct device *dev;
+	const struct portmux_ops *ops;
+};
+
+/**
+ * enum portmux_role - role of the port
+ */
+enum portmux_role {
+	PORTMUX_UNKNOWN,
+	PORTMUX_HOST,
+	PORTMUX_DEVICE,
+};
+
+/**
+ * struct portmux_dev - A mux device
+ *
+ * @desc: the descriptor of the mux
+ * @dev: device of this mux
+ * @mux_mutex: lock to serialize port switch operation
+ * @mux_state: state of the mux
+ */
+struct portmux_dev {
+	const struct portmux_desc *desc;
+	struct device dev;
+
+	/* lock for mux_state */
+	struct mutex mux_mutex;
+	enum portmux_role mux_state;
+};
+
+/*
+ * Functions for mux driver
+ */
+struct portmux_dev *portmux_register(struct portmux_desc *desc);
+void portmux_unregister(struct portmux_dev *pdev);
+#ifdef CONFIG_PM_SLEEP
+void portmux_complete(struct portmux_dev *pdev);
+#endif
+
+/*
+ * Functions for mux consumer
+ */
+#if defined(CONFIG_USB_PORTMUX)
+int portmux_switch(struct portmux_dev *pdev, enum portmux_role role);
+#else
+static inline int portmux_switch(struct portmux_dev *pdev,
+				 enum portmux_role role)
+{
+	return 0;
+}
+#endif
+
+#endif /* __LINUX_USB_PORTMUX_H */

Signature · 6 lines

2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 6 lines · Lu Baolu

Several Intel platforms implement USB dual role by having completely separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share a single USB port. There is another external port mux which controls where the data lines should go. While the USB controllers are part of the silicon, the port mux design are platform specific.

Quoted 9 lines

This patch adds the generic code to handle such multiple roles of a usb port. It exports the necessary interfaces for other components to register or unregister a usb mux device, and to control its role. It registers the mux device with sysfs as well, so that users are able to control the port role from user space.

Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to swap usb roles as well. This code could also be leveraged for those archs.

Sorry to review this so late, from my point, it is a dual-role switch driver too, we are reviewing USB OTG/dual-role framework [1], it is not necessary to create another framework to do it. And USB OTG framework has already tested at Renesas's platform [2].

[1] http://www.spinics.net/lists/linux-usb/msg140835.html
[2] http://www.spinics.net/lists/linux-usb/msg140827.html

Quoted 17 lines · Peter Chen

Quoted 6 lines · Lu Baolu

Several Intel platforms implement USB dual role by having completely separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share a single USB port. There is another external port mux which controls where the data lines should go. While the USB controllers are part of the silicon, the port mux design are platform specific.

Quoted 9 lines

This patch adds the generic code to handle such multiple roles of a usb port. It exports the necessary interfaces for other components to register or unregister a usb mux device, and to control its role. It registers the mux device with sysfs as well, so that users are able to control the port role from user space.

Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to swap usb roles as well. This code could also be leveraged for those archs.

Quoted 7 lines

Sorry to review this so late, from my point, it is a dual-role switch driver too, we are reviewing USB OTG/dual-role framework [1], it is not necessary to create another framework to do it. And USB OTG framework has already tested at Renesas's platform [2].

[1] http://www.spinics.net/lists/linux-usb/msg140835.html
[2] http://www.spinics.net/lists/linux-usb/msg140827.html

We really can't marry dual-role capability with OTG. That OTG framework can be used be when the hardware actually supports the protocols defined in the OTG spec starting from SRP. In other cases it must not be used.

OTG relies heavily on existence of the ID pin, but with Type-C connectors we do not have it. Therefore USB Type-C defines competing support for example for the role swapping. With USB Type-C connectors OTG will never be supported.

So let's not mix USB dual-role capability with OTG.

Quoted 12 lines · Lu Baolu

Quoted 12 lines

Several Intel platforms implement USB dual role by having completely separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share a single USB port. There is another external port mux which controls where the data lines should go. While the USB controllers are part of the silicon, the port mux design are platform specific.

This patch adds the generic code to handle such multiple roles of a usb port. It exports the necessary interfaces for other components to register or unregister a usb mux device, and to control its role. It registers the mux device with sysfs as well, so that users are able to control the port role from user space.

Quoted 3 lines

Quoted 3 lines

Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to swap usb roles as well. This code could also be leveraged for those archs.

Sorry to review this so late,

from my point,it is a dual-role switch
driver too,

we are reviewing USB OTG/dual-role framework [1], it is
not necessary to create another framework to do it. And USB OTG framework has already tested at Renesas's platform [2].

[1] http://www.spinics.net/lists/linux-usb/msg140835.html
[2] http://www.spinics.net/lists/linux-usb/msg140827.html

Peter

Hi Peter,

Quoted 13 lines · Peter Chen

Quoted 12 lines · Lu Baolu

Quoted 12 lines

Several Intel platforms implement USB dual role by having completely separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share a single USB port. There is another external port mux which controls where the data lines should go. While the USB controllers are part of the silicon, the port mux design are platform specific.

This patch adds the generic code to handle such multiple roles of a usb port. It exports the necessary interfaces for other components to register or unregister a usb mux device, and to control its role. It registers the mux device with sysfs as well, so that users are able to control the port role from user space.

Quoted 4 lines

Quoted 3 lines

Quoted 3 lines

Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to swap usb roles as well. This code could also be leveraged for those archs.

Sorry to review this so late,

It doesn't matter. Thanks for review. Comments are always welcome.:-)

Quoted 2 lines

from my point,it is a dual-role switch
driver too,

No, it's not a dual-role switch driver, but a driver for USB port multiplexing.

One example of port multiplexing can be found in several Intel SOC and PCH chips, inside of which, there are two independent USB controllers: host and device. They might share a single port and this port could be configured to route the line to one of these two controllers. This patch introduced a generic framework for port mux drivers. It aids the drivers to handle port mux by providing interfaces to 1) register/unregister a mux device; 2) lookup the mux device; and 3) switch the port.

Port multiplexing isn't equal to USB dual role. There are other cases in today's systems. In several Intel PCH chips, there equips two USB host controllers: ehci and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all USB ports work even running an old version of OS which lacks of USB3 support. In theory, we can create a driver for the port mux and switch the ports between xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)

Another case is xHCI debug capability. The xHCI host controller might equip a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit is independent of xhci host controller. But it shares its port with xhci. Software could switch the port between xhci and the debugging unit through the registers defined in xHCI spec.

Quoted 4 lines · Lu Baolu

Quoted 2 lines

from my point,it is a dual-role switch
driver too,

No, it's not a dual-role switch driver, but a driver for USB port multiplexing.

Quoted 8 lines

One example of port multiplexing can be found in several Intel SOC and PCH chips, inside of which, there are two independent USB controllers: host and device. They might share a single port and this port could be configured to route the line to one of these two controllers. This patch introduced a generic framework for port mux drivers. It aids the drivers to handle port mux by providing interfaces to 1) register/unregister a mux device; 2) lookup the mux device; and 3) switch the port.

For this case, I can't see it is different with dual-role switch.

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

Quoted 7 lines

Port multiplexing isn't equal to USB dual role. There are other cases in today's systems. In several Intel PCH chips, there equips two USB host controllers: ehci and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all USB ports work even running an old version of OS which lacks of USB3 support. In theory, we can create a driver for the port mux and switch the ports between xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)

Quoted 6 lines

Another case is xHCI debug capability. The xHCI host controller might equip a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit is independent of xhci host controller. But it shares its port with xhci. Software could switch the port between xhci and the debugging unit through the registers defined in xHCI spec.

Yes, above two are different with dual role switch. But in your code and Kconfig, it seems this framework is dedicated for dual-role. Eg:

I think a general dual role port mux is necessary, it can be used to manage different dual-role switch method, eg

- ID pin
- External connector through GPIO
- SoC register
- sysfs
- type-C events

But this code is better co-work with OTG/Dual-role framework, we'd better have only interface that the user can know which role for the current port.

-----Original Message-----
From: [email protected] [mailto:linux-usb- [email protected]] On Behalf Of Lu Baolu
Sent: Sunday, June 05, 2016 2:56 PM
To: Peter Chen <[email protected]>
Cc: [email protected]; Mathias Nyman <[email protected]>; Greg Kroah-Hartman <[email protected]>; Lee Jones <[email protected]>; Heikki Krogerus <[email protected]>; Liam Girdwood <[email protected]>; Mark Brown <[email protected]>; [email protected]; [email protected]; Roger Quadros <[email protected]>
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Hi Peter,

Quoted 3 lines · Peter Chen

Quoted 2 lines · Lu Baolu

Quoted 1 line

from my point,it is a dual-role switch driver too,

No, it's not a dual-role switch driver, but a driver for USB port

multiplexing.

Quoted 8 lines

Quoted 8 lines

One example of port multiplexing can be found in several Intel SOC and PCH chips, inside of which, there are two independent USB
controllers: host and device. They might share a single port and this port could be configured to route the line to one of these two controllers. This patch introduced a generic framework for port mux drivers. It aids the drivers to handle port mux by providing interfaces to 1) register/unregister a mux device; 2) lookup the mux

device; and 3) switch the port.

Quoted 2 lines

Quoted 1 line

For this case, I can't see it is different with dual-role switch.

Port mux is part of dual role switch, but not the whole thing.

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 3 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

Quoted 5 lines

Quoted 5 lines

Port multiplexing isn't equal to USB dual role. There are other cases in today's systems. In several Intel PCH chips, there equips two USB host controllers: ehci and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all USB ports work even running an old

version of OS which lacks of USB3 support.

Quoted 13 lines

Quoted 10 lines

In theory, we can create a driver for the port mux and switch the ports between xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)

Another case is xHCI debug capability. The xHCI host controller might equip a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit is independent of xhci host controller. But it shares its port with xhci. Software could switch the port between xhci and the debugging unit through the registers defined in xHCI spec.

Yes, above two are different with dual role switch. But in your code and Kconfig, it seems this framework is dedicated for dual-role. Eg:

drivers/usb/mux/Kconfig line 8

Quoted 4 lines

drivers/usb/mux/Kconfig new file +4 -0

+menuconfig USB_PORTMUX
+       bool "USB dual role port MUX support"
+       help
+         Generic USB dual role port mux support.

Above two cases are examples for port multiplexing, but I don't think we need drivers for them. At this moment, this framework is only for dual role port mux devices.

Quoted 3 lines

I think a general dual role port mux is necessary, it can be used to manage different dual-role switch method, eg

Yes, I agree.

Quoted 5 lines

- ID pin
- External connector through GPIO
- SoC register
- sysfs
- type-C events

ID pin and type-C events are the *reasons* which trigger the port mux switch. Currently, on our platforms, gpio, registers and sysfs are methods to control the mux.

Quoted 5 lines

But this code is better co-work with OTG/Dual-role framework, we'd better have only interface that the user can know which role for the current port.

OTG/Dual-role framework and portmux framework are not overlapped.

From previous discussion, I still think this is a special dual role

The sysfs interface shouldn't be overlapped as well. Say, I have a port mux device and I have a driver for it. I am able to read the status of my port mux device through sysfs. This is not part of OTG/Dual-role as far as I can see.

Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

Quoted 16 lines

Port mux is part of dual role switch, but not the whole thing.

Quoted 15 lines

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 3 lines

Quoted 3 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

Quoted 4 lines

Quoted 4 lines

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

I also think current OTG/Dual role framework can support your case, if you find there is any limitation of it which can't meet your requirement, we should improve it, Roger also provide an example of dual role switch with USB3 based on his OTG core.

-----Original Message-----
From: Lu Baolu [mailto:[email protected]]
Sent: Sunday, June 05, 2016 4:47 PM
To: Jun Li <[email protected]>; Peter Chen <[email protected]>
Cc: [email protected]; Mathias Nyman <[email protected]>; Greg Kroah-Hartman <[email protected]>; Lee Jones <[email protected]>; Heikki Krogerus <[email protected]>; Liam Girdwood <[email protected]>; Mark Brown <[email protected]>; [email protected]; [email protected]; Roger Quadros <[email protected]>
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Hi,

Quoted 20 lines · Jun Li

Quoted 20 lines

Port mux is part of dual role switch, but not the whole thing.

Quoted 19 lines

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 3 lines

Quoted 3 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

Quoted 5 lines

I also think current OTG/Dual role framework can support your case, if you find there is any limitation of it which can't meet your requirement, we should improve it, Roger also provide an example of dual role switch with
USB3 based on his OTG core.

Why do we need an OTG framework to support a device driver?

Is it something like a bus or class driver?

Best regards,
Lu Baolu

Quoted 40 lines

-----Original Message-----
From: Lu Baolu [mailto:[email protected]]
Sent: Sunday, June 05, 2016 4:47 PM
To: Jun Li <[email protected]>; Peter Chen <[email protected]>
Cc: [email protected]; Mathias Nyman <[email protected]>; Greg Kroah-Hartman <[email protected]>; Lee Jones <[email protected]>; Heikki Krogerus <[email protected]>; Liam Girdwood <[email protected]>; Mark Brown <[email protected]>; [email protected]; [email protected]; Roger Quadros <[email protected]>
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Hi,

Quoted 23 lines · Jun Li

Quoted 18 lines

Port mux is part of dual role switch, but not the whole thing.

Quoted 17 lines

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 3 lines

Quoted 3 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

I also think current OTG/Dual role framework can support your case, if you find there is any limitation of it which can't meet your requirement, we should improve it, Roger also provide an example of dual role switch with
USB3 based on his OTG core.

Quoted 1 line

Why do we need an OTG framework to support a device driver?

Currently there are many controller drivers which are dual role capable, all has its specific approach/implementation, your case is another one, actually there are common part we can share and reuse, Roger is introducing a common framework which cooperates into usb core and udc-core. With that, each OTG/dual role user only need take care of its small specific part.

Quoted 2 lines

Is it something like a bus or class driver?

It's not actually a driver, instead, it's more like a lib or helper routines. You just need register your host and gadget into OTG core, and define the ops routines if required, OTG state machine will help you do the switch.

Hi,

Quoted 20 lines · Jun Li

Quoted 20 lines

Port mux is part of dual role switch, but not the whole thing.

Quoted 19 lines

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 3 lines

Quoted 3 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

Quoted 4 lines

I also think current OTG/Dual role framework can support your case, if you find there is any limitation of it which can't meet your requirement, we should improve it, Roger also provide an example of dual role switch with USB3 based on his OTG core.

Why do we need an OTG framework to support a device driver?

Is it something like a bus or class driver?

Quoted 21 lines · Lu Baolu

Hi,

Quoted 18 lines · Jun Li

Quoted 18 lines

Port mux is part of dual role switch, but not the whole thing.

Quoted 17 lines

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 3 lines

Quoted 3 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

Quoted 5 lines

Quoted 4 lines

I also think current OTG/Dual role framework can support your case, if you find there is any limitation of it which can't meet your requirement, we should improve it, Roger also provide an example of dual role switch with USB3 based on his OTG core.

Why do we need an OTG framework to support a device driver?

Just like you said above, OTG framework can manage role switch, the role switch may need to start or stop host/gadget driver according to different hardware signals or user input.

Quoted 2 lines

Is it something like a bus or class driver?

The DRD/OTG framework uses the same device structure with the caller, the caller can be a dual-role controller driver (like dwc3, chipidea, etc), or a separate switch driver which like your mux port driver.

From my point of view, this isn't the right way to handle a port

Hi Peter,

Quoted 26 lines · Peter Chen

Quoted 25 lines · Lu Baolu

Hi,

Quoted 22 lines · Jun Li

Quoted 18 lines

Port mux is part of dual role switch, but not the whole thing.

Quoted 17 lines

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 3 lines

Quoted 3 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

I also think current OTG/Dual role framework can support your case, if you find there is any limitation of it which can't meet your requirement, we should improve it, Roger also provide an example of dual role switch with USB3 based on his OTG core.

Quoted 4 lines

Quoted 1 line

Why do we need an OTG framework to support a device driver?

Just like you said above, OTG framework can manage role switch, the role switch may need to start or stop host/gadget driver according to different hardware signals or user input.

We don't have any OTG or dual-role (reduced OTG) capable controllers. So we don't need to aid OTG framework to start/stop host/gadget drivers.

Quoted 6 lines

Quoted 2 lines

Is it something like a bus or class driver?

The DRD/OTG framework uses the same device structure with the caller, the caller can be a dual-role controller driver (like dwc3, chipidea, etc), or a separate switch driver which like your mux port driver.

From my point of view, this isn't the right way to handle a port mux device.

We have many kinds of port mux devices across multiple archs, we should have a generic framework for them, so that consumers, (like OTG framework) can manipulate port mux devices through a common interfaces. Just like we already have frameworks for PHY, VBUS regulator and ...

Hi Peter,

Quoted 13 lines · Peter Chen

Quoted 4 lines · Lu Baolu

Quoted 2 lines

from my point,it is a dual-role switch
driver too,

No, it's not a dual-role switch driver, but a driver for USB port multiplexing.

Quoted 8 lines

One example of port multiplexing can be found in several Intel SOC and PCH chips, inside of which, there are two independent USB controllers: host and device. They might share a single port and this port could be configured to route the line to one of these two controllers. This patch introduced a generic framework for port mux drivers. It aids the drivers to handle port mux by providing interfaces to 1) register/unregister a mux device; 2) lookup the mux device; and 3) switch the port.

Quoted 1 line

For this case, I can't see it is different with dual-role switch.

Port mux is part of dual role switch, but not the whole thing.

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

Quoted 4 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

Quoted 17 lines

Quoted 8 lines

Port multiplexing isn't equal to USB dual role. There are other cases in today's systems. In several Intel PCH chips, there equips two USB host controllers: ehci and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all USB ports work even running an old version of OS which lacks of USB3 support. In theory, we can create a driver for the port mux and switch the ports between xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)

Quoted 6 lines

Another case is xHCI debug capability. The xHCI host controller might equip a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit is independent of xhci host controller. But it shares its port with xhci. Software could switch the port between xhci and the debugging unit through the registers defined in xHCI spec.

Yes, above two are different with dual role switch. But in your code and Kconfig, it seems this framework is dedicated for dual-role. Eg:

drivers/usb/mux/Kconfig line 8

Quoted 4 lines

drivers/usb/mux/Kconfig new file +4 -0

+menuconfig USB_PORTMUX
+       bool "USB dual role port MUX support"
+       help
+         Generic USB dual role port mux support.

Above two cases are examples for port multiplexing, but I don't think we need drivers for them. At this moment, this framework is only for dual role port mux devices.

Quoted 3 lines

I think a general dual role port mux is necessary, it can be used to manage different dual-role switch method, eg

Yes, I agree.

Quoted 5 lines

- ID pin
- External connector through GPIO
- SoC register
- sysfs
- type-C events

ID pin and type-C events are the *reasons* which trigger the port mux switch. Currently, on our platforms, gpio, registers and sysfs are methods to control the mux.

Quoted 4 lines

But this code is better co-work with OTG/Dual-role framework, we'd better have only interface that the user can know which role for the current port.

OTG/Dual-role framework and portmux framework are not overlapped. The sysfs interface shouldn't be overlapped as well. Say, I have a port mux device and I have a driver for it. I am able to read the status of my port mux device through sysfs. This is not part of OTG/Dual-role as far as I can see.

Quoted 25 lines · Lu Baolu

Hi Peter,

Quoted 13 lines · Peter Chen

Quoted 4 lines · Lu Baolu

Quoted 2 lines

from my point,it is a dual-role switch
driver too,

No, it's not a dual-role switch driver, but a driver for USB port multiplexing.

Quoted 8 lines

One example of port multiplexing can be found in several Intel SOC and PCH chips, inside of which, there are two independent USB controllers: host and device. They might share a single port and this port could be configured to route the line to one of these two controllers. This patch introduced a generic framework for port mux drivers. It aids the drivers to handle port mux by providing interfaces to 1) register/unregister a mux device; 2) lookup the mux device; and 3) switch the port.

Quoted 1 line

For this case, I can't see it is different with dual-role switch.

Port mux is part of dual role switch, but not the whole thing.

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

Quoted 4 lines

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

You have admitted port mux is one of the ports of dual-role switch, Then, how they can co-work with each other? If can't, the dual-role switch framework needs another input events management for switching.

Quoted 8 lines

Quoted 4 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

Yes, it is the same. GPIO pin or device registers is like ID pin event.

Quoted 9 lines

But this code is better co-work with OTG/Dual-role framework, we'd better have only interface that the user can know which role for the current port.
OTG/Dual-role framework and portmux framework are not overlapped. The sysfs interface shouldn't be overlapped as well. Say, I have a port mux device and I have a driver for it. I am able to read the status of my port mux device through sysfs. This is not part of OTG/Dual-role as far as I can see.

Then how the user wants to switch the role through the mux driver's sysfs or dual-role switch sysfs?

Hi Peter,

Quoted 26 lines · Peter Chen

Quoted 25 lines · Lu Baolu

Hi Peter,

Quoted 13 lines · Peter Chen

Quoted 4 lines · Lu Baolu

Quoted 2 lines

from my point,it is a dual-role switch
driver too,

No, it's not a dual-role switch driver, but a driver for USB port multiplexing.

Quoted 8 lines

One example of port multiplexing can be found in several Intel SOC and PCH chips, inside of which, there are two independent USB controllers: host and device. They might share a single port and this port could be configured to route the line to one of these two controllers. This patch introduced a generic framework for port mux drivers. It aids the drivers to handle port mux by providing interfaces to 1) register/unregister a mux device; 2) lookup the mux device; and 3) switch the port.

Quoted 1 line

For this case, I can't see it is different with dual-role switch.

Port mux is part of dual role switch, but not the whole thing.

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

Quoted 7 lines

Quoted 4 lines

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

You have admitted port mux is one of the ports of dual-role switch, Then, how they can co-work with each other? If can't, the dual-role switch framework needs another input events management for switching.

My point is we need a generic framework for the port mux devices, just like we have that for PHY and regulator. OTG framework manages the port mux devices through the common interfaces provided by the port mux framework.

If we integrate the port mux device support into OTG itself, this will force every use case of port mux to rely on the big OTG framework, although what it needs is only a single driver. That causes unnecessary software complexity.

Quoted 12 lines

Quoted 9 lines

Quoted 5 lines

Your
case is just like Renesas case, which uses two different drivers between peripheral and host[1].

In my case, the port mux devices are physical devices and they can be controlled through GPIO pins or device registers. They are independent of both peripheral and host controllers.

Yes, it is the same. GPIO pin or device registers is like ID pin event.

<snip>

Quoted 9 lines

Quoted 9 lines

But this code is better co-work with OTG/Dual-role framework, we'd better have only interface that the user can know which role for the current port.
OTG/Dual-role framework and portmux framework are not overlapped. The sysfs interface shouldn't be overlapped as well. Say, I have a port mux device and I have a driver for it. I am able to read the status of my port mux device through sysfs. This is not part of OTG/Dual-role as far as I can see.

Quoted 3 lines

Then how the user wants to switch the role through the mux driver's sysfs or dual-role switch sysfs?

It depends. If you have an OTG/DRD capable controllers, you need to do this through OTG sysfs; otherwise you only need to switch the port.

For Mux devices implementing dual-role, the mux device driver _must_ use OTG/dual-role core API so that a common ABI is presented to user space for OTG/dual-role.

I haven't yet looked at the mux framework but if we take care of the above point then we are not introducing any redundancy.

Hi Roger

Quoted 4 lines

For Mux devices implementing dual-role, the mux device driver _must_ use OTG/dual-role core API so that a common ABI is presented to user space for OTG/dual-role.

That's the only point we have concern, do dual role switch through OTG/dual-role core, not do it by itself.

Quoted 4 lines

I haven't yet looked at the mux framework but if we take care of the above point then we are not introducing any redundancy.

Roger, actually this is my worry on OTG core: those dual role switch users just tends to do it simply by itself(straightforward and easy), not through the OTG core(some complicated in first look), this is just an example for us to convince people to select a better way:)

Li Jun

-----Original Message-----
From: Lu Baolu [mailto:[email protected]]
Sent: Tuesday, June 07, 2016 2:27 PM
To: Jun Li <[email protected]>; Roger Quadros <[email protected]>; Peter Chen <[email protected]>
Cc: [email protected]; Mathias Nyman <[email protected]>; Greg Kroah-Hartman <[email protected]>; Lee Jones <[email protected]>; Heikki Krogerus <[email protected]>; Liam Girdwood <[email protected]>; Mark Brown <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Hi Jun,

Quoted 6 lines · Jun Li

Hi Roger

Quoted 4 lines

For Mux devices implementing dual-role, the mux device driver _must_ use OTG/dual-role core API so that a common ABI is presented to user space for OTG/dual-role.

Quoted 2 lines

That's the only point we have concern, do dual role switch through OTG/dual-role core, not do it by itself.

That really depends on how do you define "dual role". Can you please provide an unambiguous definition of "dual role" used in OTG/dual-role framework?

Best regards,
Lu Baolu

Quoted 12 lines

Quoted 4 lines

I haven't yet looked at the mux framework but if we take care of the above point then we are not introducing any redundancy.

Roger, actually this is my worry on OTG core: those dual role switch users just tends to do it simply by itself(straightforward and easy), not through the OTG core(some complicated in first look), this is just an example for us to convince people to select a better way:)

Li Jun

Quoted 7 lines

Quoted 7 lines · Jun Li

Quoted 6 lines

Quoted 6 lines

Hi Roger

Quoted 4 lines

Quoted 4 lines

For Mux devices implementing dual-role, the mux device driver _must_ use OTG/dual-role core API so that a common ABI is presented to user space for OTG/dual-role.

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

That's the only point we have concern, do dual role switch through OTG/dual-role core, not do it by itself.

That really depends on how do you define "dual role". Can you please provide an unambiguous definition of "dual role" used in OTG/dual-role framework?

Host and peripheral.

Hi,

Quoted 13 lines · Jun Li

Quoted 13 lines

Quoted 13 lines · Jun Li

Quoted 8 lines

Quoted 8 lines

Hi Roger

Quoted 4 lines

Quoted 4 lines

For Mux devices implementing dual-role, the mux device driver _must_ use OTG/dual-role core API so that a common ABI is presented to user space for OTG/dual-role.

That's the only point we have concern, do dual role switch through OTG/dual-role core, not do it by itself.

That really depends on how do you define "dual role". Can you please provide an unambiguous definition of "dual role" used in OTG/dual-role framework?

Quoted 2 lines

Host and peripheral.

This is definitely ambiguous.

By reading OTG/dual-role code, my understanding is that "dual-role" is a "reduced OTG" which is for DRD devices lacking of some OTG negotiation protocols.

We really can't say "it's the scope of OTG/dual-role" whenever it comes to "host and peripheral".

Hi Roger

Quoted 4 lines

For Mux devices implementing dual-role, the mux device driver _must_ use OTG/dual-role core API so that a common ABI is presented to user space for OTG/dual-role.

That's the only point we have concern, do dual role switch through OTG/dual-role core, not do it by itself.

Quoted 3 lines

I haven't yet looked at the mux framework but if we take care of the above point then we are not introducing any redundancy.

Roger, actually this is my worry on OTG core: those dual role switch users just tends to do it simply by itself(straightforward and easy), not through the OTG core(some complicated in first look),

this is just an example for us to convince people to select a better way:)

Hi,

Quoted 9 lines · Jun Li

Hi Roger

Quoted 4 lines

For Mux devices implementing dual-role, the mux device driver _must_ use OTG/dual-role core API so that a common ABI is presented to user space for OTG/dual-role.

That's the only point we have concern, do dual role switch through OTG/dual-role core, not do it by itself.

Quoted 6 lines

Quoted 3 lines

I haven't yet looked at the mux framework but if we take care of the above point then we are not introducing any redundancy.

Roger, actually this is my worry on OTG core: those dual role switch users just tends to do it simply by itself(straightforward and easy), not through the OTG core(some complicated in first look),

I'm sorry, but I'm really confused.

Why do we need to drop "straightforward and easy", but have to run an *unnecessary* OTG state machine? Don't you think that will (1) add *unnecessary* software complexity; (2) increase *unnecessary* memory footprint; and (3) increase the debugging efforts?

Quoted 2 lines

this is just an example for us to convince people to select a better way:)

Sure. Let's take my case for an example.

My system has a third-party port mux, which is not part any USB controllers. Also, my system doesn't have any DRD capable devices. I need a "straightforward and easy" driver for it. Otherwise, the system could not be waken up from system suspend.

But you said I must run an unnecessary OTG state machine, even thought it has nothing to do with my system, only because the two sides of my port mux device is a host and peripheral controller.

Quoted 3 lines

But you said I must run an unnecessary OTG state machine, even thought it has nothing to do with my system, only because the two sides of my port mux device is a host and peripheral controller.

We have a minimal dual-role state machine that just looks at ID pin and toggles the port role.

How are you switching the port mux between host and peripheral? Only by sysfs or do you have a GPIO for ID pin as well?

What happens to the gadget controller when the port is muxed to the host controller? Is it stopped or it continues to run?

Hi,

Quoted 6 lines · Roger Quadros

Quoted 3 lines

But you said I must run an unnecessary OTG state machine, even thought it has nothing to do with my system, only because the two sides of my port mux device is a host and peripheral controller.

We have a minimal dual-role state machine that just looks at ID pin and toggles the port role.

I don't know if we want to bring all that extra baggage just to write a few bits in a single register. Even for DWC3-only dual-role (what Synopsys licenses as part of some DWC3 instantiations), the OTG/DRD layer is a bit overkill.

If you take my testing/next, for example, we have everything we need for dual-role; except for OTG/DRD IRQ handler. Just look at how we implement ->suspend()/->resume() and it's be clear that we're just missing one step.

I might be able to find some time to implement a proof of concept which would allow your platforms to get dual-role with code we already have, but I need DWC3's OTG support which, I'm assuming, you already have :-)

If you wanna try something offline, just ping me ;-) I'll be happy to help.

Quoted 3 lines

How are you switching the port mux between host and peripheral? Only by sysfs or do you have a GPIO for ID pin as well?

depends. Some SoCs have GPIO-controller muxes while some just have mux's select signals (one for ID, one for VBUS) mapped on xHCI's address space.

Quoted 2 lines

What happens to the gadget controller when the port is muxed to the host controller? Is it stopped or it continues to run?

it continues running, but that's pretty irrelevant for Intel's dual-role

setup. We have an actual physical (inside the die, though) mux which muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only DWC3.

Quoted 6 lines

I might be able to find some time to implement a proof of concept which would allow your platforms to get dual-role with code we already have, but I need DWC3's OTG support which, I'm assuming, you already have :-)

If you wanna try something offline, just ping me ;-) I'll be happy to help.

What you are proposing is a dwc3 only solution. With the otg/dual-role series we are trying to be generic as much as possible.

Whether controller drivers want to use it or not is upto the driver maintainers but we should at least ensure that user space ABI if any, is consistent across different implementations.

Quoted 7 lines

Quoted 2 lines

How are you switching the port mux between host and peripheral? Only by sysfs or do you have a GPIO for ID pin as well?

depends. Some SoCs have GPIO-controller muxes while some just have mux's select signals (one for ID, one for VBUS) mapped on xHCI's address space.

Quoted 4 lines

Quoted 2 lines

What happens to the gadget controller when the port is muxed to the host controller? Is it stopped or it continues to run?

it continues running, but that's pretty irrelevant for Intel's dual-role

Isn't that unnecessary waste of power? Or you have firmware assisted low power mode?

Quoted 4 lines

setup. We have an actual physical (inside the die, though) mux which muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only DWC3.

Probably irrelevant for Intel's dual-role but many platforms that share the port can't have device controller running when port is in host mode and vice versa.

So there has to be a central point of control where the respective controllers are started/stopped.

That is the other point we are trying to address with the common otg/dual-role code.

Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we need to stop the host controller for device mode, right?

If so then who will deal with start/stop of the controllers then?

-----Original Message-----
From: Felipe Balbi [mailto:[email protected]]
Sent: Tuesday, June 07, 2016 11:05 PM
To: Roger Quadros <[email protected]>; Lu Baolu <[email protected]>; Jun Li <[email protected]>; Peter Chen <[email protected]>
Cc: Mathias Nyman <[email protected]>; Greg Kroah-Hartman <[email protected]>; Lee Jones <[email protected]>; Heikki Krogerus <[email protected]>; Liam Girdwood <[email protected]>; Mark Brown <[email protected]>; linux- [email protected]; [email protected]
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Hi,

Quoted 10 lines · Roger Quadros

Quoted 7 lines

I might be able to find some time to implement a proof of concept which would allow your platforms to get dual-role with code we already have, but I need DWC3's OTG support which, I'm assuming, you already have :-)

If you wanna try something offline, just ping me ;-) I'll be happy to help.

What you are proposing is a dwc3 only solution. With the otg/dual-role series we are trying to be generic as much as possible.

Well, if there is a need for that, sure. Take MUSB for instance. It makes use of nothing of the sorts, because it doesn't have to.

Quoted 3 lines

Whether controller drivers want to use it or not is upto the driver maintainers but we should at least ensure that user space ABI if any, is consistent across different implementations.

Role decisions should not be exposed to userspace unless as debug feature (using e.g. DebugFS). That should be done either by the HW or within the kernel.

If we're discussing userspace ABI here, there's something very wrong with OTG/DRD layer design.

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

How are you switching the port mux between host and peripheral? Only by sysfs or do you have a GPIO for ID pin as well?

depends. Some SoCs have GPIO-controller muxes while some just have mux's select signals (one for ID, one for VBUS) mapped on xHCI's address space.

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

What happens to the gadget controller when the port is muxed to the host controller? Is it stopped or it continues to run?

it continues running, but that's pretty irrelevant for Intel's dual-role

Isn't that unnecessary waste of power? Or you have firmware assisted low power mode?

that's an implementation detail which brings nothing to this discussion, right? :-)

We can, certainly, put the other side to D3.

Quoted 8 lines

Quoted 4 lines

setup. We have an actual physical (inside the die, though) mux which muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only DWC3.

Probably irrelevant for Intel's dual-role but many platforms that share the port can't have device controller running when port is in host mode and vice versa.

but that doesn't mean we need an entire new layer added to the kernel ;-)

DWC3 already gives us all the information necessary to make a decision on which role we should assume. Just consider your options. Here's how things would look like without any OTG/DRD layer:

-> DWC3 OTG IRQ
-> readl(OSTS);
-> if (OSTS & BIT(4))
-> dwc3_host_exit(); __dwc3_gadget_start();
-> else
-> __dwc3_gadget_stop(); dwc3_host_init();

Can you draw something similar for your proposed OTG/DRD layer?

I remember there were at least two schedule_work(). IIRC it looked something like below:

-> DWC3 OTG IRQ
-> readl(OSTS);
-> if (OSTS & BIT(4))

   -> otg_set_mode(PERIPHERAL);
    -> schedule_work();
     -> otg_ops->stop_host();
      -> usb_del_hcd();
     -> otg_ops->start_peripheral();
      -> usb_gadget_add_udc();
  -> else
   -> otg_set_mode(HOST);
    -> schedule_work();
     -> otg_ops->stop_peripheral();
      -> usb_gadget_del_udc();
     -> otg_ops->start_host();
      -> usb_add_hcd();

I'm probably missing some steps there.

Quoted 2 lines

So there has to be a central point of control where the respective controllers are started/stopped.

some implementations might need this, yes. DWC3 and MUSB don't seem to be this type of system.

Quoted 5 lines

That is the other point we are trying to address with the common otg/dual-role code.

Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we need to stop the host controller for device mode, right?

yes, see above. We already have that code.

Quoted 1 line

If so then who will deal with start/stop of the controllers then?

dwc3 itself.

--
balbi

Quoted 2 lines

maintainers but we should at least ensure that user space ABI if any, is consistent across different implementations.

Role decisions should not be exposed to userspace unless as debug feature (using e.g. DebugFS). That should be done either by the HW or within the kernel.
In many cases the role decision is made by usersapce, this also should be covered.
This patchset also expose it to userspace but I think it isn't for debug:
/sys/bus/platform/devices/.../portmux.N/state

Please don't use this interface for host/gadget role switch, and the document doesn't tell you to do so as well. This is only designed to put the port mux device to a right direction. Host/gadget dual role switch includes other elements, like ID pin detection, type-c events, VBUS management and so on.

Best regards,
Lu Baolu

Hi,

Quoted 4 lines · Roger Quadros

Quoted 4 lines

I might be able to find some time to implement a proof of concept which would allow your platforms to get dual-role with code we already have, but I need DWC3's OTG support which, I'm assuming, you already have :-)

Quoted 5 lines

Quoted 2 lines

If you wanna try something offline, just ping me ;-) I'll be happy to help.

What you are proposing is a dwc3 only solution. With the otg/dual-role series we are trying to be generic as much as possible.

Well, if there is a need for that, sure. Take MUSB for instance. It makes use of nothing of the sorts, because it doesn't have to.

Quoted 3 lines

Whether controller drivers want to use it or not is upto the driver maintainers but we should at least ensure that user space ABI if any, is consistent across different implementations.

Role decisions should not be exposed to userspace unless as debug feature (using e.g. DebugFS). That should be done either by the HW or within the kernel.

If we're discussing userspace ABI here, there's something very wrong with OTG/DRD layer design.

Hi,

Quoted 9 lines · Roger Quadros

Quoted 6 lines

I might be able to find some time to implement a proof of concept which would allow your platforms to get dual-role with code we already have, but I need DWC3's OTG support which, I'm assuming, you already have :-)

If you wanna try something offline, just ping me ;-) I'll be happy to help.

What you are proposing is a dwc3 only solution. With the otg/dual-role series we are trying to be generic as much as possible.

Well, if there is a need for that, sure. Take MUSB for instance. It makes use of nothing of the sorts, because it doesn't have to.

Quoted 3 lines

Whether controller drivers want to use it or not is upto the driver maintainers but we should at least ensure that user space ABI if any, is consistent across different implementations.

Role decisions should not be exposed to userspace unless as debug feature (using e.g. DebugFS). That should be done either by the HW or within the kernel.

If we're discussing userspace ABI here, there's something very wrong with OTG/DRD layer design.

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

How are you switching the port mux between host and peripheral? Only by sysfs or do you have a GPIO for ID pin as well?

depends. Some SoCs have GPIO-controller muxes while some just have mux's select signals (one for ID, one for VBUS) mapped on xHCI's address space.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

What happens to the gadget controller when the port is muxed to the host controller? Is it stopped or it continues to run?

it continues running, but that's pretty irrelevant for Intel's dual-role

Isn't that unnecessary waste of power? Or you have firmware assisted low power mode?

that's an implementation detail which brings nothing to this discussion, right? :-)

We can, certainly, put the other side to D3.

Quoted 8 lines

Quoted 4 lines

setup. We have an actual physical (inside the die, though) mux which muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only DWC3.

Probably irrelevant for Intel's dual-role but many platforms that share the port can't have device controller running when port is in host mode and vice versa.

but that doesn't mean we need an entire new layer added to the kernel ;-)

DWC3 already gives us all the information necessary to make a decision on which role we should assume. Just consider your options. Here's how things would look like without any OTG/DRD layer:

-> DWC3 OTG IRQ
-> readl(OSTS);
-> if (OSTS & BIT(4))
-> dwc3_host_exit(); __dwc3_gadget_start();
-> else
-> __dwc3_gadget_stop(); dwc3_host_init();

Can you draw something similar for your proposed OTG/DRD layer?

I remember there were at least two schedule_work(). IIRC it looked something like below:

-> DWC3 OTG IRQ
-> readl(OSTS);
-> if (OSTS & BIT(4))

   -> otg_set_mode(PERIPHERAL);
    -> schedule_work();
     -> otg_ops->stop_host();
      -> usb_del_hcd();
     -> otg_ops->start_peripheral();
      -> usb_gadget_add_udc();
  -> else
   -> otg_set_mode(HOST);
    -> schedule_work();
     -> otg_ops->stop_peripheral();
      -> usb_gadget_del_udc();
     -> otg_ops->start_host();
      -> usb_add_hcd();

I'm probably missing some steps there.

Quoted 3 lines

So there has to be a central point of control where the respective controllers are started/stopped.

some implementations might need this, yes. DWC3 and MUSB don't seem to be this type of system.

Quoted 6 lines

That is the other point we are trying to address with the common otg/dual-role code.

Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we need to stop the host controller for device mode, right?

yes, see above. We already have that code.

Quoted 2 lines

If so then who will deal with start/stop of the controllers then?

dwc3 itself.

Hi Peter,

Quoted 26 lines · Peter Chen

Quoted 25 lines · Lu Baolu

Hi Peter,

Quoted 13 lines · Peter Chen

Quoted 4 lines · Lu Baolu

Quoted 2 lines

from my point,it is a dual-role switch
driver too,

No, it's not a dual-role switch driver, but a driver for USB port multiplexing.

Quoted 8 lines

One example of port multiplexing can be found in several Intel SOC and PCH chips, inside of which, there are two independent USB controllers: host and device. They might share a single port and this port could be configured to route the line to one of these two controllers. This patch introduced a generic framework for port mux drivers. It aids the drivers to handle port mux by providing interfaces to 1) register/unregister a mux device; 2) lookup the mux device; and 3) switch the port.

Quoted 1 line

For this case, I can't see it is different with dual-role switch.

Port mux is part of dual role switch, but not the whole thing.

Dual role switch includes at least below things:
- ID or type-C event detection
- port mux
- VBUS management
- start/stop host/device controllers

Quoted 7 lines

Quoted 4 lines

An OTG/Dual-role framework can be used to keep all these things run together with an internal state machine. But it's not duplicated with a generic framework for port mux and the port mux drivers.

You have admitted port mux is one of the ports of dual-role switch, Then, how they can co-work with each other? If can't, the dual-role switch framework needs another input events management for switching.

My point is we need a generic framework for the port mux devices, just like we have that for PHY and regulator. OTG framework manages the port mux devices through the common interfaces provided by the port mux framework.

If we integrate the port mux device support into OTG itself, this will force every use case of port mux to rely on the big OTG framework, although what it needs is only a single driver. That causes unnecessary software complexity.

Quoted 10 lines

Quoted 10 lines

But this code is better co-work with OTG/Dual-role framework, we'd better have only interface that the user can know which role for the current port.
OTG/Dual-role framework and portmux framework are not overlapped. The sysfs interface shouldn't be overlapped as well. Say, I have a port mux device and I have a driver for it. I am able to read the status of my port mux device through sysfs. This is not part of OTG/Dual-role as far as I can see.

Quoted 3 lines

Then how the user wants to switch the role through the mux driver's sysfs or dual-role switch sysfs?

It depends. If you have an OTG/DRD capable controllers, you need to do this through OTG sysfs; otherwise you only need to switch the port.

Quoted 13 lines

Quoted 13 lines

Quoted 12 lines

Quoted 12 lines

Quoted 9 lines

Quoted 9 lines

But this code is better co-work with OTG/Dual-role framework, we'd better have only interface that the user can know which role for the current port.
OTG/Dual-role framework and portmux framework are not overlapped. The sysfs interface shouldn't be overlapped as well. Say, I have a port mux device and I have a driver for it. I am able to read the status of my port mux device through sysfs. This is not part of OTG/Dual-role as far as I can see.

Then how the user wants to switch the role through the mux driver's sysfs or dual-role switch sysfs?

Quoted 3 lines

Quoted 3 lines

It depends. If you have an OTG/DRD capable controllers, you need to do this through OTG sysfs; otherwise you only need to switch the port.

The user may not know the detail, they will do role switch according to sysfs documentation. Yes, in your role switch case, only port mux is enough, but for others, it needs other operations.

I agree with Roger that the dual-role switch part in your code is better to use OTG framework to reduce redundancy.

In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI.

This patch adds the support to detect and create the platform device for the port mux switch.

Quoted 7 lines · Lu Baolu

In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI.

This patch adds the support to detect and create the platform device for the port mux switch.

Why do you need a platform device for this? You do nothing with this device, why create it at all?

And why is it a platform device, isn't is really a PCI device? Why would you ever find a "platform" device below a PCI device? Don't abuse platform devices for things that aren't. It makes me want to delete that whole interface more and more...

Hi Greg,

Quoted 10 lines · Greg Kroah-Hartman

Quoted 7 lines · Lu Baolu

In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI.

This patch adds the support to detect and create the platform device for the port mux switch.

Why do you need a platform device for this? You do nothing with this device, why create it at all?

In this patch series, I have a generic framework for port mux devices and two port mux drivers sitting on top the generic code.

In this patch, I create a platform device for the real mux device in Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux into the generic framework and handle the power management things in driver's pm entries (otherwise, the system can't be waken up from system suspend).

Quoted 4 lines

And why is it a platform device, isn't is really a PCI device? Why would you ever find a "platform" device below a PCI device? Don't abuse platform devices for things that aren't. It makes me want to delete that whole interface more and more...

Port mux devices are physical devices in Intel Cherry Trail and Broxton SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI space. OS kernel can enumerate it by looking up the xhci extended capability list with a vendor specific capability ID.

Quoted 26 lines · Lu Baolu

Hi Greg,

Quoted 10 lines · Greg Kroah-Hartman

Quoted 7 lines · Lu Baolu

In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI.

This patch adds the support to detect and create the platform device for the port mux switch.

Why do you need a platform device for this? You do nothing with this device, why create it at all?

In this patch series, I have a generic framework for port mux devices and two port mux drivers sitting on top the generic code.

In this patch, I create a platform device for the real mux device in Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux into the generic framework and handle the power management things in driver's pm entries (otherwise, the system can't be waken up from system suspend).:)

Quoted 4 lines

And why is it a platform device, isn't is really a PCI device? Why would you ever find a "platform" device below a PCI device? Don't abuse platform devices for things that aren't. It makes me want to delete that whole interface more and more...

Quoted 4 lines

Port mux devices are physical devices in Intel Cherry Trail and Broxton SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI space. OS kernel can enumerate it by looking up the xhci extended capability list with a vendor specific capability ID.

A physical device that maps registers into PCI space seems like a PCI device of some type to me :)

Again, I hate platform devices for obvious reasons like this...

Hi Greg,

Quoted 34 lines · Greg Kroah-Hartman

Quoted 22 lines · Lu Baolu

Hi Greg,

Quoted 10 lines · Greg Kroah-Hartman

Quoted 7 lines · Lu Baolu

In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI.

This patch adds the support to detect and create the platform device for the port mux switch.

Why do you need a platform device for this? You do nothing with this device, why create it at all?

In this patch series, I have a generic framework for port mux devices and two port mux drivers sitting on top the generic code.

In this patch, I create a platform device for the real mux device in Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux into the generic framework and handle the power management things in driver's pm entries (otherwise, the system can't be waken up from system suspend).:)

Quoted 8 lines

Quoted 4 lines

And why is it a platform device, isn't is really a PCI device? Why would you ever find a "platform" device below a PCI device? Don't abuse platform devices for things that aren't. It makes me want to delete that whole interface more and more...

Port mux devices are physical devices in Intel Cherry Trail and Broxton SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI space. OS kernel can enumerate it by looking up the xhci extended capability list with a vendor specific capability ID.

A physical device that maps registers into PCI space seems like a PCI device of some type to me :)

Quoted 2 lines

Again, I hate platform devices for obvious reasons like this...

It's not PCI configure space, but xhci's io memory. XHCI spec reserves a range in its extended capability list for vendor specific things. Intel's platform leverages this for the port mux device register mapping. It looks odd though. :)

Hi Greg,

Quoted 37 lines · Lu Baolu

Hi Greg,

Quoted 34 lines · Greg Kroah-Hartman

Quoted 22 lines · Lu Baolu

Hi Greg,

Quoted 10 lines · Greg Kroah-Hartman

Quoted 7 lines · Lu Baolu

In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI.

This patch adds the support to detect and create the platform device for the port mux switch.

Why do you need a platform device for this? You do nothing with this device, why create it at all?

In this patch series, I have a generic framework for port mux devices and two port mux drivers sitting on top the generic code.

In this patch, I create a platform device for the real mux device in Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux into the generic framework and handle the power management things in driver's pm entries (otherwise, the system can't be waken up from system suspend).:)

Quoted 8 lines

Quoted 4 lines

And why is it a platform device, isn't is really a PCI device? Why would you ever find a "platform" device below a PCI device? Don't abuse platform devices for things that aren't. It makes me want to delete that whole interface more and more...

Port mux devices are physical devices in Intel Cherry Trail and Broxton SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI space. OS kernel can enumerate it by looking up the xhci extended capability list with a vendor specific capability ID.

A physical device that maps registers into PCI space seems like a PCI device of some type to me :)

Quoted 6 lines

Quoted 2 lines

Again, I hate platform devices for obvious reasons like this...

It's not PCI configure space, but xhci's io memory. XHCI spec reserves a range in its extended capability list for vendor specific things. Intel's platform leverages this for the port mux device register mapping. It looks odd though. :)

A gentle ping. :)

This port mux is not a PCI device. It only leverages the vendor specific capability defined in xhci specification for enumeration.

Quoted 42 lines · Lu Baolu

Hi Greg,

Quoted 39 lines · Lu Baolu

Hi Greg,

Quoted 36 lines · Greg Kroah-Hartman

Quoted 22 lines · Lu Baolu

Hi Greg,

Quoted 10 lines · Greg Kroah-Hartman

Quoted 7 lines · Lu Baolu

In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI.

This patch adds the support to detect and create the platform device for the port mux switch.

Why do you need a platform device for this? You do nothing with this device, why create it at all?

In this patch series, I have a generic framework for port mux devices and two port mux drivers sitting on top the generic code.

In this patch, I create a platform device for the real mux device in Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux into the generic framework and handle the power management things in driver's pm entries (otherwise, the system can't be waken up from system suspend).:)

Quoted 8 lines

Quoted 4 lines

And why is it a platform device, isn't is really a PCI device? Why would you ever find a "platform" device below a PCI device? Don't abuse platform devices for things that aren't. It makes me want to delete that whole interface more and more...

Port mux devices are physical devices in Intel Cherry Trail and Broxton SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI space. OS kernel can enumerate it by looking up the xhci extended capability list with a vendor specific capability ID.

A physical device that maps registers into PCI space seems like a PCI device of some type to me :)

Again, I hate platform devices for obvious reasons like this...

Quoted 5 lines

Quoted 4 lines

It's not PCI configure space, but xhci's io memory. XHCI spec reserves a range in its extended capability list for vendor specific things. Intel's platform leverages this for the port mux device register mapping. It looks odd though. :)

A gentle ping. :)

For what? This patchset is long gone from my queue for the other various things that came up with it, what can I do with it now?

Quoted 3 lines

This port mux is not a PCI device. It only leverages the vendor specific capability defined in xhci specification for enumeration.

It's still crap :)

I don't know, and don't really remember the patch anymore anyway, remember, I have the sort-term memory of a squirrel, you need repost patches, with a proper changelog for me to be able to do anything...

greg k-h