linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/7] usb: add support for Intel dual role port mux
@ 2016-06-02  1:37 Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 1/7] regulator: fixed: add support for ACPI interface Lu Baolu
                   ` (6 more replies)
  0 siblings, 7 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, Lu Baolu

Intel SOC chips are featured with USB dual role. The host role
is provided by Intel xHCI IP, and the gadget role is provided
by IP from designware. Tablet platform designs always share a
single port for both host and gadget controllers.  There is a
mux to switch the port to the right controller according to the
cable type. OS needs to provide the callback to control the mux
when a plug-in event raises. The method to control the mux is
platform dependent. At least three types of implementation can
be found across current devices. 1) GPIO pins; 2) a unit which
can be controlled by memory mapped registers; 3) ACPI ASL code.

This patch series adds supports for Intel dual role port mux.
It includes:
(1) A generic framework for port mux devices. It exports interfaces
    to register and unregister a port mux device. It also exports
    an interface for the port mux consumers (e.x. PHY and charger
    drivers) to switch the port role according to the events they
    have detected.
(2) Drivers for GPIO controlled port mux which could be found
    on Baytrail devices. A mfd driver is used to split the GPIOs
    into a USB gpio extcon device, a fixed regulator for gpio
    controlled USB VCC, and a USB mux device. Driver for USB
    gpio extcon device is already in upstream Linux. This patch
    series includes a driver for GPIO USB mux.
(3) Drivers for USB port mux controlled through memory mapped
    registers and the logic to create the mux device. This type
    of dual role port mux could be found in Cherry Trail and
    Broxton devices.

All patches have been verified on Intel's BayTrail, CherryTrail,
and Broxton P platforms.

Lu Baolu (7):
  regulator: fixed: add support for ACPI interface
  usb: mux: add generic code for dual role port mux
  usb: mux: add driver for Intel gpio controlled port mux
  usb: mux: add driver for Intel drcfg controlled port mux
  mfd: intel_vuport: Add Intel virtual USB port MFD Driver
  usb: pci-quirks: add Intel USB drcfg mux device
  MAINTAINERS: add maintainer entry for Intel USB dual role mux drivers

Change log:

v9->v10:
 - rebase the patch series on top of 4.7-rc1.

v8->v9:
 - Patch "mfd: intel_vuport: Add Intel virtual USB port MFD Driver"
   has been ACK-ed by "Lee Jones <lee.jones@linaro.org>".
 - remove extcon dependency out of the generic mux code to support
   systems which have multiple port muxes.
 - Add dev.release for mux device, otherwise, the mux driver module
   couldn't be removed.
 - Use the refreshed device property interface updated in Commit (f4d0526
   device property: don't bother the drivers with struct property_set)

v7->v8:
 - In patch "regulator: fixed: add support for ACPI interface", the
   fixed regulator gpio name has been changed to "gpio".
 - In patch "MAINTAINERS: add maintainer entry for Intel USB dual role
   mux drivers", filenames have been updated.

v6->v7:
 - Two patches have been picked up by extcon maintainer. Hence,
   remove them since this version.
   - extcon: usb-gpio: add device binding for platform device
   - extcon: usb-gpio: add support for ACPI gpio interface
 - Below patch has been removed from this series because it's unnecessary.
   - regulator: fixed: add device binding for platform device
 - In patch "regulator: fixed: add support for ACPI interface",
   a static gpio name is used to get the regulator gpio.
 - In patch "mfd: intel_vuport: Add Intel virtual USB port MFD Driver",
   unnecessary "gpio-name" string property has been removed.

v5->v6:
 Work internally with Felipe to improve the whole patch series.
 Below changes have been made since last version.
 - rework the common code to make it a generic interface for mux devices;
 - split the vbus gpio handling to a fixed regulator device;
 - removed unnecessary filtering for state change;
 - removed unnecessary WARN statement;
 - removed globals in mux drivers;
 - removed unnecessary register polling and waiting in drcfg driver;

v4->v5:
 - Change the extcon interfaces with the new ones suggested by
   2a9de9c0f08d6 (extcon: Use the unique id for external connector
   instead of string)
 - remove patch "usb: pci-quirks: add Intel USB drcfg mux device"
   from this serial due to that it's not driver staff. Will be
   submitted seperately.

v3->v4:
 - Check all patches with "checkpatch.pl --strict", and fix all
   CHECKs;
 - Change sysfs node from "intel_mux" to "port_mux";
 - Refines below confusing functions:
   intel_usb_mux_register() -> intel_usb_mux_bind_cable()
   intel_usb_mux_unregister() -> intel_usb_mux_unbind_cable();
 - Remove unnecessary struct intel_mux_dev.

v2->v3:
 - uvport mfd driver got reviewed by Lee Jones, the following
   changes were made accordingly.
 - seperate uvport driver from the mux drivers in MAINTAINERS file
 - refine the description in Kconfig
 - refine the mfd_cell structure data

v1->v2:
 - move mux driver from drivers/usb/misc to drivers/usb/mux;
 - replace debugfs with sysfs for user level mux control;
 - remove unnecessary register restore if mux registeration failed;
 - Add "Acked-by: Chanwoo Choi <cw00.choi@samsung.com>" to extcon changes;
 - Make the file names and exported function names more specific;
 - Remove the usb_mux_get_dev() interface;
 - Move "struct intel_usb_mux" from .h to .c file;
 - Fix various kbuild robot warnings.

 Documentation/ABI/testing/sysfs-bus-platform |  18 +++
 MAINTAINERS                                  |  10 ++
 drivers/mfd/Kconfig                          |   8 ++
 drivers/mfd/Makefile                         |   1 +
 drivers/mfd/intel-vuport.c                   |  86 ++++++++++++
 drivers/regulator/fixed.c                    |  46 ++++++
 drivers/usb/Kconfig                          |   2 +
 drivers/usb/Makefile                         |   1 +
 drivers/usb/host/pci-quirks.c                |  42 +++++-
 drivers/usb/host/xhci-ext-caps.h             |   2 +
 drivers/usb/mux/Kconfig                      |  28 ++++
 drivers/usb/mux/Makefile                     |   6 +
 drivers/usb/mux/portmux-core.c               | 202 +++++++++++++++++++++++++++
 drivers/usb/mux/portmux-intel-drcfg.c        | 162 +++++++++++++++++++++
 drivers/usb/mux/portmux-intel-gpio.c         | 183 ++++++++++++++++++++++++
 include/linux/usb/portmux.h                  |  90 ++++++++++++
 16 files changed, 886 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mfd/intel-vuport.c
 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 drivers/usb/mux/portmux-intel-drcfg.c
 create mode 100644 drivers/usb/mux/portmux-intel-gpio.c
 create mode 100644 include/linux/usb/portmux.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v10 1/7] regulator: fixed: add support for ACPI interface
  2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
@ 2016-06-02  1:37 ` Lu Baolu
  2016-06-08  4:42   ` Greg Kroah-Hartman
  2016-06-02  1:37 ` [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, 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 <baolu.lu@linux.intel.com>
---
 drivers/regulator/fixed.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index ff62d69..207ab40 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -30,6 +30,9 @@
 #include <linux/of_gpio.h>
 #include <linux/regulator/of_regulator.h>
 #include <linux/regulator/machine.h>
+#include <linux/acpi.h>
+#include <linux/property.h>
+#include <linux/gpio/consumer.h>
 
 struct fixed_voltage_data {
 	struct regulator_desc desc;
@@ -104,6 +107,44 @@ of_get_fixed_voltage_config(struct device *dev,
 	return config;
 }
 
+/**
+ * acpi_get_fixed_voltage_config - extract fixed_voltage_config structure info
+ * @dev: device requesting for fixed_voltage_config
+ * @desc: regulator description
+ *
+ * Populates fixed_voltage_config structure by extracting data through ACPI
+ * interface, returns a pointer to the populated structure of NULL if memory
+ * alloc fails.
+ */
+static struct fixed_voltage_config *
+acpi_get_fixed_voltage_config(struct device *dev,
+			      const struct regulator_desc *desc)
+{
+	struct fixed_voltage_config *config;
+	const char *supply_name;
+	struct gpio_desc *gpiod;
+	int ret;
+
+	config = devm_kzalloc(dev, sizeof(*config), GFP_KERNEL);
+	if (!config)
+		return ERR_PTR(-ENOMEM);
+
+	ret = device_property_read_string(dev, "supply-name", &supply_name);
+	if (!ret)
+		config->supply_name = supply_name;
+
+	gpiod = gpiod_get(dev, "gpio", GPIOD_ASIS);
+	if (IS_ERR(gpiod))
+		return ERR_PTR(-ENODEV);
+
+	config->gpio = desc_to_gpio(gpiod);
+	config->enable_high = device_property_read_bool(dev,
+							"enable-active-high");
+	gpiod_put(gpiod);
+
+	return config;
+}
+
 static struct regulator_ops fixed_voltage_ops = {
 };
 
@@ -124,6 +165,11 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 						     &drvdata->desc);
 		if (IS_ERR(config))
 			return PTR_ERR(config);
+	} else if (ACPI_HANDLE(&pdev->dev)) {
+		config = acpi_get_fixed_voltage_config(&pdev->dev,
+						       &drvdata->desc);
+		if (IS_ERR(config))
+			return PTR_ERR(config);
 	} else {
 		config = dev_get_platdata(&pdev->dev);
 	}
