Re: [PATCH 09/18] usb: dwc2: Add dwc2_core_reset()

[Doug Anderson]

John,

On Wed, Dec 2, 2015 at 11:13 AM, John Youn  wrote:
> dwc2_core_reset() was previously renamed to
> dwc2_core_reset_and_force_mode(). Now add back dwc2_core_reset() which
> performs only a basic core reset without forcing the mode.
>
> Signed-off-by: John Youn 
> ---
>  drivers/usb/dwc2/core.c | 22 ++
>  drivers/usb/dwc2/core.h |  1 +
>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index 24f9e80..b7a733a 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -478,14 +478,12 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg 
> *hsotg)
>  }
>
>  /*
> - * Do core a soft reset of the core.  Be careful with this because it
> - * resets all the internal state machines of the core.
> + * Performs a base controller soft reset.

Wouldn't the "be careful" message still be relevant?  Presumably this
makes the mode no longer match what may be stored in hsotg->dr_mode,
so it probably wouldn't hurt to at least mention
dwc2_core_reset_and_force_mode() in the comment?

>   */
> -int dwc2_core_reset_and_force_mode(struct dwc2_hsotg *hsotg)
> +int dwc2_core_reset(struct dwc2_hsotg *hsotg)
>  {
> u32 greset;
> int count = 0;
> -   u32 gusbcfg;
>
> dev_vdbg(hsotg->dev, "%s()\n", __func__);
>
> @@ -517,6 +515,22 @@ int dwc2_core_reset_and_force_mode(struct dwc2_hsotg 
> *hsotg)
> }
> } while (!(greset & GRSTCTL_AHBIDLE));
>
> +   return 0;
> +}
> +
> +/*
> + * Do core a soft reset of the core.  Be careful with this because it
> + * resets all the internal state machines of the core.

Nothing differentiates this comment from dwc2_core_reset() other than
the warning message, and that warning message seems like it also
belongs on the other one.  Maybe at least a comment like "This will
apply mode forces as per the stored hsotg->dr_mode"

> + */
> +int dwc2_core_reset_and_force_mode(struct dwc2_hsotg *hsotg)
> +{
> +   int retval;
> +   u32 gusbcfg;
> +
> +   retval = dwc2_core_reset(hsotg);
> +   if (retval)
> +   return retval;
> +
> if (hsotg->dr_mode == USB_DR_MODE_HOST) {
> gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
> gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 0b4c036..53c992a 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -889,6 +889,7 @@ enum dwc2_halt_status {
>   * The following functions support initialization of the core driver 
> component
>   * and the DWC_otg controller
>   */
> +extern int dwc2_core_reset(struct dwc2_hsotg *hsotg);
>  extern int dwc2_core_reset_and_force_mode(struct dwc2_hsotg *hsotg);
>  extern void dwc2_core_host_init(struct dwc2_hsotg *hsotg);
>  extern int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg);

Other than function commenting, this seems sane.  Works across many
reboots on a farm of rk3288-based devices on a 3.14 kernel.

Reviewed-by: Douglas Anderson 
Tested-by: Douglas Anderson 
--
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 09/18] usb: dwc2: Add dwc2_core_reset()

[John Youn]

dwc2_core_reset() was previously renamed to
dwc2_core_reset_and_force_mode(). Now add back dwc2_core_reset() which
performs only a basic core reset without forcing the mode.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.c | 22 ++
 drivers/usb/dwc2/core.h |  1 +
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 24f9e80..b7a733a 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -478,14 +478,12 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg 
*hsotg)
 }
 
 /*
- * Do core a soft reset of the core.  Be careful with this because it
- * resets all the internal state machines of the core.
+ * Performs a base controller soft reset.
  */
-int dwc2_core_reset_and_force_mode(struct dwc2_hsotg *hsotg)
+int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 {
u32 greset;
int count = 0;
-   u32 gusbcfg;
 
dev_vdbg(hsotg->dev, "%s()\n", __func__);
 
@@ -517,6 +515,22 @@ int dwc2_core_reset_and_force_mode(struct dwc2_hsotg 
*hsotg)
}
} while (!(greset & GRSTCTL_AHBIDLE));
 
+   return 0;
+}
+
+/*
+ * Do core a soft reset of the core.  Be careful with this because it
+ * resets all the internal state machines of the core.
+ */
+int dwc2_core_reset_and_force_mode(struct dwc2_hsotg *hsotg)
+{
+   int retval;
+   u32 gusbcfg;
+
+   retval = dwc2_core_reset(hsotg);
+   if (retval)
+   return retval;
+
if (hsotg->dr_mode == USB_DR_MODE_HOST) {
gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 0b4c036..53c992a 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -889,6 +889,7 @@ enum dwc2_halt_status {
  * The following functions support initialization of the core driver component
  * and the DWC_otg controller
  */
+extern int dwc2_core_reset(struct dwc2_hsotg *hsotg);
 extern int dwc2_core_reset_and_force_mode(struct dwc2_hsotg *hsotg);
 extern void dwc2_core_host_init(struct dwc2_hsotg *hsotg);
 extern int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg);
-- 
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