usb: lpc32xx: add host USB driver

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 10-Aug-15 8:24 AM
To: slemieux.tyco at gmail.com
Cc: u-boot at lists.denx.de; scottwood at freescale.com; vz at mleia.com; albert.u.boot at aribaud.net; LEMIEUX, SYLVAIN
Subject: Re: [PATCH v6 5/5] usb: lpc32xx: add host USB driver

Quoted 10 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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.

Quoted 1 line

Signed-off-by Sylvain Lemieux <[email protected]>

Acked-by Marek Vasut <[email protected]>

btw I think patch 4/5 didn't reach mean.
Also, if you want me to pick the USB pieces, let me know, though I wouldn't mind if the whole series was picked up by someone (who?).

Best regards,
Marek Vasut

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

---
Changes from v5 to v6:
* Addressed Marek's comments on LPC32xx USB driver:
- return "-ETIMEDOUT" on timeout.
* Addressed Vladimir's comments on LPC32xx USB driver:
- follow alphabetic order when adding "lpc32xx_usb_init()".
- use "CONFIG_SYS_HZ" when verifying if timer expired.
- use "wait_for_bit()" when verify the I2C OTG clock status.

Changes from v4 to v5:
* Addressed Marek's comments on LPC32xx USB driver:
- update "wait_for_bit()" implementation to use "get_timer()".
- use return value of "wait_for_bit()" instead of returning -1.
* Updated multiline comments style.

Changes from v3 to v4:
* Addressed Marek's comments on LPC32xx USB driver:
- use same "wait_for_bit()" implementation as drivers/usb/host/dwc2.c
- use a const variable to define a mask to make the code more clear to read.
* Fixed legacy USB driver; use "otg_clk_sts" register to verify clock status instead of control register.

Changes from v2 to v3:
* Addressed Marek's comments on LPC32xx USB driver:
- use "get_timer()" to handle timeout (usbpll_setup).
- submit i2c driver update into a separate patch.
- use "u32" for 4 bytes registers definition.
- Move pin mux code to setup file (i.e. "device.c").
* Updated ISP1301 register definition (set & clear) instead of using an extra mask for the clear address.

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.

Update to the legacy 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 volatile in register definition taken from "lpc3250.h" header file.
5) Update driver for latest u-boot USB API.
6) Use the peripheral clock to compute the I2C divider.

The legacy BSP patch (u-boot-2009.03_lpc32x0-v1.07.patch.tar.bz2) was downloaded from the LPCLinux Web site.

 arch/arm/cpu/arm926ejs/lpc32xx/devices.c      |   6 +
 arch/arm/include/asm/arch-lpc32xx/clk.h       |  12 ++
 arch/arm/include/asm/arch-lpc32xx/sys_proto.h |   1 +
 drivers/usb/host/Makefile                     |   1 +
 drivers/usb/host/ohci-lpc32xx.c               | 235 ++++++++++++++++++++++++++
 5 files changed, 255 insertions(+)
 create mode 100644 drivers/usb/host/ohci-lpc32xx.c

arch/arm/cpu/arm926ejs/lpc32xx/devices.c +6 -0

index f0af851..d9fa280 100644
@@ -72,6 +72,12 @@ void lpc32xx_slc_nand_init(void)
 	writel(CLK_NAND_SLC | CLK_NAND_SLC_SELECT, &clk->flashclk_ctrl);
 }
 
+void lpc32xx_usb_init(void)
+{
+	/* Do not route the UART 5 Tx/Rx pins to the USB D+ and USB D- pins. */
+	clrbits_le32(&ctrl->ctrl, UART_CTRL_UART5_USB_MODE);
+}
+
 void lpc32xx_i2c_init(unsigned int devnum)
 {
 	/* Enable I2C interface */

arch/arm/include/asm/arch-lpc32xx/clk.h +12 -0

index 663f6bc..d21310e 100644
@@ -167,6 +167,18 @@ struct clk_pm_regs {
 /* SDRAMCLK register bits */
 #define CLK_SDRAM_DDR_SEL		(1 << 1)
 
+/* USB control register definitions */
+#define CLK_USBCTRL_PLL_STS		(1 << 0)
+#define CLK_USBCTRL_FDBK_PLUS1(n)	(((n) & 0xFF) << 1)
+#define CLK_USBCTRL_POSTDIV_2POW(n)	(((n) & 0x3) << 11)
+#define CLK_USBCTRL_PLL_PWRUP		(1 << 16)
+#define CLK_USBCTRL_CLK_EN1		(1 << 17)
+#define CLK_USBCTRL_CLK_EN2		(1 << 18)
+#define CLK_USBCTRL_BUS_KEEPER		(0x1 << 19)
+#define CLK_USBCTRL_USBHSTND_EN		(1 << 21)
+#define CLK_USBCTRL_USBDVND_EN		(1 << 22)
+#define CLK_USBCTRL_HCLK_EN		(1 << 24)
+
 unsigned int get_sys_clk_rate(void);
 unsigned int get_hclk_pll_rate(void);
 unsigned int get_hclk_clk_div(void);

arch/arm/include/asm/arch-lpc32xx/sys_proto.h +1 -0

index d6e5e68..eb8010f 100644
@@ -16,6 +16,7 @@ void lpc32xx_mlc_nand_init(void);
 void lpc32xx_slc_nand_init(void);
 void lpc32xx_i2c_init(unsigned int devnum);
 void lpc32xx_ssp_init(void);
+void lpc32xx_usb_init(void);
 #if defined(CONFIG_SPL_BUILD)
 void ddr_init(const struct emc_dram_settings *dram);
 #endif

drivers/usb/host/Makefile +1 -0

index 4d35d3e..9dfdc94 100644
@@ -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

drivers/usb/host/ohci-lpc32xx.c new file +74 -0

new file mode 100644
index 0000000..acf356b
@@ -0,0 +1,235 @@
+/*
+ * Copyright (C) 2008 by NXP Semiconductors
+ * @Author: Based on code by Kevin Wells
+ * @Descr: USB driver - Embedded Artists LPC3250 OEM Board support functions
+ *
+ * Copyright (c) 2015 Tyco Fire Protection Products.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <errno.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 {
+	u32 otg_i2c_txrx;   /* OTG I2C Tx/Rx Data FIFO */
+	u32 otg_i2c_stat;   /* OTG I2C Status Register */
+	u32 otg_i2c_ctrl;   /* OTG I2C Control Register */
+	u32 otg_i2c_clk_hi; /* OTG I2C Clock Divider high */
+	u32 otg_i2c_clk_lo; /* OTG I2C Clock Divider low */
+};
+
+/* OTG controller module register structures */
+struct otg_regs {
+	u32 reserved1[64];
+	u32 otg_int_sts;    /* OTG int status register */
+	u32 otg_int_enab;   /* OTG int enable register */
+	u32 otg_int_set;    /* OTG int set register */
+	u32 otg_int_clr;    /* OTG int clear register */
+	u32 otg_sts_ctrl;   /* OTG status/control register */
+	u32 otg_timer;      /* OTG timer register */
+	u32 reserved2[122];
+	struct otgi2c_regs otg_i2c;
+	u32 reserved3[824];
+	u32 otg_clk_ctrl;   /* OTG clock control reg */
+	u32 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 */
+
+/* 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_SET		0x04
+#define ISP1301_I2C_MODE_CONTROL_1_CLR		0x05
+#define ISP1301_I2C_MODE_CONTROL_2_SET		0x12
+#define ISP1301_I2C_MODE_CONTROL_2_CLR		0x13
+#define ISP1301_I2C_OTG_CONTROL_1_SET		0x06

drivers/usb/host/ohci-lpc32xx.c new file +9 -0

+	u32 val;
+	unsigned long start = get_timer(0);
+
+	while (1) {
+		val = readl(reg);
+		if (!set)
+			val = ~val;
+
+		if ((val & mask) == mask)

drivers/usb/host/ohci-lpc32xx.c new file +72 -0

+	return -ETIMEDOUT;
+}
+
+static int isp1301_set_value(int reg, u8 value)
+{
+	return i2c_write(ISP1301_I2C_ADDR, reg, 1, &value, 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_CLR, MC1_UART_EN);
+
+	isp1301_set_value(ISP1301_I2C_MODE_CONTROL_1_CLR, ~MC1_SPEED_REG);
+	isp1301_set_value(ISP1301_I2C_MODE_CONTROL_1_SET, MC1_SPEED_REG);
+	isp1301_set_value(ISP1301_I2C_MODE_CONTROL_2_CLR, ~0);
+	isp1301_set_value(ISP1301_I2C_MODE_CONTROL_2_SET,
+			  MC2_BI_DI | MC2_PSW_EN | MC2_SPD_SUSP_CTRL);
+
+	isp1301_set_value(ISP1301_I2C_OTG_CONTROL_1_CLR, ~0);
+	isp1301_set_value(ISP1301_I2C_MODE_CONTROL_1_SET, MC1_DAT_SE0);
+	isp1301_set_value(ISP1301_I2C_OTG_CONTROL_1_SET,
+			  OTG1_DM_PULLDOWN | OTG1_DP_PULLDOWN);
+	isp1301_set_value(ISP1301_I2C_OTG_CONTROL_1_CLR,
+			  OTG1_DM_PULLUP | OTG1_DP_PULLUP);
+	isp1301_set_value(ISP1301_I2C_INTERRUPT_LATCH_CLR, ~0);
+	isp1301_set_value(ISP1301_I2C_INTERRUPT_FALLING_CLR, ~0);
+	isp1301_set_value(ISP1301_I2C_INTERRUPT_RISING_CLR, ~0);
+
+	/* Enable usb_need_clk clock after transceiver is initialized */
+	setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_USBDVND_EN);
+}
+
+static int usbpll_setup(void)
+{
+	u32 ret;
+
+	/* 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 ret;
+
+	/* enable PLL output */
+	setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_CLK_EN2);
+
+	return 0;
+}
+
+int usb_cpu_init(void)
+{
+	u32 ret;
+
+	/*

drivers/usb/host/ohci-lpc32xx.c new file +30 -0

+
+	/* setup USB clocks and PLL */
+	ret = usbpll_setup();
+	if (ret)
+		return ret;
+
+	/* enable usb_host_need_clk */
+	setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_USBHSTND_EN);
+
+	/* enable all needed USB clocks */
+	const u32 mask = OTG_CLK_AHB_EN | OTG_CLK_OTG_EN |
+			 OTG_CLK_I2C_EN | OTG_CLK_HOST_EN;
+	writel(mask, &otg->otg_clk_ctrl);
+
+	ret = wait_for_bit(&otg->otg_clk_sts, mask, 1);
+	if (ret)
+		return ret;
+
+	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);

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