-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 1/7] regulator: fixed: add support for ACPI interface Lu Baolu
@ 2016-06-02  1:37 ` Lu Baolu
  2016-06-03  7:41   ` Peter Chen
  2016-06-02  1:37 ` [PATCH v10 3/7] usb: mux: add driver for Intel gpio controlled " Lu Baolu
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, 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.

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 <baolu.lu@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 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

diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform
index 5172a61..b994e4e 100644
--- a/Documentation/ABI/testing/sysfs-bus-platform
+++ b/Documentation/ABI/testing/sysfs-bus-platform
@@ -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 <baolu.lu@linux.intel.com>
+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.
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8689dcb..328916e 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -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
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index dca7856..9a92338 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -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/
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
new file mode 100644
index 0000000..ba778f2
--- /dev/null
+++ b/drivers/usb/mux/Kconfig
@@ -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.
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
new file mode 100644
index 0000000..f85df92
--- /dev/null
+++ b/drivers/usb/mux/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for USB port mux drivers
+#
+obj-$(CONFIG_USB_PORTMUX)		+= portmux-core.o
diff --git a/drivers/usb/mux/portmux-core.c b/drivers/usb/mux/portmux-core.c
new file mode 100644
index 0000000..75fbb45
--- /dev/null
+++ b/drivers/usb/mux/portmux-core.c
@@ -0,0 +1,202 @@
+/**
+ * portmux-core.c - USB Port Mux support
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * 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
diff --git a/include/linux/usb/portmux.h b/include/linux/usb/portmux.h
new file mode 100644
index 0000000..093620a
--- /dev/null
+++ b/include/linux/usb/portmux.h
@@ -0,0 +1,90 @@
+/**
+ * portmux.h - USB Port Mux definitions
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * 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 */
-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v10 3/7] usb: mux: add driver for Intel gpio controlled port mux
  2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 1/7] regulator: fixed: add support for ACPI interface Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu
@ 2016-06-02  1:37 ` Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 4/7] usb: mux: add driver for Intel drcfg " Lu Baolu
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, Lu Baolu, David Cohen

In some Intel platforms, a single usb port is shared between USB host
and device controller. The shared port is under control of GPIO pins.

This patch adds the support for USB GPIO controlled port mux.

[baolu: removed .owner per platform_no_drv_owner.cocci]
[baolu: extcon usage reviewed by Chanwoo Choi]
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/usb/mux/Kconfig              |  13 +++
 drivers/usb/mux/Makefile             |   1 +
 drivers/usb/mux/portmux-intel-gpio.c | 183 +++++++++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 drivers/usb/mux/portmux-intel-gpio.c

diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
index ba778f2..41b0f72 100644
--- a/drivers/usb/mux/Kconfig
+++ b/drivers/usb/mux/Kconfig
@@ -6,3 +6,16 @@ menuconfig USB_PORTMUX
 	bool "USB dual role port MUX support"
 	help
 	  Generic USB dual role port mux support.
+
+if USB_PORTMUX
+
+config INTEL_MUX_GPIO
+tristate "Intel dual role port mux controlled by GPIOs"
+	depends on GPIOLIB
+	depends on REGULATOR
+	depends on X86 && ACPI
+	help
+	  Say Y here to enable support for Intel dual role port mux
+	  controlled by GPIOs.
+
+endif
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
index f85df92..4eb5582 100644
--- a/drivers/usb/mux/Makefile
+++ b/drivers/usb/mux/Makefile
@@ -2,3 +2,4 @@
 # Makefile for USB port mux drivers
 #
 obj-$(CONFIG_USB_PORTMUX)		+= portmux-core.o
