> >> @@ -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 > > > > 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? >
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/ > >> help > >> Say Y here if your system has a dual role high speed USB > >> controller based on ChipIdea silicon IP. It supports: > >> diff --git a/drivers/usb/chipidea/core.c > >> b/drivers/usb/chipidea/core.c index > >> 33ae87f..8fa0991 100644 > >> --- a/drivers/usb/chipidea/core.c > >> +++ b/drivers/usb/chipidea/core.c > >> @@ -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 > >> --- a/drivers/usb/chipidea/host.c > >> +++ b/drivers/usb/chipidea/host.c > >> @@ -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; > > > > 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. > Thanks, I see it judges NULL pointer, then it is OK. > >> /** > >> * 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; > > > > If CONFIG_USB_CHIPIDEA_HOST is not defined, it may cause build error > > 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
