| Message ID | 20160914014246.31847-1-stephen.boyd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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?
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.
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>; }; }; }; };
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
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.
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.
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
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.
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
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.
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?
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.
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
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.
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");
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