+obj-$(CONFIG_INTEL_MUX_GPIO)		+= portmux-intel-gpio.o
diff --git a/drivers/usb/mux/portmux-intel-gpio.c b/drivers/usb/mux/portmux-intel-gpio.c
new file mode 100644
index 0000000..cada490
--- /dev/null
+++ b/drivers/usb/mux/portmux-intel-gpio.c
@@ -0,0 +1,183 @@
+/*
+ * USB Dual Role Port Mux driver controlled by gpios
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ * Author: David Cohen <david.a.cohen@linux.intel.com>
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * 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/acpi.h>
+#include <linux/module.h>
+#include <linux/extcon.h>
+#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/usb/portmux.h>
+#include <linux/regulator/consumer.h>
+
+struct vuport {
+	struct extcon_dev *edev;
+	struct notifier_block nb;
+	struct portmux_desc desc;
+	struct portmux_dev *pdev;
+	struct regulator *regulator;
+	struct gpio_desc *gpio_usb_mux;
+};
+
+/*
+ * id == 0, HOST connected, USB port should be set to peripheral
+ * id == 1, HOST disconnected, USB port should be set to host
+ *
+ * Peripheral: set USB mux to peripheral and disable VBUS
+ * Host: set USB mux to host and enable VBUS
+ */
+static inline int vuport_set_port(struct device *dev, int id)
+{
+	struct vuport *vup;
+
+	dev_dbg(dev, "USB PORT ID: %s\n", id ? "HOST" : "PERIPHERAL");
+
+	vup = dev_get_drvdata(dev);
+
+	gpiod_set_value_cansleep(vup->gpio_usb_mux, !id);
+
+	if (!id ^ regulator_is_enabled(vup->regulator))
+		return id ? regulator_disable(vup->regulator) :
+				regulator_enable(vup->regulator);
+
+	return 0;
+}
+
+static int vuport_cable_set(struct device *dev)
+{
+	return vuport_set_port(dev, 1);
+}
+
+static int vuport_cable_unset(struct device *dev)
+{
+	return vuport_set_port(dev, 0);
+}
+
+static const struct portmux_ops vuport_ops = {
+	.set_host_cb = vuport_cable_set,
+	.set_device_cb = vuport_cable_unset,
+};
+
+static int vuport_notifier(struct notifier_block *nb,
+			   unsigned long event, void *ptr)
+{
+	struct vuport *vup;
+	int state;
+
+	vup = container_of(nb, struct vuport, nb);
+	state = extcon_get_cable_state_(vup->edev, EXTCON_USB_HOST);
+	if (state < 0)
+		return state;
+
+	return portmux_switch(vup->pdev, state ? PORTMUX_HOST : PORTMUX_DEVICE);
+}
+
+static int vuport_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct vuport *vup;
+	int ret;
+
+	vup = devm_kzalloc(dev, sizeof(*vup), GFP_KERNEL);
+	if (!vup)
+		return -ENOMEM;
+
+	vup->regulator = devm_regulator_get_exclusive(dev,
+						      "regulator-usb-gpio");
+	if (IS_ERR_OR_NULL(vup->regulator))
+		return -EPROBE_DEFER;
+
+	vup->edev = extcon_get_extcon_dev("extcon-usb-gpio");
+	if (IS_ERR_OR_NULL(vup->edev))
+		return -EPROBE_DEFER;
+
+	vup->gpio_usb_mux = devm_gpiod_get_optional(dev,
+						    "usb_mux", GPIOD_ASIS);
+	if (IS_ERR(vup->gpio_usb_mux))
+		return PTR_ERR(vup->gpio_usb_mux);
+
+	vup->desc.dev = dev;
+	vup->desc.name = "intel-mux-gpio";
+	vup->desc.ops = &vuport_ops;
+	dev_set_drvdata(dev, vup);
+	vup->pdev = portmux_register(&vup->desc);
+	if (IS_ERR(vup->pdev))
+		return PTR_ERR(vup->pdev);
+
+	vup->nb.notifier_call = vuport_notifier;
+	ret = extcon_register_notifier(vup->edev, EXTCON_USB_HOST,
+				       &vup->nb);
+	if (ret < 0) {
+		portmux_unregister(vup->pdev);
+		return ret;
+	}
+
+	vuport_notifier(&vup->nb, 0, NULL);
+
+	return 0;
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+	struct vuport *vup;
+
+	vup = platform_get_drvdata(pdev);
+	extcon_unregister_notifier(vup->edev, EXTCON_USB_HOST, &vup->nb);
+	portmux_unregister(vup->pdev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * In case a micro A cable was plugged in while device was sleeping,
+ * we missed the interrupt. We need to poll usb id gpio when waking the
+ * driver to detect the missed event.
+ * We use 'complete' callback to give time to all extcon listeners to
+ * resume before we send new events.
+ */
+static void vuport_complete(struct device *dev)
+{
+	struct vuport *vup;
+
+	vup = dev_get_drvdata(dev);
+	vuport_notifier(&vup->nb, 0, NULL);
+}
+
+static const struct dev_pm_ops vuport_pm_ops = {
+	.complete = vuport_complete,
+};
+#endif
+
+static const struct platform_device_id vuport_platform_ids[] = {
+	{ .name = "intel-mux-gpio", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, vuport_platform_ids);
+
+static struct platform_driver vuport_driver = {
+	.driver = {
+		.name = "intel-mux-gpio",
+#ifdef CONFIG_PM_SLEEP
+		.pm = &vuport_pm_ops,
+#endif
+	},
+	.probe = vuport_probe,
+	.remove = vuport_remove,
+	.id_table = vuport_platform_ids,
+};
+
+module_platform_driver(vuport_driver);
+
+MODULE_AUTHOR("David Cohen <david.a.cohen@linux.intel.com>");
+MODULE_AUTHOR("Lu Baolu <baolu.lu@linux.intel.com>");
+MODULE_DESCRIPTION("Intel USB gpio mux driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v10 4/7] usb: mux: add driver for Intel drcfg controlled port mux
  2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
                   ` (2 preceding siblings ...)
  2016-06-02  1:37 ` [PATCH v10 3/7] usb: mux: add driver for Intel gpio controlled " Lu Baolu
@ 2016-06-02  1:37 ` Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 5/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver Lu Baolu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, Lu Baolu, Wu Hao

Several Intel PCHs and SOCs have an internal mux that is used to
share one USB port between device controller and host controller.
The mux is handled through the Dual Role Configuration Register.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 drivers/usb/mux/Kconfig               |   7 ++
 drivers/usb/mux/Makefile              |   1 +
 drivers/usb/mux/portmux-intel-drcfg.c | 162 ++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+)
 create mode 100644 drivers/usb/mux/portmux-intel-drcfg.c

diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
index 41b0f72..98b54d2 100644
--- a/drivers/usb/mux/Kconfig
+++ b/drivers/usb/mux/Kconfig
@@ -18,4 +18,11 @@ tristate "Intel dual role port mux controlled by GPIOs"
 	  Say Y here to enable support for Intel dual role port mux
 	  controlled by GPIOs.
 
+config INTEL_MUX_DRCFG
+	tristate "Intel dual role port mux controlled by register"
+	depends on X86
+	help
+	  Say Y here to enable support for Intel dual role port mux
+	  controlled by the Dual Role Configuration Register.
+
 endif
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
index 4eb5582..0f102b5 100644
--- a/drivers/usb/mux/Makefile
+++ b/drivers/usb/mux/Makefile
@@ -3,3 +3,4 @@
 #
 obj-$(CONFIG_USB_PORTMUX)		+= portmux-core.o
 obj-$(CONFIG_INTEL_MUX_GPIO)		+= portmux-intel-gpio.o
+obj-$(CONFIG_INTEL_MUX_DRCFG)		+= portmux-intel-drcfg.o
diff --git a/drivers/usb/mux/portmux-intel-drcfg.c b/drivers/usb/mux/portmux-intel-drcfg.c
new file mode 100644
index 0000000..67b11d2
--- /dev/null
+++ b/drivers/usb/mux/portmux-intel-drcfg.c
@@ -0,0 +1,162 @@
+/**
+ * intel-mux-drcfg.c - Driver for Intel USB mux via register
+ *
+ * Copyright (C) 2016 Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * 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/module.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/usb.h>
+#include <linux/usb/portmux.h>
+
+#define INTEL_MUX_CFG0		0x00
+#define CFG0_SW_IDPIN		BIT(20)
+#define CFG0_SW_IDPIN_EN	BIT(21)
+#define CFG0_SW_VBUS_VALID	BIT(24)
+
+struct intel_mux_drcfg {
+	struct portmux_desc desc;
+	struct device *dev;
+	void __iomem *regs;
+	struct portmux_dev *pdev;
+};
+
+static inline int intel_mux_drcfg_switch(struct device *dev, bool host)
+{
+	u32 data;
+	struct intel_mux_drcfg *mux;
+
+	mux = dev_get_drvdata(dev);
+
+	/* Check and set mux to SW controlled mode */
+	data = readl(mux->regs + INTEL_MUX_CFG0);
+	if (!(data & CFG0_SW_IDPIN_EN)) {
+		data |= CFG0_SW_IDPIN_EN;
+		writel(data, mux->regs + INTEL_MUX_CFG0);
+	}
+
+	/*
+	 * Configure CFG0 to switch the mux and VBUS_VALID bit is
+	 * required for device mode.
+	 */
+	data = readl(mux->regs + INTEL_MUX_CFG0);
+	if (host)
+		data &= ~(CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
+	else
+		data |= (CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
+	writel(data, mux->regs + INTEL_MUX_CFG0);
+
+	return 0;
+}
+
+static int intel_mux_drcfg_set_host(struct device *dev)
+{
+	dev_dbg(dev, "drcfg mux switch to HOST\n");
+
+	return intel_mux_drcfg_switch(dev, true);
+}
+
+static int intel_mux_drcfg_set_device(struct device *dev)
+{
+	dev_dbg(dev, "drcfg mux switch to DEVICE\n");
+
+	return intel_mux_drcfg_switch(dev, false);
+}
+
+static const struct portmux_ops drcfg_ops = {
+	.set_host_cb = intel_mux_drcfg_set_host,
+	.set_device_cb = intel_mux_drcfg_set_device,
+};
+
+static int intel_mux_drcfg_probe(struct platform_device *pdev)
+{
+	struct intel_mux_drcfg *mux;
+	struct device *dev = &pdev->dev;
+	u64 start, size;
+	int ret;
+
+	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
+
+	ret = device_property_read_u64(dev, "reg-start", &start);
+	ret |= device_property_read_u64(dev, "reg-size", &size);
+	if (ret)
+		return -ENODEV;
+
+	mux->regs = devm_ioremap_nocache(dev, start, size);
+	if (!mux->regs)
+		return -ENOMEM;
+
+	mux->desc.dev = dev;
+	mux->desc.name = "intel-mux-drcfg";
+	mux->desc.ops = &drcfg_ops;
+	dev_set_drvdata(dev, mux);
+	mux->pdev = portmux_register(&mux->desc);
+
+	return PTR_ERR_OR_ZERO(mux->pdev);
+}
+
+static int intel_mux_drcfg_remove(struct platform_device *pdev)
+{
+	struct intel_mux_drcfg *mux;
+
+	mux = platform_get_drvdata(pdev);
+	portmux_unregister(mux->pdev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * In case a micro A cable was plugged in while device was sleeping,
+ * we missed the interrupt. We need to poll usb id state when waking
+ * the driver to detect the missed event.
+ * We use 'complete' callback to give time to all extcon listeners to
+ * resume before we send new events.
+ */
+static void intel_mux_drcfg_complete(struct device *dev)
+{
+	struct intel_mux_drcfg *mux;
+
+	mux = dev_get_drvdata(dev);
+	portmux_complete(mux->pdev);
+}
+
+static const struct dev_pm_ops intel_mux_drcfg_pm_ops = {
+	.complete = intel_mux_drcfg_complete,
+};
+#endif
+
+static const struct platform_device_id intel_mux_drcfg_platform_ids[] = {
+	{ .name = "intel-mux-drcfg", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, intel_mux_drcfg_platform_ids);
+
+static struct platform_driver intel_mux_drcfg_driver = {
+	.probe		= intel_mux_drcfg_probe,
+	.remove		= intel_mux_drcfg_remove,
+	.driver		= {
+		.name	= "intel-mux-drcfg",
+#ifdef CONFIG_PM_SLEEP
+		.pm	= &intel_mux_drcfg_pm_ops,
+#endif
+	},
+	.id_table = intel_mux_drcfg_platform_ids,
+};
+
+module_platform_driver(intel_mux_drcfg_driver);
+
+MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
+MODULE_AUTHOR("Lu Baolu <baolu.lu@linux.intel.com>");
+MODULE_DESCRIPTION("Intel USB drcfg mux driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v10 5/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver
  2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
                   ` (3 preceding siblings ...)
  2016-06-02  1:37 ` [PATCH v10 4/7] usb: mux: add driver for Intel drcfg " Lu Baolu
@ 2016-06-02  1:37 ` Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Lu Baolu
  2016-06-02  1:37 ` [PATCH v10 7/7] MAINTAINERS: add maintainer entry for Intel USB dual role mux drivers Lu Baolu
  6 siblings, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, Lu Baolu

Some Intel platforms have an USB port mux controlled by GPIOs.
There's a single ACPI platform device that provides 1) USB ID
extcon device; 2) USB vbus regulator device; and 3) USB port
switch device. This MFD driver will split these 3 devices for
their respective drivers.

[baolu: removed .owner per platform_no_drv_owner.cocci]
Suggested-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/Kconfig        |  8 +++++
 drivers/mfd/Makefile       |  1 +
 drivers/mfd/intel-vuport.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+)
 create mode 100644 drivers/mfd/intel-vuport.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1bcf601..94affb3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1603,5 +1603,13 @@ config MFD_VEXPRESS_SYSREG
 	  System Registers are the platform configuration block
 	  on the ARM Ltd. Versatile Express board.
 
+config MFD_INTEL_VUPORT
+	tristate "Intel virtual USB port controller"
+	select MFD_CORE
+	depends on X86 && ACPI
+	help
+	  Say Y here to enable support for Intel's dual role port mux
+	  controlled by GPIOs.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 42a66e1..b86d4f9 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -205,3 +205,4 @@ intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)	+= intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
 obj-$(CONFIG_MFD_MT6397)	+= mt6397-core.o
+obj-$(CONFIG_MFD_INTEL_VUPORT)	+= intel-vuport.o
diff --git a/drivers/mfd/intel-vuport.c b/drivers/mfd/intel-vuport.c
new file mode 100644
index 0000000..eb27a8f
--- /dev/null
+++ b/drivers/mfd/intel-vuport.c
@@ -0,0 +1,86 @@
+/*
+ * MFD driver for Intel virtual USB port
+ *
+ * Copyright(c) 2016 Intel Corporation.
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * 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/acpi.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/mfd/core.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+
+/* ACPI GPIO Mappings */
+static const struct acpi_gpio_params id_gpio = { 0, 0, false };
+static const struct acpi_gpio_params vbus_gpio = { 1, 0, false };
+static const struct acpi_gpio_params mux_gpio = { 2, 0, false };
+static const struct acpi_gpio_mapping acpi_usb_gpios[] = {
+	{ "id-gpios", &id_gpio, 1 },
+	{ "gpio-gpios", &vbus_gpio, 1 },
+	{ "usb_mux-gpios", &mux_gpio, 1 },
+	{ },
+};
+
+static struct property_entry reg_properties[] = {
+	PROPERTY_ENTRY_STRING("supply-name", "regulator-usb-gpio"),
+	{ },
+};
+
+static const struct mfd_cell intel_vuport_mfd_cells[] = {
+	{ .name = "extcon-usb-gpio", },
+	{
+		.name = "reg-fixed-voltage",
+		.properties = reg_properties,
+	},
+	{ .name = "intel-mux-gpio", },
+};
+
+static int vuport_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev), acpi_usb_gpios);
+	if (ret)
+		return ret;
+
+	return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
+				intel_vuport_mfd_cells,
+				ARRAY_SIZE(intel_vuport_mfd_cells), NULL, 0,
+				NULL);
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+	mfd_remove_devices(&pdev->dev);
+	acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
+
+	return 0;
+}
+
+static struct acpi_device_id vuport_acpi_match[] = {
+	{ "INT3496" },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, vuport_acpi_match);
+
+static struct platform_driver vuport_driver = {
+	.driver = {
+		.name = "intel-vuport",
+		.acpi_match_table = ACPI_PTR(vuport_acpi_match),
+	},
+	.probe = vuport_probe,
+	.remove = vuport_remove,
+};
+
+module_platform_driver(vuport_driver);
+
+MODULE_AUTHOR("Lu Baolu <baolu.lu@linux.intel.com>");
+MODULE_DESCRIPTION("Intel virtual USB port");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
                   ` (4 preceding siblings ...)
  2016-06-02  1:37 ` [PATCH v10 5/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver Lu Baolu
@ 2016-06-02  1:37 ` Lu Baolu
  2016-06-08  4:45   ` Greg Kroah-Hartman
  2016-06-02  1:37 ` [PATCH v10 7/7] MAINTAINERS: add maintainer entry for Intel USB dual role mux drivers Lu Baolu
  6 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, 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.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 drivers/usb/host/pci-quirks.c    | 42 +++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci-ext-caps.h |  2 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 35af362..7e3194f 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,6 +16,8 @@
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/platform_device.h>
+
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
@@ -78,6 +80,9 @@
 #define USB_INTEL_USB3_PSSEN   0xD8
 #define USB_INTEL_USB3PRM      0xDC
 
+#define DEVICE_ID_INTEL_BROXTON_P_XHCI		0x5aa8
+#define DEVICE_ID_INTEL_CHERRYVIEW_XHCI		0x22b5
+
 /*
  * amd_chipset_gen values represent AMD different chipset generations
  */
@@ -956,6 +961,36 @@ void usb_disable_xhci_ports(struct pci_dev *xhci_pdev)
 }
 EXPORT_SYMBOL_GPL(usb_disable_xhci_ports);
 
+static void create_intel_usb_mux_device(struct pci_dev *xhci_pdev,
+					void __iomem *base)
+{
+	int ret;
+	struct platform_device *plat_dev;
+	struct property_entry pentry[] = {
+		PROPERTY_ENTRY_U64("reg-start",
+				   pci_resource_start(xhci_pdev, 0) + 0x80d8),
+		PROPERTY_ENTRY_U64("reg-size", 8),
+		{ },
+	};
+
+	if (!xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_INTEL_USB_MUX))
+		return;
+
+	plat_dev = platform_device_alloc("intel-mux-drcfg",
+					 PLATFORM_DEVID_NONE);
+	if (!plat_dev)
+		return;
+
+	plat_dev->dev.parent = &xhci_pdev->dev;
+	platform_device_add_properties(plat_dev, pentry);
+	ret = platform_device_add(plat_dev);
+	if (ret) {
+		dev_warn(&xhci_pdev->dev,
+			 "failed to create mux device with error %d", ret);
+		platform_device_put(plat_dev);
+	}
+}
+
 /**
  * PCI Quirks for xHCI.
  *
@@ -1022,9 +1057,14 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
 	writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
 
 hc_init:
-	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
 		usb_enable_intel_xhci_ports(pdev);
 
+		if (pdev->device == DEVICE_ID_INTEL_BROXTON_P_XHCI ||
+		    pdev->device == DEVICE_ID_INTEL_CHERRYVIEW_XHCI)
+			create_intel_usb_mux_device(pdev, base);
+	}
+
 	op_reg_base = base + XHCI_HC_LENGTH(readl(base));
 
 	/* Wait for the host controller to be ready before writing any
diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index e0244fb..e368ccb 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -51,6 +51,8 @@
 #define XHCI_EXT_CAPS_ROUTE	5
 /* IDs 6-9 reserved */
 #define XHCI_EXT_CAPS_DEBUG	10
+/* Vendor defined 192-255 */
+#define XHCI_EXT_CAPS_INTEL_USB_MUX	192
 /* USB Legacy Support Capability - section 7.1.1 */
 #define XHCI_HC_BIOS_OWNED	(1 << 16)
 #define XHCI_HC_OS_OWNED	(1 << 24)
-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v10 7/7] MAINTAINERS: add maintainer entry for Intel USB dual role mux drivers
  2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
                   ` (5 preceding siblings ...)
  2016-06-02  1:37 ` [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Lu Baolu
@ 2016-06-02  1:37 ` Lu Baolu
  6 siblings, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-02  1:37 UTC (permalink / raw)
  To: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown
  Cc: linux-usb, linux-kernel, Lu Baolu

Add a maintainer entry for Intel USB dual role mux drivers and
add myself as a maintainer.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7304d2e..56a98d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6111,6 +6111,16 @@ S:	Maintained
 F:	arch/x86/include/asm/pmc_core.h
 F:	drivers/platform/x86/intel_pmc_core*
 
+INTEL USB DUAL ROLE PORT MUX DRIVERS
+M:	Lu Baolu <baolu.lu@linux.intel.com>
+L:	linux-usb@vger.kernel.org
+S:	Supported
+F:	include/linux/usb/portmux.h
+F:	drivers/usb/mux/portmux-core.c
+F:	drivers/usb/mux/portmux-intel-gpio.c
+F:	drivers/usb/mux/portmux-intel-drcfg.c
+F:	drivers/mfd/intel-vuport.c
+
 IOC3 ETHERNET DRIVER
 M:	Ralf Baechle <ralf@linux-mips.org>
 L:	linux-mips@linux-mips.org
-- 
2.1.4

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-02  1:37 ` [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu
@ 2016-06-03  7:41   ` Peter Chen
  2016-06-03  8:16     ` Heikki Krogerus
  2016-06-03 16:06     ` Lu Baolu
  0 siblings, 2 replies; 51+ messages in thread
From: Peter Chen @ 2016-06-03  7:41 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> 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.
> 

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

> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@kernel.org>
> ---
>  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
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform
> index 5172a61..b994e4e 100644
> --- a/Documentation/ABI/testing/sysfs-bus-platform
> +++ b/Documentation/ABI/testing/sysfs-bus-platform
> @@ -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 <baolu.lu@linux.intel.com>
> +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.
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8689dcb..328916e 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -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
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index dca7856..9a92338 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -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/
> diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
> new file mode 100644
> index 0000000..ba778f2
> --- /dev/null
> +++ b/drivers/usb/mux/Kconfig
> @@ -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.
> diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
> new file mode 100644
> index 0000000..f85df92
> --- /dev/null
> +++ b/drivers/usb/mux/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for USB port mux drivers
> +#
> +obj-$(CONFIG_USB_PORTMUX)		+= portmux-core.o
> diff --git a/drivers/usb/mux/portmux-core.c b/drivers/usb/mux/portmux-core.c
> new file mode 100644
> index 0000000..75fbb45
> --- /dev/null
> +++ b/drivers/usb/mux/portmux-core.c
> @@ -0,0 +1,202 @@
> +/**
> + * portmux-core.c - USB Port Mux support
> + *
> + * Copyright (C) 2016 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@linux.intel.com>
> + *
> + * 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
> diff --git a/include/linux/usb/portmux.h b/include/linux/usb/portmux.h
> new file mode 100644
> index 0000000..093620a
> --- /dev/null
> +++ b/include/linux/usb/portmux.h
> @@ -0,0 +1,90 @@
> +/**
> + * portmux.h - USB Port Mux definitions
> + *
> + * Copyright (C) 2016 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@linux.intel.com>
> + *
> + * 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 */
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-03  7:41   ` Peter Chen
@ 2016-06-03  8:16     ` Heikki Krogerus
  2016-06-03  9:20       ` Peter Chen
  2016-06-03 16:06     ` Lu Baolu
  1 sibling, 1 reply; 51+ messages in thread
From: Heikki Krogerus @ 2016-06-03  8:16 UTC (permalink / raw)
  To: Peter Chen
  Cc: Lu Baolu, felipe.balbi, Mathias Nyman, Greg Kroah-Hartman,
	Lee Jones, Liam Girdwood, Mark Brown, linux-usb, linux-kernel,
	Roger Quadros

On Fri, Jun 03, 2016 at 03:41:13PM +0800, Peter Chen wrote:
> On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> > 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.
> > 
> 
> 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.


Thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-03  8:16     ` Heikki Krogerus
@ 2016-06-03  9:20       ` Peter Chen
  0 siblings, 0 replies; 51+ messages in thread
From: Peter Chen @ 2016-06-03  9:20 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Lu Baolu, felipe.balbi, Mathias Nyman, Greg Kroah-Hartman,
	Lee Jones, Liam Girdwood, Mark Brown, linux-usb, linux-kernel,
	Roger Quadros

On Fri, Jun 03, 2016 at 11:16:32AM +0300, Heikki Krogerus wrote:
> On Fri, Jun 03, 2016 at 03:41:13PM +0800, Peter Chen wrote:
> > On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> > > 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.
> > > 
> > 
> > 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.
> 

Well, DRD/OTG framework is mainly used for dual-role switch, no
matter what input is, you can use id pin, sysfs, or Type-C events.
It is long term target, currently, it only supports id pin.
In future, we can expend it to support more input events.
Microsoft also has similar framework for it:

https://msdn.microsoft.com/en-us/library/windows/hardware/dn957036(v=vs.85).aspx

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-03  7:41   ` Peter Chen
  2016-06-03  8:16     ` Heikki Krogerus
@ 2016-06-03 16:06     ` Lu Baolu
  2016-06-04  2:28       ` Peter Chen
  1 sibling, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-03 16:06 UTC (permalink / raw)
  To: Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi Peter,

On 06/03/2016 03:41 PM, Peter Chen wrote:
> On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
>> > 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.
>> > 
> Sorry to review this so late,

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

> 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.

Best regards,
Lu Baolu

> 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
>

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-03 16:06     ` Lu Baolu
@ 2016-06-04  2:28       ` Peter Chen
  2016-06-05  6:55         ` Lu Baolu
  0 siblings, 1 reply; 51+ messages in thread
From: Peter Chen @ 2016-06-04  2:28 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/03/2016 03:41 PM, Peter Chen wrote:
> > On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> >> > 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.
> >> > 
> > Sorry to review this so late,
> 
> It doesn't matter. Thanks for review. Comments are always welcome.:-)
> 
> > 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.
> 

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].

> 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.
> 

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:

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

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.

[1] https://lkml.org/lkml/2016/4/7/115
-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-04  2:28       ` Peter Chen
@ 2016-06-05  6:55         ` Lu Baolu
  2016-06-05  8:33           ` Jun Li
  2016-06-06  1:25           ` Peter Chen
  0 siblings, 2 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-05  6:55 UTC (permalink / raw)
  To: Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi Peter,

On 06/04/2016 10:28 AM, Peter Chen wrote:
> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
>>> 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.
>>
> 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.

> 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.


>> 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.
>>
> 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:
>
> +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.

>
> 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.

> - 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.

>
> 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.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-05  6:55         ` Lu Baolu
@ 2016-06-05  8:33           ` Jun Li
  2016-06-05  8:46             ` Lu Baolu
  2016-06-06  1:25           ` Peter Chen
  1 sibling, 1 reply; 51+ messages in thread
From: Jun Li @ 2016-06-05  8:33 UTC (permalink / raw)
  To: Lu Baolu, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Lu Baolu
> Sent: Sunday, June 05, 2016 2:56 PM
> To: Peter Chen <hzpeterchen@gmail.com>
> Cc: felipe.balbi@linux.intel.com; Mathias Nyman <mathias.nyman@intel.com>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Lee Jones
> <lee.jones@linaro.org>; Heikki Krogerus <heikki.krogerus@linux.intel.com>;
> Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Roger Quadros
> <rogerq@ti.com>
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi Peter,
> 
> On 06/04/2016 10:28 AM, Peter Chen wrote:
> > On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> >>> 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.
> >>
> > 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.
> 
> > 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.

> 
> >> 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.
> >>
> > 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:
> >
> > +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.
> 
> >
> > 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.
> 
> > - 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.

Those methods also can be mapped/added into OTG/dual role core framework.

> 
> >
> > 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
switch:) which should be covered by general OTG/dual role core(this
core framework is target to support one port with both host and
device capable by any HW/IC implementations).

