Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1685427 > unrolled thread

[PATCH 0/3] USB Mux support for Chipidea

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] USB Mux support for Chipidea Stephen Boyd <stephen.boyd@linaro.org> - 2017-07-12 03:10 +0200
    [PATCH 1/3] usb: misc: Add a driver for TC7USB40MU Stephen Boyd <stephen.boyd@linaro.org> - 2017-07-12 03:10 +0200
    [PATCH 2/3] usb: chipidea: Hook into mux framework to toggle usb switch Stephen Boyd <stephen.boyd@linaro.org> - 2017-07-12 03:10 +0200
      Re: [PATCH 2/3] usb: chipidea: Hook into mux framework to toggle usb switch Stephen Boyd <stephen.boyd@linaro.org> - 2017-07-14 00:40 +0200
        Re: [PATCH 2/3] usb: chipidea: Hook into mux framework to toggle usb  switch Rob Herring <robh@kernel.org> - 2017-07-17 19:30 +0200
    Re: [PATCH 0/3] USB Mux support for Chipidea Stephen Boyd <stephen.boyd@linaro.org> - 2017-07-14 00:40 +0200
      Re: [PATCH 0/3] USB Mux support for Chipidea Stephen Boyd <stephen.boyd@linaro.org> - 2017-07-14 23:30 +0200
      Re: [PATCH 0/3] USB Mux support for Chipidea Rob Herring <robh@kernel.org> - 2017-07-17 19:20 +0200

#1685427 — [PATCH 0/3] USB Mux support for Chipidea

This patchset adds support for the TC7USB40MU usb mux found on 
db410c 96boards platforms via the new multiplexer framework and
hooks that into the chipidea driver. This allows us to properly
control host or device mode on this board via the sysfs knob.

So far I've only tested this on db410c, and there are some rough
edges to finish off before it can merge. Also I'm experiencing
odd behavior with switching the role while gadget is enabled and
the micro-usb cable is kept connected. Not sure what's wrong but
it seems like the gadget never gets disconnected? I'll investigate
more.

TODO:

 1. The mux framework has to be selected for consumers to use it. We'll
    need some stubs in the consumer header file to allow compilation to
    continue without mux always enabled by consumers.

 2. We probably need some sort of mux_control_get_optional() API so that
    we know if there was an error getting the mux control, instead of just
    ignoring errors. For now I can pass up EPROBE_DEFER errors and ignore
    other errors and consider it "missing from DT".

 3. Maybe we can get rid of the mux driver and just use mux-gpio.c with
    a compatible string update? I split it off because we may want to
    support the "S" pin on the TC7USB40MU one day that shuts off both
    mux outputs.

 4. The userspace side of things is murky. What is expected to go and toggle
    the host/gadget side of things in userspace at this very specific location
    for chipidea devices?

Stephen Boyd (3):
  usb: misc: Add a driver for TC7USB40MU
  usb: chipidea: Hook into mux framework to toggle usb switch
  arm64: dts: qcom: Collapse usb support into one node

 .../devicetree/bindings/usb/ci-hdrc-usb2.txt       |  8 +++
 .../devicetree/bindings/usb/toshiba,tc7usb40mu.txt | 31 +++++++++
 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi          | 39 +++++++-----
 arch/arm64/boot/dts/qcom/msm8916.dtsi              | 62 +++++++++---------
 drivers/usb/chipidea/core.c                        | 17 +++++
 drivers/usb/chipidea/host.c                        | 10 +++
 drivers/usb/chipidea/udc.c                         | 11 ++++
 drivers/usb/misc/Kconfig                           | 11 ++++
 drivers/usb/misc/Makefile                          |  1 +
 drivers/usb/misc/tc7usb40mu.c                      | 74 ++++++++++++++++++++++
 include/linux/usb/chipidea.h                       | 14 ++++
 11 files changed, 228 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
 create mode 100644 drivers/usb/misc/tc7usb40mu.c

-- 
2.10.0.297.gf6727b0

[toc] | [next] | [standalone]

