[U-Boot] [PATCH v4 5/5] usb: lpc32xx: add host USB driver

[LEMIEUX, SYLVAIN]

Hi Marek,

Is it OK to only submit the patch that need to be update as a new
version or I always have to resubmit the complete set of patches?

See comments and question below.


Sylvain

> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
>
> On Tuesday, August 04, 2015 at 11:04:42 PM, slemieux.tyco at gmail.com wrote:
> > From: Sylvain Lemieux <slemieux at tycoint.com>
> >
> > Incorporate USB driver from legacy LPCLinux NXP BSP.
> > The files taken from the legacy patch are:
> > - lpc32xx USB driver
> > - lpc3250 header file USB registers definition.
> >
> > The legacy driver was updated and clean-up as part
> > of the integration with the latest u-boot.
> >
> > Signed-off-by: Sylvain Lemieux <slemieux at tycoint.com>
> > ---
>
> Hi!
>
> > +static int wait_for_bit(void *reg, const uint32_t mask, bool set)
> > +{
> > +   unsigned int timeout = 1000000;
> > +   uint32_t val;
> > +
> > +   while (--timeout) {
>
> Where did the get_timer() stuff disappear ? :'-(

I reused the "wait_for_bit()" from " drivers/usb/host/dwc2.c";
this implement does not use "get_timer()".

Should I update this driver implementation of "wait_for_bit()" to use
"get_timer()" or keep it the same as the implementation from "dwc2.c"?

>
> > +           val = readl(reg);
> > +           if (!set)
> > +                   val = ~val;
> > +
> > +           if ((val & mask) == mask)
> > +                   return 0;
> > +
> > +           udelay(1);
> > +   }
> > +
> > +   debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n",
> > +         __func__, reg, mask, set);
> > +
> > +   return -ETIMEDOUT;
> > +}
>
> [...]
>
> > +static int usbpll_setup(void)
> > +{
> > +   uint32_t ret;
>
> u32, please fix all around.

Will do.

>
> > +   /* make sure clocks are disabled */
> > +   clrbits_le32(&clk_pwr->usb_ctrl,
> > +                CLK_USBCTRL_CLK_EN1 | CLK_USBCTRL_CLK_EN2);
> > +
> > +   /* start PLL clock input */
> > +   setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_CLK_EN1);
> > +
> > +   /* Setup PLL. */
> > +   setbits_le32(&clk_pwr->usb_ctrl,
> > +                CLK_USBCTRL_FDBK_PLUS1(192 - 1));
> > +   setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_POSTDIV_2POW(0x01));
> > +   setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_PLL_PWRUP);
> > +
> > +   ret = wait_for_bit(&clk_pwr->usb_ctrl, CLK_USBCTRL_PLL_STS, 1);
> > +   if (ret)
> > +           return -1;
> > +
> > +   /* enable PLL output */
> > +   setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_CLK_EN2);
> > +
> > +   return 0;
> > +}
>
> [...]
>
> > +   ret = wait_for_bit(&otg->otg_clk_sts, mask, 1);
> > +   if (ret)
> > +           return -1;
>
> return ret; should be just find in these cases.
>
Will do.

> > +   setbits_le32(&otg->otg_sts_ctrl, OTG_HOST_EN);
> > +   isp1301_set_value(ISP1301_I2C_OTG_CONTROL_1_SET, OTG1_VBUS_DRV);
> > +
> > +   return 0;
> > +}
> > +
> > +int usb_cpu_stop(void)
> > +{
> > +   /* vbus off */
> > +   isp1301_set_value(ISP1301_I2C_OTG_CONTROL_1_SET, OTG1_VBUS_DRV);
> > +
> > +   clrbits_le32(&otg->otg_sts_ctrl, OTG_HOST_EN);
> > +
> > +   clrbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_HCLK_EN);
> > +
> > +   return 0;
> > +}
> > +
> > +int usb_cpu_init_fail(void)
> > +{
> > +   return usb_cpu_stop();
> > +}

________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.