Li Jun

> 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 majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-05  8:33           ` Jun Li
@ 2016-06-05  8:46             ` Lu Baolu
  2016-06-06  1:08               ` Jun Li
  2016-06-06  2:05               ` Peter Chen
  0 siblings, 2 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-05  8:46 UTC (permalink / raw)
  To: Jun Li, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi,

On 06/05/2016 04:33 PM, Jun Li wrote:
>> 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.
>> > 
>>> > > 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.

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-05  8:46             ` Lu Baolu
@ 2016-06-06  1:08               ` Jun Li
  2016-06-06  2:30                 ` Lu Baolu
  2016-06-06  2:05               ` Peter Chen
  1 sibling, 1 reply; 51+ messages in thread
From: Jun Li @ 2016-06-06  1:08 UTC (permalink / raw)
  To: Lu Baolu, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros



> -----Original Message-----
> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
> Sent: Sunday, June 05, 2016 4:47 PM
> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: felipe.balbi@linux.intel.com; Mathias Nyman <mathias.nyman@intel.com>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Lee Jones
> <lee.jones@linaro.org>; Heikki Krogerus <heikki.krogerus@linux.intel.com>;
> Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Roger Quadros
> <rogerq@ti.com>
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi,
> 
> On 06/05/2016 04:33 PM, Jun Li wrote:
> >> 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.
> >> >
> >>> > > 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.
> 
> 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.
  
