Skip to contentSkip to conversation

usb: chipidea: Hook into mux framework to toggle usb switch

PATCH In review Yossi Mansharoff 3 participants 7 messages Apr 17, 2018 – May 14, 2018

Subsystems (2)
USB Chipidea USB high speed dual role controller
lore
Share & tools
lore

Yossi Mansharoff

author
#0 lore raw

PATCH v2 1/1 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 <[email protected]>
Cc Peter Chen <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc <[email protected]>
Signed-off-by Stephen Boyd <[email protected]>
Signed-off-by Yossi Mansharoff <[email protected]>
---
 drivers/usb/chipidea/Kconfig |  2 ++
 drivers/usb/chipidea/core.c  |  5 +++++
 drivers/usb/chipidea/host.c  |  7 +++++++
 drivers/usb/chipidea/udc.c   | 13 ++++++++++++-
 include/linux/usb/chipidea.h |  2 ++
 5 files changed, 28 insertions(+), 1 deletion(-)
drivers/usb/chipidea/Kconfig +2 -0
index 785f0ed..6a8e635 100644
@@ -3,6 +3,8 @@ config USB_CHIPIDEA
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
 	select EXTCON
 	select RESET_CONTROLLER
+	select MULTIPLEXER
+	select MUX_GPIO
 	help
 	  Say Y here if your system has a dual role high speed USB
 	  controller based on ChipIdea silicon IP. It supports:
drivers/usb/chipidea/core.c +5 -0
index 33ae87f..8fa0991 100644
@@ -61,6 +61,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"

@@ -687,6 +688,10 @@ 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;
 
+	platdata->usb_switch = devm_mux_control_get_optional(dev, "usb_switch");
+	if (IS_ERR(platdata->usb_switch))
+		return PTR_ERR(platdata->usb_switch);
+
 	ext_id = ERR_PTR(-ENODEV);
 	ext_vbus = ERR_PTR(-ENODEV);
 	if (of_property_read_bool(dev->of_node, "extcon")) {
drivers/usb/chipidea/host.c +7 -0
index af45aa32..d9d2d00 100644
@@ -13,6 +13,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"
 

@@ -161,6 +162,10 @@ static int host_start(struct ci_hdrc *ci)
 		if (ci_otg_is_fsm_mode(ci)) {
 			otg->host = &hcd->self;
 			hcd->self.otg_port = 1;
+		} else {
+			ret = mux_control_select(ci->platdata->usb_switch, 1);
+			if (ret)
+				goto disable_reg;
 		}
 	}
 

@@ -181,6 +186,8 @@ static void host_stop(struct ci_hdrc *ci)
 	struct usb_hcd *hcd = ci->hcd;
 
 	if (hcd) {
+		if (!ci_otg_is_fsm_mode(ci))
+			mux_control_deselect(ci->platdata->usb_switch);
 		if (ci->platdata->notify_event)
 			ci->platdata->notify_event(ci,
 				CI_HDRC_CONTROLLER_STOPPED_EVENT);
drivers/usb/chipidea/udc.c +12 -1
index 9852ec5..209d3f6 100644
@@ -19,6 +19,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"

@@ -1965,16 +1966,26 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
 
 static int udc_id_switch_for_device(struct ci_hdrc *ci)
 {
+	int ret = 0;
+
 	if (ci->is_otg)
 		/* Clear and enable BSV irq */
 		hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
 					OTGSC_BSVIS | OTGSC_BSVIE);
 
-	return 0;
+	if (!ci_otg_is_fsm_mode(ci))
+		ret = mux_control_select(ci->platdata->usb_switch, 0);
+
+	if (ci->is_otg && ret)
+		hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
+
+	return ret;
 }
 
 static void udc_id_switch_for_host(struct ci_hdrc *ci)
 {
+	mux_control_deselect(ci->platdata->usb_switch);
+
 	/*
 	 * host doesn't care B_SESSION_VALID event
 	 * so clear and disbale BSV irq
include/linux/usb/chipidea.h +2 -0
index 07f9936..9ea55a1 100644
@@ -10,6 +10,7 @@
 #include <linux/usb/otg.h>
 
 struct ci_hdrc;
+struct mux_control;
 
 /**
  * struct ci_hdrc_cable - structure for external connector cable state tracking

@@ -76,6 +77,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 mux_control		*usb_switch;
 	u32			phy_clkgate_delay_us;
 };
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw
Quoted 9 lines · Yossi Mansharoff

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.

Ok, so this is v2. Please describe what is different from v1. I have told you before that this information helps.

Quoted 1 line

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

This link returns 404 for me.

Cheers,
Peter

author
#2 lore raw
Quoted 14 lines · Peter Rosin
Quoted 9 lines · Yossi Mansharoff

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.

Ok, so this is v2. Please describe what is different from v1. I have told you before that this information helps.

Quoted 6 lines
Quoted 1 line

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

This link returns 404 for me.

Cheers,
Peter

Hi,
This patch was split apart from the original patch into two patches one for chipidea and the other for bindings.
this patch has no other changes two the code.

I will update the link.

thanks
Yossi

#3 lore raw
Quoted 24 lines · On 2018-04-18 07:48, [email protected] wrote:
Quoted 14 lines · Peter Rosin
Quoted 9 lines · Yossi Mansharoff

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.

Ok, so this is v2. Please describe what is different from v1. I have told you before that this information helps.

Quoted 7 lines
Quoted 2 lines

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

This link returns 404 for me.

Cheers,
Peter

Quoted 6 lines

Hi,
This patch was split apart from the original patch into two patches one for chipidea and the other for bindings.
this patch has no other changes two the code.

I will update the link.

Just a note: I will not feed the mux_control_get_optional patch upstream until I feel confident that this patch is going also heading upstream.

Cheers,
Peter

#4 lore raw
In reply to Yossi Mansharoff

drivers/usb/chipidea/Kconfig config USB_CHIPIDEA · line 7

Quoted 11 lines
drivers/usb/chipidea/Kconfig +0 -0
@@ -3,6 +3,8 @@ config USB_CHIPIDEA
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD

&& !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA

drivers/usb/chipidea/Kconfig +2 -0
 	select EXTCON
 	select RESET_CONTROLLER
+	select MULTIPLEXER
+	select MUX_GPIO

The above two configurations are only used at your specific platforms, please add them at either your platform defconfig or the related hardware driver's Kconfig.

Quoted 39 lines
drivers/usb/chipidea/Kconfig +0 -0
 	help
 	  Say Y here if your system has a dual role high speed USB
 	  controller based on ChipIdea silicon IP. It supports:
drivers/usb/chipidea/core.c index renamed from drivers/usb/chipidea/core.c +0 -0

33ae87f..8fa0991 100644

drivers/usb/chipidea/core.c +5 -0
@@ -61,6 +61,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"

@@ -687,6 +688,10 @@ 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;
 
+	platdata->usb_switch = devm_mux_control_get_optional(dev, "usb_switch");
+	if (IS_ERR(platdata->usb_switch))
+		return PTR_ERR(platdata->usb_switch);
+
 	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 af45aa32..d9d2d00 100644

drivers/usb/chipidea/host.c +1 -0
@@ -13,6 +13,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"

drivers/usb/chipidea/host.c static int host_start(struct ci_hdrc *ci) · line 168

Quoted 7 lines
drivers/usb/chipidea/host.c +4 -0
@@ -161,6 +162,10 @@ static int host_start(struct ci_hdrc *ci)
 		if (ci_otg_is_fsm_mode(ci)) {
 			otg->host = &hcd->self;
 			hcd->self.otg_port = 1;
+		} else {
+			ret = mux_control_select(ci->platdata->usb_switch, 1);
+			if (ret)
+				goto disable_reg;

What will happen if ci->platdata->usb_switch is NULL?

drivers/usb/chipidea/host.c static void host_stop(struct ci_hdrc *ci) · line 190

Quoted 9 lines
drivers/usb/chipidea/host.c +2 -0
 		}
 	}
 

@@ -181,6 +186,8 @@ static void host_stop(struct ci_hdrc *ci)
 	struct usb_hcd *hcd = ci->hcd;
 
 	if (hcd) {
+		if (!ci_otg_is_fsm_mode(ci))
+			mux_control_deselect(ci->platdata->usb_switch);

Ditto.

Quoted 19 lines
drivers/usb/chipidea/host.c +0 -0
 		if (ci->platdata->notify_event)
 			ci->platdata->notify_event(ci,
 				CI_HDRC_CONTROLLER_STOPPED_EVENT);
drivers/usb/chipidea/udc.c index renamed from drivers/usb/chipidea/udc.c +0 -0

9852ec5..209d3f6 100644

drivers/usb/chipidea/udc.c +2 -0
@@ -19,6 +19,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"

@@ -1965,16 +1966,26 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
 
 static int udc_id_switch_for_device(struct ci_hdrc *ci)  {
+	int ret = 0;

drivers/usb/chipidea/udc.c void ci_hdrc_gadget_destroy(struct ci_hdrc *ci) · line 1977

Quoted 10 lines
drivers/usb/chipidea/udc.c +4 -1
+
 	if (ci->is_otg)
 		/* Clear and enable BSV irq */
 		hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
 					OTGSC_BSVIS | OTGSC_BSVIE);
 
