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

On Monday, August 03, 2015 at 09:47:49 PM, slemieux.tyco@gmail.com wrote:

From: Sylvain Lemieux slemieux@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@tycoint.com

Looks good, so I'm only nitpicking :)

[...]

diff --git a/drivers/usb/host/ohci-lpc32xx.c b/drivers/usb/host/ohci-lpc32xx.c new file mode 100644 index 0000000..b1a9e9e --- /dev/null +++ b/drivers/usb/host/ohci-lpc32xx.c

[...]

+static int usbpll_setup(void) +{

• unsigned long start;
• /* 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);
• start = get_timer(0);
• while (1) {

• if ((readl(clk_pwr->usb_ctrl) & CLK_USBCTRL_PLL_STS) != 0)
  

• 	break;
  

• if (get_timer(start) > 10000) {
  

• 	printf("usbpll_setup: ERROR PLL doesn't lock\n");
  

• 	return -1;
  

• }
  

• udelay(1);
  

• }

Maybe you can pick drivers/usb/host/dwc2.c wait_for_bit() and copy it into this driver, since you're having two such timeouts in this driver. Extra point for implementing generic wait_for_bit() :)

• /* enable PLL output */
• setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_CLK_EN2);
• return 0;

+}

+int usb_cpu_init(void) +{

• unsigned long start;
• /* USB pins routing setup is done by "lpc32xx_usb_init()" and should

• * be call by board "board_init()" or "misc_init_r()" functions. */
  

• /* 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);
  

• }
  

• }
• /* Configure ISP1301 */
• isp1301_configure();
• /* setup USB clocks and PLL */
• if (usbpll_setup() == -1)

• return -1;
  

• /* enable usb_host_need_clk */
• setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_USBHSTND_EN);
• /* enable all needed USB clocks */
• writel(OTG_CLK_AHB_EN | OTG_CLK_OTG_EN |

•       OTG_CLK_I2C_EN | OTG_CLK_HOST_EN,
  

•       &otg->otg_clk_ctrl);
  

• start = get_timer(0);
• while (1) {

• if ((readl(&otg->otg_clk_ctrl) &
  

•      (OTG_CLK_AHB_EN | OTG_CLK_OTG_EN |
  

•       OTG_CLK_I2C_EN | OTG_CLK_HOST_EN)) ==
  

•     (OTG_CLK_AHB_EN | OTG_CLK_OTG_EN |
  

•      OTG_CLK_I2C_EN | OTG_CLK_HOST_EN))
  

• 	break;
  

You can introduce a variable here, const u32 mask = x | y | z | w; and possibly another one for the reg and then do: reg = readl(); if (reg & mask == mask) break;

That's a bit clearer :)

• if (get_timer(start) > 100)
  

• 	return -1;
  

• udelay(1);
  

• }
• 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();

+}