> 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.

Li Jun
 
> 
> Best regards,
> Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-05  6:55         ` Lu Baolu
  2016-06-05  8:33           ` Jun Li
@ 2016-06-06  1:25           ` Peter Chen
  2016-06-06  3:04             ` Lu Baolu
  1 sibling, 1 reply; 51+ messages in thread
From: Peter Chen @ 2016-06-06  1:25 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/04/2016 10:28 AM, Peter Chen wrote:
> > On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> >>> 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.
> >>
> > 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.

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.

> 
> > 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.

> 
> >> 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.
> >>
> > 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:
> >
> > +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.
> 
> >
> > 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.
> 
> > - 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.
> 
> >
> > 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?

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-05  8:46             ` Lu Baolu
  2016-06-06  1:08               ` Jun Li
@ 2016-06-06  2:05               ` Peter Chen
  2016-06-06  2:45                 ` Lu Baolu
  1 sibling, 1 reply; 51+ messages in thread
From: Peter Chen @ 2016-06-06  2:05 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Jun Li, felipe.balbi, Mathias Nyman, Greg Kroah-Hartman,
	Lee Jones, Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
> Hi,
> 
> On 06/05/2016 04:33 PM, Jun Li wrote:
> >> 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.
> >> > 
> >>> > > 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.
> 
> 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.

> 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.

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  1:08               ` Jun Li
@ 2016-06-06  2:30                 ` Lu Baolu
  0 siblings, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-06  2:30 UTC (permalink / raw)
  To: Jun Li, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi Jun,

On 06/06/2016 09:08 AM, Jun Li wrote:
>
>> -----Original Message-----
>> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
>> Sent: Sunday, June 05, 2016 4:47 PM
>> To: Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
>> Cc: felipe.balbi@linux.intel.com; Mathias Nyman <mathias.nyman@intel.com>;
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Lee Jones
>> <lee.jones@linaro.org>; Heikki Krogerus <heikki.krogerus@linux.intel.com>;
>> Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
>> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Roger Quadros
>> <rogerq@ti.com>
>> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
>> mux
>>
>> Hi,
>>
>> On 06/05/2016 04:33 PM, Jun Li wrote:
>>>> 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.
>>>>>
>>>>>>> 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.
>> 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.

Intel's USB controllers aren't dual role capable, and we don't
need any dual role capable drivers either. It's just two USB
controllers which shares a single port and it has a physical
port mux device which could control the route of lines. We
only need drivers for the port mux devices.

>   
>> 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.