Review

Quoted 2 lines

(quoted diff) +2 -0

+static int wait_for_bit(void *reg, const u32 mask, bool set)
+{

(set == false) argument is not in use, and hence there is a piece of dead code in the function.

Quoted 13 lines

drivers/usb/host/ohci-lpc32xx.c new file +13 -0

+	u32 val;
+	unsigned long start = get_timer(0);
+
+	while (1) {
+		val = readl(reg);
+		if (!set)
+			val = ~val;
+
+		if ((val & mask) == mask)
+			return 0;
+
+		if (get_timer(start) > CONFIG_SYS_HZ)
+			break;

drivers/usb/host/ohci-lpc32xx.c line 103

Quoted 6 lines

drivers/usb/host/ohci-lpc32xx.c new file +6 -0

+
+		udelay(1);
+	}
+
+	debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n",
+	      __func__, reg, mask, set);

I would recommend on error path always to display this message to a user.

Quoted 2 lines

(quoted diff) +2 -0

+	return -ETIMEDOUT;
+}

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 12-Aug-15 3:42 PM

Quoted 14 lines · Vladimir Zapolskiy

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

[...]

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+static int wait_for_bit(void *reg, const u32 mask, bool set)
+{

(set == false) argument is not in use, and hence there is a piece of dead code in the function.

I'd prefer this to be the way it is, since this function can be extracted and made into generic code (probably in subsequent patch).

Quoted 13 lines

Quoted 13 lines

+ u32 val;
+ unsigned long start = get_timer(0);

drivers/usb/host/ohci-lpc32xx.c new file +11 -0

+
+ while (1) {
+         val = readl(reg);
+         if (!set)
+                 val = ~val;
+
+         if ((val & mask) == mask)
+                 return 0;
+
+         if (get_timer(start) > CONFIG_SYS_HZ)
+                 break;

Quoted 8 lines

drivers/usb/host/ohci-lpc32xx.c line 103

Quoted 6 lines

drivers/usb/host/ohci-lpc32xx.c new file +6 -0

+
+         udelay(1);
+ }
+
+ debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n",
+       __func__, reg, mask, set);

I would recommend on error path always to display this message to a user.

Yeah.

Quoted 2 lines

Quoted 2 lines

+ return -ETIMEDOUT;
+}

[...]

Hi Vladimir,

Quoted 28 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 12-Aug-15 3:42 PM

Quoted 14 lines · Vladimir Zapolskiy

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

