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

[LEMIEUX, SYLVAIN]

Hi Mark,

Thanks for the feedback;

> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Marek Vasut
> Sent: 17-Jul-15 6:09 PM
>
> On Friday, July 17, 2015 at 10:48:54 PM, slemieux.tyco at gmail.com wrote:
> > From: Sylvain Lemieux <slemieux at tycoint.com>
>
> Hi!
>
> > Incorporate DMA driver from legacy LPCLinux NXP BSP.
> > The files taken from the legacy patch are:
> > - lpc32xx USB driver
> > - lpc3250 header file USB registers definition.
> >
> > Updated driver to integrate with the latest u-boot:
> > 1) Fixed checkpatch script output in legacy code.
> > 2) Use LPC32xx definition from "cpu.h" and "clk.h".
> > 3) Incorporate USB specific register definition from "lpc3250.h" header
> > file from legacy BSP patch from LPCLinux.
> > 4) Use u-boot API for register access to remove the use of volatile in
> >    register definition taken from "lpc3250.h" header file.
> > 5) Update driver for latest u-boot USB interface (board_usb init &
> > board_usb_cleanup). 6) Use the peripheral clock to compute the I2C
> > divider.
>
>
> [...]
>
> > +static int i2c_wait_reset(int timeout)
> > +{
> > +   while (timeout > 0 && (readl(&otg->otg_i2c.otg_i2c_ctrl) & I2C_RESET)) {
> > +           udelay(1000);
> > +           timeout--;
> > +   }
> > +
> > +   return timeout <= 0;
> > +}
> > +
> > +static int isp1301_set_value(int reg, int value)
> > +{
> > +   int n = 0;
> > +
> > +   /* send isp1301 address */
> > +   writel((ISP1301_I2C_ADDR << 1) | I2C_START_BIT,
> > +          &otg->otg_i2c.otg_i2c_txrx);
> > +
> > +   /* offset to write to */
> > +   writel((reg & 0xff) | I2C_WRITE, &otg->otg_i2c.otg_i2c_txrx);
> > +   /* value to write */
> > +   writel((value & 0xff) | I2C_STOP_BIT, &otg->otg_i2c.otg_i2c_txrx);
> > +
> > +   /* wait for transmit done (TDI) */
> > +   while (((readl(&otg->otg_i2c.otg_i2c_stat) & I2C_TDI) != I2C_TDI) &&
> > +          n++ < 100000)
> > +           ;
>
> Can you please rework the timeouts to use get_timer() ? It looks like this:
>
> start = get_timer(0);
> do {
>       if (readl(...) == FOO)
>               break;
>       udelay(1);      /* Don't do tightloop and poke WDT */
> } while (get_timer(start) < DELAY_IN_MSECS);
>
> > +   if (n >= 100000) {
> > +           printf("isp1301_set_value: ERROR TDI not set\n");
> > +           return -1;
> > +   }
> > +
> > +   /* clear TDI */
> > +   setbits_le32(&otg->otg_i2c.otg_i2c_stat, I2C_TDI);
> > +
> > +   return 0;
> > +}
>

The initial posting was a porting effort only;
no problem, I will do the rework.

> Don't you want to split this code out and make it into an actual i2c controller?
> Then, the USB driver would use regular i2c_*() calls to configure the device on
> the other end of the i2c bus. That'd be really cool and convenient.
>

I can look at this; but, I will not be able to do it before the following week.
I will ensure to post my update before the merge window close.

There is already a LPC32xx I2C available in u-boot;
minor change will be require (ex. source clock is different for the USB I2C).

> [...]
>
> > +int board_usb_init(int index, enum usb_init_type init)
> > +{
> > +   /* Remove warning. */
> > +   (void)index;
> > +
> > +   if (init != USB_INIT_HOST)
> > +           return -1;
> > +
> > +   /* enable AHB slave USB clock */
> > +   setbits_le32(&clk_pwr->usb_ctrl,
> > +                CLK_USBCTRL_HCLK_EN | CLK_USBCTRL_BUS_KEEPER);
> > +
> > +   /* enable I2C clock in OTG block if it isn't */
> > +   if ((readl(&otg->otg_clk_sts) & OTG_CLK_I2C_EN) != OTG_CLK_I2C_EN) {
> > +           writel(OTG_CLK_I2C_EN, &otg->otg_clk_ctrl);
> > +
> > +           while (readl(&otg->otg_clk_sts) != OTG_CLK_I2C_EN)
> > +                   ;
>
> Ew..., such ugly infinite loops are a big no-no, please add a timeout here.
>

Will do as part of the rework to improve the result of this porting effort.

Sylvain

________________________________

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.