As I said above, we don't have any dual role capable controllers.
We don't need to have any dual role capable host or gadget drivers
to register into OTG core. We only provide drivers for port mux
devices, just like what we already have for phy or regulator
devices.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  2:05               ` Peter Chen
@ 2016-06-06  2:45                 ` Lu Baolu
  2016-06-06  6:48                   ` Peter Chen
  0 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-06  2:45 UTC (permalink / raw)
  To: Peter Chen
  Cc: Jun Li, felipe.balbi, Mathias Nyman, Greg Kroah-Hartman,
	Lee Jones, Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi Peter,

On 06/06/2016 10:05 AM, Peter Chen wrote:
> On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
>> Hi,
>>
>> On 06/05/2016 04:33 PM, Jun Li wrote:
>>>> 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.
>>>>>
>>>>>>> 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.
>> 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.

>
>> 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 ...

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  1:25           ` Peter Chen
@ 2016-06-06  3:04             ` Lu Baolu
  2016-06-06  7:02               ` Roger Quadros
  2016-06-06  7:02               ` Peter Chen
  0 siblings, 2 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-06  3:04 UTC (permalink / raw)
  To: Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi Peter,

On 06/06/2016 09:25 AM, Peter Chen wrote:
> On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
>> Hi Peter,
>>
>> On 06/04/2016 10:28 AM, Peter Chen wrote:
>>> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
>>>>> 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.
>>>>
>>> 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.
> 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.

>
>>> 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>

>> 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?
>

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.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  2:45                 ` Lu Baolu
@ 2016-06-06  6:48                   ` Peter Chen
  0 siblings, 0 replies; 51+ messages in thread
From: Peter Chen @ 2016-06-06  6:48 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Jun Li, felipe.balbi, Mathias Nyman, Greg Kroah-Hartman,
	Lee Jones, Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

On Mon, Jun 06, 2016 at 10:45:34AM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/06/2016 10:05 AM, Peter Chen wrote:
> > On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
> >> Hi,
> >>
> >> On 06/05/2016 04:33 PM, Jun Li wrote:
> >>>> 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.
> >>>>>
> >>>>>>> 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.
> >> 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.
> 

In your case, the related APIs are NULL.

> >
> >> 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 ...
> 

But, in your framework, it will finish the role switch like OTG
framework does.

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  3:04             ` Lu Baolu
@ 2016-06-06  7:02               ` Roger Quadros
  2016-06-07  3:03                 ` Jun Li
  2016-06-06  7:02               ` Peter Chen
  1 sibling, 1 reply; 51+ messages in thread
From: Roger Quadros @ 2016-06-06  7:02 UTC (permalink / raw)
  To: Lu Baolu, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

Hi,

On 06/06/16 06:04, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/06/2016 09:25 AM, Peter Chen wrote:
>> On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
>>> Hi Peter,
>>>
>>> On 06/04/2016 10:28 AM, Peter Chen wrote:
>>>> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
>>>>>> 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.
>>>>>
>>>> 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.
>> 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.

I agree with Lu here.

Intel platforms seem to actually have a Mux device and we need a device driver for that.
OTG/dual-role core cannot directly handle the Mux device.

The Mux device can be used not only for dual-role but for other things so we can't
force it to use just OTG/dual-role.

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.

> 
>>
>>>> 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>
> 
>>> 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?
>>
> 
> 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.
> 

--
cheers,
-roger

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  3:04             ` Lu Baolu
  2016-06-06  7:02               ` Roger Quadros
@ 2016-06-06  7:02               ` Peter Chen
  2016-06-06  7:35                 ` Lu Baolu
  1 sibling, 1 reply; 51+ messages in thread
From: Peter Chen @ 2016-06-06  7:02 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

On Mon, Jun 06, 2016 at 11:04:48AM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/06/2016 09:25 AM, Peter Chen wrote:
> > On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
> >> Hi Peter,
> >>
> >> On 06/04/2016 10:28 AM, Peter Chen wrote:
> >>> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> >>>>> 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.
> >>>>
> >>> 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.
> > 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.
> 

<snip>

> 
> >> 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?
> >
> 
> 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.

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  7:02               ` Peter Chen
@ 2016-06-06  7:35                 ` Lu Baolu
  0 siblings, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-06  7:35 UTC (permalink / raw)
  To: Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel, Roger Quadros

Hi Peter,

On 06/06/2016 03:02 PM, Peter Chen wrote:
>>>> > >> 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?
>>> > >
>> > 
>> > 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.

So we need to make it clear in Documentation/ABI/testing/sysfs-bus-platform.

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

I agree that we should use dual-role framework for role switch. Actually,
my code doesn't do this work. It only adds a generic framework for port
mux device and two mux device drivers used in Intel platform.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-06  7:02               ` Roger Quadros
@ 2016-06-07  3:03                 ` Jun Li
  2016-06-07  6:27                   ` Lu Baolu
  2016-06-07  9:53                   ` Lu Baolu
  0 siblings, 2 replies; 51+ messages in thread
From: Jun Li @ 2016-06-07  3:03 UTC (permalink / raw)
  To: Roger Quadros, Lu Baolu, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

Hi Roger

>  
> 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.

> 
> 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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07  3:03                 ` Jun Li
@ 2016-06-07  6:27                   ` Lu Baolu
  2016-06-07  6:34                     ` Jun Li
  2016-06-07  9:53                   ` Lu Baolu
  1 sibling, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-07  6:27 UTC (permalink / raw)
  To: Jun Li, Roger Quadros, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

Hi Jun,

On 06/07/2016 11:03 AM, Jun Li wrote:
> Hi Roger
>
>>  
>> 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?

Best regards,
Lu Baolu

>
>> 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
>

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07  6:27                   ` Lu Baolu
@ 2016-06-07  6:34                     ` Jun Li
  2016-06-07  9:27                       ` Lu Baolu
  0 siblings, 1 reply; 51+ messages in thread
From: Jun Li @ 2016-06-07  6:34 UTC (permalink / raw)
  To: Lu Baolu, Roger Quadros, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

Hi Baolu

> -----Original Message-----
> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
> Sent: Tuesday, June 07, 2016 2:27 PM
> To: Jun Li <jun.li@nxp.com>; Roger Quadros <rogerq@ti.com>; Peter Chen
> <hzpeterchen@gmail.com>
> Cc: felipe.balbi@linux.intel.com; Mathias Nyman <mathias.nyman@intel.com>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Lee Jones
> <lee.jones@linaro.org>; Heikki Krogerus <heikki.krogerus@linux.intel.com>;
> Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi Jun,
> 
> On 06/07/2016 11:03 AM, Jun Li wrote:
> > Hi Roger
> >
> >>
> >> 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?

Host and peripheral.

> 
> Best regards,
> Lu Baolu
> 
> >
> >> 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
> >

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07  6:34                     ` Jun Li
@ 2016-06-07  9:27                       ` Lu Baolu
  2016-06-07 12:49                         ` Roger Quadros
  0 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-07  9:27 UTC (permalink / raw)
  To: Jun Li, Roger Quadros, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

Hi,

On 06/07/2016 02:34 PM, Jun Li wrote:
>> > On 06/07/2016 11:03 AM, Jun Li wrote:
>>> > > Hi Roger
>>> > >
>>>> > >>
>>>> > >> 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?
> 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".

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07  3:03                 ` Jun Li
  2016-06-07  6:27                   ` Lu Baolu
@ 2016-06-07  9:53                   ` Lu Baolu
  2016-06-07 12:58                     ` Roger Quadros
  1 sibling, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-07  9:53 UTC (permalink / raw)
  To: Jun Li, Roger Quadros, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

Hi,

On 06/07/2016 11:03 AM, Jun Li wrote:
> Hi Roger
>
>>  
>> 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.
>
>> 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?

> 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.

Why?

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07  9:27                       ` Lu Baolu
@ 2016-06-07 12:49                         ` Roger Quadros
  0 siblings, 0 replies; 51+ messages in thread
From: Roger Quadros @ 2016-06-07 12:49 UTC (permalink / raw)
  To: Lu Baolu, Jun Li, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

On 07/06/16 12:27, Lu Baolu wrote:
> Hi,
> 
> On 06/07/2016 02:34 PM, Jun Li wrote:
>>>> On 06/07/2016 11:03 AM, Jun Li wrote:
>>>>>> Hi Roger
>>>>>>
>>>>>>>>
>>>>>>>> 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?
>> 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.

DRD means dual role with zero OTG features, which is similar to just host and
peripheral mode.

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

What other combination you foresee?

cheers,
-roger

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07  9:53                   ` Lu Baolu
@ 2016-06-07 12:58                     ` Roger Quadros
  2016-06-07 13:04                       ` Felipe Balbi
  0 siblings, 1 reply; 51+ messages in thread
From: Roger Quadros @ 2016-06-07 12:58 UTC (permalink / raw)
  To: Lu Baolu, Jun Li, Peter Chen
  Cc: felipe.balbi, Mathias Nyman, Greg Kroah-Hartman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, Mark Brown, linux-usb,
	linux-kernel

On 07/06/16 12:53, Lu Baolu wrote:
> Hi,
> 
> On 06/07/2016 11:03 AM, Jun Li wrote:
>> Hi Roger
>>
>>>  
>>> 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.
>>
>>> 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?
> 
>> 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.

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?

