All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-14  1:42 ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  1:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-arm-msm, linux-kernel, linux-usb,
	robh+dt, MyungJoo Ham, Chanwoo Choi, devicetree

On the db410c 96boards platform we have a TC7USB40MU[1] on the
board to mux the D+/D- lines from the SoC between a micro usb
"device" port and a USB hub for "host" roles. Upon a role switch,
we need to change this mux to forward the D+/D- lines to either
the port or the hub. Therefore, introduce a driver for this
device that intercepts extcon USB_HOST events and logically
asserts a gpio to mux the "host" D+/D- lines when a host cable is
attached. When the cable goes away, it will logically deassert
the gpio and mux the "device" lines.

[1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---

Should I make the extcon part optional? I could see a case where there are two
"OTG" ports connected to the mux (or two hubs), and for some reason the
software may want to mux between them at runtime. If we mandate an extcon,
that won't be possible to support. Perhaps it would be better to have
the node, but connect it to the usb controller with a phandle (maybe of_graph
endpoints would be useful too) so that when the controller wants to mux over
a port it can do so.

Muxing the ports this way based on ID cable is pretty much a software
design decision. We could mux the ports during the role switch, and the
role switch can be entirely userspace driven with the chipidea controller
that I'm using (see the role switching support in the "role" file for
debugfs support in that driver). So extcon cables don't come into the picture
in that scenario.

 .../devicetree/bindings/usb/toshiba,tc7usb40mu.txt |  34 +++++++
 drivers/usb/misc/Kconfig                           |   9 ++
 drivers/usb/misc/Makefile                          |   1 +
 drivers/usb/misc/tc7usb40mu.c                      | 107 +++++++++++++++++++++
 4 files changed, 151 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
 create mode 100644 drivers/usb/misc/tc7usb40mu.c

diff --git a/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
new file mode 100644
index 000000000000..18e6607408fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
@@ -0,0 +1,34 @@
+Toshiba TC7USB40MU
+
+This device muxes USB D+/D- lines between two outputs called 1D+/1D- and 2D+/2D-.
+When the switch pin is asserted, we mux out 2D+/2D-, and when it's deasserted we
+select 1D+/1D-.
+
+This can be used to mux USB D+/D- lines between a USB hub and an OTG port.
+
+PROPERTIES
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "toshiba,tc7usb40mu"
+
+- switch-gpios:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: Should contain the gpio used to toggle the switch. Logically
+                asserting the gpio will cause the device to mux the "host"
+                D+/D- lines instead of the "device" lines.
+
+- extcon:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: Should contain the extcon device for USB_HOST cable events
+
+Example:
+
+	usb-switch {
+		compatible = "toshiba,tc7usb40mu";
+		switch-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
+		extcon = <&usb_id>;
+	};
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 47b357760afc..3da568c751d2 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -46,6 +46,15 @@ config USB_SEVSEG
 	  To compile this driver as a module, choose M here: the
 	  module will be called usbsevseg.
 
+config USB_TC7USB40MU
+	tristate "TC7USB40MU USB mux support"
+	depends on (GPIOLIB && EXTCON) || COMPILE_TEST
+	help
+	  Say Y here if you have a TC7USB40MU by Toshiba. If a USB ID cable is
+	  present, a gpio will be asserted to mux out "host" D+/D- lines and when
+	  the ID cable is removed, a gpio will be deasserted to mux out "device"
+	  D+/D- lines.
+
 config USB_RIO500
 	tristate "USB Diamond Rio500 support"
 	help
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 3d1992750da4..d8f9ad1dee13 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
 obj-$(CONFIG_USB_RIO500)		+= rio500.o
 obj-$(CONFIG_USB_TEST)			+= usbtest.o
 obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
+obj-$(CONFIG_USB_TC7USB40MU)		+= tc7usb40mu.o
 obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
 obj-$(CONFIG_USB_USS720)		+= uss720.o
 obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
