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.poul...@linaro.org>
> ---
> 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
>