-	return 0;
+	if (!ci_otg_is_fsm_mode(ci))
+		ret = mux_control_select(ci->platdata->usb_switch, 0);
+

Ditto

Quoted 26 lines
drivers/usb/chipidea/udc.c +6 -0
+	if (ci->is_otg && ret)
+		hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS,
 OTGSC_BSVIS);
+
+	return ret;
 }
 
 static void udc_id_switch_for_host(struct ci_hdrc *ci)  {
+	mux_control_deselect(ci->platdata->usb_switch);
+
 	/*
 	 * host doesn't care B_SESSION_VALID event
 	 * so clear and disbale BSV irq
include/linux/usb/chipidea.h index renamed from include/linux/usb/chipidea.h +0 -0

07f9936..9ea55a1 100644

include/linux/usb/chipidea.h +1 -0
@@ -10,6 +10,7 @@
 #include <linux/usb/otg.h>
 
 struct ci_hdrc;
+struct mux_control;
 
 /**
  * struct ci_hdrc_cable - structure for external connector cable state tracking @@ -

76,6 +77,7 @@ struct ci_hdrc_platform_data {

include/linux/usb/chipidea.h struct ci_hdrc_platform_data { · line 81

Quoted 5 lines
include/linux/usb/chipidea.h +1 -0
 	/* VBUS and ID signal state tracking, using extcon framework */
 	struct ci_hdrc_cable		vbus_extcon;
 	struct ci_hdrc_cable		id_extcon;
+	struct mux_control		*usb_switch;
 	u32			phy_clkgate_delay_us;

If CONFIG_USB_CHIPIDEA_HOST is not defined, it may cause build error

Peter

#5 lore raw
Quoted 14 lines · Peter Chen

drivers/usb/chipidea/Kconfig config USB_CHIPIDEA · line 7

Quoted 11 lines
drivers/usb/chipidea/Kconfig +0 -0
@@ -3,6 +3,8 @@ config USB_CHIPIDEA
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD

&& !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA

drivers/usb/chipidea/Kconfig +2 -0
 	select EXTCON
 	select RESET_CONTROLLER
+	select MULTIPLEXER
+	select MUX_GPIO

The above two configurations are only used at your specific platforms, please add them at either your platform defconfig or the related hardware driver's Kconfig.

"select MUX_GPIO" is indeed questionable and should be somewhere else because this driver will work with any other mux as well. It's simply something else that requires it. If it was the case that MUX_GPIO is indeed required then the whole use of the mux subsystem is questionable and the thing controlled might as well be controlled directly with the GPIO line. The mux subsystem is good when a single mux "controller" is shared between several unrelated drivers utilizing different muxes controlled by that same mux "controller" (think several muxes controlled by the same GPIO line/lines). The mux subsystem is also useful when the driver does not want to handle/know how the specific mux is controlled. That said, it's of course not wrong to use the mux subsystem in cases like this either, but I think it might be much easier and direct to just twiddle the single GPIO line directly here?

Or do you expect some future HW variant that will use some other means to control this mux?

Quoted 38 lines
Quoted 38 lines
drivers/usb/chipidea/Kconfig +0 -0
 	help
 	  Say Y here if your system has a dual role high speed USB
 	  controller based on ChipIdea silicon IP. It supports:
drivers/usb/chipidea/core.c index renamed from drivers/usb/chipidea/core.c +0 -0

33ae87f..8fa0991 100644

drivers/usb/chipidea/core.c +5 -0
@@ -61,6 +61,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"

@@ -687,6 +688,10 @@ 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;
 
+	platdata->usb_switch = devm_mux_control_get_optional(dev, "usb_switch");
+	if (IS_ERR(platdata->usb_switch))
+		return PTR_ERR(platdata->usb_switch);
+
 	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 af45aa32..d9d2d00 100644

drivers/usb/chipidea/host.c +1 -0
@@ -13,6 +13,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"
Quoted 10 lines

drivers/usb/chipidea/host.c static int host_start(struct ci_hdrc *ci) · line 168

Quoted 8 lines
drivers/usb/chipidea/host.c +4 -0
@@ -161,6 +162,10 @@ static int host_start(struct ci_hdrc *ci)
 		if (ci_otg_is_fsm_mode(ci)) {
 			otg->host = &hcd->self;
 			hcd->self.otg_port = 1;
+		} else {
+			ret = mux_control_select(ci->platdata->usb_switch, 1);
+			if (ret)
+				goto disable_reg;

What will happen if ci->platdata->usb_switch is NULL?

What has not been mentioned in this patch is that it depends on another patch which is not yet upstream. You can google for

mux: add mux_control_get_optional() API

to get an idea (it's also in linux-next). Anyway, with that patch this is not a problem.

Quoted 67 lines

drivers/usb/chipidea/host.c static void host_stop(struct ci_hdrc *ci) · line 190

Quoted 9 lines
drivers/usb/chipidea/host.c +2 -0
 		}
 	}
 

@@ -181,6 +186,8 @@ static void host_stop(struct ci_hdrc *ci)
 	struct usb_hcd *hcd = ci->hcd;
 
 	if (hcd) {
+		if (!ci_otg_is_fsm_mode(ci))
+			mux_control_deselect(ci->platdata->usb_switch);

Ditto.

Quoted 19 lines
drivers/usb/chipidea/host.c +0 -0
 		if (ci->platdata->notify_event)
 			ci->platdata->notify_event(ci,
 				CI_HDRC_CONTROLLER_STOPPED_EVENT);
drivers/usb/chipidea/udc.c index renamed from drivers/usb/chipidea/udc.c +0 -0

9852ec5..209d3f6 100644

drivers/usb/chipidea/udc.c +2 -0
@@ -19,6 +19,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"

@@ -1965,16 +1966,26 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
 
 static int udc_id_switch_for_device(struct ci_hdrc *ci)  {
+	int ret = 0;

drivers/usb/chipidea/udc.c void ci_hdrc_gadget_destroy(struct ci_hdrc *ci) · line 1977

Quoted 10 lines
drivers/usb/chipidea/udc.c +4 -1
+
 	if (ci->is_otg)
 		/* Clear and enable BSV irq */
 		hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
 					OTGSC_BSVIS | OTGSC_BSVIE);
 
-	return 0;
+	if (!ci_otg_is_fsm_mode(ci))
+		ret = mux_control_select(ci->platdata->usb_switch, 0);
+

Ditto

Quoted 23 lines
drivers/usb/chipidea/udc.c +6 -0
+	if (ci->is_otg && ret)
+		hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS,
 OTGSC_BSVIS);
+
+	return ret;
 }
 
 static void udc_id_switch_for_host(struct ci_hdrc *ci)  {
+	mux_control_deselect(ci->platdata->usb_switch);
+
 	/*
 	 * host doesn't care B_SESSION_VALID event
 	 * so clear and disbale BSV irq
include/linux/usb/chipidea.h index renamed from include/linux/usb/chipidea.h +0 -0

07f9936..9ea55a1 100644

include/linux/usb/chipidea.h +1 -0
@@ -10,6 +10,7 @@
 #include <linux/usb/otg.h>
 
 struct ci_hdrc;
+struct mux_control;
Quoted 10 lines

include/linux/usb/chipidea.h struct ci_hdrc_platform_data { · line 81

Quoted 8 lines
include/linux/usb/chipidea.h +0 -0
 /**
  * struct ci_hdrc_cable - structure for external connector cable state tracking @@ -

76,6 +77,7 @@ struct ci_hdrc_platform_data {

include/linux/usb/chipidea.h +1 -0
 	/* VBUS and ID signal state tracking, using extcon framework */
 	struct ci_hdrc_cable		vbus_extcon;
 	struct ci_hdrc_cable		id_extcon;
+	struct mux_control		*usb_switch;
 	u32			phy_clkgate_delay_us;

If CONFIG_USB_CHIPIDEA_HOST is not defined, it may cause build error

How is that related? There is a forward declaration above?

Cheers,
peda

#6 lore raw
Quoted 25 lines
Quoted 11 lines

drivers/usb/chipidea/Kconfig config USB_CHIPIDEA · line 7

Quoted 8 lines
(quoted diff) +0 -0
@@ -3,6 +3,8 @@ config USB_CHIPIDEA
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD &&

!USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA

drivers/usb/chipidea/Kconfig +2 -0
 	select EXTCON
 	select RESET_CONTROLLER
+	select MULTIPLEXER
+	select MUX_GPIO

The above two configurations are only used at your specific platforms, please add them at either your platform defconfig or the related hardware driver's

Kconfig.

"select MUX_GPIO" is indeed questionable and should be somewhere else because this driver will work with any other mux as well. It's simply something else that requires it. If it was the case that MUX_GPIO is indeed required then the whole use of the mux subsystem is questionable and the thing controlled might as well be controlled directly with the GPIO line. The mux subsystem is good when a single mux "controller" is shared between several unrelated drivers utilizing different muxes controlled by that same mux "controller" (think several muxes controlled by the same GPIO line/lines). The mux subsystem is also useful when the driver does not want to handle/know how the specific mux is controlled. That said, it's of course not wrong to use the mux subsystem in cases like this either, but I think it might be much easier and direct to just twiddle the single GPIO line directly here?

Quoted 3 lines

Or do you expect some future HW variant that will use some other means to control this mux?

No, this mux is only used at Yossi's boards, I expect these two configurations is under MUX's hardware driver Kconfig if it is existed, or under its platform defconfig. Someone has already complained too many selects increase the code size:

https://patchwork.kernel.org/patch/10349293/

Quoted 51 lines
Quoted 40 lines
Quoted 40 lines
drivers/usb/chipidea/Kconfig +0 -0
 	help
 	  Say Y here if your system has a dual role high speed USB
 	  controller based on ChipIdea silicon IP. It supports:
(quoted diff) +0 -0

b/drivers/usb/chipidea/core.c index
33ae87f..8fa0991 100644

drivers/usb/chipidea/core.c +0 -0
@@ -61,6 +61,7 @@
 #include <linux/of.h>
 #include <linux/regulator/consumer.h>  #include

<linux/usb/ehci_def.h>

drivers/usb/chipidea/core.c +5 -0
+#include <linux/mux/consumer.h>
 
 #include "ci.h"
 #include "udc.h"

@@ -687,6 +688,10 @@ 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;
 
+	platdata->usb_switch = devm_mux_control_get_optional(dev, "usb_switch");
+	if (IS_ERR(platdata->usb_switch))
+		return PTR_ERR(platdata->usb_switch);
+
 	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 af45aa32..d9d2d00
100644

drivers/usb/chipidea/host.c +1 -0
@@ -13,6 +13,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"
Quoted 10 lines

drivers/usb/chipidea/host.c static int host_start(struct ci_hdrc *ci) · line 168

Quoted 8 lines
drivers/usb/chipidea/host.c +4 -0
@@ -161,6 +162,10 @@ static int host_start(struct ci_hdrc *ci)
 		if (ci_otg_is_fsm_mode(ci)) {
 			otg->host = &hcd->self;
 			hcd->self.otg_port = 1;
+		} else {
+			ret = mux_control_select(ci->platdata->usb_switch, 1);
+			if (ret)
+				goto disable_reg;

What will happen if ci->platdata->usb_switch is NULL?

Quoted 7 lines

What has not been mentioned in this patch is that it depends on another patch which is not yet upstream. You can google for

mux: add mux_control_get_optional() API

to get an idea (it's also in linux-next). Anyway, with that patch this is not a problem.

Thanks, I see it judges NULL pointer, then it is OK.

Quoted 12 lines
Quoted 11 lines

drivers/usb/chipidea/host.c

Quoted 9 lines
drivers/usb/chipidea/host.c +0 -0
 /**
  * struct ci_hdrc_cable - structure for external connector cable

state tracking @@ -
76,6 +77,7 @@ struct ci_hdrc_platform_data {

drivers/usb/chipidea/host.c +1 -0
 	/* VBUS and ID signal state tracking, using extcon framework */
 	struct ci_hdrc_cable		vbus_extcon;
 	struct ci_hdrc_cable		id_extcon;
+	struct mux_control		*usb_switch;
 	u32			phy_clkgate_delay_us;

If CONFIG_USB_CHIPIDEA_HOST is not defined, it may cause build error

Quoted 2 lines

How is that related? There is a forward declaration above?

Sorry, I did not notice drivers/usb/chipidea/core.c also includes <linux/mux/consumer.h>.

Peter