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

[Marek Vasut <marex@denx.de>]

On Wednesday, July 29, 2015 at 06:14:10 PM, slemieux.tyco at gmail.com wrote:

Hi!

> 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 to integrate with the latest u-boot.
> 
> Signed-off-by: Sylvain Lemieux <slemieux@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.aribaud@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);
> +		}
> +	}
[...]