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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



John,

On Wed, Dec 2, 2015 at 11:13 AM, John Youn <John.Youn@xxxxxxxxxxxx> 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 <johnyoun@xxxxxxxxxxxx>
> ---
>  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 <dianders@xxxxxxxxxxxx>
Tested-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux