Re: [PATCH 14/18] usb: dwc2: Add functions to set and clear force mode

[Doug Anderson]

John,

On Wed, Dec 2, 2015 at 11:14 AM, John Youn  wrote:
> Added functions to set force mode for host and device. These functions
> will check the current mode and only force if needed.
>
> Signed-off-by: John Youn 
> ---
>  drivers/usb/dwc2/core.c | 54 
> +
>  1 file changed, 54 insertions(+)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index 7090050..caaff42 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -3105,6 +3105,60 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
> dwc2_set_param_hibernation(hsotg, params->hibernation);
>  }
>
> +/*
> + * Force host mode if not in host mode. Returns true if the mode was
> + * forced.
> + */
> +static bool dwc2_force_host_if_needed(struct dwc2_hsotg *hsotg)
> +{
> +   u32 gusbcfg;
> +
> +   if (dwc2_is_host_mode(hsotg))
> +   return false;
> +
> +   gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
> +   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
> +   dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
> +   usleep_range(25000, 5);
> +
> +   return true;
> +}
> +
> +/*
> + * Force device mode if not in device mode. Returns true if the mode
> + * was forced.
> + */
> +static bool dwc2_force_device_if_needed(struct dwc2_hsotg *hsotg)
> +{
> +   u32 gusbcfg;
> +
> +   if (dwc2_is_device_mode(hsotg))
> +   return false;
> +
> +   gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
> +   gusbcfg |= GUSBCFG_FORCEDEVMODE;
> +   dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
> +   usleep_range(25000, 5);
> +
> +   return true;
> +}
> +
> +/*
> + * Clears the force mode bits.
> + */
> +static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
> +{
> +   u32 gusbcfg;
> +
> +   gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
> +   dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
> +   usleep_range(25000, 5);
> +}
> +

Other than same comments as "usb: dwc2: Reduce delay when forcing mode
in reset" (decrease range to 25000, 3 or use msleep), this looks
good to me.

Optionally I'd think it might be worth it to add:
  WARN_ON(hsotg->dr_mode == USB_DR_MODE_HOST)
...in dwc2_force_device_if_needed() and in dwc2_clear_force_mode()

...and
  WARN_ON(hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
...in dwc2_force_host_if_needed() and in dwc2_clear_force_mode()

AKA: make it clear that the caller was expected to check these things.

This is tested on some rk3288-based devices with a 3.14 kernel.

Reviewed-by: Douglas Anderson 
Tested-by: Douglas Anderson 

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/18] usb: dwc2: Add functions to set and clear force mode

[John Youn]

Added functions to set force mode for host and device. These functions
will check the current mode and only force if needed.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.c | 54 +
 1 file changed, 54 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 7090050..caaff42 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -3105,6 +3105,60 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
dwc2_set_param_hibernation(hsotg, params->hibernation);
 }
 
+/*
+ * Force host mode if not in host mode. Returns true if the mode was
+ * forced.
+ */
+static bool dwc2_force_host_if_needed(struct dwc2_hsotg *hsotg)
+{
+   u32 gusbcfg;
+
+   if (dwc2_is_host_mode(hsotg))
+   return false;
+
+   gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
+   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
+   dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
+   usleep_range(25000, 5);
+
+   return true;
+}
+
+/*
+ * Force device mode if not in device mode. Returns true if the mode
+ * was forced.
+ */
+static bool dwc2_force_device_if_needed(struct dwc2_hsotg *hsotg)
+{
+   u32 gusbcfg;
+
+   if (dwc2_is_device_mode(hsotg))
+   return false;
+
+   gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
+   gusbcfg |= GUSBCFG_FORCEDEVMODE;
+   dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
+   usleep_range(25000, 5);
+
+   return true;
+}
+
+/*
+ * Clears the force mode bits.
+ */
+static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
+{
+   u32 gusbcfg;
+
+   gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
+   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
+   dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
+   usleep_range(25000, 5);
+}
+
 /**
  * During device initialization, read various hardware configuration
  * registers and interpret the contents.
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html