#1685428 — [PATCH 1/3] usb: misc: Add a driver for TC7USB40MU

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. Introduce a driver for this device that
hooks into the generic mux framework logically asserts a gpio to
mux the D+/D- lines to the 2D+/2D- output when the state is set
to "1". Similary, deassert the gpio and mux the D+/D- lines to
the 1D+/1D- output when the state is set to "0".

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

Cc: Peter Rosin <peda@axentia.se>
Cc: Peter Chen <peter.chen@nxp.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---

We may be able to ignore this patch and just use mux-gpio.c file. I'll
investigate that approach but I'd like to keep the compatible string
in case we need to do something later.

 .../devicetree/bindings/usb/toshiba,tc7usb40mu.txt | 31 +++++++++
 drivers/usb/misc/Kconfig                           | 11 ++++
 drivers/usb/misc/Makefile                          |  1 +
 drivers/usb/misc/tc7usb40mu.c                      | 74 ++++++++++++++++++++++
 4 files changed, 117 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..5c6b2b39825f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
@@ -0,0 +1,31 @@
+Toshiba TC7USB40MU
+
+This device muxes USB D+/D- lines between two outputs called 1D+/1D- and
+2D+/2D-. When the switch pin is asserted, the device muxes out 2D+/2D-, and
+when it's deasserted it muxes out 1D+/1D-.
+
+This can be used to mux USB D+/D- lines between a USB hub and a micro-USB port
+to provide host mode and device modes with the same USB controller.
+
+PROPERTIES
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "toshiba,tc7usb40mu"
+
+- mux-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 2D+/2D-
+                lines and deasserting the gpio will cause the device to mux
+		the 1D+/1D- lines.
+
+Example:
+
+	usb-switch {
+		compatible = "toshiba,tc7usb40mu";
+		mux-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
+		#mux-control-cells = <0>;
+	};
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 0f9f25db9163..df44c9d16eb1 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -46,6 +46,17 @@ 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 && MULTIPLEXER) || COMPILE_TEST
+	help
+	  Say Y here if you have a TC7USB40MU by Toshiba. A USB controller
+	  driver can then use the mux controller provided by this driver to
+	  route the D+/D- lines to two different devices downstream. For
+	  example, one downstream device could be a micro-USB port, and the
+	  other could be a USB hub, allowing a device to provide either
+	  device or host mode via a single USB controller.
+
 config USB_RIO500
 	tristate "USB Diamond Rio500 support"
 	help
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 7fdb45fc976f..42268fcb8a60 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..fbccd2fc0030
--- /dev/null
+++ b/drivers/usb/misc/tc7usb40mu.c
@@ -0,0 +1,74 @@
+/**
+ * Copyright (C) 2016-2017 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/gpio/consumer.h>
+#include <linux/mux/driver.h>
+
+struct tc7usb40mu_drv {
+	struct gpio_desc *gpio;
+};
+
+static int tc7usb40mu_mux_set(struct mux_control *mux, int state)
+{
+	struct tc7usb40mu_drv *drv = mux_chip_priv(mux->chip);
+
+	gpiod_set_value_cansleep(drv->gpio, state);
+
+	return 0;
+}
+
+static const struct mux_control_ops tc7usb40mu_mux_ops = {
+	.set = tc7usb40mu_mux_set,
+};
+
+static int tc7usb40mu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct tc7usb40mu_drv *drv;
+	struct mux_chip *mux_chip;
+
+	mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*drv));
+	if (!mux_chip)
+		return -ENOMEM;
+
+	mux_chip->ops = &tc7usb40mu_mux_ops;
+	mux_chip->mux->states = 2; /* 1D+/1D- and 2D+/2D- */
+
+	drv = mux_chip_priv(mux_chip);
+	drv->gpio = devm_gpiod_get(dev, "mux", GPIOD_ASIS);
+	if (IS_ERR(drv->gpio))
+		return PTR_ERR(drv->gpio);
+
+	return devm_mux_chip_register(dev, mux_chip);
+}
+
+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,
+	.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.10.0.297.gf6727b0

[toc] | [prev] | [next] | [standalone]

#1685432 — [PATCH 2/3] usb: chipidea: Hook into mux framework to toggle usb switch

