Hi Marek, Thanks for the feedback. I will do the change and submit a new revision of the patch.
For your question regarding reference to UART 5 inside the USB driver; * It is possible to also route the UART5 Tx/Rx pin to the USB D+ and D- pins; the driver ensure this is not selected. For details, refer to UM10326, "LPC32x0 and LPC32x0/01 User manual - Rev. 3" table 459 and paragraph 17.2.3.2.6. * I will add comments in the code. Sylvain Lemieux > From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Marek Vasut > > On Wednesday, July 29, 2015 at 06:14:10 PM, slemieux.t...@gmail.com wrote: > > Hi! > > > From: Sylvain Lemieux <slemi...@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 to integrate with the latest u-boot. > > > > Signed-off-by: Sylvain Lemieux <slemi...@tycoint.com> > > --- > > Changes from v1 to v2: > > * Addressed Marek's comments on LPC32xx USB driver: > > - use "get_timer()" to handle timeout. > > - Split USB and I2C driver. > > * Updated LPC32xx I2C driver to support the I2C that is part > > of the USB module. > > * Removed ISP1301 USB transceiver I2C registers definition > > that are not used. > > * Use "cpu" initialization & stop functions API instead of the "board" API. > > [...] > > I like this , but please split the change to the i2c driver from the addition > of the USB driver into two separate patches. > > > unsigned int get_hclk_clk_div(void); > > diff --git a/drivers/i2c/lpc32xx_i2c.c b/drivers/i2c/lpc32xx_i2c.c > > index 98106fa..be166b0 100644 > > --- a/drivers/i2c/lpc32xx_i2c.c > > +++ b/drivers/i2c/lpc32xx_i2c.c > > @@ -1,7 +1,7 @@ > > /* > > * LPC32xx I2C interface driver > > * > > - * (C) Copyright 2014 DENX Software Engineering GmbH > > + * (C) Copyright 2014-2015 DENX Software Engineering GmbH > > * Written-by: Albert ARIBAUD - 3ADEV <albert.arib...@3adev.fr> > > * > > * SPDX-License-Identifier: GPL-2.0+ > > [...] > > > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile > > index 4d35d3e..9dfdc94 100644 > > --- a/drivers/usb/host/Makefile > > +++ b/drivers/usb/host/Makefile > > @@ -20,6 +20,7 @@ obj-$(CONFIG_USB_SL811HS) += sl811-hcd.o > > obj-$(CONFIG_USB_OHCI_S3C24XX) += ohci-s3c24xx.o > > obj-$(CONFIG_USB_OHCI_EP93XX) += ohci-ep93xx.o > > obj-$(CONFIG_USB_OHCI_SUNXI) += ohci-sunxi.o > > +obj-$(CONFIG_USB_OHCI_LPC32XX) += ohci-lpc32xx.o > > > > # echi > > obj-$(CONFIG_USB_EHCI) += ehci-hcd.o > > diff --git a/drivers/usb/host/ohci-lpc32xx.c > > b/drivers/usb/host/ohci-lpc32xx.c new file mode 100644 > > index 0000000..fb51d42 > > --- /dev/null > > +++ b/drivers/usb/host/ohci-lpc32xx.c > > @@ -0,0 +1,245 @@ > > +/* > > + * Copyright (C) 2008-2015 by NXP Semiconductors > > + * All rights reserved. > > + * > > + * @Author: Based on code by Kevin Wells > > + * @Descr: USB driver - Embedded Artists LPC3250 OEM Board support > > functions + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +#include <common.h> > > +#include <asm/io.h> > > +#include <asm/arch/cpu.h> > > +#include <asm/arch/clk.h> > > +#include <usb.h> > > +#include <i2c.h> > > + > > + > > +/* OTG I2C controller module register structures */ > > +struct otgi2c_regs { > > + unsigned int otg_i2c_txrx; /* OTG I2C Tx/Rx Data FIFO */ > > + unsigned int otg_i2c_stat; /* OTG I2C Status Register */ > > + unsigned int otg_i2c_ctrl; /* OTG I2C Control Register */ > > + unsigned int otg_i2c_clk_hi; /* OTG I2C Clock Divider high */ > > + unsigned int otg_i2c_clk_lo; /* OTG I2C Clock Divider low */ > > +}; > > Please replace that unsigned int with u32, here we are certain that > the register is 4-byte big. > > > +/* OTG controller module register structures */ > > +struct otg_regs { > > + unsigned int reserved1[64]; > > + unsigned int otg_int_sts; /* OTG int status register */ > > + unsigned int otg_int_enab; /* OTG int enable register */ > > + unsigned int otg_int_set; /* OTG int set register */ > > + unsigned int otg_int_clr; /* OTG int clear register */ > > + unsigned int otg_sts_ctrl; /* OTG status/control register */ > > + unsigned int otg_timer; /* OTG timer register */ > > + unsigned int reserved2[122]; > > + struct otgi2c_regs otg_i2c; > > + unsigned int reserved3[824]; > > + unsigned int otg_clk_ctrl; /* OTG clock control reg */ > > + unsigned int otg_clk_sts; /* OTG clock status reg */ > > +}; > > + > > +/* otg_sts_ctrl register definitions */ > > +#define OTG_HOST_EN (1 << 0) /* Enable host mode */ > > + > > +/* otg_clk_ctrl and otg_clk_sts register definitions */ > > +#define OTG_CLK_AHB_EN (1 << 4) /* Enable AHB clock */ > > +#define OTG_CLK_OTG_EN (1 << 3) /* Enable OTG clock */ > > +#define OTG_CLK_I2C_EN (1 << 2) /* Enable I2C clock */ > > +#define OTG_CLK_HOST_EN (1 << 0) /* Enable host clock */ > > + > > +/* UART control structure */ > > +struct uartctrl_regs { > > + unsigned int ctrl; /* General UART control register */ > > + unsigned int clkmode; /* UART clock control register */ > > + unsigned int loop; /* UART loopmode enable/disable */ > > +}; > > Huh? Is this an UART driver now ? :) > > > +/* UART ctrl register definitions */ > > +#define UART_U5_ROUTE_TO_USB (1 << 0) > > + > > +/* ISP1301 USB transceiver I2C registers */ > > +#define MC1_SPEED_REG (1 << 0) > > +#define MC1_DAT_SE0 (1 << 2) > > +#define MC1_UART_EN (1 << 6) > > + > > +#define MC2_SPD_SUSP_CTRL (1 << 1) > > +#define MC2_BI_DI (1 << 2) > > +#define MC2_PSW_EN (1 << 6) > > + > > +#define OTG1_DP_PULLUP (1 << 0) > > +#define OTG1_DM_PULLUP (1 << 1) > > +#define OTG1_DP_PULLDOWN (1 << 2) > > +#define OTG1_DM_PULLDOWN (1 << 3) > > +#define OTG1_VBUS_DRV (1 << 5) > > + > > +#define ISP1301_I2C_ADDR CONFIG_USB_ISP1301_I2C_ADDR > > + > > +#define ISP1301_I2C_MODE_CONTROL_1 0x4 > > +#define ISP1301_I2C_MODE_CONTROL_2 0x12 > > +#define ISP1301_I2C_OTG_CONTROL_1 0x6 > > +#define ISP1301_I2C_INTERRUPT_LATCH 0xA > > +#define ISP1301_I2C_INTERRUPT_FALLING 0xC > > +#define ISP1301_I2C_INTERRUPT_RISING 0xE > > +#define ISP1301_I2C_REG_CLEAR_ADDR 1 > > [...] > > > +static void isp1301_configure(void) > > +{ > > + i2c_set_bus_num(I2C_2); > > + > > + /* LPC32XX only supports DAT_SE0 USB mode */ > > + /* This sequence is important */ > > + > > + /* Disable transparent UART mode first */ > > + isp1301_set_value((ISP1301_I2C_MODE_CONTROL_1 > > + | ISP1301_I2C_REG_CLEAR_ADDR), MC1_UART_EN); > > The parenthesis around (ISP... | ISP...) are unnecessary :) > > [...] > > > +static int usbpll_setup(void) > > +{ > > + int n = 0; > > + > > + /* 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); > > + while ((readl(clk_pwr->usb_ctrl) & CLK_USBCTRL_PLL_STS) == 0) { > > + if (n++ >= 100000) { > > + printf("usbpll_setup: ERROR PLL doesn't lock\n"); > > + return -1; > > + } > > Please use get_timer(0) kind of loop: > > #define DELAY 1000 /* delay in mS */ > > ulong start = get_timer(0); > while (1) { > if (cond) { handle the cond } > if (get_timer(start) > DELAY) { timeout } > udelay(1); /* Poke WDT */ > } > > You actually use that below already in usb_cpu_init() . > > > + } > > + > > + /* enable PLL output */ > > + setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_CLK_EN2); > > + > > + return 0; > > +} > > + > > +int usb_cpu_init(void) > > +{ > > + unsigned long start; > > + > > + /* 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); > > + > > + start = get_timer(0); > > + while (1) { > > + if (readl(&otg->otg_clk_sts) == OTG_CLK_I2C_EN) > > + break; > > + > > + if (get_timer(start) > 100) > > + return -1; > > + > > + udelay(1); > > + } > > + } > [...] > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot ________________________________ 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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot