John,
On Wed, Dec 2, 2015 at 11:15 AM, John Youn <John.Youn@xxxxxxxxxxxx> wrote:
> This commit adds separate functions for getting the host and device
> specific hardware params. These functions check whether the parameters
> need to be read at all, depending on dr_mode, and they force the mode
> only if necessary. This saves some delays during probe.
>
> Signed-off-by: John Youn <johnyoun@xxxxxxxxxxxx>
> ---
> drivers/usb/dwc2/core.c | 93 +++++++++++++++++++++++++++++++++++++------------
> drivers/usb/dwc2/core.h | 1 +
> 2 files changed, 71 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index caaff42..91d63a8 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -3159,17 +3159,76 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
> usleep_range(25000, 50000);
> }
>
> +/*
> + * Gets host hardware parameters. Forces host mode if not currently in
> + * host mode. Should be called immediately after a core soft reset in
> + * order to get the reset values.
> + */
> +static void dwc2_get_host_hwparams(struct dwc2_hsotg *hsotg)
> +{
> + struct dwc2_hw_params *hw = &hsotg->hw_params;
> + u32 gnptxfsiz;
> + u32 hptxfsiz;
> + bool forced;
> +
> + if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
> + return;
> +
> + forced = dwc2_force_host_if_needed(hsotg);
> +
> + gnptxfsiz = dwc2_readl(hsotg->regs + GNPTXFSIZ);
> + hptxfsiz = dwc2_readl(hsotg->regs + HPTXFSIZ);
> + dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz);
> + dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz);
> +
> + if (forced)
> + dwc2_clear_force_mode(hsotg);
> +
> + hw->host_nperio_tx_fifo_size = (gnptxfsiz & FIFOSIZE_DEPTH_MASK) >>
> + FIFOSIZE_DEPTH_SHIFT;
> + hw->host_perio_tx_fifo_size = (hptxfsiz & FIFOSIZE_DEPTH_MASK) >>
> + FIFOSIZE_DEPTH_SHIFT;
> +}
> +
> +/*
> + * Gets device hardware parameters. Forces device mode if not
> + * currently in device mode. Should be called immediately after a core
> + * soft reset in order to get the reset values.
> + */
> +static void dwc2_get_dev_hwparams(struct dwc2_hsotg *hsotg)
> +{
> + struct dwc2_hw_params *hw = &hsotg->hw_params;
> + bool forced;
> + u32 gnptxfsiz;
> +
> + if (hsotg->dr_mode == USB_DR_MODE_HOST)
> + return;
> +
> + forced = dwc2_force_device_if_needed(hsotg);
Interesting. You're getting a new parameter that's never been needed
in the code before and (as far as I can tell) isn't used anywhere in
your series. I presume this is in prep for a future patch that uses
this?
At the moment you're burning a decent delay to read this unused
parameter (potentially up to 100ms), so hopefully there's a good use
for it eventually...
> +
> + gnptxfsiz = dwc2_readl(hsotg->regs + GNPTXFSIZ);
> + dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz);
> +
> + if (forced)
> + dwc2_clear_force_mode(hsotg);
> +
> + hw->dev_nperio_tx_fifo_size = (gnptxfsiz & FIFOSIZE_DEPTH_MASK) >>
> + FIFOSIZE_DEPTH_SHIFT;
> +}
> +
> /**
> * During device initialization, read various hardware configuration
> * registers and interpret the contents.
> + *
> + * This should be called during driver probe. It will perform a core
> + * soft reset in order to get the reset values of the parameters.
> */
> int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
> {
> struct dwc2_hw_params *hw = &hsotg->hw_params;
> unsigned width;
> u32 hwcfg1, hwcfg2, hwcfg3, hwcfg4;
> - u32 hptxfsiz, grxfsiz, gnptxfsiz;
> - u32 gusbcfg = 0;
> + u32 grxfsiz;
> int retval;
>
> /*
> @@ -3206,23 +3265,6 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
> dev_dbg(hsotg->dev, "hwcfg4=%08x\n", hwcfg4);
> dev_dbg(hsotg->dev, "grxfsiz=%08x\n", grxfsiz);
>
> - /* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value */
> - if (hsotg->dr_mode != USB_DR_MODE_HOST) {
> - gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
> - dwc2_writel(gusbcfg | GUSBCFG_FORCEHOSTMODE,
> - hsotg->regs + GUSBCFG);
> - usleep_range(25000, 50000);
> - }
> -
> - gnptxfsiz = dwc2_readl(hsotg->regs + GNPTXFSIZ);
> - hptxfsiz = dwc2_readl(hsotg->regs + HPTXFSIZ);
> - dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz);
> - dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz);
> - if (hsotg->dr_mode != USB_DR_MODE_HOST) {
> - dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
> - usleep_range(25000, 50000);
> - }
> -
optional nit: To make the function change less, it seems like
dwc2_get_host_hwparams() and dwc2_get_dev_hwparams() could be here
instead of below. Then your forcing of modes / etc could happen in
the same relative place that they happened before.
This does work though. Aside from nits:
This is tested on some rk3288-based devices with 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