[...]

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+static int wait_for_bit(void *reg, const u32 mask, bool set)
+{

(set == false) argument is not in use, and hence there is a piece of dead code in the function.

Quoted 3 lines

I'd prefer this to be the way it is, since this function can be extracted and made into generic code (probably in subsequent patch).

Vladimir, are you OK with keeping the code as-is? It will make it easier when we attempt to create a generic "wait_for_bit()" function.

Quoted 28 lines

Quoted 13 lines

Quoted 13 lines

+ u32 val;
+ unsigned long start = get_timer(0);

drivers/usb/host/ohci-lpc32xx.c new file +11 -0

+
+ while (1) {
+         val = readl(reg);
+         if (!set)
+                 val = ~val;
+
+         if ((val & mask) == mask)
+                 return 0;
+
+         if (get_timer(start) > CONFIG_SYS_HZ)
+                 break;

Quoted 8 lines

drivers/usb/host/ohci-lpc32xx.c line 103

Quoted 6 lines

drivers/usb/host/ohci-lpc32xx.c new file +6 -0

+
+         udelay(1);
+ }
+
+ debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n",
+       __func__, reg, mask, set);

I would recommend on error path always to display this message to a user.

Yeah.

Quoted 2 lines

Quoted 2 lines

+ return -ETIMEDOUT;
+}

[...]

Quoted 30 lines · SYLVAIN

Hi Vladimir,

Quoted 28 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 12-Aug-15 3:42 PM

Quoted 14 lines · Vladimir Zapolskiy

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

[...]

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+static int wait_for_bit(void *reg, const u32 mask, bool set)
+{

(set == false) argument is not in use, and hence there is a piece of dead code in the function.

Quoted 6 lines

Quoted 2 lines

I'd prefer this to be the way it is, since this function can be extracted and made into generic code (probably in subsequent patch).

Vladimir, are you OK with keeping the code as-is? It will make it easier when we attempt to create a generic "wait_for_bit()" function.

Okay, if maintainer says, let it remain :)

-----Original Message-----
From: Vladimir Zapolskiy [mailto:vz at mleia.com]
Sent: 12-Aug-15 1:55 PM

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

---

drivers/usb/host/ohci-lpc32xx.c line 84

Quoted 3 lines

drivers/usb/host/ohci-lpc32xx.c new file +3 -0

+
+static int wait_for_bit(void *reg, const u32 mask, bool set)
+{

(set == false) argument is not in use, and hence there is a piece of dead code in the function.

Quoted 13 lines

drivers/usb/host/ohci-lpc32xx.c new file +13 -0

+   u32 val;
+   unsigned long start = get_timer(0);
+
+   while (1) {
+           val = readl(reg);
+           if (!set)
+                   val = ~val;
+
+           if ((val & mask) == mask)
+                   return 0;
+
+           if (get_timer(start) > CONFIG_SYS_HZ)
+                   break;

drivers/usb/host/ohci-lpc32xx.c line 103

Quoted 6 lines

drivers/usb/host/ohci-lpc32xx.c new file +6 -0

+
+           udelay(1);
+   }
+
+   debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n",
+         __func__, reg, mask, set);

I would recommend on error path always to display this message to a user.

drivers/usb/host/ohci-lpc32xx.c line 186

Quoted 3 lines

drivers/usb/host/ohci-lpc32xx.c new file +3 -0

+
+   /* enable I2C clock in OTG block if it isn't */
+   if ((readl(&otg->otg_clk_sts) & OTG_CLK_I2C_EN) != OTG_CLK_I2C_EN) {

if (!(readl(&otg->otg_clk_sts) & OTG_CLK_I2C_EN))

drivers/usb/host/ohci-lpc32xx.c line 191

Quoted 5 lines

drivers/usb/host/ohci-lpc32xx.c new file +5 -0

+           writel(OTG_CLK_I2C_EN, &otg->otg_clk_ctrl);
+
+           ret = wait_for_bit(&otg->otg_clk_sts, OTG_CLK_I2C_EN, 1);
+           if (ret)
+                   return ret;

Or actually the embracing check may be dropped and writel(OTG_CLK_I2C_EN) / wait_for_bit() are done unconditionally.

drivers/usb/host/ohci-lpc32xx.c line 195

Quoted 5 lines

drivers/usb/host/ohci-lpc32xx.c new file +4 -0

+   }
+
+   /* Configure ISP1301 */
+   isp1301_configure();

Not sure if there are any LPC32xx boards with USB host/device, but without ISP1301 phy. Let it be here until such a board emerges, if Marek does not object.

In general I wonder, won't drivers/usb/phy/ be a better place for this driver or bigger part of it?

Quoted 1 line

Feel free to add my

Tested-by Vladimir Zapolskiy <[email protected]>

--
With best wishes,
Vladimir

Hi Vladimir and Marek,

Quoted 18 lines

-----Original Message-----
From: Vladimir Zapolskiy [mailto:vz at mleia.com]
Sent: 12-Aug-15 1:55 PM

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

---

Quoted 9 lines

drivers/usb/host/ohci-lpc32xx.c line 195

Quoted 4 lines

drivers/usb/host/ohci-lpc32xx.c new file +4 -0

+   }
+
+   /* Configure ISP1301 */
+   isp1301_configure();

Not sure if there are any LPC32xx boards with USB host/device, but without ISP1301 phy. Let it be here until such a board emerges, if Marek does not object.

Quoted 2 lines

In general I wonder, won't drivers/usb/phy/ be a better place for this driver or bigger part of it?

The Embedded Artists LPC3250 OEM Board is having the USB with the ISP1301;
http://www.embeddedartists.com/products/kits/lpc3250_kit.php
The legacy NXP LPC32xx BSP included board support files for it.

-----Original Message-----
From: Vladimir Zapolskiy [mailto:vz at mleia.com]
Sent: 12-Aug-15 8:05 PM

Hi Sylvain,

Quoted 20 lines · SYLVAIN

Hi Vladimir and Marek,

Quoted 18 lines

-----Original Message-----
From: Vladimir Zapolskiy [mailto:vz at mleia.com]
Sent: 12-Aug-15 1:55 PM

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

---

[snip]

Quoted 9 lines

Quoted 9 lines

drivers/usb/host/ohci-lpc32xx.c line 195

Quoted 4 lines

drivers/usb/host/ohci-lpc32xx.c new file +4 -0

+   }
+
+   /* Configure ISP1301 */
+   isp1301_configure();

Not sure if there are any LPC32xx boards with USB host/device, but without ISP1301 phy. Let it be here until such a board emerges, if Marek does not object.

Quoted 7 lines

Quoted 2 lines

In general I wonder, won't drivers/usb/phy/ be a better place for this driver or bigger part of it?

The Embedded Artists LPC3250 OEM Board is having the USB with the ISP1301;
http://www.embeddedartists.com/products/kits/lpc3250_kit.php
The legacy NXP LPC32xx BSP included board support files for it.

it seems to be a usual design pattern, my devkit3250 board also has ISP1301 phy. The question is if fusion of a particular external phy device driver and a particular host device driver are permitted in form of a host driver only.

This is a question in area of device driver(s) maintenance, therefore Marek should rule it. For me as a user I'm satisfied with the approach, when on-SoC host device and external phy device share one driver for both. Just when an LPC32xx powered board with USB emerges without ISP1301 phy or other way round ISP1301 phy is used in junction with another host/device, this time the driver will be split.

--
With best wishes,
Vladimir

Quoted 53 lines

-----Original Message-----
From: Vladimir Zapolskiy [mailto:vz at mleia.com]
Sent: 12-Aug-15 8:05 PM

Hi Sylvain,

Quoted 20 lines · SYLVAIN

Hi Vladimir and Marek,

Quoted 18 lines

-----Original Message-----
From: Vladimir Zapolskiy [mailto:vz at mleia.com]
Sent: 12-Aug-15 1:55 PM

Hi Sylvain,

Quoted 11 lines · slemieux.tyco at gmail.com

From: Sylvain Lemieux <[email protected]>

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 <[email protected]>

---

[snip]

Quoted 9 lines

Quoted 9 lines

drivers/usb/host/ohci-lpc32xx.c line 195

Quoted 4 lines

drivers/usb/host/ohci-lpc32xx.c new file +4 -0

+   }
+
+   /* Configure ISP1301 */
+   isp1301_configure();

Not sure if there are any LPC32xx boards with USB host/device, but without ISP1301 phy. Let it be here until such a board emerges, if Marek does not object.

Quoted 7 lines

Quoted 2 lines

In general I wonder, won't drivers/usb/phy/ be a better place for this driver or bigger part of it?

The Embedded Artists LPC3250 OEM Board is having the USB with the ISP1301;
http://www.embeddedartists.com/products/kits/lpc3250_kit.php
The legacy NXP LPC32xx BSP included board support files for it.

it seems to be a usual design pattern, my devkit3250 board also has ISP1301 phy. The question is if fusion of a particular external phy device driver and a particular host device driver are permitted in form of a host driver only.

Quoted 7 lines

This is a question in area of device driver(s) maintenance, therefore Marek should rule it. For me as a user I'm satisfied with the approach, when on-SoC host device and external phy device share one driver for both. Just when an LPC32xx powered board with USB emerges without ISP1301 phy or other way round ISP1301 phy is used in junction with another host/device, this time the driver will be split.

The maintainer, Marek, previously ack the patch;
https://www.mail-archive.com/u-boot at lists.denx.de/msg180934.html I am assuming it is OK to keep the current approach.

I will add your tested, as per this message listed below and the ack from Marek when I submit revision 7, later today.
https://www.mail-archive.com/u-boot at lists.denx.de/msg180934.html