cheers,
-roger

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07 12:58                     ` Roger Quadros
@ 2016-06-07 13:04                       ` Felipe Balbi
  2016-06-07 14:02                         ` Roger Quadros
  0 siblings, 1 reply; 51+ messages in thread
From: Felipe Balbi @ 2016-06-07 13:04 UTC (permalink / raw)
  To: Roger Quadros, Lu Baolu, Jun Li, Peter Chen
  Cc: Mathias Nyman, Greg Kroah-Hartman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
>> 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.

> 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.

> 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.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07 13:04                       ` Felipe Balbi
@ 2016-06-07 14:02                         ` Roger Quadros
  2016-06-07 15:05                           ` Felipe Balbi
  0 siblings, 1 reply; 51+ messages in thread
From: Roger Quadros @ 2016-06-07 14:02 UTC (permalink / raw)
  To: Felipe Balbi, Lu Baolu, Jun Li, Peter Chen
  Cc: Mathias Nyman, Greg Kroah-Hartman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 3042 bytes --]

On 07/06/16 16:04, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>> 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.

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.

> 
>> 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.
> 
>> 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?

> 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?

So for Intel port-mux case it seems that OTG/dual-role is overcomplicated
and I wouldn't force you to use it. It is upto Peter to decide how he wants
dual-role users to behave.

cheers,
-roger


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07 14:02                         ` Roger Quadros
@ 2016-06-07 15:05                           ` Felipe Balbi
  2016-06-08  3:04                             ` Jun Li
                                               ` (2 more replies)
  0 siblings, 3 replies; 51+ messages in thread
From: Felipe Balbi @ 2016-06-07 15:05 UTC (permalink / raw)
  To: Roger Quadros, Lu Baolu, Jun Li, Peter Chen
  Cc: Mathias Nyman, Greg Kroah-Hartman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

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


Hi,

Roger Quadros <rogerq@ti.com> writes:
>> 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.

> 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.

>>> 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.
>> 
>>> 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.

>> 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.    

> 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.

> 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.

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

dwc3 itself.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07 15:05                           ` Felipe Balbi
@ 2016-06-08  3:04                             ` Jun Li
       [not found]                               ` <5757A8CB.90402@linux.intel.com>
  2016-06-08  6:25                             ` Peter Chen
  2016-06-08  7:58                             ` Roger Quadros
  2 siblings, 1 reply; 51+ messages in thread
From: Jun Li @ 2016-06-08  3:04 UTC (permalink / raw)
  To: Felipe Balbi, Roger Quadros, Lu Baolu, Peter Chen
  Cc: Mathias Nyman, Greg Kroah-Hartman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

Hi,
> -----Original Message-----
> From: Felipe Balbi [mailto:felipe.balbi@linux.intel.com]
> Sent: Tuesday, June 07, 2016 11:05 PM
> To: Roger Quadros <rogerq@ti.com>; Lu Baolu <baolu.lu@linux.intel.com>;
> Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: Mathias Nyman <mathias.nyman@intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Lee Jones <lee.jones@linaro.org>; Heikki
> Krogerus <heikki.krogerus@linux.intel.com>; Liam Girdwood
> <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; linux-
> usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
> >> 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.
> 
> > 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.

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

Li Jun
> 
> If we're discussing userspace ABI here, there's something very wrong with
> OTG/DRD layer design.
> 
> >>> 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.
> >>
> >>> 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.
> 
> >> 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.
> 
> > 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.
> 
> > 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.
> 
> > If so then who will deal with start/stop of the controllers then?
> 
> dwc3 itself.
> 
> --
> balbi

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 1/7] regulator: fixed: add support for ACPI interface
  2016-06-02  1:37 ` [PATCH v10 1/7] regulator: fixed: add support for ACPI interface Lu Baolu
@ 2016-06-08  4:42   ` Greg Kroah-Hartman
  2016-06-08 13:43     ` Mark Brown
  2016-06-09  2:43     ` Lu Baolu
  0 siblings, 2 replies; 51+ messages in thread
From: Greg Kroah-Hartman @ 2016-06-08  4:42 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

On Thu, Jun 02, 2016 at 09:37:23AM +0800, Lu Baolu wrote:
> 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 <baolu.lu@linux.intel.com>
> ---
>  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 :(

greg k-h

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-02  1:37 ` [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Lu Baolu
@ 2016-06-08  4:45   ` Greg Kroah-Hartman
  2016-06-08  7:56     ` Lu Baolu
  0 siblings, 1 reply; 51+ messages in thread
From: Greg Kroah-Hartman @ 2016-06-08  4:45 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
> 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...

greg k-h

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
       [not found]                               ` <5757A8CB.90402@linux.intel.com>
@ 2016-06-08  6:20                                 ` Jun Li
  0 siblings, 0 replies; 51+ messages in thread
From: Jun Li @ 2016-06-08  6:20 UTC (permalink / raw)
  To: Lu Baolu, Felipe Balbi, Roger Quadros, Peter Chen
  Cc: Mathias Nyman, Greg Kroah-Hartman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

Hi, Baolu

From: Lu Baolu [mailto:baolu.lu@linux.intel.com] 
Sent: Wednesday, June 08, 2016 1:11 PM
To: Jun Li <jun.li@nxp.com>; Felipe Balbi <felipe.balbi@linux.intel.com>; Roger Quadros <rogerq@ti.com>; Peter Chen <hzpeterchen@gmail.com>
Cc: Mathias Nyman <mathias.nyman@intel.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Lee Jones <lee.jones@linaro.org>; Heikki Krogerus <heikki.krogerus@linux.intel.com>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Hi,

[I have to resend my reply. The previous reply was failed to deliver
to usb mailing list. Sorry for inconvenience.]
On 06/08/2016 11:04 AM, Jun Li wrote:
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.
> 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.

Confused, then what's the purpose of it? How to use it?
Below is all about it in document, it's seems telling me can do that,
but you say no:)

+What:          /sys/bus/platform/devices/.../portmux.N/name
+               /sys/bus/platform/devices/.../portmux.N/state
+Date:          April 2016
+Contact:       Lu Baolu <baolu.lu@linux.intel.com>
+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.

> Best regards,
> Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07 15:05                           ` Felipe Balbi
  2016-06-08  3:04                             ` Jun Li
@ 2016-06-08  6:25                             ` Peter Chen
  2016-06-08  7:58                             ` Roger Quadros
  2 siblings, 0 replies; 51+ messages in thread
From: Peter Chen @ 2016-06-08  6:25 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Roger Quadros, Lu Baolu, Jun Li, Mathias Nyman,
	Greg Kroah-Hartman, Lee Jones, Heikki Krogerus, Liam Girdwood,
	Mark Brown, linux-usb, linux-kernel

On Tue, Jun 07, 2016 at 06:05:25PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
> >> 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.
> 

Indeed, some centralized IP drivers like MUSB, chipidea, dwc3 do not
need this framework for role switch. But there are some common stuffs,
like OTG FSM (fully/simplified), manage roles and sysfs for role switch,
these things can be in a framework, the purpose of this framework is
easy for dual-role switch function.

Besides, when the host and device driver are in different folders for
platform, eg host/ and gadget/udc/, a role switch driver is needed if 
we need dual role function.

Recently, the dual-role function is more and more common for USB, a
framework can avoid duplicated work and let switch be standardized.

> > 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.

Currently, there are some use cases which need to switch role on the
fly (will be more for type-c in future), a sysfs for role switch is
necessary.

-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-08  4:45   ` Greg Kroah-Hartman
@ 2016-06-08  7:56     ` Lu Baolu
  2016-06-08 15:45       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-08  7:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

Hi Greg,

On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
>> 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).

> 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.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
  2016-06-07 15:05                           ` Felipe Balbi
  2016-06-08  3:04                             ` Jun Li
  2016-06-08  6:25                             ` Peter Chen
@ 2016-06-08  7:58                             ` Roger Quadros
  2 siblings, 0 replies; 51+ messages in thread
From: Roger Quadros @ 2016-06-08  7:58 UTC (permalink / raw)
  To: Felipe Balbi, Lu Baolu, Jun Li, Peter Chen
  Cc: Mathias Nyman, Greg Kroah-Hartman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 4950 bytes --]

On 07/06/16 18:05, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>> 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.
> 
>> 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.

Not really. There can be a need for user space application to control the
port role. Consider Apple carplay for instance or even full OTG support
which has user selectable 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?
>>>
>>> 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.
>>>
>>>> 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.
> 
>>> 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?

What about B_IDLE state? We don't want either peripheral or host to run when no cable
is inserted.

Have you tested if it works? I'd be happy to test if you can prepare a patch
to get dual-role working on dwc3 without the 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.    

As a user you just need to do this

        reg = read(OSTS);
        dwc->otg->fsm.id = !!(reg & STS_ID);
        dwc->otg->fsm.b_sess_vld = !!(reg &STS_BSESVLD);
        usb_otg_sync_inputs(dwc->otg);

And the layer does the rest.

But as I said earlier. I have absolutely no issues if dwc3 doesn't use that layer
as long as dual-role works on our platforms.

> 
>> 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.

OK.
> 
>> 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.

Just code is not enough. We need to know if it works :).

> 
>> If so then who will deal with start/stop of the controllers then?
> 
> dwc3 itself.
> 
OK.