diff --git a/drivers/usb/misc/tc7usb40mu.c b/drivers/usb/misc/tc7usb40mu.c
new file mode 100644
index 000000000000..9edcfe577ae4
--- /dev/null
+++ b/drivers/usb/misc/tc7usb40mu.c
@@ -0,0 +1,107 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/extcon.h>
+#include <linux/gpio/consumer.h>
+#include <linux/notifier.h>
+
+struct tc7usb40mu_drv {
+	struct gpio_desc *gpio;
+	struct extcon_dev *edev;
+	struct notifier_block notify;
+};
+
+static int tc7usb40mu_notify(struct notifier_block *nb, unsigned long event,
+			     void *ptr)
+{
+	struct tc7usb40mu_drv *drv;
+
+	drv = container_of(nb, struct tc7usb40mu_drv, notify);
+	if (event)
+		gpiod_set_value_cansleep(drv->gpio, 1); /* USB HUB */
+	else
+		gpiod_set_value_cansleep(drv->gpio, 0); /* device connector */
+
+	return NOTIFY_OK;
+}
+
+static int tc7usb40mu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct tc7usb40mu_drv *drv;
+	int state, ret;
+	enum gpiod_flags flags;
+
+	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	drv->edev = extcon_get_edev_by_phandle(dev, 0);
+	if (IS_ERR(drv->edev))
+		return PTR_ERR(drv->edev);
+
+	/*
+	 * TODO: This can race with extcon changing state before we request the
+	 * gpio or the extcon changing state before we register the notifier
+	 */
+	state = extcon_get_cable_state_(drv->edev, EXTCON_USB_HOST);
+	if (state)
+		flags = GPIOD_OUT_HIGH;
+	else
+		flags = GPIOD_OUT_LOW;
+
+	drv->gpio = devm_gpiod_get(dev, "switch", flags);
+	if (IS_ERR(drv->gpio))
+		return PTR_ERR(drv->gpio);
+
+	drv->notify.notifier_call = tc7usb40mu_notify;
+	ret = extcon_register_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, drv);
+
+	return 0;
+}
+
+static int tc7usb40mu_remove(struct platform_device *pdev)
+{
+	struct tc7usb40mu_drv *drv;
+
+	drv = platform_get_drvdata(pdev);
+	extcon_unregister_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
+
+	return 0;
+}
+
+static const struct of_device_id tc7usb40mu_dt_match[] = {
+	{ .compatible = "toshiba,tc7usb40mu", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tc7usb40mu_dt_match);
+
+static struct platform_driver tc7usb40mu_driver = {
+	.probe		= tc7usb40mu_probe,
+	.remove		= tc7usb40mu_remove,
+	.driver		= {
+		.name	= "tc7usb40mu",
+		.of_match_table = tc7usb40mu_dt_match,
+	},
+};
+module_platform_driver(tc7usb40mu_driver);
+
+MODULE_AUTHOR("Stephen Boyd <stephen.boyd@linaro.org>");
+MODULE_DESCRIPTION("TC7USB40MU USB multiplexer driver");
+MODULE_LICENSE("GPL");
-- 
2.9.0.rc2.8.ga28705d

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-14  1:42 ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  1:42 UTC (permalink / raw)
  To: linux-arm-kernel

On the db410c 96boards platform we have a TC7USB40MU[1] on the
board to mux the D+/D- lines from the SoC between a micro usb
"device" port and a USB hub for "host" roles. Upon a role switch,
we need to change this mux to forward the D+/D- lines to either
the port or the hub. Therefore, introduce a driver for this
device that intercepts extcon USB_HOST events and logically
asserts a gpio to mux the "host" D+/D- lines when a host cable is
attached. When the cable goes away, it will logically deassert
the gpio and mux the "device" lines.

[1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---

Should I make the extcon part optional? I could see a case where there are two
"OTG" ports connected to the mux (or two hubs), and for some reason the
software may want to mux between them at runtime. If we mandate an extcon,
that won't be possible to support. Perhaps it would be better to have
the node, but connect it to the usb controller with a phandle (maybe of_graph
endpoints would be useful too) so that when the controller wants to mux over
a port it can do so.

Muxing the ports this way based on ID cable is pretty much a software
design decision. We could mux the ports during the role switch, and the
role switch can be entirely userspace driven with the chipidea controller
that I'm using (see the role switching support in the "role" file for
debugfs support in that driver). So extcon cables don't come into the picture
in that scenario.

 .../devicetree/bindings/usb/toshiba,tc7usb40mu.txt |  34 +++++++
 drivers/usb/misc/Kconfig                           |   9 ++
 drivers/usb/misc/Makefile                          |   1 +
 drivers/usb/misc/tc7usb40mu.c                      | 107 +++++++++++++++++++++
 4 files changed, 151 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
 create mode 100644 drivers/usb/misc/tc7usb40mu.c

diff --git a/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
new file mode 100644
index 000000000000..18e6607408fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
@@ -0,0 +1,34 @@
+Toshiba TC7USB40MU
+
+This device muxes USB D+/D- lines between two outputs called 1D+/1D- and 2D+/2D-.
+When the switch pin is asserted, we mux out 2D+/2D-, and when it's deasserted we
+select 1D+/1D-.
+
+This can be used to mux USB D+/D- lines between a USB hub and an OTG port.
+
+PROPERTIES
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "toshiba,tc7usb40mu"
+
+- switch-gpios:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: Should contain the gpio used to toggle the switch. Logically
+                asserting the gpio will cause the device to mux the "host"
+                D+/D- lines instead of the "device" lines.
+
+- extcon:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: Should contain the extcon device for USB_HOST cable events
+
+Example:
+
+	usb-switch {
+		compatible = "toshiba,tc7usb40mu";
+		switch-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
+		extcon = <&usb_id>;
+	};
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 47b357760afc..3da568c751d2 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -46,6 +46,15 @@ config USB_SEVSEG
 	  To compile this driver as a module, choose M here: the
 	  module will be called usbsevseg.
 
+config USB_TC7USB40MU
+	tristate "TC7USB40MU USB mux support"
+	depends on (GPIOLIB && EXTCON) || COMPILE_TEST
+	help
+	  Say Y here if you have a TC7USB40MU by Toshiba. If a USB ID cable is
+	  present, a gpio will be asserted to mux out "host" D+/D- lines and when
+	  the ID cable is removed, a gpio will be deasserted to mux out "device"
+	  D+/D- lines.
+
 config USB_RIO500
 	tristate "USB Diamond Rio500 support"
 	help
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 3d1992750da4..d8f9ad1dee13 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
 obj-$(CONFIG_USB_RIO500)		+= rio500.o
 obj-$(CONFIG_USB_TEST)			+= usbtest.o
 obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
+obj-$(CONFIG_USB_TC7USB40MU)		+= tc7usb40mu.o
 obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
 obj-$(CONFIG_USB_USS720)		+= uss720.o
 obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
diff --git a/drivers/usb/misc/tc7usb40mu.c b/drivers/usb/misc/tc7usb40mu.c
new file mode 100644
index 000000000000..9edcfe577ae4
--- /dev/null
+++ b/drivers/usb/misc/tc7usb40mu.c
@@ -0,0 +1,107 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/extcon.h>
+#include <linux/gpio/consumer.h>
+#include <linux/notifier.h>
+
+struct tc7usb40mu_drv {
+	struct gpio_desc *gpio;
+	struct extcon_dev *edev;
+	struct notifier_block notify;
+};
+
+static int tc7usb40mu_notify(struct notifier_block *nb, unsigned long event,
+			     void *ptr)
+{
+	struct tc7usb40mu_drv *drv;
+
+	drv = container_of(nb, struct tc7usb40mu_drv, notify);
+	if (event)
+		gpiod_set_value_cansleep(drv->gpio, 1); /* USB HUB */
+	else
+		gpiod_set_value_cansleep(drv->gpio, 0); /* device connector */
+
+	return NOTIFY_OK;
+}
+
+static int tc7usb40mu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct tc7usb40mu_drv *drv;
+	int state, ret;
+	enum gpiod_flags flags;
+
+	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	drv->edev = extcon_get_edev_by_phandle(dev, 0);
+	if (IS_ERR(drv->edev))
+		return PTR_ERR(drv->edev);
+
+	/*
+	 * TODO: This can race with extcon changing state before we request the
+	 * gpio or the extcon changing state before we register the notifier
+	 */
+	state = extcon_get_cable_state_(drv->edev, EXTCON_USB_HOST);
+	if (state)
+		flags = GPIOD_OUT_HIGH;
+	else
+		flags = GPIOD_OUT_LOW;
+
+	drv->gpio = devm_gpiod_get(dev, "switch", flags);
+	if (IS_ERR(drv->gpio))
+		return PTR_ERR(drv->gpio);
+
+	drv->notify.notifier_call = tc7usb40mu_notify;
+	ret = extcon_register_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, drv);
+
+	return 0;
+}
+
+static int tc7usb40mu_remove(struct platform_device *pdev)
+{
+	struct tc7usb40mu_drv *drv;
+
+	drv = platform_get_drvdata(pdev);
+	extcon_unregister_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
+
+	return 0;
+}
+
+static const struct of_device_id tc7usb40mu_dt_match[] = {
+	{ .compatible = "toshiba,tc7usb40mu", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tc7usb40mu_dt_match);
+
+static struct platform_driver tc7usb40mu_driver = {
+	.probe		= tc7usb40mu_probe,
+	.remove		= tc7usb40mu_remove,
+	.driver		= {
+		.name	= "tc7usb40mu",
+		.of_match_table = tc7usb40mu_dt_match,
+	},
+};
+module_platform_driver(tc7usb40mu_driver);
+
+MODULE_AUTHOR("Stephen Boyd <stephen.boyd@linaro.org>");
+MODULE_DESCRIPTION("TC7USB40MU USB multiplexer driver");
+MODULE_LICENSE("GPL");
-- 
2.9.0.rc2.8.ga28705d

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  1:42 ` Stephen Boyd
@ 2016-09-14  3:32   ` Peter Chen
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-14  3:32 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, robh+dt, MyungJoo Ham, Chanwoo Choi,
	devicetree

On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> On the db410c 96boards platform we have a TC7USB40MU[1] on the
> board to mux the D+/D- lines from the SoC between a micro usb
> "device" port and a USB hub for "host" roles. Upon a role switch,
> we need to change this mux to forward the D+/D- lines to either
> the port or the hub. Therefore, introduce a driver for this
> device that intercepts extcon USB_HOST events and logically
> asserts a gpio to mux the "host" D+/D- lines when a host cable is
> attached. When the cable goes away, it will logically deassert
> the gpio and mux the "device" lines.

Would you please draw the design? It can also help me review your
chipidea patch well.

1. How many ports on the board?
2. How the lines are connected on the board?

Peter
> 
> [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> 
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> 
> Should I make the extcon part optional? I could see a case where there are two
> "OTG" ports connected to the mux (or two hubs), and for some reason the
> software may want to mux between them at runtime. If we mandate an extcon,
> that won't be possible to support. Perhaps it would be better to have
> the node, but connect it to the usb controller with a phandle (maybe of_graph
> endpoints would be useful too) so that when the controller wants to mux over
> a port it can do so.
> 
> Muxing the ports this way based on ID cable is pretty much a software
> design decision. We could mux the ports during the role switch, and the
> role switch can be entirely userspace driven with the chipidea controller
> that I'm using (see the role switching support in the "role" file for
> debugfs support in that driver). So extcon cables don't come into the picture
> in that scenario.
> 
>  .../devicetree/bindings/usb/toshiba,tc7usb40mu.txt |  34 +++++++
>  drivers/usb/misc/Kconfig                           |   9 ++
>  drivers/usb/misc/Makefile                          |   1 +
>  drivers/usb/misc/tc7usb40mu.c                      | 107 +++++++++++++++++++++
>  4 files changed, 151 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
>  create mode 100644 drivers/usb/misc/tc7usb40mu.c
> 
> diff --git a/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> new file mode 100644
> index 000000000000..18e6607408fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> @@ -0,0 +1,34 @@
> +Toshiba TC7USB40MU
> +
> +This device muxes USB D+/D- lines between two outputs called 1D+/1D- and 2D+/2D-.
> +When the switch pin is asserted, we mux out 2D+/2D-, and when it's deasserted we
> +select 1D+/1D-.
> +
> +This can be used to mux USB D+/D- lines between a USB hub and an OTG port.
> +
> +PROPERTIES
> +
> +- compatible:
> +    Usage: required
> +    Value type: <string>
> +    Definition: Should contain "toshiba,tc7usb40mu"
> +
> +- switch-gpios:
> +    Usage: required
> +    Value type: <prop-encoded-array>
> +    Definition: Should contain the gpio used to toggle the switch. Logically
> +                asserting the gpio will cause the device to mux the "host"
> +                D+/D- lines instead of the "device" lines.
> +
> +- extcon:
> +    Usage: required
> +    Value type: <prop-encoded-array>
> +    Definition: Should contain the extcon device for USB_HOST cable events
> +
> +Example:
> +
> +	usb-switch {
> +		compatible = "toshiba,tc7usb40mu";
> +		switch-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
> +		extcon = <&usb_id>;
> +	};
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 47b357760afc..3da568c751d2 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -46,6 +46,15 @@ config USB_SEVSEG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called usbsevseg.
>  
> +config USB_TC7USB40MU
> +	tristate "TC7USB40MU USB mux support"
> +	depends on (GPIOLIB && EXTCON) || COMPILE_TEST
> +	help
> +	  Say Y here if you have a TC7USB40MU by Toshiba. If a USB ID cable is
> +	  present, a gpio will be asserted to mux out "host" D+/D- lines and when
> +	  the ID cable is removed, a gpio will be deasserted to mux out "device"
> +	  D+/D- lines.
> +
>  config USB_RIO500
>  	tristate "USB Diamond Rio500 support"
>  	help
> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> index 3d1992750da4..d8f9ad1dee13 100644
> --- a/drivers/usb/misc/Makefile
> +++ b/drivers/usb/misc/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
>  obj-$(CONFIG_USB_RIO500)		+= rio500.o
>  obj-$(CONFIG_USB_TEST)			+= usbtest.o
>  obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
> +obj-$(CONFIG_USB_TC7USB40MU)		+= tc7usb40mu.o
>  obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
>  obj-$(CONFIG_USB_USS720)		+= uss720.o
>  obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
> diff --git a/drivers/usb/misc/tc7usb40mu.c b/drivers/usb/misc/tc7usb40mu.c
> new file mode 100644
> index 000000000000..9edcfe577ae4
> --- /dev/null
> +++ b/drivers/usb/misc/tc7usb40mu.c
> @@ -0,0 +1,107 @@
> +/**
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/extcon.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/notifier.h>
> +
> +struct tc7usb40mu_drv {
> +	struct gpio_desc *gpio;
> +	struct extcon_dev *edev;
> +	struct notifier_block notify;
> +};
> +
> +static int tc7usb40mu_notify(struct notifier_block *nb, unsigned long event,
> +			     void *ptr)
> +{
> +	struct tc7usb40mu_drv *drv;
> +
> +	drv = container_of(nb, struct tc7usb40mu_drv, notify);
> +	if (event)
> +		gpiod_set_value_cansleep(drv->gpio, 1); /* USB HUB */
> +	else
> +		gpiod_set_value_cansleep(drv->gpio, 0); /* device connector */
> +
> +	return NOTIFY_OK;
> +}
> +
> +static int tc7usb40mu_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct tc7usb40mu_drv *drv;
> +	int state, ret;
> +	enum gpiod_flags flags;
> +
> +	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	drv->edev = extcon_get_edev_by_phandle(dev, 0);
> +	if (IS_ERR(drv->edev))
> +		return PTR_ERR(drv->edev);
> +
> +	/*
> +	 * TODO: This can race with extcon changing state before we request the
> +	 * gpio or the extcon changing state before we register the notifier
> +	 */
> +	state = extcon_get_cable_state_(drv->edev, EXTCON_USB_HOST);
> +	if (state)
> +		flags = GPIOD_OUT_HIGH;
> +	else
> +		flags = GPIOD_OUT_LOW;
> +
> +	drv->gpio = devm_gpiod_get(dev, "switch", flags);
> +	if (IS_ERR(drv->gpio))
> +		return PTR_ERR(drv->gpio);
> +
> +	drv->notify.notifier_call = tc7usb40mu_notify;
> +	ret = extcon_register_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, drv);
> +
> +	return 0;
> +}
> +
> +static int tc7usb40mu_remove(struct platform_device *pdev)
> +{
> +	struct tc7usb40mu_drv *drv;
> +
> +	drv = platform_get_drvdata(pdev);
> +	extcon_unregister_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tc7usb40mu_dt_match[] = {
> +	{ .compatible = "toshiba,tc7usb40mu", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc7usb40mu_dt_match);
> +
> +static struct platform_driver tc7usb40mu_driver = {
> +	.probe		= tc7usb40mu_probe,
> +	.remove		= tc7usb40mu_remove,
> +	.driver		= {
> +		.name	= "tc7usb40mu",
> +		.of_match_table = tc7usb40mu_dt_match,
> +	},
> +};
> +module_platform_driver(tc7usb40mu_driver);
> +
> +MODULE_AUTHOR("Stephen Boyd <stephen.boyd@linaro.org>");
> +MODULE_DESCRIPTION("TC7USB40MU USB multiplexer driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.9.0.rc2.8.ga28705d
> 
> --
> 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] 35+ messages in thread

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-14  3:32   ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-14  3:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> On the db410c 96boards platform we have a TC7USB40MU[1] on the
> board to mux the D+/D- lines from the SoC between a micro usb
> "device" port and a USB hub for "host" roles. Upon a role switch,
> we need to change this mux to forward the D+/D- lines to either
> the port or the hub. Therefore, introduce a driver for this
> device that intercepts extcon USB_HOST events and logically
> asserts a gpio to mux the "host" D+/D- lines when a host cable is
> attached. When the cable goes away, it will logically deassert
> the gpio and mux the "device" lines.

Would you please draw the design? It can also help me review your
chipidea patch well.

1. How many ports on the board?
2. How the lines are connected on the board?

Peter
> 
> [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> 
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> 
> Should I make the extcon part optional? I could see a case where there are two
> "OTG" ports connected to the mux (or two hubs), and for some reason the
> software may want to mux between them at runtime. If we mandate an extcon,
> that won't be possible to support. Perhaps it would be better to have
> the node, but connect it to the usb controller with a phandle (maybe of_graph
> endpoints would be useful too) so that when the controller wants to mux over
> a port it can do so.
> 
> Muxing the ports this way based on ID cable is pretty much a software
> design decision. We could mux the ports during the role switch, and the
> role switch can be entirely userspace driven with the chipidea controller
> that I'm using (see the role switching support in the "role" file for
> debugfs support in that driver). So extcon cables don't come into the picture
> in that scenario.
> 
>  .../devicetree/bindings/usb/toshiba,tc7usb40mu.txt |  34 +++++++
>  drivers/usb/misc/Kconfig                           |   9 ++
>  drivers/usb/misc/Makefile                          |   1 +
>  drivers/usb/misc/tc7usb40mu.c                      | 107 +++++++++++++++++++++
>  4 files changed, 151 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
>  create mode 100644 drivers/usb/misc/tc7usb40mu.c
> 
> diff --git a/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> new file mode 100644
> index 000000000000..18e6607408fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> @@ -0,0 +1,34 @@
> +Toshiba TC7USB40MU
> +
> +This device muxes USB D+/D- lines between two outputs called 1D+/1D- and 2D+/2D-.
> +When the switch pin is asserted, we mux out 2D+/2D-, and when it's deasserted we
> +select 1D+/1D-.
> +
> +This can be used to mux USB D+/D- lines between a USB hub and an OTG port.
> +
> +PROPERTIES
> +
> +- compatible:
> +    Usage: required
> +    Value type: <string>
> +    Definition: Should contain "toshiba,tc7usb40mu"
> +
> +- switch-gpios:
> +    Usage: required
> +    Value type: <prop-encoded-array>
> +    Definition: Should contain the gpio used to toggle the switch. Logically
> +                asserting the gpio will cause the device to mux the "host"
> +                D+/D- lines instead of the "device" lines.
> +
> +- extcon:
> +    Usage: required
> +    Value type: <prop-encoded-array>
> +    Definition: Should contain the extcon device for USB_HOST cable events
> +
> +Example:
> +
> +	usb-switch {
> +		compatible = "toshiba,tc7usb40mu";
> +		switch-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
> +		extcon = <&usb_id>;
> +	};
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 47b357760afc..3da568c751d2 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -46,6 +46,15 @@ config USB_SEVSEG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called usbsevseg.
>  
> +config USB_TC7USB40MU
> +	tristate "TC7USB40MU USB mux support"
> +	depends on (GPIOLIB && EXTCON) || COMPILE_TEST
> +	help
> +	  Say Y here if you have a TC7USB40MU by Toshiba. If a USB ID cable is
> +	  present, a gpio will be asserted to mux out "host" D+/D- lines and when
> +	  the ID cable is removed, a gpio will be deasserted to mux out "device"
> +	  D+/D- lines.
> +
>  config USB_RIO500
>  	tristate "USB Diamond Rio500 support"
>  	help
> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> index 3d1992750da4..d8f9ad1dee13 100644
> --- a/drivers/usb/misc/Makefile
> +++ b/drivers/usb/misc/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
>  obj-$(CONFIG_USB_RIO500)		+= rio500.o
>  obj-$(CONFIG_USB_TEST)			+= usbtest.o
>  obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
> +obj-$(CONFIG_USB_TC7USB40MU)		+= tc7usb40mu.o
>  obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
>  obj-$(CONFIG_USB_USS720)		+= uss720.o
>  obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
> diff --git a/drivers/usb/misc/tc7usb40mu.c b/drivers/usb/misc/tc7usb40mu.c
> new file mode 100644
> index 000000000000..9edcfe577ae4
> --- /dev/null
> +++ b/drivers/usb/misc/tc7usb40mu.c
> @@ -0,0 +1,107 @@
> +/**
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/extcon.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/notifier.h>
> +
> +struct tc7usb40mu_drv {
> +	struct gpio_desc *gpio;
> +	struct extcon_dev *edev;
> +	struct notifier_block notify;
> +};
> +
> +static int tc7usb40mu_notify(struct notifier_block *nb, unsigned long event,
> +			     void *ptr)
> +{
> +	struct tc7usb40mu_drv *drv;
> +
> +	drv = container_of(nb, struct tc7usb40mu_drv, notify);
> +	if (event)
> +		gpiod_set_value_cansleep(drv->gpio, 1); /* USB HUB */
> +	else
> +		gpiod_set_value_cansleep(drv->gpio, 0); /* device connector */
> +
> +	return NOTIFY_OK;
> +}
> +
> +static int tc7usb40mu_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct tc7usb40mu_drv *drv;
> +	int state, ret;
> +	enum gpiod_flags flags;
> +
> +	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	drv->edev = extcon_get_edev_by_phandle(dev, 0);
> +	if (IS_ERR(drv->edev))
> +		return PTR_ERR(drv->edev);
> +
> +	/*
> +	 * TODO: This can race with extcon changing state before we request the
> +	 * gpio or the extcon changing state before we register the notifier
> +	 */
> +	state = extcon_get_cable_state_(drv->edev, EXTCON_USB_HOST);
> +	if (state)
> +		flags = GPIOD_OUT_HIGH;
> +	else
> +		flags = GPIOD_OUT_LOW;
> +
> +	drv->gpio = devm_gpiod_get(dev, "switch", flags);
> +	if (IS_ERR(drv->gpio))
> +		return PTR_ERR(drv->gpio);
> +
> +	drv->notify.notifier_call = tc7usb40mu_notify;
> +	ret = extcon_register_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, drv);
> +
> +	return 0;
> +}
> +
> +static int tc7usb40mu_remove(struct platform_device *pdev)
> +{
> +	struct tc7usb40mu_drv *drv;
> +
> +	drv = platform_get_drvdata(pdev);
> +	extcon_unregister_notifier(drv->edev, EXTCON_USB_HOST, &drv->notify);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tc7usb40mu_dt_match[] = {
> +	{ .compatible = "toshiba,tc7usb40mu", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc7usb40mu_dt_match);
> +
> +static struct platform_driver tc7usb40mu_driver = {
> +	.probe		= tc7usb40mu_probe,
> +	.remove		= tc7usb40mu_remove,
> +	.driver		= {
> +		.name	= "tc7usb40mu",
> +		.of_match_table = tc7usb40mu_dt_match,
> +	},
> +};
> +module_platform_driver(tc7usb40mu_driver);
> +
> +MODULE_AUTHOR("Stephen Boyd <stephen.boyd@linaro.org>");
> +MODULE_DESCRIPTION("TC7USB40MU USB multiplexer driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.9.0.rc2.8.ga28705d
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  3:32   ` Peter Chen
@ 2016-09-14  5:58     ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  5:58 UTC (permalink / raw)
  To: Peter Chen
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Chanwoo Choi, robh+dt, MyungJoo Ham, linux-arm-msm,
	linux-arm-kernel

Quoting Peter Chen (2016-09-13 20:32:00)
> On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > board to mux the D+/D- lines from the SoC between a micro usb
> > "device" port and a USB hub for "host" roles. Upon a role switch,
> > we need to change this mux to forward the D+/D- lines to either
> > the port or the hub. Therefore, introduce a driver for this
> > device that intercepts extcon USB_HOST events and logically
> > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > attached. When the cable goes away, it will logically deassert
> > the gpio and mux the "device" lines.
> 
> Would you please draw the design? It can also help me review your
> chipidea patch well.
> 
> 1. How many ports on the board?
> 2. How the lines are connected on the board?
> 

The schematic for the db410c is publically available here[2][3].

There's also the 96boards spec[4] which talks about this switch based
design a little bit. See the section titled "Single USB port Example".

[2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
[3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
[4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-14  5:58     ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  5:58 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Peter Chen (2016-09-13 20:32:00)
> On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > board to mux the D+/D- lines from the SoC between a micro usb
> > "device" port and a USB hub for "host" roles. Upon a role switch,
> > we need to change this mux to forward the D+/D- lines to either
> > the port or the hub. Therefore, introduce a driver for this
> > device that intercepts extcon USB_HOST events and logically
> > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > attached. When the cable goes away, it will logically deassert
> > the gpio and mux the "device" lines.
> 
> Would you please draw the design? It can also help me review your
> chipidea patch well.
> 
> 1. How many ports on the board?
> 2. How the lines are connected on the board?
> 

The schematic for the db410c is publically available here[2][3].

There's also the 96boards spec[4] which talks about this switch based
design a little bit. See the section titled "Single USB port Example".

[2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
[3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
[4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  5:58     ` Stephen Boyd
@ 2016-09-14  8:03       ` Peter Chen
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-14  8:03 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, robh+dt, MyungJoo Ham, Chanwoo Choi,
	devicetree

On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-13 20:32:00)
> > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > board to mux the D+/D- lines from the SoC between a micro usb
> > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > we need to change this mux to forward the D+/D- lines to either
> > > the port or the hub. Therefore, introduce a driver for this
> > > device that intercepts extcon USB_HOST events and logically
> > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > attached. When the cable goes away, it will logically deassert
> > > the gpio and mux the "device" lines.
> > 
> > Would you please draw the design? It can also help me review your
> > chipidea patch well.
> > 
> > 1. How many ports on the board?
> > 2. How the lines are connected on the board?
> > 
> 
> The schematic for the db410c is publically available here[2][3].
> 
> There's also the 96boards spec[4] which talks about this switch based
> design a little bit. See the section titled "Single USB port Example".
> 
> [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf

Ok, I see several use cases for this role switch

1. Using the hardware switch (218-4LPST)
In this case, you can set USB_SW_SEL as input gpio, and use
extcon-usb-gpio.c like before, just set this gpio as active
low at dts.

2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)
I can't find hardware relationship between each other, maybe I miss
something.
This use case (design) seems strange, usually, we use ID pin controls
vbus, but seldom use vbus pin control ID.
How you would like to implement it? When the USB cable is connected
(between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
as low? If disconnected, you set USB_SW_SEL as high?
When the USB controller works at Host mode, what will happen if the user
connects USB cable at device port?

3. Using sysfs to switch the role
Set USB_SW_SEL according to "role" at debugfs

Which one you would like to implement? Or anything else I miss?

-- 

Best Regards,
Peter Chen

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-14  8:03       ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-14  8:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-13 20:32:00)
> > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > board to mux the D+/D- lines from the SoC between a micro usb
> > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > we need to change this mux to forward the D+/D- lines to either
> > > the port or the hub. Therefore, introduce a driver for this
> > > device that intercepts extcon USB_HOST events and logically
> > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > attached. When the cable goes away, it will logically deassert
> > > the gpio and mux the "device" lines.
> > 
> > Would you please draw the design? It can also help me review your
> > chipidea patch well.
> > 
> > 1. How many ports on the board?
> > 2. How the lines are connected on the board?
> > 
> 
> The schematic for the db410c is publically available here[2][3].
> 
> There's also the 96boards spec[4] which talks about this switch based
> design a little bit. See the section titled "Single USB port Example".
> 
> [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf

Ok, I see several use cases for this role switch

1. Using the hardware switch (218-4LPST)
In this case, you can set USB_SW_SEL as input gpio, and use
extcon-usb-gpio.c like before, just set this gpio as active
low at dts.

2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)
I can't find hardware relationship between each other, maybe I miss
something.
This use case (design) seems strange, usually, we use ID pin controls
vbus, but seldom use vbus pin control ID.
How you would like to implement it? When the USB cable is connected
(between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
as low? If disconnected, you set USB_SW_SEL as high?
When the USB controller works at Host mode, what will happen if the user
connects USB cable at device port?

3. Using sysfs to switch the role
Set USB_SW_SEL according to "role" at debugfs

Which one you would like to implement? Or anything else I miss?

-- 

Best Regards,
Peter Chen

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  8:03       ` Peter Chen
@ 2016-09-14  8:45         ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  8:45 UTC (permalink / raw)
  To: Peter Chen
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Chanwoo Choi, robh+dt, MyungJoo Ham, linux-arm-msm,
	linux-arm-kernel

Quoting Peter Chen (2016-09-14 01:03:21)
> On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-13 20:32:00)
> > > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > we need to change this mux to forward the D+/D- lines to either
> > > > the port or the hub. Therefore, introduce a driver for this
> > > > device that intercepts extcon USB_HOST events and logically
> > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > attached. When the cable goes away, it will logically deassert
> > > > the gpio and mux the "device" lines.
> > > 
> > > Would you please draw the design? It can also help me review your
> > > chipidea patch well.
> > > 
> > > 1. How many ports on the board?
> > > 2. How the lines are connected on the board?
> > > 
> > 
> > The schematic for the db410c is publically available here[2][3].
> > 
> > There's also the 96boards spec[4] which talks about this switch based
> > design a little bit. See the section titled "Single USB port Example".
> > 
> > [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf
> 
> Ok, I see several use cases for this role switch
> 
> 1. Using the hardware switch (218-4LPST)
> In this case, you can set USB_SW_SEL as input gpio, and use
> extcon-usb-gpio.c like before, just set this gpio as active
> low at dts.

Nice! I didn't think of this case but it's good that we can support
that with some work.

> 
> 2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)

This is pretty much what has been implemented. USB_HS_ID is an
extcon-usb-gpio.c device.

> I can't find hardware relationship between each other, maybe I miss
> something.

I believe USB_SW_SEL is the physical switch (218-4LPST) while
USB_SW_SEL_PM is the software controllable part using a GPIO. USB_HS_ID
is just the vbus line from the uB connector and that line goes straight
into the SoC via a GPIO.

> This use case (design) seems strange, usually, we use ID pin controls
> vbus, but seldom use vbus pin control ID.
> How you would like to implement it? When the USB cable is connected
> (between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
> as low? If disconnected, you set USB_SW_SEL as high?

Right. The documented behavior is to detect the micro-usb cable and
drive USB_SW_SEL low. When the cable is unplugged we drive USB_SW_SEL
high. Maybe that should be changed though? If we always sampled
USB_SW_SEL as a USB_HOST extcon and the vbus line as a USB extcon then
we could allow the user to decide either with the physical switch or
with some sort of software control to toggle that gpio.

> When the USB controller works at Host mode, what will happen if the user
> connects USB cable at device port?

The devices on the two type-A connectors will be disconnected and we'll
switch from host mode to device mode.

> 
> 3. Using sysfs to switch the role
> Set USB_SW_SEL according to "role" at debugfs

sysfs isn't debugfs, but yes I wonder if we need to worry about the
debugfs role switching support here and toggle the gpio manually without
involving extcon. That would mean we need to have a way for the chipidea
controller to toggle this gpio/mux itself.

> 
> Which one you would like to implement? Or anything else I miss?
> 

Mostly #2, but I'm concerned that the DT binding is going to force that
decision on others who may have the same switch and want to do #1 or #3.
So how to design it in a way that makes it work in all cases? Also, what
to do if the USB_SW_SEL switch is driven high by the physical switch?
That will "override" any software control we might be able to use, so
hopefully we can detect this somehow and prevent the role switch from
happening.

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-14  8:45         ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  8:45 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Peter Chen (2016-09-14 01:03:21)
> On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-13 20:32:00)
> > > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > we need to change this mux to forward the D+/D- lines to either
> > > > the port or the hub. Therefore, introduce a driver for this
> > > > device that intercepts extcon USB_HOST events and logically
> > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > attached. When the cable goes away, it will logically deassert
> > > > the gpio and mux the "device" lines.
> > > 
> > > Would you please draw the design? It can also help me review your
> > > chipidea patch well.
> > > 
> > > 1. How many ports on the board?
> > > 2. How the lines are connected on the board?
> > > 
> > 
> > The schematic for the db410c is publically available here[2][3].
> > 
> > There's also the 96boards spec[4] which talks about this switch based
> > design a little bit. See the section titled "Single USB port Example".
> > 
> > [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf
> 
> Ok, I see several use cases for this role switch
> 
> 1. Using the hardware switch (218-4LPST)
> In this case, you can set USB_SW_SEL as input gpio, and use
> extcon-usb-gpio.c like before, just set this gpio as active
> low at dts.

Nice! I didn't think of this case but it's good that we can support
that with some work.

> 
> 2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)

This is pretty much what has been implemented. USB_HS_ID is an
extcon-usb-gpio.c device.

> I can't find hardware relationship between each other, maybe I miss
> something.

I believe USB_SW_SEL is the physical switch (218-4LPST) while
USB_SW_SEL_PM is the software controllable part using a GPIO. USB_HS_ID
is just the vbus line from the uB connector and that line goes straight
into the SoC via a GPIO.

> This use case (design) seems strange, usually, we use ID pin controls
> vbus, but seldom use vbus pin control ID.
> How you would like to implement it? When the USB cable is connected
> (between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
> as low? If disconnected, you set USB_SW_SEL as high?

Right. The documented behavior is to detect the micro-usb cable and
drive USB_SW_SEL low. When the cable is unplugged we drive USB_SW_SEL
high. Maybe that should be changed though? If we always sampled
USB_SW_SEL as a USB_HOST extcon and the vbus line as a USB extcon then
we could allow the user to decide either with the physical switch or
with some sort of software control to toggle that gpio.

> When the USB controller works at Host mode, what will happen if the user
> connects USB cable at device port?

The devices on the two type-A connectors will be disconnected and we'll
switch from host mode to device mode.

> 
> 3. Using sysfs to switch the role
> Set USB_SW_SEL according to "role" at debugfs

sysfs isn't debugfs, but yes I wonder if we need to worry about the
debugfs role switching support here and toggle the gpio manually without
involving extcon. That would mean we need to have a way for the chipidea
controller to toggle this gpio/mux itself.

> 
> Which one you would like to implement? Or anything else I miss?
> 

Mostly #2, but I'm concerned that the DT binding is going to force that
decision on others who may have the same switch and want to do #1 or #3.
So how to design it in a way that makes it work in all cases? Also, what
to do if the USB_SW_SEL switch is driven high by the physical switch?
That will "override" any software control we might be able to use, so
hopefully we can detect this somehow and prevent the role switch from
happening.

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  1:42 ` Stephen Boyd
@ 2016-09-14  8:55   ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  8:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-arm-msm, linux-kernel, linux-usb,
	robh+dt, MyungJoo Ham, Chanwoo Choi, devicetree, Peter Chen

Quoting Stephen Boyd (2016-09-13 18:42:46)
> On the db410c 96boards platform we have a TC7USB40MU[1] on the
> board to mux the D+/D- lines from the SoC between a micro usb
> "device" port and a USB hub for "host" roles. Upon a role switch,
> we need to change this mux to forward the D+/D- lines to either
> the port or the hub. Therefore, introduce a driver for this
> device that intercepts extcon USB_HOST events and logically
> asserts a gpio to mux the "host" D+/D- lines when a host cable is
> attached. When the cable goes away, it will logically deassert
> the gpio and mux the "device" lines.
> 
> [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> 
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> 
> Should I make the extcon part optional? I could see a case where there are two
> "OTG" ports connected to the mux (or two hubs), and for some reason the
> software may want to mux between them at runtime. If we mandate an extcon,
> that won't be possible to support. Perhaps it would be better to have
> the node, but connect it to the usb controller with a phandle (maybe of_graph
> endpoints would be useful too) so that when the controller wants to mux over
> a port it can do so.

Here's some dts mock-up on top of the db410c for the of_graph stuff. I
haven't written any code around it, but the idea is to allow the binding
to specify how the mux is connected to upstream and downstream D+/D-
lines. This way, we can do some dt parsing of the endpoints and their
parent nodes to figure out if the mux needs to be set high or low to use
a device connector or a usb hub based on if the id cable is present.
Maybe I'm over thinking things though and we could just have a DT
property for that.

	soc {
		usb@78d9000 {
			extcon = <&usb_id>, <&usb_id>;
			usb-controller; // needed?

			ports {
				#address-cells = <1>;
				#size-cells = <0>;

				port@0 {
					#address-cells = <1>;
					#size-cells = <0>;
					reg = <0>;

					usb_output: endpoint@0 { // USB D+/D-
						reg = <0>;
						remote-endpoint = <&usb_switch_input>;
					};
				};
			};
		};
	};

	usb2513 {
		compatible = "smsc,usb3503";
		reset-gpios = <&pm8916_gpios 3 GPIO_ACTIVE_LOW>;
		initial-mode = <1>;
		usb-hub; // indicate this is a hub

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0>;

				usb_hub_input: endpoint@0 { // USB{DP,DM}_UP
					reg = <0>;
					remote-endpoint = <&usb_switch_hub_ep>;
				};

				usb_hub_output1: endpoint@1 { // USB{DP,DM}_DN1
					reg = <1>;
					remote-endpoint = <&usb_a2_connector>;
				};

				usb_hub_output2: endpoint@2 { // USB{DP,DM}_DN2
					reg = <2>;
					remote-endpoint = <&usb_a1_connector>;
				};

				usb_hub_output3: endpoint@3 { // USB{DP,DM}_DN3
					reg = <3>;
					// goes to expansion connector
				};
			};
		};
	};

	usb_id: usb-id {
		compatible = "linux,extcon-usb-gpio";
		id-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
		pinctrl-names = "default";
		pinctrl-0 = <&usb_id_default>;
	};

	usb-switch {
		compatible = "toshiba,tc7usb40mu";
		switch-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>;
		extcon = <&usb_id>;
		pinctrl-names = "default";
		pinctrl-0 = <&usb_sw_sel_pm>;

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0>;

				usb_switch_input: endpoint@0 { // D+/D-
					reg = <0>;
					remote-endpoint = <&usb_output>;
				};

				usb_switch_device_ep: endpoint@1 { // D1+/D1-
					reg = <1>;
					remote-endpoint = <&usb_ub_connector>;
				};

				usb_switch_hub_ep: endpoint@2 { // D2+/D2-
					reg = <2>;
					remote-endpoint = <&usb_hub_input>;
				};
			};
		};
	};

	uB-connector {
		compatible = "usb-ub-connector";
		#address-cells = <1>;
		#size-cells = <0>;
		usb-connector;
		port@0 {
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0>;
			usb_ub_connector: endpoint@0 {
				reg = <0>;
				remote-endpoint = <&usb_switch_device_ep>;
			};
		};
	};

	usb-A-connector1 {
		compatible = "usb-A-connector";
		#address-cells = <1>;
		#size-cells = <0>;
		usb-connector;
		port@0 {
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0>;
			usb_a1_connector: endpoint@0 {
				reg = <0>;
				remote-endpoint = <&usb_hub_output2>;
			};
		};
	};

	usb-A-connector2 {
		compatible = "usb-A-connector";
		#address-cells = <1>;
		#size-cells = <0>;
		usb-connector;
		port@0 {
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0>;
			usb_a2_connector: endpoint@0 {
				reg = <0>;
				remote-endpoint = <&usb_hub_output1>;
			};
		};
	};
};

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-14  8:55   ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-14  8:55 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Stephen Boyd (2016-09-13 18:42:46)
> On the db410c 96boards platform we have a TC7USB40MU[1] on the
> board to mux the D+/D- lines from the SoC between a micro usb
> "device" port and a USB hub for "host" roles. Upon a role switch,
> we need to change this mux to forward the D+/D- lines to either
> the port or the hub. Therefore, introduce a driver for this
> device that intercepts extcon USB_HOST events and logically
> asserts a gpio to mux the "host" D+/D- lines when a host cable is
> attached. When the cable goes away, it will logically deassert
> the gpio and mux the "device" lines.
> 
> [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> 
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> 
> Should I make the extcon part optional? I could see a case where there are two
> "OTG" ports connected to the mux (or two hubs), and for some reason the
> software may want to mux between them at runtime. If we mandate an extcon,
> that won't be possible to support. Perhaps it would be better to have
> the node, but connect it to the usb controller with a phandle (maybe of_graph
> endpoints would be useful too) so that when the controller wants to mux over
> a port it can do so.

Here's some dts mock-up on top of the db410c for the of_graph stuff. I
haven't written any code around it, but the idea is to allow the binding
to specify how the mux is connected to upstream and downstream D+/D-
lines. This way, we can do some dt parsing of the endpoints and their
parent nodes to figure out if the mux needs to be set high or low to use
a device connector or a usb hub based on if the id cable is present.
Maybe I'm over thinking things though and we could just have a DT
property for that.

	soc {
		usb at 78d9000 {
			extcon = <&usb_id>, <&usb_id>;
			usb-controller; // needed?

			ports {
				#address-cells = <1>;
				#size-cells = <0>;

				port at 0 {
					#address-cells = <1>;
					#size-cells = <0>;
					reg = <0>;

					usb_output: endpoint at 0 { // USB D+/D-
						reg = <0>;
						remote-endpoint = <&usb_switch_input>;
					};
				};
			};
		};
	};

	usb2513 {
		compatible = "smsc,usb3503";
		reset-gpios = <&pm8916_gpios 3 GPIO_ACTIVE_LOW>;
		initial-mode = <1>;
		usb-hub; // indicate this is a hub

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port at 0 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0>;

				usb_hub_input: endpoint at 0 { // USB{DP,DM}_UP
					reg = <0>;
					remote-endpoint = <&usb_switch_hub_ep>;
				};

				usb_hub_output1: endpoint at 1 { // USB{DP,DM}_DN1
					reg = <1>;
					remote-endpoint = <&usb_a2_connector>;
				};

				usb_hub_output2: endpoint at 2 { // USB{DP,DM}_DN2
					reg = <2>;
					remote-endpoint = <&usb_a1_connector>;
				};

				usb_hub_output3: endpoint at 3 { // USB{DP,DM}_DN3
					reg = <3>;
					// goes to expansion connector
				};
			};
		};
	};

	usb_id: usb-id {
		compatible = "linux,extcon-usb-gpio";
		id-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
		pinctrl-names = "default";
		pinctrl-0 = <&usb_id_default>;
	};

	usb-switch {
		compatible = "toshiba,tc7usb40mu";
		switch-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>;
		extcon = <&usb_id>;
		pinctrl-names = "default";
		pinctrl-0 = <&usb_sw_sel_pm>;

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port at 0 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0>;

				usb_switch_input: endpoint at 0 { // D+/D-
					reg = <0>;
					remote-endpoint = <&usb_output>;
				};

				usb_switch_device_ep: endpoint at 1 { // D1+/D1-
					reg = <1>;
					remote-endpoint = <&usb_ub_connector>;
				};

				usb_switch_hub_ep: endpoint at 2 { // D2+/D2-
					reg = <2>;
					remote-endpoint = <&usb_hub_input>;
				};
			};
		};
	};

	uB-connector {
		compatible = "usb-ub-connector";
		#address-cells = <1>;
		#size-cells = <0>;
		usb-connector;
		port at 0 {
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0>;
			usb_ub_connector: endpoint at 0 {
				reg = <0>;
				remote-endpoint = <&usb_switch_device_ep>;
			};
		};
	};

	usb-A-connector1 {
		compatible = "usb-A-connector";
		#address-cells = <1>;
		#size-cells = <0>;
		usb-connector;
		port at 0 {
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0>;
			usb_a1_connector: endpoint at 0 {
				reg = <0>;
				remote-endpoint = <&usb_hub_output2>;
			};
		};
	};

	usb-A-connector2 {
		compatible = "usb-A-connector";
		#address-cells = <1>;
		#size-cells = <0>;
		usb-connector;
		port at 0 {
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0>;
			usb_a2_connector: endpoint at 0 {
				reg = <0>;
				remote-endpoint = <&usb_hub_output1>;
			};
		};
	};
};

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  8:45         ` Stephen Boyd
@ 2016-09-15  9:01           ` Peter Chen
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-15  9:01 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, robh+dt, MyungJoo Ham, Chanwoo Choi,
	devicetree

On Wed, Sep 14, 2016 at 01:45:04AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-14 01:03:21)
> > On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> > > Quoting Peter Chen (2016-09-13 20:32:00)
> > > > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > device that intercepts extcon USB_HOST events and logically
> > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > attached. When the cable goes away, it will logically deassert
> > > > > the gpio and mux the "device" lines.
> > > > 
> > > > Would you please draw the design? It can also help me review your
> > > > chipidea patch well.
> > > > 
> > > > 1. How many ports on the board?
> > > > 2. How the lines are connected on the board?
> > > > 
> > > 
> > > The schematic for the db410c is publically available here[2][3].
> > > 
> > > There's also the 96boards spec[4] which talks about this switch based
> > > design a little bit. See the section titled "Single USB port Example".
> > > 
> > > [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf
> > 
> > Ok, I see several use cases for this role switch
> > 
> > 1. Using the hardware switch (218-4LPST)
> > In this case, you can set USB_SW_SEL as input gpio, and use
> > extcon-usb-gpio.c like before, just set this gpio as active
> > low at dts.
> 
> Nice! I didn't think of this case but it's good that we can support
> that with some work.
> 

After looking more, I find the main purpose of this switch is
what it writes, FORCE DSI SELECTION TO USB HOST MODE

When the switch is on, the system is host-only.
When the switch is off, the default role is device mode, and
switch the role through gpio USB_SW_SEL_PM by software.

> > 
> > 2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)
> 
> This is pretty much what has been implemented. USB_HS_ID is an
> extcon-usb-gpio.c device.

At some designs, this gpio is tied to ID pin at MicroB connector, and
using a Mirco-AB cable to switch the role.

> 
> > I can't find hardware relationship between each other, maybe I miss
> > something.
> 
> I believe USB_SW_SEL is the physical switch (218-4LPST) while
> USB_SW_SEL_PM is the software controllable part using a GPIO. USB_HS_ID
> is just the vbus line from the uB connector and that line goes straight
> into the SoC via a GPIO.
> 
> > This use case (design) seems strange, usually, we use ID pin controls
> > vbus, but seldom use vbus pin control ID.
> > How you would like to implement it? When the USB cable is connected
> > (between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
> > as low? If disconnected, you set USB_SW_SEL as high?
> 
> Right. The documented behavior is to detect the micro-usb cable and
> drive USB_SW_SEL low. When the cable is unplugged we drive USB_SW_SEL
> high. Maybe that should be changed though? If we always sampled
> USB_SW_SEL as a USB_HOST extcon and the vbus line as a USB extcon then
> we could allow the user to decide either with the physical switch or
> with some sort of software control to toggle that gpio.
> 
> > When the USB controller works at Host mode, what will happen if the user
> > connects USB cable at device port?
> 
> The devices on the two type-A connectors will be disconnected and we'll
> switch from host mode to device mode.
> 
> > 
> > 3. Using sysfs to switch the role
> > Set USB_SW_SEL according to "role" at debugfs
> 
> sysfs isn't debugfs, but yes I wonder if we need to worry about the
> debugfs role switching support here and toggle the gpio manually without
> involving extcon. That would mean we need to have a way for the chipidea
> controller to toggle this gpio/mux itself.
> 
> > 
> > Which one you would like to implement? Or anything else I miss?
> > 
> 
> Mostly #2, but I'm concerned that the DT binding is going to force that
> decision on others who may have the same switch and want to do #1 or #3.
> So how to design it in a way that makes it work in all cases? Also, what
> to do if the USB_SW_SEL switch is driven high by the physical switch?
> That will "override" any software control we might be able to use, so
> hopefully we can detect this somehow and prevent the role switch from
> happening.

>From my point, the physical switch is only used to force host mode. The
#1 and #2 can't be supported at the same time. For #3, it is not the
use case for this design. #3 is usually used for the single port which
needs to support switching role on the fly without disconnection.
So, you may only need to consider #2, you can't use extcon-usb-gpio.c
directly since you need to set one gpio to mux the dp/dm, Baolu Lu had
USB MUX patch set before which may satisfy your requirement. [1]

[1] https://lkml.org/lkml/2016/5/30/36

-- 

Best Regards,
Peter Chen

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-15  9:01           ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-15  9:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 14, 2016 at 01:45:04AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-14 01:03:21)
> > On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> > > Quoting Peter Chen (2016-09-13 20:32:00)
> > > > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > device that intercepts extcon USB_HOST events and logically
> > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > attached. When the cable goes away, it will logically deassert
> > > > > the gpio and mux the "device" lines.
> > > > 
> > > > Would you please draw the design? It can also help me review your
> > > > chipidea patch well.
> > > > 
> > > > 1. How many ports on the board?
> > > > 2. How the lines are connected on the board?
> > > > 
> > > 
> > > The schematic for the db410c is publically available here[2][3].
> > > 
> > > There's also the 96boards spec[4] which talks about this switch based
> > > design a little bit. See the section titled "Single USB port Example".
> > > 
> > > [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf
> > 
> > Ok, I see several use cases for this role switch
> > 
> > 1. Using the hardware switch (218-4LPST)
> > In this case, you can set USB_SW_SEL as input gpio, and use
> > extcon-usb-gpio.c like before, just set this gpio as active
> > low at dts.
> 
> Nice! I didn't think of this case but it's good that we can support
> that with some work.
> 

After looking more, I find the main purpose of this switch is
what it writes, FORCE DSI SELECTION TO USB HOST MODE

When the switch is on, the system is host-only.
When the switch is off, the default role is device mode, and
switch the role through gpio USB_SW_SEL_PM by software.

> > 
> > 2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)
> 
> This is pretty much what has been implemented. USB_HS_ID is an
> extcon-usb-gpio.c device.

At some designs, this gpio is tied to ID pin at MicroB connector, and
using a Mirco-AB cable to switch the role.

> 
> > I can't find hardware relationship between each other, maybe I miss
> > something.
> 
> I believe USB_SW_SEL is the physical switch (218-4LPST) while
> USB_SW_SEL_PM is the software controllable part using a GPIO. USB_HS_ID
> is just the vbus line from the uB connector and that line goes straight
> into the SoC via a GPIO.
> 
> > This use case (design) seems strange, usually, we use ID pin controls
> > vbus, but seldom use vbus pin control ID.
> > How you would like to implement it? When the USB cable is connected
> > (between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
> > as low? If disconnected, you set USB_SW_SEL as high?
> 
> Right. The documented behavior is to detect the micro-usb cable and
> drive USB_SW_SEL low. When the cable is unplugged we drive USB_SW_SEL
> high. Maybe that should be changed though? If we always sampled
> USB_SW_SEL as a USB_HOST extcon and the vbus line as a USB extcon then
> we could allow the user to decide either with the physical switch or
> with some sort of software control to toggle that gpio.
> 
> > When the USB controller works at Host mode, what will happen if the user
> > connects USB cable at device port?
> 
> The devices on the two type-A connectors will be disconnected and we'll
> switch from host mode to device mode.
> 
> > 
> > 3. Using sysfs to switch the role
> > Set USB_SW_SEL according to "role" at debugfs
> 
> sysfs isn't debugfs, but yes I wonder if we need to worry about the
> debugfs role switching support here and toggle the gpio manually without
> involving extcon. That would mean we need to have a way for the chipidea
> controller to toggle this gpio/mux itself.
> 
> > 
> > Which one you would like to implement? Or anything else I miss?
> > 
> 
> Mostly #2, but I'm concerned that the DT binding is going to force that
> decision on others who may have the same switch and want to do #1 or #3.
> So how to design it in a way that makes it work in all cases? Also, what
> to do if the USB_SW_SEL switch is driven high by the physical switch?
> That will "override" any software control we might be able to use, so
> hopefully we can detect this somehow and prevent the role switch from
> happening.

>From my point, the physical switch is only used to force host mode. The
#1 and #2 can't be supported at the same time. For #3, it is not the
use case for this design. #3 is usually used for the single port which
needs to support switching role on the fly without disconnection.
So, you may only need to consider #2, you can't use extcon-usb-gpio.c
directly since you need to set one gpio to mux the dp/dm, Baolu Lu had
USB MUX patch set before which may satisfy your requirement. [1]

[1] https://lkml.org/lkml/2016/5/30/36

-- 

Best Regards,
Peter Chen

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-15  9:01           ` Peter Chen
@ 2016-09-16  1:16             ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-16  1:16 UTC (permalink / raw)
  To: Peter Chen
  Cc: devicetree, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Chanwoo Choi, robh+dt, MyungJoo Ham, linux-arm-msm,
	linux-arm-kernel

Quoting Peter Chen (2016-09-15 02:01:58)
> On Wed, Sep 14, 2016 at 01:45:04AM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-14 01:03:21)
> > > On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> > > > Quoting Peter Chen (2016-09-13 20:32:00)
> > > > > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > > device that intercepts extcon USB_HOST events and logically
> > > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > > attached. When the cable goes away, it will logically deassert
> > > > > > the gpio and mux the "device" lines.
> > > > > 
> > > > > Would you please draw the design? It can also help me review your
> > > > > chipidea patch well.
> > > > > 
> > > > > 1. How many ports on the board?
> > > > > 2. How the lines are connected on the board?
> > > > > 
> > > > 
> > > > The schematic for the db410c is publically available here[2][3].
> > > > 
> > > > There's also the 96boards spec[4] which talks about this switch based
> > > > design a little bit. See the section titled "Single USB port Example".
> > > > 
> > > > [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > > [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > > [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf
> > > 
> > > Ok, I see several use cases for this role switch
> > > 
> > > 1. Using the hardware switch (218-4LPST)
> > > In this case, you can set USB_SW_SEL as input gpio, and use
> > > extcon-usb-gpio.c like before, just set this gpio as active
> > > low at dts.
> > 
> > Nice! I didn't think of this case but it's good that we can support
> > that with some work.
> > 
> 
> After looking more, I find the main purpose of this switch is
> what it writes, FORCE DSI SELECTION TO USB HOST MODE
> 
> When the switch is on, the system is host-only.
> When the switch is off, the default role is device mode, and
> switch the role through gpio USB_SW_SEL_PM by software.

Correct.

> 
> > > 
> > > 2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)
> > 
> > This is pretty much what has been implemented. USB_HS_ID is an
> > extcon-usb-gpio.c device.
> 
> At some designs, this gpio is tied to ID pin at MicroB connector, and
> using a Mirco-AB cable to switch the role.
> 
> > 
> > > I can't find hardware relationship between each other, maybe I miss
> > > something.
> > 
> > I believe USB_SW_SEL is the physical switch (218-4LPST) while
> > USB_SW_SEL_PM is the software controllable part using a GPIO. USB_HS_ID
> > is just the vbus line from the uB connector and that line goes straight
> > into the SoC via a GPIO.
> > 
> > > This use case (design) seems strange, usually, we use ID pin controls
> > > vbus, but seldom use vbus pin control ID.
> > > How you would like to implement it? When the USB cable is connected
> > > (between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
> > > as low? If disconnected, you set USB_SW_SEL as high?
> > 
> > Right. The documented behavior is to detect the micro-usb cable and
> > drive USB_SW_SEL low. When the cable is unplugged we drive USB_SW_SEL
> > high. Maybe that should be changed though? If we always sampled
> > USB_SW_SEL as a USB_HOST extcon and the vbus line as a USB extcon then
> > we could allow the user to decide either with the physical switch or
> > with some sort of software control to toggle that gpio.
> > 
> > > When the USB controller works at Host mode, what will happen if the user
> > > connects USB cable at device port?
> > 
> > The devices on the two type-A connectors will be disconnected and we'll
> > switch from host mode to device mode.
> > 
> > > 
> > > 3. Using sysfs to switch the role
> > > Set USB_SW_SEL according to "role" at debugfs
> > 
> > sysfs isn't debugfs, but yes I wonder if we need to worry about the
> > debugfs role switching support here and toggle the gpio manually without
> > involving extcon. That would mean we need to have a way for the chipidea
> > controller to toggle this gpio/mux itself.
> > 
> > > 
> > > Which one you would like to implement? Or anything else I miss?
> > > 
> > 
> > Mostly #2, but I'm concerned that the DT binding is going to force that
> > decision on others who may have the same switch and want to do #1 or #3.
> > So how to design it in a way that makes it work in all cases? Also, what
> > to do if the USB_SW_SEL switch is driven high by the physical switch?
> > That will "override" any software control we might be able to use, so
> > hopefully we can detect this somehow and prevent the role switch from
> > happening.
> 
> From my point, the physical switch is only used to force host mode. The
> #1 and #2 can't be supported at the same time.

Sure. I mean to say support all use cases with the same DT binding.

> For #3, it is not the
> use case for this design. #3 is usually used for the single port which
> needs to support switching role on the fly without disconnection.
> So, you may only need to consider #2, you can't use extcon-usb-gpio.c
> directly since you need to set one gpio to mux the dp/dm, Baolu Lu had
> USB MUX patch set before which may satisfy your requirement. [1]

Ok. Did the usb mux patches go anywhere? It seemed to get tangled up in
DRD framework and I haven't been following along. I'll look into these
patches more.

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-16  1:16             ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-16  1:16 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Peter Chen (2016-09-15 02:01:58)
> On Wed, Sep 14, 2016 at 01:45:04AM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-14 01:03:21)
> > > On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> > > > Quoting Peter Chen (2016-09-13 20:32:00)
> > > > > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > > device that intercepts extcon USB_HOST events and logically
> > > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > > attached. When the cable goes away, it will logically deassert
> > > > > > the gpio and mux the "device" lines.
> > > > > 
> > > > > Would you please draw the design? It can also help me review your
> > > > > chipidea patch well.
> > > > > 
> > > > > 1. How many ports on the board?
> > > > > 2. How the lines are connected on the board?
> > > > > 
> > > > 
> > > > The schematic for the db410c is publically available here[2][3].
> > > > 
> > > > There's also the 96boards spec[4] which talks about this switch based
> > > > design a little bit. See the section titled "Single USB port Example".
> > > > 
> > > > [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > > [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > > > [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf
> > > 
> > > Ok, I see several use cases for this role switch
> > > 
> > > 1. Using the hardware switch (218-4LPST)
> > > In this case, you can set USB_SW_SEL as input gpio, and use
> > > extcon-usb-gpio.c like before, just set this gpio as active
> > > low at dts.
> > 
> > Nice! I didn't think of this case but it's good that we can support
> > that with some work.
> > 
> 
> After looking more, I find the main purpose of this switch is
> what it writes, FORCE DSI SELECTION TO USB HOST MODE
> 
> When the switch is on, the system is host-only.
> When the switch is off, the default role is device mode, and
> switch the role through gpio USB_SW_SEL_PM by software.

Correct.

> 
> > > 
> > > 2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)
> > 
> > This is pretty much what has been implemented. USB_HS_ID is an
> > extcon-usb-gpio.c device.
> 
> At some designs, this gpio is tied to ID pin at MicroB connector, and
> using a Mirco-AB cable to switch the role.
> 
> > 
> > > I can't find hardware relationship between each other, maybe I miss
> > > something.
> > 
> > I believe USB_SW_SEL is the physical switch (218-4LPST) while
> > USB_SW_SEL_PM is the software controllable part using a GPIO. USB_HS_ID
> > is just the vbus line from the uB connector and that line goes straight
> > into the SoC via a GPIO.
> > 
> > > This use case (design) seems strange, usually, we use ID pin controls
> > > vbus, but seldom use vbus pin control ID.
> > > How you would like to implement it? When the USB cable is connected
> > > (between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
> > > as low? If disconnected, you set USB_SW_SEL as high?
> > 
> > Right. The documented behavior is to detect the micro-usb cable and
> > drive USB_SW_SEL low. When the cable is unplugged we drive USB_SW_SEL
> > high. Maybe that should be changed though? If we always sampled
> > USB_SW_SEL as a USB_HOST extcon and the vbus line as a USB extcon then
> > we could allow the user to decide either with the physical switch or
> > with some sort of software control to toggle that gpio.
> > 
> > > When the USB controller works at Host mode, what will happen if the user
> > > connects USB cable at device port?
> > 
> > The devices on the two type-A connectors will be disconnected and we'll
> > switch from host mode to device mode.
> > 
> > > 
> > > 3. Using sysfs to switch the role
> > > Set USB_SW_SEL according to "role" at debugfs
> > 
> > sysfs isn't debugfs, but yes I wonder if we need to worry about the
> > debugfs role switching support here and toggle the gpio manually without
> > involving extcon. That would mean we need to have a way for the chipidea
> > controller to toggle this gpio/mux itself.
> > 
> > > 
> > > Which one you would like to implement? Or anything else I miss?
> > > 
> > 
> > Mostly #2, but I'm concerned that the DT binding is going to force that
> > decision on others who may have the same switch and want to do #1 or #3.
> > So how to design it in a way that makes it work in all cases? Also, what
> > to do if the USB_SW_SEL switch is driven high by the physical switch?
> > That will "override" any software control we might be able to use, so
> > hopefully we can detect this somehow and prevent the role switch from
> > happening.
> 
> From my point, the physical switch is only used to force host mode. The
> #1 and #2 can't be supported at the same time.

Sure. I mean to say support all use cases with the same DT binding.

> For #3, it is not the
> use case for this design. #3 is usually used for the single port which
> needs to support switching role on the fly without disconnection.
> So, you may only need to consider #2, you can't use extcon-usb-gpio.c
> directly since you need to set one gpio to mux the dp/dm, Baolu Lu had
> USB MUX patch set before which may satisfy your requirement. [1]

Ok. Did the usb mux patches go anywhere? It seemed to get tangled up in
DRD framework and I haven't been following along. I'll look into these
patches more.

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-16  1:16             ` Stephen Boyd
@ 2016-09-17  0:20               ` Peter Chen
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-17  0:20 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, robh+dt, MyungJoo Ham, Chanwoo Choi,
	devicetree

On Thu, Sep 15, 2016 at 06:16:38PM -0700, Stephen Boyd wrote:
> 
> > For #3, it is not the
> > use case for this design. #3 is usually used for the single port which
> > needs to support switching role on the fly without disconnection.
> > So, you may only need to consider #2, you can't use extcon-usb-gpio.c
> > directly since you need to set one gpio to mux the dp/dm, Baolu Lu had
> > USB MUX patch set before which may satisfy your requirement. [1]
> 
> Ok. Did the usb mux patches go anywhere? It seemed to get tangled up in
> DRD framework and I haven't been following along. I'll look into these
> patches more.

DRD framework is denied by Felipe due to only one user can be benefit
from it (chipidea OTG), I don't know the further status of USB MUX,
maybe you can ask for it.

-- 

Best Regards,
Peter Chen

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-17  0:20               ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-17  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 15, 2016 at 06:16:38PM -0700, Stephen Boyd wrote:
> 
> > For #3, it is not the
> > use case for this design. #3 is usually used for the single port which
> > needs to support switching role on the fly without disconnection.
> > So, you may only need to consider #2, you can't use extcon-usb-gpio.c
> > directly since you need to set one gpio to mux the dp/dm, Baolu Lu had
> > USB MUX patch set before which may satisfy your requirement. [1]
> 
> Ok. Did the usb mux patches go anywhere? It seemed to get tangled up in
> DRD framework and I haven't been following along. I'll look into these
> patches more.

DRD framework is denied by Felipe due to only one user can be benefit
from it (chipidea OTG), I don't know the further status of USB MUX,
maybe you can ask for it.

-- 

Best Regards,
Peter Chen

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  8:55   ` Stephen Boyd
@ 2016-09-17  1:16     ` Peter Chen
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-17  1:16 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, robh+dt, MyungJoo Ham, Chanwoo Choi,
	devicetree, Peter Chen

On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> Quoting Stephen Boyd (2016-09-13 18:42:46)
> > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > board to mux the D+/D- lines from the SoC between a micro usb
> > "device" port and a USB hub for "host" roles. Upon a role switch,
> > we need to change this mux to forward the D+/D- lines to either
> > the port or the hub. Therefore, introduce a driver for this
> > device that intercepts extcon USB_HOST events and logically
> > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > attached. When the cable goes away, it will logically deassert
> > the gpio and mux the "device" lines.
> > 
> > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > 
> > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> > 
> > Should I make the extcon part optional? I could see a case where there are two
> > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > software may want to mux between them at runtime. If we mandate an extcon,
> > that won't be possible to support. Perhaps it would be better to have
> > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > endpoints would be useful too) so that when the controller wants to mux over
> > a port it can do so.
> 
> Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> haven't written any code around it, but the idea is to allow the binding
> to specify how the mux is connected to upstream and downstream D+/D-
> lines. This way, we can do some dt parsing of the endpoints and their
> parent nodes to figure out if the mux needs to be set high or low to use
> a device connector or a usb hub based on if the id cable is present.
> Maybe I'm over thinking things though and we could just have a DT
> property for that.
> 
> 	soc {
> 		usb@78d9000 {
> 			extcon = <&usb_id>, <&usb_id>;

Why you have two same extcon phandler? From my mind, one should id,
another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
vbus support, how you support vbus detection for
connection/disconnection with PC for your chipidea msm patch set?

> 			usb-controller; // needed?

Not needed. You only need to describe controller, mux ic, and hub 
(if you need platform stuffs the driver needs to know).

Peter
> 
> 			ports {
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 
> 				port@0 {
> 					#address-cells = <1>;
> 					#size-cells = <0>;
> 					reg = <0>;
> 
> 					usb_output: endpoint@0 { // USB D+/D-
> 						reg = <0>;
> 						remote-endpoint = <&usb_switch_input>;
> 					};
> 				};
> 			};
> 		};
> 	};
> 
> 	usb2513 {
> 		compatible = "smsc,usb3503";
> 		reset-gpios = <&pm8916_gpios 3 GPIO_ACTIVE_LOW>;
> 		initial-mode = <1>;
> 		usb-hub; // indicate this is a hub
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port@0 {
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 				reg = <0>;
> 
> 				usb_hub_input: endpoint@0 { // USB{DP,DM}_UP
> 					reg = <0>;
> 					remote-endpoint = <&usb_switch_hub_ep>;
> 				};
> 
> 				usb_hub_output1: endpoint@1 { // USB{DP,DM}_DN1
> 					reg = <1>;
> 					remote-endpoint = <&usb_a2_connector>;
> 				};
> 
> 				usb_hub_output2: endpoint@2 { // USB{DP,DM}_DN2
> 					reg = <2>;
> 					remote-endpoint = <&usb_a1_connector>;
> 				};
> 
> 				usb_hub_output3: endpoint@3 { // USB{DP,DM}_DN3
> 					reg = <3>;
> 					// goes to expansion connector
> 				};
> 			};
> 		};
> 	};
> 
> 	usb_id: usb-id {
> 		compatible = "linux,extcon-usb-gpio";
> 		id-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
> 		pinctrl-names = "default";
> 		pinctrl-0 = <&usb_id_default>;
> 	};
> 
> 	usb-switch {
> 		compatible = "toshiba,tc7usb40mu";
> 		switch-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>;
> 		extcon = <&usb_id>;
> 		pinctrl-names = "default";
> 		pinctrl-0 = <&usb_sw_sel_pm>;
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port@0 {
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 				reg = <0>;
> 
> 				usb_switch_input: endpoint@0 { // D+/D-
> 					reg = <0>;
> 					remote-endpoint = <&usb_output>;
> 				};
> 
> 				usb_switch_device_ep: endpoint@1 { // D1+/D1-
> 					reg = <1>;
> 					remote-endpoint = <&usb_ub_connector>;
> 				};
> 
> 				usb_switch_hub_ep: endpoint@2 { // D2+/D2-
> 					reg = <2>;
> 					remote-endpoint = <&usb_hub_input>;
> 				};
> 			};
> 		};
> 	};
> 
> 	uB-connector {
> 		compatible = "usb-ub-connector";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		usb-connector;
> 		port@0 {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			reg = <0>;
> 			usb_ub_connector: endpoint@0 {
> 				reg = <0>;
> 				remote-endpoint = <&usb_switch_device_ep>;
> 			};
> 		};
> 	};
> 
> 	usb-A-connector1 {
> 		compatible = "usb-A-connector";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		usb-connector;
> 		port@0 {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			reg = <0>;
> 			usb_a1_connector: endpoint@0 {
> 				reg = <0>;
> 				remote-endpoint = <&usb_hub_output2>;
> 			};
> 		};
> 	};
> 
> 	usb-A-connector2 {
> 		compatible = "usb-A-connector";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		usb-connector;
> 		port@0 {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			reg = <0>;
> 			usb_a2_connector: endpoint@0 {
> 				reg = <0>;
> 				remote-endpoint = <&usb_hub_output1>;
> 			};
> 		};
> 	};
> };
> --
> 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] 35+ messages in thread

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-17  1:16     ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-17  1:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> Quoting Stephen Boyd (2016-09-13 18:42:46)
> > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > board to mux the D+/D- lines from the SoC between a micro usb
> > "device" port and a USB hub for "host" roles. Upon a role switch,
> > we need to change this mux to forward the D+/D- lines to either
> > the port or the hub. Therefore, introduce a driver for this
> > device that intercepts extcon USB_HOST events and logically
> > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > attached. When the cable goes away, it will logically deassert
> > the gpio and mux the "device" lines.
> > 
> > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > 
> > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> > 
> > Should I make the extcon part optional? I could see a case where there are two
> > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > software may want to mux between them at runtime. If we mandate an extcon,
> > that won't be possible to support. Perhaps it would be better to have
> > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > endpoints would be useful too) so that when the controller wants to mux over
> > a port it can do so.
> 
> Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> haven't written any code around it, but the idea is to allow the binding
> to specify how the mux is connected to upstream and downstream D+/D-
> lines. This way, we can do some dt parsing of the endpoints and their
> parent nodes to figure out if the mux needs to be set high or low to use
> a device connector or a usb hub based on if the id cable is present.
> Maybe I'm over thinking things though and we could just have a DT
> property for that.
> 
> 	soc {
> 		usb at 78d9000 {
> 			extcon = <&usb_id>, <&usb_id>;

Why you have two same extcon phandler? From my mind, one should id,
another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
vbus support, how you support vbus detection for
connection/disconnection with PC for your chipidea msm patch set?

> 			usb-controller; // needed?

Not needed. You only need to describe controller, mux ic, and hub 
(if you need platform stuffs the driver needs to know).

Peter
> 
> 			ports {
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 
> 				port at 0 {
> 					#address-cells = <1>;
> 					#size-cells = <0>;
> 					reg = <0>;
> 
> 					usb_output: endpoint at 0 { // USB D+/D-
> 						reg = <0>;
> 						remote-endpoint = <&usb_switch_input>;
> 					};
> 				};
> 			};
> 		};
> 	};
> 
> 	usb2513 {
> 		compatible = "smsc,usb3503";
> 		reset-gpios = <&pm8916_gpios 3 GPIO_ACTIVE_LOW>;
> 		initial-mode = <1>;
> 		usb-hub; // indicate this is a hub
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port at 0 {
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 				reg = <0>;
> 
> 				usb_hub_input: endpoint at 0 { // USB{DP,DM}_UP
> 					reg = <0>;
> 					remote-endpoint = <&usb_switch_hub_ep>;
> 				};
> 
> 				usb_hub_output1: endpoint at 1 { // USB{DP,DM}_DN1
> 					reg = <1>;
> 					remote-endpoint = <&usb_a2_connector>;
> 				};
> 
> 				usb_hub_output2: endpoint at 2 { // USB{DP,DM}_DN2
> 					reg = <2>;
> 					remote-endpoint = <&usb_a1_connector>;
> 				};
> 
> 				usb_hub_output3: endpoint at 3 { // USB{DP,DM}_DN3
> 					reg = <3>;
> 					// goes to expansion connector
> 				};
> 			};
> 		};
> 	};
> 
> 	usb_id: usb-id {
> 		compatible = "linux,extcon-usb-gpio";
> 		id-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
> 		pinctrl-names = "default";
> 		pinctrl-0 = <&usb_id_default>;
> 	};
> 
> 	usb-switch {
> 		compatible = "toshiba,tc7usb40mu";
> 		switch-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>;
> 		extcon = <&usb_id>;
> 		pinctrl-names = "default";
> 		pinctrl-0 = <&usb_sw_sel_pm>;
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port at 0 {
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 				reg = <0>;
> 
> 				usb_switch_input: endpoint at 0 { // D+/D-
> 					reg = <0>;
> 					remote-endpoint = <&usb_output>;
> 				};
> 
> 				usb_switch_device_ep: endpoint at 1 { // D1+/D1-
> 					reg = <1>;
> 					remote-endpoint = <&usb_ub_connector>;
> 				};
> 
> 				usb_switch_hub_ep: endpoint at 2 { // D2+/D2-
> 					reg = <2>;
> 					remote-endpoint = <&usb_hub_input>;
> 				};
> 			};
> 		};
> 	};
> 
> 	uB-connector {
> 		compatible = "usb-ub-connector";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		usb-connector;
> 		port at 0 {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			reg = <0>;
> 			usb_ub_connector: endpoint at 0 {
> 				reg = <0>;
> 				remote-endpoint = <&usb_switch_device_ep>;
> 			};
> 		};
> 	};
> 
> 	usb-A-connector1 {
> 		compatible = "usb-A-connector";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		usb-connector;
> 		port at 0 {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			reg = <0>;
> 			usb_a1_connector: endpoint at 0 {
> 				reg = <0>;
> 				remote-endpoint = <&usb_hub_output2>;
> 			};
> 		};
> 	};
> 
> 	usb-A-connector2 {
> 		compatible = "usb-A-connector";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		usb-connector;
> 		port at 0 {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 			reg = <0>;
> 			usb_a2_connector: endpoint at 0 {
> 				reg = <0>;
> 				remote-endpoint = <&usb_hub_output1>;
> 			};
> 		};
> 	};
> };
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-17  1:16     ` Peter Chen
@ 2016-09-22 18:51       ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-22 18:51 UTC (permalink / raw)
  To: Peter Chen
  Cc: devicetree, Peter Chen, Greg Kroah-Hartman, linux-usb,
	linux-kernel, Chanwoo Choi, robh+dt, MyungJoo Ham, linux-arm-msm,
	linux-arm-kernel

Quoting Peter Chen (2016-09-16 18:16:05)
> On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > board to mux the D+/D- lines from the SoC between a micro usb
> > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > we need to change this mux to forward the D+/D- lines to either
> > > the port or the hub. Therefore, introduce a driver for this
> > > device that intercepts extcon USB_HOST events and logically
> > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > attached. When the cable goes away, it will logically deassert
> > > the gpio and mux the "device" lines.
> > > 
> > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > 
> > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > Cc: <devicetree@vger.kernel.org>
> > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > ---
> > > 
> > > Should I make the extcon part optional? I could see a case where there are two
> > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > software may want to mux between them at runtime. If we mandate an extcon,
> > > that won't be possible to support. Perhaps it would be better to have
> > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > endpoints would be useful too) so that when the controller wants to mux over
> > > a port it can do so.
> > 
> > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > haven't written any code around it, but the idea is to allow the binding
> > to specify how the mux is connected to upstream and downstream D+/D-
> > lines. This way, we can do some dt parsing of the endpoints and their
> > parent nodes to figure out if the mux needs to be set high or low to use
> > a device connector or a usb hub based on if the id cable is present.
> > Maybe I'm over thinking things though and we could just have a DT
> > property for that.
> > 
> >       soc {
> >               usb@78d9000 {
> >                       extcon = <&usb_id>, <&usb_id>;
> 
> Why you have two same extcon phandler? From my mind, one should id,
> another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> vbus support, how you support vbus detection for
> connection/disconnection with PC for your chipidea msm patch set?

This was already in the dts files for db410c. In the chipidea binding
one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
that great on this board because we only have on gpio for this.

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-22 18:51       ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-22 18:51 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Peter Chen (2016-09-16 18:16:05)
> On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > board to mux the D+/D- lines from the SoC between a micro usb
> > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > we need to change this mux to forward the D+/D- lines to either
> > > the port or the hub. Therefore, introduce a driver for this
> > > device that intercepts extcon USB_HOST events and logically
> > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > attached. When the cable goes away, it will logically deassert
> > > the gpio and mux the "device" lines.
> > > 
> > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > 
> > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > Cc: <devicetree@vger.kernel.org>
> > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > ---
> > > 
> > > Should I make the extcon part optional? I could see a case where there are two
> > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > software may want to mux between them at runtime. If we mandate an extcon,
> > > that won't be possible to support. Perhaps it would be better to have
> > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > endpoints would be useful too) so that when the controller wants to mux over
> > > a port it can do so.
> > 
> > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > haven't written any code around it, but the idea is to allow the binding
> > to specify how the mux is connected to upstream and downstream D+/D-
> > lines. This way, we can do some dt parsing of the endpoints and their
> > parent nodes to figure out if the mux needs to be set high or low to use
> > a device connector or a usb hub based on if the id cable is present.
> > Maybe I'm over thinking things though and we could just have a DT
> > property for that.
> > 
> >       soc {
> >               usb at 78d9000 {
> >                       extcon = <&usb_id>, <&usb_id>;
> 
> Why you have two same extcon phandler? From my mind, one should id,
> another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> vbus support, how you support vbus detection for
> connection/disconnection with PC for your chipidea msm patch set?

This was already in the dts files for db410c. In the chipidea binding
one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
that great on this board because we only have on gpio for this.

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-14  8:55   ` Stephen Boyd
@ 2016-09-23 14:35     ` Rob Herring
  -1 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2016-09-23 14:35 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, MyungJoo Ham, Chanwoo Choi, devicetree,
	Peter Chen

On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> Quoting Stephen Boyd (2016-09-13 18:42:46)
> > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > board to mux the D+/D- lines from the SoC between a micro usb
> > "device" port and a USB hub for "host" roles. Upon a role switch,
> > we need to change this mux to forward the D+/D- lines to either
> > the port or the hub. Therefore, introduce a driver for this
> > device that intercepts extcon USB_HOST events and logically
> > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > attached. When the cable goes away, it will logically deassert
> > the gpio and mux the "device" lines.
> > 
> > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > 
> > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> > 
> > Should I make the extcon part optional? I could see a case where there are two
> > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > software may want to mux between them at runtime. If we mandate an extcon,
> > that won't be possible to support. Perhaps it would be better to have
> > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > endpoints would be useful too) so that when the controller wants to mux over
> > a port it can do so.

I've mentioned my opinion on extcon before. The first clue that it needs 
work is a Linux subsystem name is used for the binding. 

> Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> haven't written any code around it, but the idea is to allow the binding
> to specify how the mux is connected to upstream and downstream D+/D-
> lines. This way, we can do some dt parsing of the endpoints and their
> parent nodes to figure out if the mux needs to be set high or low to use
> a device connector or a usb hub based on if the id cable is present.
> Maybe I'm over thinking things though and we could just have a DT
> property for that.

I think the connector nodes are on the right track, but of-graph doesn't 
work here because we already have a way to describe USB buses in DT. 
Following that, would something like this work for you? The vbus-supply 
and id-gpios are just examples and may not always be there like if the 
hub controls each port's vbus directly.

usb-controller@1234 {
	usb-switch@0 {
		compatible = "toshiba,tc7usb40mu";
		hub@0 {
			compatible = "some-hub";
			port@0 {
				compatible = "usb-A-connector"
				vbus-supply = ...;
			};
			port@1 {
				compatible = "usb-A-connector"
				vbus-supply = ...;
			};

		};
		connector@1 {
			compatible = "usb-ub-connector";
			vbus-supply = ...;
			id-gpios = <>;
		};
	};
};

Rob

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-23 14:35     ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2016-09-23 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> Quoting Stephen Boyd (2016-09-13 18:42:46)
> > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > board to mux the D+/D- lines from the SoC between a micro usb
> > "device" port and a USB hub for "host" roles. Upon a role switch,
> > we need to change this mux to forward the D+/D- lines to either
> > the port or the hub. Therefore, introduce a driver for this
> > device that intercepts extcon USB_HOST events and logically
> > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > attached. When the cable goes away, it will logically deassert
> > the gpio and mux the "device" lines.
> > 
> > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > 
> > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> > 
> > Should I make the extcon part optional? I could see a case where there are two
> > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > software may want to mux between them at runtime. If we mandate an extcon,
> > that won't be possible to support. Perhaps it would be better to have
> > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > endpoints would be useful too) so that when the controller wants to mux over
> > a port it can do so.

I've mentioned my opinion on extcon before. The first clue that it needs 
work is a Linux subsystem name is used for the binding. 

> Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> haven't written any code around it, but the idea is to allow the binding
> to specify how the mux is connected to upstream and downstream D+/D-
> lines. This way, we can do some dt parsing of the endpoints and their
> parent nodes to figure out if the mux needs to be set high or low to use
> a device connector or a usb hub based on if the id cable is present.
> Maybe I'm over thinking things though and we could just have a DT
> property for that.

I think the connector nodes are on the right track, but of-graph doesn't 
work here because we already have a way to describe USB buses in DT. 
Following that, would something like this work for you? The vbus-supply 
and id-gpios are just examples and may not always be there like if the 
hub controls each port's vbus directly.

usb-controller at 1234 {
	usb-switch at 0 {
		compatible = "toshiba,tc7usb40mu";
		hub at 0 {
			compatible = "some-hub";
			port at 0 {
				compatible = "usb-A-connector"
				vbus-supply = ...;
			};
			port at 1 {
				compatible = "usb-A-connector"
				vbus-supply = ...;
			};

		};
		connector at 1 {
			compatible = "usb-ub-connector";
			vbus-supply = ...;
			id-gpios = <>;
		};
	};
};

Rob

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-22 18:51       ` Stephen Boyd
@ 2016-09-26  3:29         ` Peter Chen
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-26  3:29 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, robh+dt, MyungJoo Ham, Chanwoo Choi,
	devicetree, Peter Chen

On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-16 18:16:05)
> > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > we need to change this mux to forward the D+/D- lines to either
> > > > the port or the hub. Therefore, introduce a driver for this
> > > > device that intercepts extcon USB_HOST events and logically
> > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > attached. When the cable goes away, it will logically deassert
> > > > the gpio and mux the "device" lines.
> > > > 
> > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > 
> > > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > > Cc: <devicetree@vger.kernel.org>
> > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > ---
> > > > 
> > > > Should I make the extcon part optional? I could see a case where there are two
> > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > that won't be possible to support. Perhaps it would be better to have
> > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > a port it can do so.
> > > 
> > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > haven't written any code around it, but the idea is to allow the binding
> > > to specify how the mux is connected to upstream and downstream D+/D-
> > > lines. This way, we can do some dt parsing of the endpoints and their
> > > parent nodes to figure out if the mux needs to be set high or low to use
> > > a device connector or a usb hub based on if the id cable is present.
> > > Maybe I'm over thinking things though and we could just have a DT
> > > property for that.
> > > 
> > >       soc {
> > >               usb@78d9000 {
> > >                       extcon = <&usb_id>, <&usb_id>;
> > 
> > Why you have two same extcon phandler? From my mind, one should id,
> > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > vbus support, how you support vbus detection for
> > connection/disconnection with PC for your chipidea msm patch set?
> 
> This was already in the dts files for db410c. In the chipidea binding
> one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> that great on this board because we only have on gpio for this.

I think extcon-usb-gpio.c needs to extend for supporting vbus event,
otherwise, the micro-b cable's connect/disconnect will introduce
EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
vbus event.

-- 

Best Regards,
Peter Chen

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-26  3:29         ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-26  3:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-16 18:16:05)
> > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > we need to change this mux to forward the D+/D- lines to either
> > > > the port or the hub. Therefore, introduce a driver for this
> > > > device that intercepts extcon USB_HOST events and logically
> > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > attached. When the cable goes away, it will logically deassert
> > > > the gpio and mux the "device" lines.
> > > > 
> > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > 
> > > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > > Cc: <devicetree@vger.kernel.org>
> > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > ---
> > > > 
> > > > Should I make the extcon part optional? I could see a case where there are two
> > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > that won't be possible to support. Perhaps it would be better to have
> > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > a port it can do so.
> > > 
> > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > haven't written any code around it, but the idea is to allow the binding
> > > to specify how the mux is connected to upstream and downstream D+/D-
> > > lines. This way, we can do some dt parsing of the endpoints and their
> > > parent nodes to figure out if the mux needs to be set high or low to use
> > > a device connector or a usb hub based on if the id cable is present.
> > > Maybe I'm over thinking things though and we could just have a DT
> > > property for that.
> > > 
> > >       soc {
> > >               usb at 78d9000 {
> > >                       extcon = <&usb_id>, <&usb_id>;
> > 
> > Why you have two same extcon phandler? From my mind, one should id,
> > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > vbus support, how you support vbus detection for
> > connection/disconnection with PC for your chipidea msm patch set?
> 
> This was already in the dts files for db410c. In the chipidea binding
> one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> that great on this board because we only have on gpio for this.

I think extcon-usb-gpio.c needs to extend for supporting vbus event,
otherwise, the micro-b cable's connect/disconnect will introduce
EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
vbus event.

-- 

Best Regards,
Peter Chen

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-26  3:29         ` Peter Chen
@ 2016-09-26 18:44           ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-26 18:44 UTC (permalink / raw)
  To: Peter Chen
  Cc: devicetree, Peter Chen, Greg Kroah-Hartman, linux-usb,
	linux-kernel, Chanwoo Choi, robh+dt, MyungJoo Ham, linux-arm-msm,
	linux-arm-kernel

