On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote: > Some hardware implementations require to configure pins differently > according to the USB role (host/device), this can be an update of the > pins routing or a simple GPIO value change. > > This patch introduces new optional "host" and "device" pinctrls. > If these pinctrls are defined by the device, they are respectively > selected on host/device role start. > > If a default pinctrl exist, it is restored on host/device role stop. > The implementation looks reasonable, but we're actually just toggling a gpio using pinctrl states. Do you see any reason not to control this mux through the gpio api? Regards, Bjorn > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx> > --- > drivers/usb/chipidea/core.c | 19 +++++++++++++++++++ > drivers/usb/chipidea/host.c | 9 +++++++++ > drivers/usb/chipidea/udc.c | 9 +++++++++ > include/linux/usb/chipidea.h | 6 ++++++ > 4 files changed, 43 insertions(+) > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > index 85fc6db..03e52fc 100644 > --- a/drivers/usb/chipidea/core.c > +++ b/drivers/usb/chipidea/core.c > @@ -46,6 +46,7 @@ > #include <linux/extcon.h> > #include <linux/phy/phy.h> > #include <linux/platform_device.h> > +#include <linux/pinctrl/consumer.h> > #include <linux/module.h> > #include <linux/idr.h> > #include <linux/interrupt.h> > @@ -723,6 +724,24 @@ static int ci_get_platdata(struct device *dev, > else > cable->connected = false; > } > + > + platdata->pctl = devm_pinctrl_get(dev); > + if (!IS_ERR(platdata->pctl)) { > + struct pinctrl_state *p; > + > + p = pinctrl_lookup_state(platdata->pctl, "default"); > + if (!IS_ERR(p)) > + platdata->pins_default = p; > + > + p = pinctrl_lookup_state(platdata->pctl, "host"); > + if (!IS_ERR(p)) > + platdata->pins_host = p; > + > + p = pinctrl_lookup_state(platdata->pctl, "device"); > + if (!IS_ERR(p)) > + platdata->pins_device = p; > + } > + > return 0; > } > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c > index af45aa32..55dbd49 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/pinctrl/consumer.h> > > #include "../host/ehci.h" > > @@ -150,6 +151,10 @@ static int host_start(struct ci_hdrc *ci) > } > } > > + if (ci->platdata->pins_host) > + pinctrl_select_state(ci->platdata->pctl, > + ci->platdata->pins_host); > + > ret = usb_add_hcd(hcd, 0, 0); > if (ret) { > goto disable_reg; > @@ -194,6 +199,10 @@ static void host_stop(struct ci_hdrc *ci) > } > ci->hcd = NULL; > ci->otg.host = NULL; > + > + if (ci->platdata->pins_host && ci->platdata->pins_default) > + pinctrl_select_state(ci->platdata->pctl, > + ci->platdata->pins_default); > } > > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > index 9852ec5..c04384e 100644 > --- a/drivers/usb/chipidea/udc.c > +++ b/drivers/usb/chipidea/udc.c > @@ -19,6 +19,7 @@ > #include <linux/usb/gadget.h> > #include <linux/usb/otg-fsm.h> > #include <linux/usb/chipidea.h> > +#include <linux/pinctrl/consumer.h> > > #include "ci.h" > #include "udc.h" > @@ -1965,6 +1966,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci) > > static int udc_id_switch_for_device(struct ci_hdrc *ci) > { > + if (ci->platdata->pins_device) > + pinctrl_select_state(ci->platdata->pctl, > + ci->platdata->pins_device); > + > if (ci->is_otg) > /* Clear and enable BSV irq */ > hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE, > @@ -1983,6 +1988,10 @@ 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 (ci->platdata->pins_device && ci->platdata->pins_default) > + pinctrl_select_state(ci->platdata->pctl, > + ci->platdata->pins_default); > } > > /** > diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h > index 07f9936..63758c3 100644 > --- a/include/linux/usb/chipidea.h > +++ b/include/linux/usb/chipidea.h > @@ -77,6 +77,12 @@ struct ci_hdrc_platform_data { > struct ci_hdrc_cable vbus_extcon; > struct ci_hdrc_cable id_extcon; > u32 phy_clkgate_delay_us; > + > + /* pins */ > + struct pinctrl *pctl; > + struct pinctrl_state *pins_default; > + struct pinctrl_state *pins_host; > + struct pinctrl_state *pins_device; > }; > > /* Default offset of capability registers */ > -- > 2.7.4 >