cheers,
-roger


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 1/7] regulator: fixed: add support for ACPI interface
  2016-06-08  4:42   ` Greg Kroah-Hartman
@ 2016-06-08 13:43     ` Mark Brown
  2016-06-08 15:46       ` Greg Kroah-Hartman
  2016-06-09  2:43     ` Lu Baolu
  1 sibling, 1 reply; 51+ messages in thread
From: Mark Brown @ 2016-06-08 13:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lu Baolu, felipe.balbi, Mathias Nyman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, linux-usb, linux-kernel

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

On Tue, Jun 07, 2016 at 09:42:48PM -0700, Greg Kroah-Hartman wrote:
> On Thu, Jun 02, 2016 at 09:37:23AM +0800, Lu Baolu wrote:

> > 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.

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-08  7:56     ` Lu Baolu
@ 2016-06-08 15:45       ` Greg Kroah-Hartman
  2016-06-09  2:39         ` Lu Baolu
  0 siblings, 1 reply; 51+ messages in thread
From: Greg Kroah-Hartman @ 2016-06-08 15:45 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote:
> Hi Greg,
> 
> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
> > On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
> >> 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).
> 
> > 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...

greg k-h

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 1/7] regulator: fixed: add support for ACPI interface
  2016-06-08 13:43     ` Mark Brown
@ 2016-06-08 15:46       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 51+ messages in thread
From: Greg Kroah-Hartman @ 2016-06-08 15:46 UTC (permalink / raw)
  To: Mark Brown
  Cc: Lu Baolu, felipe.balbi, Mathias Nyman, Lee Jones,
	Heikki Krogerus, Liam Girdwood, linux-usb, linux-kernel

On Wed, Jun 08, 2016 at 02:43:27PM +0100, Mark Brown wrote:
> On Tue, Jun 07, 2016 at 09:42:48PM -0700, Greg Kroah-Hartman wrote:
> > On Thu, Jun 02, 2016 at 09:37:23AM +0800, Lu Baolu wrote:
> 
> > > 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.
> 
> 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.

I wouldn't consider it, my response was to the developer of the patch,
sorry for the confusion,

greg k-h

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-08 15:45       ` Greg Kroah-Hartman
@ 2016-06-09  2:39         ` Lu Baolu
  2016-06-16  0:27           ` Lu Baolu
  0 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-09  2:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

Hi Greg,

On 06/08/2016 11:45 PM, Greg Kroah-Hartman wrote:
> On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote:
>> Hi Greg,
>>
>> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
>>> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
>>>> 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).:)
>>
>>> 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...
>

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. :)

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 1/7] regulator: fixed: add support for ACPI interface
  2016-06-08  4:42   ` Greg Kroah-Hartman
  2016-06-08 13:43     ` Mark Brown
@ 2016-06-09  2:43     ` Lu Baolu
  1 sibling, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-09  2:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

Hi Felipe and Heikki,

On 06/08/2016 12:42 PM, Greg Kroah-Hartman wrote:
> On Thu, Jun 02, 2016 at 09:37:23AM +0800, Lu Baolu wrote:
>> 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 <baolu.lu@linux.intel.com>
>> ---
>>  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 :(
>

You have reviewed the whole patches in this series. Are you willing
to ack them?

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-09  2:39         ` Lu Baolu
@ 2016-06-16  0:27           ` Lu Baolu
  2016-06-18  0:58             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 51+ messages in thread
From: Lu Baolu @ 2016-06-16  0:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

Hi Greg,

On 06/09/2016 10:39 AM, Lu Baolu wrote:
> Hi Greg,
>
> On 06/08/2016 11:45 PM, Greg Kroah-Hartman wrote:
>> On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote:
>>> Hi Greg,
>>>
>>> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
>>>> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
>>>>> 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).:)
>>>
>>>> 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...
>>
> 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.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-16  0:27           ` Lu Baolu
@ 2016-06-18  0:58             ` Greg Kroah-Hartman
  2016-06-19  9:52               ` Lu Baolu
  0 siblings, 1 reply; 51+ messages in thread
From: Greg Kroah-Hartman @ 2016-06-18  0:58 UTC (permalink / raw)
  To: Lu Baolu
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

On Thu, Jun 16, 2016 at 08:27:41AM +0800, Lu Baolu wrote:
> Hi Greg,
> 
> On 06/09/2016 10:39 AM, Lu Baolu wrote:
> > Hi Greg,
> >
> > On 06/08/2016 11:45 PM, Greg Kroah-Hartman wrote:
> >> On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote:
> >>> Hi Greg,
> >>>
> >>> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
> >>>> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
> >>>>> 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).:)
> >>>
> >>>> 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...
> >>
> > 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?

> 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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
  2016-06-18  0:58             ` Greg Kroah-Hartman
@ 2016-06-19  9:52               ` Lu Baolu
  0 siblings, 0 replies; 51+ messages in thread
From: Lu Baolu @ 2016-06-19  9:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: felipe.balbi, Mathias Nyman, Lee Jones, Heikki Krogerus,
	Liam Girdwood, Mark Brown, linux-usb, linux-kernel

Hi Greg,

On 06/18/2016 08:58 AM, Greg Kroah-Hartman wrote:
> On Thu, Jun 16, 2016 at 08:27:41AM +0800, Lu Baolu wrote:
>> Hi Greg,
>>
>> On 06/09/2016 10:39 AM, Lu Baolu wrote:
>>> Hi Greg,
>>>
>>> On 06/08/2016 11:45 PM, Greg Kroah-Hartman wrote:
>>>> On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote:
>>>>> Hi Greg,
>>>>>
>>>>> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
>>>>>> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
>>>>>>> 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).:)
>>>>>
>>>>>> 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...
>>>>
>>> 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?

I see now. Thanks for your reply.

Best regards,
Lu Baolu

>
>> 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
>

^ permalink raw reply	[flat|nested] 51+ messages in thread

end of thread, other threads:[~2016-06-19  9:53 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-02  1:37 [PATCH v10 0/7] usb: add support for Intel dual role port mux Lu Baolu
2016-06-02  1:37 ` [PATCH v10 1/7] regulator: fixed: add support for ACPI interface Lu Baolu
2016-06-08  4:42   ` Greg Kroah-Hartman
2016-06-08 13:43     ` Mark Brown
2016-06-08 15:46       ` Greg Kroah-Hartman
2016-06-09  2:43     ` Lu Baolu
2016-06-02  1:37 ` [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu
2016-06-03  7:41   ` Peter Chen
2016-06-03  8:16     ` Heikki Krogerus
2016-06-03  9:20       ` Peter Chen
2016-06-03 16:06     ` Lu Baolu
2016-06-04  2:28       ` Peter Chen
2016-06-05  6:55         ` Lu Baolu
2016-06-05  8:33           ` Jun Li
2016-06-05  8:46             ` Lu Baolu
2016-06-06  1:08               ` Jun Li
2016-06-06  2:30                 ` Lu Baolu
2016-06-06  2:05               ` Peter Chen
2016-06-06  2:45                 ` Lu Baolu
2016-06-06  6:48                   ` Peter Chen
2016-06-06  1:25           ` Peter Chen
2016-06-06  3:04             ` Lu Baolu
2016-06-06  7:02               ` Roger Quadros
2016-06-07  3:03                 ` Jun Li
2016-06-07  6:27                   ` Lu Baolu
2016-06-07  6:34                     ` Jun Li
2016-06-07  9:27                       ` Lu Baolu
2016-06-07 12:49                         ` Roger Quadros
2016-06-07  9:53                   ` Lu Baolu
2016-06-07 12:58                     ` Roger Quadros
2016-06-07 13:04                       ` Felipe Balbi
2016-06-07 14:02                         ` Roger Quadros
2016-06-07 15:05                           ` Felipe Balbi
2016-06-08  3:04                             ` Jun Li
     [not found]                               ` <5757A8CB.90402@linux.intel.com>
2016-06-08  6:20                                 ` Jun Li
2016-06-08  6:25                             ` Peter Chen
2016-06-08  7:58                             ` Roger Quadros
2016-06-06  7:02               ` Peter Chen
2016-06-06  7:35                 ` Lu Baolu
2016-06-02  1:37 ` [PATCH v10 3/7] usb: mux: add driver for Intel gpio controlled " Lu Baolu
2016-06-02  1:37 ` [PATCH v10 4/7] usb: mux: add driver for Intel drcfg " Lu Baolu
2016-06-02  1:37 ` [PATCH v10 5/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver Lu Baolu
2016-06-02  1:37 ` [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Lu Baolu
2016-06-08  4:45   ` Greg Kroah-Hartman
2016-06-08  7:56     ` Lu Baolu
2016-06-08 15:45       ` Greg Kroah-Hartman
2016-06-09  2:39         ` Lu Baolu
2016-06-16  0:27           ` Lu Baolu
2016-06-18  0:58             ` Greg Kroah-Hartman
2016-06-19  9:52               ` Lu Baolu
2016-06-02  1:37 ` [PATCH v10 7/7] MAINTAINERS: add maintainer entry for Intel USB dual role mux drivers Lu Baolu

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).