Quoting Peter Chen (2016-09-25 20:29:27)
> On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-16 18:16:05)
> > > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > device that intercepts extcon USB_HOST events and logically
> > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > attached. When the cable goes away, it will logically deassert
> > > > > the gpio and mux the "device" lines.
> > > > > 
> > > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > > 
> > > > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > > > Cc: <devicetree@vger.kernel.org>
> > > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > > ---
> > > > > 
> > > > > Should I make the extcon part optional? I could see a case where there are two
> > > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > > that won't be possible to support. Perhaps it would be better to have
> > > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > > a port it can do so.
> > > > 
> > > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > > haven't written any code around it, but the idea is to allow the binding
> > > > to specify how the mux is connected to upstream and downstream D+/D-
> > > > lines. This way, we can do some dt parsing of the endpoints and their
> > > > parent nodes to figure out if the mux needs to be set high or low to use
> > > > a device connector or a usb hub based on if the id cable is present.
> > > > Maybe I'm over thinking things though and we could just have a DT
> > > > property for that.
> > > > 
> > > >       soc {
> > > >               usb@78d9000 {
> > > >                       extcon = <&usb_id>, <&usb_id>;
> > > 
> > > Why you have two same extcon phandler? From my mind, one should id,
> > > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > > vbus support, how you support vbus detection for
> > > connection/disconnection with PC for your chipidea msm patch set?
> > 
> > This was already in the dts files for db410c. In the chipidea binding
> > one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> > understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> > and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> > that great on this board because we only have on gpio for this.
> 
> I think extcon-usb-gpio.c needs to extend for supporting vbus event,
> otherwise, the micro-b cable's connect/disconnect will introduce
> EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
> vbus event.
> 

Sorry, I'm lost now. extcon-usb-gpio.c already supports EXTCON_USB as an
event. Is the problem that we're using two of the same phandles in the
binding?

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-26 18:44           ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-26 18:44 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Peter Chen (2016-09-25 20:29:27)
> On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-16 18:16:05)
> > > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > device that intercepts extcon USB_HOST events and logically
> > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > attached. When the cable goes away, it will logically deassert
> > > > > the gpio and mux the "device" lines.
> > > > > 
> > > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > > 
> > > > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > > > Cc: <devicetree@vger.kernel.org>
> > > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > > ---
> > > > > 
> > > > > Should I make the extcon part optional? I could see a case where there are two
> > > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > > that won't be possible to support. Perhaps it would be better to have
> > > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > > a port it can do so.
> > > > 
> > > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > > haven't written any code around it, but the idea is to allow the binding
> > > > to specify how the mux is connected to upstream and downstream D+/D-
> > > > lines. This way, we can do some dt parsing of the endpoints and their
> > > > parent nodes to figure out if the mux needs to be set high or low to use
> > > > a device connector or a usb hub based on if the id cable is present.
> > > > Maybe I'm over thinking things though and we could just have a DT
> > > > property for that.
> > > > 
> > > >       soc {
> > > >               usb at 78d9000 {
> > > >                       extcon = <&usb_id>, <&usb_id>;
> > > 
> > > Why you have two same extcon phandler? From my mind, one should id,
> > > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > > vbus support, how you support vbus detection for
> > > connection/disconnection with PC for your chipidea msm patch set?
> > 
> > This was already in the dts files for db410c. In the chipidea binding
> > one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> > understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> > and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> > that great on this board because we only have on gpio for this.
> 
> I think extcon-usb-gpio.c needs to extend for supporting vbus event,
> otherwise, the micro-b cable's connect/disconnect will introduce
> EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
> vbus event.
> 

Sorry, I'm lost now. extcon-usb-gpio.c already supports EXTCON_USB as an
event. Is the problem that we're using two of the same phandles in the
binding?

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-23 14:35     ` Rob Herring
@ 2016-09-26 18:59       ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-26 18:59 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Peter Chen, Greg Kroah-Hartman, linux-usb,
	linux-kernel, Chanwoo Choi, MyungJoo Ham, linux-arm-msm,
	linux-arm-kernel

Quoting Rob Herring (2016-09-23 07:35:13)
> On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > board to mux the D+/D- lines from the SoC between a micro usb
> > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > we need to change this mux to forward the D+/D- lines to either
> > > the port or the hub. Therefore, introduce a driver for this
> > > device that intercepts extcon USB_HOST events and logically
> > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > attached. When the cable goes away, it will logically deassert
> > > the gpio and mux the "device" lines.
> > > 
> > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > 
> > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > Cc: <devicetree@vger.kernel.org>
> > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > ---
> > > 
> > > Should I make the extcon part optional? I could see a case where there are two
> > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > software may want to mux between them at runtime. If we mandate an extcon,
> > > that won't be possible to support. Perhaps it would be better to have
> > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > endpoints would be useful too) so that when the controller wants to mux over
> > > a port it can do so.
> 
> I've mentioned my opinion on extcon before. The first clue that it needs 
> work is a Linux subsystem name is used for the binding. 
> 
> > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > haven't written any code around it, but the idea is to allow the binding
> > to specify how the mux is connected to upstream and downstream D+/D-
> > lines. This way, we can do some dt parsing of the endpoints and their
> > parent nodes to figure out if the mux needs to be set high or low to use
> > a device connector or a usb hub based on if the id cable is present.
> > Maybe I'm over thinking things though and we could just have a DT
> > property for that.
> 
> I think the connector nodes are on the right track, but of-graph doesn't 
> work here because we already have a way to describe USB buses in DT. 
> Following that, would something like this work for you? The vbus-supply 
> and id-gpios are just examples and may not always be there like if the 
> hub controls each port's vbus directly.

My philosophical problem with this is that I don't view this usb-switch
as a usb device. It isn't addressable via the typical USB addressing
scheme. It's just a simple chip wired down on the board that muxes two
wires without considering what goes across those wires.

I would agree if this switch was a usb device itself that had a vid/pid
that we could talk to to switch the mux. But that isn't the case here.

> 
> usb-controller@1234 {
>         usb-switch@0 {
>                 compatible = "toshiba,tc7usb40mu";
>                 hub@0 {
>                         compatible = "some-hub";
>                         port@0 {
>                                 compatible = "usb-A-connector"
>                                 vbus-supply = ...;
>                         };
>                         port@1 {
>                                 compatible = "usb-A-connector"
>                                 vbus-supply = ...;
>                         };
> 
>                 };
>                 connector@1 {
>                         compatible = "usb-ub-connector";
>                         vbus-supply = ...;
>                         id-gpios = <>;
>                 };
>         };
> };
> 

What do we do about a hub downstream of the mux like usb3503? If that's
on the i2c bus and we need to do some initial setup, shouldn't we put
the hub under the i2c bus (because that's the addressing scheme) instead
of under the switch and then use of-graph to describe the connections
that aren't being used for addressing? My understanding of of-graph is
pretty weak so perhaps I missed something.

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-26 18:59       ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-26 18:59 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Rob Herring (2016-09-23 07:35:13)
> On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > board to mux the D+/D- lines from the SoC between a micro usb
> > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > we need to change this mux to forward the D+/D- lines to either
> > > the port or the hub. Therefore, introduce a driver for this
> > > device that intercepts extcon USB_HOST events and logically
> > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > attached. When the cable goes away, it will logically deassert
> > > the gpio and mux the "device" lines.
> > > 
> > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > 
> > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > Cc: <devicetree@vger.kernel.org>
> > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > ---
> > > 
> > > Should I make the extcon part optional? I could see a case where there are two
> > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > software may want to mux between them at runtime. If we mandate an extcon,
> > > that won't be possible to support. Perhaps it would be better to have
> > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > endpoints would be useful too) so that when the controller wants to mux over
> > > a port it can do so.
> 
> I've mentioned my opinion on extcon before. The first clue that it needs 
> work is a Linux subsystem name is used for the binding. 
> 
> > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > haven't written any code around it, but the idea is to allow the binding
> > to specify how the mux is connected to upstream and downstream D+/D-
> > lines. This way, we can do some dt parsing of the endpoints and their
> > parent nodes to figure out if the mux needs to be set high or low to use
> > a device connector or a usb hub based on if the id cable is present.
> > Maybe I'm over thinking things though and we could just have a DT
> > property for that.
> 
> I think the connector nodes are on the right track, but of-graph doesn't 
> work here because we already have a way to describe USB buses in DT. 
> Following that, would something like this work for you? The vbus-supply 
> and id-gpios are just examples and may not always be there like if the 
> hub controls each port's vbus directly.

My philosophical problem with this is that I don't view this usb-switch
as a usb device. It isn't addressable via the typical USB addressing
scheme. It's just a simple chip wired down on the board that muxes two
wires without considering what goes across those wires.

I would agree if this switch was a usb device itself that had a vid/pid
that we could talk to to switch the mux. But that isn't the case here.

> 
> usb-controller at 1234 {
>         usb-switch at 0 {
>                 compatible = "toshiba,tc7usb40mu";
>                 hub at 0 {
>                         compatible = "some-hub";
>                         port at 0 {
>                                 compatible = "usb-A-connector"
>                                 vbus-supply = ...;
>                         };
>                         port at 1 {
>                                 compatible = "usb-A-connector"
>                                 vbus-supply = ...;
>                         };
> 
>                 };
>                 connector at 1 {
>                         compatible = "usb-ub-connector";
>                         vbus-supply = ...;
>                         id-gpios = <>;
>                 };
>         };
> };
> 

What do we do about a hub downstream of the mux like usb3503? If that's
on the i2c bus and we need to do some initial setup, shouldn't we put
the hub under the i2c bus (because that's the addressing scheme) instead
of under the switch and then use of-graph to describe the connections
that aren't being used for addressing? My understanding of of-graph is
pretty weak so perhaps I missed something.

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-26 18:44           ` Stephen Boyd
  (?)
@ 2016-09-27  4:53             ` Peter Chen
  -1 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-27  4:53 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	MyungJoo Ham, Chanwoo Choi, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Peter Chen

On Mon, Sep 26, 2016 at 11:44:50AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-25 20:29:27)
> > On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> > > Quoting Peter Chen (2016-09-16 18:16:05)
> > > > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > > device that intercepts extcon USB_HOST events and logically
> > > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > > attached. When the cable goes away, it will logically deassert
> > > > > > the gpio and mux the "device" lines.
> > > > > > 
> > > > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > > > 
> > > > > > Cc: MyungJoo Ham <myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > > > > Cc: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > > > > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > > > > > Signed-off-by: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > > > > ---
> > > > > > 
> > > > > > Should I make the extcon part optional? I could see a case where there are two
> > > > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > > > that won't be possible to support. Perhaps it would be better to have
> > > > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > > > a port it can do so.
> > > > > 
> > > > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > > > haven't written any code around it, but the idea is to allow the binding
> > > > > to specify how the mux is connected to upstream and downstream D+/D-
> > > > > lines. This way, we can do some dt parsing of the endpoints and their
> > > > > parent nodes to figure out if the mux needs to be set high or low to use
> > > > > a device connector or a usb hub based on if the id cable is present.
> > > > > Maybe I'm over thinking things though and we could just have a DT
> > > > > property for that.
> > > > > 
> > > > >       soc {
> > > > >               usb@78d9000 {
> > > > >                       extcon = <&usb_id>, <&usb_id>;
> > > > 
> > > > Why you have two same extcon phandler? From my mind, one should id,
> > > > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > > > vbus support, how you support vbus detection for
> > > > connection/disconnection with PC for your chipidea msm patch set?
> > > 
> > > This was already in the dts files for db410c. In the chipidea binding
> > > one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> > > understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> > > and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> > > that great on this board because we only have on gpio for this.
> > 
> > I think extcon-usb-gpio.c needs to extend for supporting vbus event,
> > otherwise, the micro-b cable's connect/disconnect will introduce
> > EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
> > vbus event.
> > 
> 
> Sorry, I'm lost now. extcon-usb-gpio.c already supports EXTCON_USB as an
> event. Is the problem that we're using two of the same phandles in the
> binding?

No, ID and VBUS are different events.

http://www.spinics.net/lists/linux-usb/msg147004.html

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-27  4:53             ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-27  4:53 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm-kernel, linux-arm-msm,
	linux-kernel, linux-usb, robh+dt, MyungJoo Ham, Chanwoo Choi,
	devicetree, Peter Chen

On Mon, Sep 26, 2016 at 11:44:50AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-25 20:29:27)
> > On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> > > Quoting Peter Chen (2016-09-16 18:16:05)
> > > > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > > device that intercepts extcon USB_HOST events and logically
> > > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > > attached. When the cable goes away, it will logically deassert
> > > > > > the gpio and mux the "device" lines.
> > > > > > 
> > > > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > > > 
> > > > > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > > > > Cc: <devicetree@vger.kernel.org>
> > > > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > > > ---
> > > > > > 
> > > > > > Should I make the extcon part optional? I could see a case where there are two
> > > > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > > > that won't be possible to support. Perhaps it would be better to have
> > > > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > > > a port it can do so.
> > > > > 
> > > > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > > > haven't written any code around it, but the idea is to allow the binding
> > > > > to specify how the mux is connected to upstream and downstream D+/D-
> > > > > lines. This way, we can do some dt parsing of the endpoints and their
> > > > > parent nodes to figure out if the mux needs to be set high or low to use
> > > > > a device connector or a usb hub based on if the id cable is present.
> > > > > Maybe I'm over thinking things though and we could just have a DT
> > > > > property for that.
> > > > > 
> > > > >       soc {
> > > > >               usb@78d9000 {
> > > > >                       extcon = <&usb_id>, <&usb_id>;
> > > > 
> > > > Why you have two same extcon phandler? From my mind, one should id,
> > > > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > > > vbus support, how you support vbus detection for
> > > > connection/disconnection with PC for your chipidea msm patch set?
> > > 
> > > This was already in the dts files for db410c. In the chipidea binding
> > > one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> > > understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> > > and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> > > that great on this board because we only have on gpio for this.
> > 
> > I think extcon-usb-gpio.c needs to extend for supporting vbus event,
> > otherwise, the micro-b cable's connect/disconnect will introduce
> > EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
> > vbus event.
> > 
> 
> Sorry, I'm lost now. extcon-usb-gpio.c already supports EXTCON_USB as an
> event. Is the problem that we're using two of the same phandles in the
> binding?

No, ID and VBUS are different events.

http://www.spinics.net/lists/linux-usb/msg147004.html

-- 

Best Regards,
Peter Chen

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-27  4:53             ` Peter Chen
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Chen @ 2016-09-27  4:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 26, 2016 at 11:44:50AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-25 20:29:27)
> > On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> > > Quoting Peter Chen (2016-09-16 18:16:05)
> > > > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > > > we need to change this mux to forward the D+/D- lines to either
> > > > > > the port or the hub. Therefore, introduce a driver for this
> > > > > > device that intercepts extcon USB_HOST events and logically
> > > > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > > > attached. When the cable goes away, it will logically deassert
> > > > > > the gpio and mux the "device" lines.
> > > > > > 
> > > > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > > > 
> > > > > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > > > > Cc: <devicetree@vger.kernel.org>
> > > > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > > > ---
> > > > > > 
> > > > > > Should I make the extcon part optional? I could see a case where there are two
> > > > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > > > that won't be possible to support. Perhaps it would be better to have
> > > > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > > > a port it can do so.
> > > > > 
> > > > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > > > haven't written any code around it, but the idea is to allow the binding
> > > > > to specify how the mux is connected to upstream and downstream D+/D-
> > > > > lines. This way, we can do some dt parsing of the endpoints and their
> > > > > parent nodes to figure out if the mux needs to be set high or low to use
> > > > > a device connector or a usb hub based on if the id cable is present.
> > > > > Maybe I'm over thinking things though and we could just have a DT
> > > > > property for that.
> > > > > 
> > > > >       soc {
> > > > >               usb at 78d9000 {
> > > > >                       extcon = <&usb_id>, <&usb_id>;
> > > > 
> > > > Why you have two same extcon phandler? From my mind, one should id,
> > > > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > > > vbus support, how you support vbus detection for
> > > > connection/disconnection with PC for your chipidea msm patch set?
> > > 
> > > This was already in the dts files for db410c. In the chipidea binding
> > > one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> > > understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> > > and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> > > that great on this board because we only have on gpio for this.
> > 
> > I think extcon-usb-gpio.c needs to extend for supporting vbus event,
> > otherwise, the micro-b cable's connect/disconnect will introduce
> > EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
> > vbus event.
> > 
> 
> Sorry, I'm lost now. extcon-usb-gpio.c already supports EXTCON_USB as an
> event. Is the problem that we're using two of the same phandles in the
> binding?

No, ID and VBUS are different events.

http://www.spinics.net/lists/linux-usb/msg147004.html

-- 

Best Regards,
Peter Chen

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

* Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
  2016-09-27  4:53             ` Peter Chen
@ 2016-09-27 21:25               ` Stephen Boyd
  -1 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-27 21:25 UTC (permalink / raw)
  To: Peter Chen
  Cc: devicetree, Peter Chen, Greg Kroah-Hartman, linux-usb,
	linux-kernel, Chanwoo Choi, robh+dt, MyungJoo Ham, linux-arm-msm,
	linux-arm-kernel

Quoting Peter Chen (2016-09-26 21:53:58)
> On Mon, Sep 26, 2016 at 11:44:50AM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-25 20:29:27)
> > > On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> > > > Quoting Peter Chen (2016-09-16 18:16:05)
> > > > > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > > > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > > > 
> > > > > >       soc {
> > > > > >               usb@78d9000 {
> > > > > >                       extcon = <&usb_id>, <&usb_id>;
> > > > > 
> > > > > Why you have two same extcon phandler? From my mind, one should id,
> > > > > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > > > > vbus support, how you support vbus detection for
> > > > > connection/disconnection with PC for your chipidea msm patch set?
> > > > 
> > > > This was already in the dts files for db410c. In the chipidea binding
> > > > one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> > > > understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> > > > and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> > > > that great on this board because we only have on gpio for this.
> > > 
> > > I think extcon-usb-gpio.c needs to extend for supporting vbus event,
> > > otherwise, the micro-b cable's connect/disconnect will introduce
> > > EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
> > > vbus event.
> > > 
> > 
> > Sorry, I'm lost now. extcon-usb-gpio.c already supports EXTCON_USB as an
> > event. Is the problem that we're using two of the same phandles in the
> > binding?
> 
> No, ID and VBUS are different events.
> 
> http://www.spinics.net/lists/linux-usb/msg147004.html
> 

Agreed. But we only register for EXTCON_USB or EXTCON_USB_HOST for each
phandle in the list respectively based on the index of the phandle. So
with or without the patch you mention I don't see how it matters.

We do it this way to trigger the role switch when the cable is
disconnected. I think the problem with just having the id phandle is
that we'll never see the vbus event, and so the role switch doesn't
work.

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

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
@ 2016-09-27 21:25               ` Stephen Boyd
  0 siblings, 0 replies; 35+ messages in thread
From: Stephen Boyd @ 2016-09-27 21:25 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Peter Chen (2016-09-26 21:53:58)
> On Mon, Sep 26, 2016 at 11:44:50AM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-25 20:29:27)
> > > On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> > > > Quoting Peter Chen (2016-09-16 18:16:05)
> > > > > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > > > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > > > 
> > > > > >       soc {
> > > > > >               usb at 78d9000 {
> > > > > >                       extcon = <&usb_id>, <&usb_id>;
> > > > > 
> > > > > Why you have two same extcon phandler? From my mind, one should id,
> > > > > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > > > > vbus support, how you support vbus detection for
> > > > > connection/disconnection with PC for your chipidea msm patch set?
> > > > 
> > > > This was already in the dts files for db410c. In the chipidea binding
> > > > one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> > > > understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> > > > and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> > > > that great on this board because we only have on gpio for this.
> > > 
> > > I think extcon-usb-gpio.c needs to extend for supporting vbus event,
> > > otherwise, the micro-b cable's connect/disconnect will introduce
> > > EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
> > > vbus event.
> > > 
> > 
> > Sorry, I'm lost now. extcon-usb-gpio.c already supports EXTCON_USB as an
> > event. Is the problem that we're using two of the same phandles in the
> > binding?
> 
> No, ID and VBUS are different events.
> 
> http://www.spinics.net/lists/linux-usb/msg147004.html
> 

Agreed. But we only register for EXTCON_USB or EXTCON_USB_HOST for each
phandle in the list respectively based on the index of the phandle. So
with or without the patch you mention I don't see how it matters.

We do it this way to trigger the role switch when the cable is
disconnected. I think the problem with just having the id phandle is
that we'll never see the vbus event, and so the role switch doesn't
work.

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

end of thread, other threads:[~2016-09-27 21:25 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14  1:42 [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU Stephen Boyd
2016-09-14  1:42 ` Stephen Boyd
2016-09-14  3:32 ` Peter Chen
2016-09-14  3:32   ` Peter Chen
2016-09-14  5:58   ` Stephen Boyd
2016-09-14  5:58     ` Stephen Boyd
2016-09-14  8:03     ` Peter Chen
2016-09-14  8:03       ` Peter Chen
2016-09-14  8:45       ` Stephen Boyd
2016-09-14  8:45         ` Stephen Boyd
2016-09-15  9:01         ` Peter Chen
2016-09-15  9:01           ` Peter Chen
2016-09-16  1:16           ` Stephen Boyd
2016-09-16  1:16             ` Stephen Boyd
2016-09-17  0:20             ` Peter Chen
2016-09-17  0:20               ` Peter Chen
2016-09-14  8:55 ` Stephen Boyd
2016-09-14  8:55   ` Stephen Boyd
2016-09-17  1:16   ` Peter Chen
2016-09-17  1:16     ` Peter Chen
2016-09-22 18:51     ` Stephen Boyd
2016-09-22 18:51       ` Stephen Boyd
2016-09-26  3:29       ` Peter Chen
2016-09-26  3:29         ` Peter Chen
2016-09-26 18:44         ` Stephen Boyd
2016-09-26 18:44           ` Stephen Boyd
2016-09-27  4:53           ` Peter Chen
2016-09-27  4:53             ` Peter Chen
2016-09-27  4:53             ` Peter Chen
2016-09-27 21:25             ` Stephen Boyd
2016-09-27 21:25               ` Stephen Boyd
2016-09-23 14:35   ` Rob Herring
2016-09-23 14:35     ` Rob Herring
2016-09-26 18:59     ` Stephen Boyd
2016-09-26 18:59       ` Stephen Boyd

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.