On the db410c 96boards platform we have a TC7USB40MU on the board
to mux the D+/D- lines coming from the controller between a micro
usb "device" port and a USB hub for "host" roles[1]. During a
role switch, we need to toggle this mux to forward the D+/D-
lines to either the port or the hub. Add the necessary code to do
the role switch in chipidea core via the generic mux framework.
Board configurations like on db410c are expected to change roles
via the sysfs API described in
Documentation/ABI/testing/sysfs-platform-chipidea-usb2.

[1] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf

Cc: Peter Rosin <peda@axentia.se>
Cc: Peter Chen <peter.chen@nxp.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt |  8 ++++++++
 drivers/usb/chipidea/core.c                            | 17 +++++++++++++++++
 drivers/usb/chipidea/host.c                            | 10 ++++++++++
 drivers/usb/chipidea/udc.c                             | 11 +++++++++++
 include/linux/usb/chipidea.h                           | 14 ++++++++++++++
 5 files changed, 60 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 0e03344e2e8b..96ce81d975d5 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -76,6 +76,11 @@ Optional properties:
   needs to make sure it does not send more than 90%
   maximum_periodic_data_per_frame. The use case is multiple transactions, but
   less frame rate.
+- mux-controls: The mux control for toggling host/device output of this
+  controller.
+- mux-control-names: Shall be "usb_switch" if mux-controls is specified.
+- usb-switch-states: Two u32's defining the state to set on the mux for the
+  host mode and device modes respectively.
 
 i.mx specific properties
 - fsl,usbmisc: phandler of non-core register device, with one
@@ -102,4 +107,7 @@ Example:
 		rx-burst-size-dword = <0x10>;
 		extcon = <0>, <&usb_id>;
 		phy-clkgate-delay-us = <400>;
+		mux-controls = <&usb_switch>;
+		mux-control-names = "usb_switch";
+		usb-switch-states = <0>, <1>;
 	};
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index b17ed3a9a304..6531d771f296 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -64,6 +64,7 @@
 #include <linux/of.h>
 #include <linux/regulator/consumer.h>
 #include <linux/usb/ehci_def.h>
+#include <linux/mux/consumer.h>
 
 #include "ci.h"
 #include "udc.h"
@@ -606,6 +607,7 @@ static int ci_get_platdata(struct device *dev,
 {
 	struct extcon_dev *ext_vbus, *ext_id;
 	struct ci_hdrc_cable *cable;
+	struct ci_hdrc_switch *usb_switch;
 	int ret;
 
 	if (!platdata->phy_mode)
@@ -690,6 +692,21 @@ static int ci_get_platdata(struct device *dev,
 	if (of_find_property(dev->of_node, "non-zero-ttctrl-ttha", NULL))
 		platdata->flags |= CI_HDRC_SET_NON_ZERO_TTHA;
 
+	if (IS_ENABLED(CONFIG_MULTIPLEXER)) {
+		usb_switch = &platdata->usb_switch;
+		usb_switch->mux = devm_mux_control_get(dev, "usb_switch");
+		if (!IS_ERR(usb_switch->mux)) {
+			if (of_property_read_u32_index(dev->of_node,
+						       "usb-switch-states",
+						       0, &usb_switch->device))
+				return -EINVAL;
+			if (of_property_read_u32_index(dev->of_node,
+						       "usb-switch-states",
+						       1, &usb_switch->host))
+				return -EINVAL;
+		}
+	}
+
 	ext_id = ERR_PTR(-ENODEV);
 	ext_vbus = ERR_PTR(-ENODEV);
 	if (of_property_read_bool(dev->of_node, "extcon")) {
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 18cb8e46262d..9fd23ecc2da3 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -25,6 +25,7 @@
 #include <linux/usb/hcd.h>
 #include <linux/usb/chipidea.h>
 #include <linux/regulator/consumer.h>
+#include <linux/mux/consumer.h>
 
 #include "../host/ehci.h"
 
@@ -123,6 +124,13 @@ static int host_start(struct ci_hdrc *ci)
 	if (usb_disabled())
 		return -ENODEV;
 
+	if (!IS_ERR(ci->platdata->usb_switch.mux)) {
+		ret = mux_control_select(ci->platdata->usb_switch.mux,
+					 ci->platdata->usb_switch.host);
+		if (ret)
+			return ret;
+	}
+
 	hcd = __usb_create_hcd(&ci_ehci_hc_driver, ci->dev->parent,
 			       ci->dev, dev_name(ci->dev), NULL);
 	if (!hcd)
@@ -205,6 +213,8 @@ static void host_stop(struct ci_hdrc *ci)
 		if (ci->platdata->reg_vbus && !ci_otg_is_fsm_mode(ci) &&
 			(ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON))
 				regulator_disable(ci->platdata->reg_vbus);
+		if (!IS_ERR(ci->platdata->usb_switch.mux))
+			mux_control_deselect(ci->platdata->usb_switch.mux);
 	}
 	ci->hcd = NULL;
 	ci->otg.host = NULL;
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index d68b125796f9..ab3355905740 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -22,6 +22,7 @@
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg-fsm.h>
 #include <linux/usb/chipidea.h>
+#include <linux/mux/consumer.h>
 
 #include "ci.h"
 #include "udc.h"
@@ -1899,6 +1900,13 @@ static int udc_start(struct ci_hdrc *ci)
 	ci->gadget.name         = ci->platdata->name;
 	ci->gadget.otg_caps	= otg_caps;
 
+	if (!IS_ERR(ci->platdata->usb_switch.mux)) {
+		retval = mux_control_select(ci->platdata->usb_switch.mux,
+					    ci->platdata->usb_switch.device);
+		if (retval)
+			return retval;
+	}
+
 	if (ci->is_otg && (otg_caps->hnp_support || otg_caps->srp_support ||
 						otg_caps->adp_support))
 		ci->gadget.is_otg = 1;
@@ -1982,6 +1990,9 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 		hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
 
 	ci->vbus_active = 0;
+
+	if (!IS_ERR(ci->platdata->usb_switch.mux))
+		mux_control_deselect(ci->platdata->usb_switch.mux);
 }
 
 /**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index c5fdfcf99828..559bd470b8c0 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -9,6 +9,7 @@
 #include <linux/usb/otg.h>
 
 struct ci_hdrc;
+struct mux_control;
 
 /**
  * struct ci_hdrc_cable - structure for external connector cable state tracking
@@ -29,6 +30,18 @@ struct ci_hdrc_cable {
 	struct notifier_block		nb;
 };
 
+/**
+ * struct ci_hdrc_switch - structure for usb mux control
+ * @mux: mux to set @host state or @device state on during role switch
+ * @host: Value to set for mux to connect D+/D- to host D+/D- lines
+ * @device: Value to set for mux to connect D+/D- to device D+/D- lines
+ */
+struct ci_hdrc_switch {
+	struct mux_control		*mux;
+	int				host;
+	int				device;
+};
+
 struct ci_hdrc_platform_data {
 	const char	*name;
 	/* offset of the capability registers */
@@ -74,6 +87,7 @@ struct ci_hdrc_platform_data {
 	/* VBUS and ID signal state tracking, using extcon framework */
 	struct ci_hdrc_cable		vbus_extcon;
 	struct ci_hdrc_cable		id_extcon;
+	struct ci_hdrc_switch		usb_switch;
 	u32			phy_clkgate_delay_us;
 };
 
-- 
2.10.0.297.gf6727b0

[toc] | [prev] | [next] | [standalone]

#1686965 — Re: [PATCH 2/3] usb: chipidea: Hook into mux framework to toggle usb switch

Quoting Peter Rosin (2017-07-11 23:45:24)
> On 2017-07-12 03:02, Stephen Boyd wrote:
> > @@ -102,4 +107,7 @@ Example:
> >               rx-burst-size-dword = <0x10>;
> >               extcon = <0>, <&usb_id>;
> >               phy-clkgate-delay-us = <400>;
> > +             mux-controls = <&usb_switch>;
> > +             mux-control-names = "usb_switch";
> > +             usb-switch-states = <0>, <1>;
> 
> I don't see the need for usb-switch-states? Just assume states 0/1 and
> if someone later needs some other states, make them add a property that
> overrides the defaults. Just document that 0 is host and 1 is device.
> 

Fine by me. Rob H do you agree?

> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 18cb8e46262d..9fd23ecc2da3 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/usb/hcd.h>
> >  #include <linux/usb/chipidea.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/mux/consumer.h>
> >  
> >  #include "../host/ehci.h"
> >  
> > @@ -123,6 +124,13 @@ static int host_start(struct ci_hdrc *ci)
> >       if (usb_disabled())
> >               return -ENODEV;
> >  
> > +     if (!IS_ERR(ci->platdata->usb_switch.mux)) {
> > +             ret = mux_control_select(ci->platdata->usb_switch.mux,
> > +                                      ci->platdata->usb_switch.host);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> 
> You *must* call mux_control_deselect to clean up if there is a failure
> later in host_start. Is that handled in some non-obvious way?

Good catch. Thanks. I'll add in the unwinding on the error path.

[toc] | [prev] | [next] | [standalone]

#1689295 — Re: [PATCH 2/3] usb: chipidea: Hook into mux framework to toggle usb switch

On Thu, Jul 13, 2017 at 03:29:43PM -0700, Stephen Boyd wrote:
> Quoting Peter Rosin (2017-07-11 23:45:24)
> > On 2017-07-12 03:02, Stephen Boyd wrote:
> > > @@ -102,4 +107,7 @@ Example:
> > >               rx-burst-size-dword = <0x10>;
> > >               extcon = <0>, <&usb_id>;
> > >               phy-clkgate-delay-us = <400>;
> > > +             mux-controls = <&usb_switch>;
> > > +             mux-control-names = "usb_switch";

Pointless to have a name when there is only 1.

> > > +             usb-switch-states = <0>, <1>;
> > 
> > I don't see the need for usb-switch-states? Just assume states 0/1 and
> > if someone later needs some other states, make them add a property that
> > overrides the defaults. Just document that 0 is host and 1 is device.
> > 
> 
> Fine by me. Rob H do you agree?

Yes.

Rob

[toc] | [prev] | [next] | [standalone]

#1686970

Quoting Peter Rosin (2017-07-11 22:04:46)
> On 2017-07-12 03:02, Stephen Boyd wrote:
> > This patchset adds support for the TC7USB40MU usb mux found on 
> > db410c 96boards platforms via the new multiplexer framework and
> > hooks that into the chipidea driver. This allows us to properly
> > control host or device mode on this board via the sysfs knob.
> > 
> > So far I've only tested this on db410c, and there are some rough
> > edges to finish off before it can merge. Also I'm experiencing
> > odd behavior with switching the role while gadget is enabled and
> > the micro-usb cable is kept connected. Not sure what's wrong but
> > it seems like the gadget never gets disconnected? I'll investigate
> > more.
> > 
> > TODO:
> > 
> >  1. The mux framework has to be selected for consumers to use it. We'll
> >     need some stubs in the consumer header file to allow compilation to
> >     continue without mux always enabled by consumers.
> 
> Instead of "depends on MULTIPLEXER", just add "select MULTIPLEXER"
> to the Kconfig. Otherwise, you'll have to convince Linus that we
> really do need a Kconfig question for the subsystem :-)
> 
> https://lkml.org/lkml/2017/7/4/118

Ok. I'll add a select to the chipidea driver.

> 
> >  2. We probably need some sort of mux_control_get_optional() API so that
> >     we know if there was an error getting the mux control, instead of just
> >     ignoring errors. For now I can pass up EPROBE_DEFER errors and ignore
> >     other errors and consider it "missing from DT".
> 
> Yes, mux_control_get_optional should be easy to add.
> 
> >  3. Maybe we can get rid of the mux driver and just use mux-gpio.c with
> >     a compatible string update? I split it off because we may want to
> >     support the "S" pin on the TC7USB40MU one day that shuts off both

Oh this is a typo. I mean "OE" pin.

> >     mux outputs.
> 
> Maybe no need for a compatible update either, if it works to do something
> like this in the DT?
> 
>         usb_switch: usb-switch {
>                 compatible = "gpio-mux";
>                 mux-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>,
>                             <&pm8916_gpios XXX GPIO_ACTIVE_XXX>;
>                 idle-state = <2>;
>                 #mux-control-cells = <0>;
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&usb_sw_sel_pm>;
>         };
> 
> But I obviously know little about how things are wired and really works,
> so that might be totally off...
> 
> Otherwise, maybe a generic mux-pinctrl driver would do the trick?
> (compare with drivers/i2c/muxes/i2c-mux-pinctrl.c)
> 

Agreed. On the board I have the "OE" pin is shorted so that it always
outputs a signal. For someone else they could do what you say and add
another pin to gpio-mux binding and things will still work.

[toc] | [prev] | [next] | [standalone]

#1687674

Quoting Stephen Boyd (2017-07-13 15:35:02)
> Quoting Peter Rosin (2017-07-11 22:04:46)
> > 
> > Maybe no need for a compatible update either, if it works to do something
> > like this in the DT?
> > 
> >         usb_switch: usb-switch {
> >                 compatible = "gpio-mux";
> >                 mux-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>,
> >                             <&pm8916_gpios XXX GPIO_ACTIVE_XXX>;
> >                 idle-state = <2>;
> >                 #mux-control-cells = <0>;
> >                 pinctrl-names = "default";
> >                 pinctrl-0 = <&usb_sw_sel_pm>;
> >         };
> > 
> > But I obviously know little about how things are wired and really works,
> > so that might be totally off...
> > 
> > Otherwise, maybe a generic mux-pinctrl driver would do the trick?
> > (compare with drivers/i2c/muxes/i2c-mux-pinctrl.c)
> > 
> 
> Agreed.

Testing looks good when I use the gpio-mux binding. The only thing I
noticed is that gpio-mux driver is requesting the gpio with
GPIOD_OUT_LOW. Is that intentional? I worry that may randomly mux the
D+/D- lines during probe if the gpio is asserted at probe time. It isn't
a problem for me right now, because the mux is power on defaulted to
have the gpio deasserted, but it may be a problem if the default
changes.

[toc] | [prev] | [next] | [standalone]

#1689290

On Thu, Jul 13, 2017 at 03:35:02PM -0700, Stephen Boyd wrote:
> Quoting Peter Rosin (2017-07-11 22:04:46)
> > On 2017-07-12 03:02, Stephen Boyd wrote:
> > > This patchset adds support for the TC7USB40MU usb mux found on 
> > > db410c 96boards platforms via the new multiplexer framework and
> > > hooks that into the chipidea driver. This allows us to properly
> > > control host or device mode on this board via the sysfs knob.
> > > 
> > > So far I've only tested this on db410c, and there are some rough
> > > edges to finish off before it can merge. Also I'm experiencing
> > > odd behavior with switching the role while gadget is enabled and
> > > the micro-usb cable is kept connected. Not sure what's wrong but
> > > it seems like the gadget never gets disconnected? I'll investigate
> > > more.
> > > 
> > > TODO:
> > > 
> > >  1. The mux framework has to be selected for consumers to use it. We'll
> > >     need some stubs in the consumer header file to allow compilation to
> > >     continue without mux always enabled by consumers.
> > 
> > Instead of "depends on MULTIPLEXER", just add "select MULTIPLEXER"
> > to the Kconfig. Otherwise, you'll have to convince Linus that we
> > really do need a Kconfig question for the subsystem :-)
> > 
> > https://lkml.org/lkml/2017/7/4/118
> 
> Ok. I'll add a select to the chipidea driver.
> 
> > 
> > >  2. We probably need some sort of mux_control_get_optional() API so that
> > >     we know if there was an error getting the mux control, instead of just
> > >     ignoring errors. For now I can pass up EPROBE_DEFER errors and ignore
> > >     other errors and consider it "missing from DT".
> > 
> > Yes, mux_control_get_optional should be easy to add.
> > 
> > >  3. Maybe we can get rid of the mux driver and just use mux-gpio.c with
> > >     a compatible string update? I split it off because we may want to
> > >     support the "S" pin on the TC7USB40MU one day that shuts off both
> 
> Oh this is a typo. I mean "OE" pin.
> 
> > >     mux outputs.
> > 
> > Maybe no need for a compatible update either, if it works to do something
> > like this in the DT?

Please keep the compatible. Use "gpio-mux" as a fallback if you wish.

Rob

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web