[U-Boot] [PATCH v3 0/3] AM335x: Add USB support in u-boot.

[U-Boot] [PATCH v3 0/3] AM335x: Add USB support in u-boot.

[Harman Sohanpal]

These patches add USB support in u-boot for AM335x.
I have tested this code on AM335x evm and beaglebone in 
the host mode using mass storage class.
For device mode, I have tested this on beaglebone using 
usbtty. CONFIG_MUSB_HCD has been enabled by default 
to make USB work in host mode.

Gene Zarkhin (1):
  AM335x : Add USB support for AM335x in u-boot

Harman Sohanpal (2):
  AM335x : Configs to add USB host support.
  musb_udc : Fix compile warning.

 drivers/usb/musb/Makefile    |    1 +
 drivers/usb/musb/am335x.c    |  121 ++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/am335x.h    |  113 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_core.h |    2 +
 drivers/usb/musb/musb_hcd.h  |    3 -
 include/configs/am335x_evm.h |   41 ++++++++++++++
 include/usb.h                |    3 +-
 7 files changed, 280 insertions(+), 4 deletions(-)
 create mode 100644 drivers/usb/musb/am335x.c
 create mode 100644 drivers/usb/musb/am335x.h

[U-Boot] [PATCH v3 1/3] AM335x : Add USB support for AM335x in u-boot

[Harman Sohanpal]

From: Gene Zarkhin <gene_zarkhin@bose.com>

Adds USB support in uboot for AM335x.
By default the USB 1 module is enabled.
The support for USB 0 can be enabled by changing the
USB base address and the phy control register address
in the header file am335x.h.
USB 1 has a full size connector so acts in host mode and
USB 0 has a mini connector so used in device mode.
By default, the support is added for host mode hence USB 1
has been enabled by default.

Signed-off-by: Gene Zarkhin <gene_zarkhin@bose.com>
Signed-off-by: Harman Sohanpal <harman_sohanpal@ti.com>
---
Changes for v2:
	- none
Changes for v3:
	- Changed commit message to specify why USB 1 
	has been enabled by default.

 drivers/usb/musb/Makefile |    1 +
 drivers/usb/musb/am335x.c |  121 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/am335x.h |  113 ++++++++++++++++++++++++++++++++++++++++++
 include/usb.h             |    3 +-
 4 files changed, 237 insertions(+), 1 deletions(-)
 create mode 100644 drivers/usb/musb/am335x.c
 create mode 100644 drivers/usb/musb/am335x.h

diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
index 20b5503..d00ec40 100644
--- a/drivers/usb/musb/Makefile
+++ b/drivers/usb/musb/Makefile
@@ -32,6 +32,7 @@ COBJS-$(CONFIG_USB_DAVINCI) += davinci.o
 COBJS-$(CONFIG_USB_OMAP3) += omap3.o
 COBJS-$(CONFIG_USB_DA8XX) += da8xx.o
 COBJS-$(CONFIG_USB_AM35X) += am35x.o
+COBJS-$(CONFIG_USB_AM335X) += am335x.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
diff --git a/drivers/usb/musb/am335x.c b/drivers/usb/musb/am335x.c
new file mode 100644
index 0000000..4b59769
--- /dev/null
+++ b/drivers/usb/musb/am335x.c
@@ -0,0 +1,121 @@
+/*
+ * am335x.c - TI's AM335x platform specific usb wrapper functions.
+ *
+ * Author: gene Zarkhin <gene_zarkhin@bose.com>
+ * Modified by: Harman Sohanpal <harman_sohanpal@ti.com>
+ *
+ * Based on drivers/usb/musb/da8xx.c
+ *
+ * Copyright (c) 2012 Texas Instruments Incorporated
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+#include <common.h>
+#include "am335x.h"
+
+/* MUSB platform configuration */
+struct musb_config musb_cfg = {
+	.regs = (struct musb_regs *)(AM335X_USB_OTG_CORE_BASE),
+	.timeout	= AM335X_USB_OTG_TIMEOUT,
+	.musb_speed	= 0,
+};
+
+/*
+ * Enable the USB phy
+ */
+static u8 phy_on(void)
+{
+	u32 timeout;
+	u32 regAddr = CM_REGISTERS + USB_CTRL0_REG_OFFSET;
+	u32 usb_ctrl_reg;
+
+	usb_ctrl_reg = readl(regAddr);
+	usb_ctrl_reg &= ~(CM_PHY_PWRDN | CM_PHY_OTG_PWRDN);
+	usb_ctrl_reg |= (OTGVDET_EN | OTGSESSENDEN);
+	writel(usb_ctrl_reg, regAddr);
+
+	timeout = musb_cfg.timeout;
+	writel(0x1, &am335x_usb_regs->ctrl);
+	udelay(6000);
+	while (timeout--) {
+		if ((readl(&am335x_usb_regs->ctrl) & SOFT_RESET_BIT) == 0)
+			return 1;
+	}
+	/* USB phy was not turned on */
+	return 0;
+}
+
+/*
+ * Disable the USB phy
+ */
+static void phy_off(void)
+{
+	u32 regAddr = CM_REGISTERS + USB_CTRL0_REG_OFFSET;
+	u32 usb_ctrl_reg;
+
+	usb_ctrl_reg = readl(regAddr);
+	usb_ctrl_reg |= (CM_PHY_PWRDN | CM_PHY_OTG_PWRDN);
+	writel(usb_ctrl_reg, regAddr);
+
+	/* Disable the USB module */
+	writel(PRCM_MODULE_DSBL, CM_PER_USB0_CLKCTRL);
+}
+
+/*
+ * This function performs platform specific initialization for usb0.
+ */
+int musb_platform_init(void)
+{
+	u32 timeout;
+	u32 revision;
+
+	/* USB */
+	/* PLL Gate set up */
+	writel(DPLL_CLKDCOLDO_GATE_CTRL, CM_CLKDCOLDO_DPLL_PER);
+
+	/* CLOCK */
+	writel(PRCM_MOD_EN, CM_PER_USB0_CLKCTRL);
+	timeout = musb_cfg.timeout;
+	while (timeout--) {
+		if (readl(CM_PER_USB0_CLKCTRL) != PRCM_MOD_EN)
+			continue;
+		else
+			break;
+	}
+	if (timeout == 0) {
+		printf("\nUSB module not enabled\nAborting");
+		return -1;
+	}
+
+	/* USB module fully functional */
+	/* start the on-chip usb phy and its pll */
+	if (phy_on() == 0)
+		return -1;
+	/* Returns zero if e.g. not clocked */
+	revision = readl(&am335x_usb_regs->revision);
+	if (revision == 0)
+		return -1;
+
+	return 0;
+}
+
+/*
+ * This function performs platform specific deinitialization for usb0.
+ */
+void musb_platform_deinit(void)
+{
+	/* Turn off the phy */
+	phy_off();
+}
diff --git a/drivers/usb/musb/am335x.h b/drivers/usb/musb/am335x.h
new file mode 100644
index 0000000..790531e
--- /dev/null
+++ b/drivers/usb/musb/am335x.h
@@ -0,0 +1,113 @@
+/*
+ * am335x.h - TI's AM335x platform specific usb wrapper definitions.
+ *
+ * Author: Gene Zarkhin <gene_zarkhin@bose.com>
+ * Modified by: Harman Sohanpal <harman_sohanpal@ti.com>
+ *
+ * Based on drivers/usb/musb/da8xx.h
+ *
+ * Copyright (c) 2012 Texas Instruments Incorporated
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef __AM335X_USB_H__
+#define __AM335X_USB_H__
+
+#include "musb_core.h"
+/* PRCM Definitions */
+#define CM_CLKDCOLDO_DPLL_PER		(CM_WKUP + 0x7c)
+#define CM_PER_USB0_CLKCTRL		(CM_PER + 0x1c)
+#define PRCM_MOD_EN			0x2
+#define PRCM_MODULE_DSBL		0x0
+
+
+/* Control Module Registers */
+#define CM_REGISTERS			CTRL_BASE
+#define USB_CTRL0_REG_OFFSET	(0x628)
+#define USB_CTRL1_REG_OFFSET	(0x680)
+
+#define PRCM_IDLEST                             0x30000
+#define DPLL_CLKDCOLDO_GATE_CTRL                0x300
+
+/* Base address of musb wrapper */
+#define AM335X_USB_OTG_BASE	0x47401800
+
+/* Base address of musb core */
+#define AM335X_USB_OTG_CORE_BASE	(AM335X_USB_OTG_BASE + 0x400)
+
+/* Timeout for AM35x usb module */
+#define AM335X_USB_OTG_TIMEOUT	0x3FFFFFF
+
+/*
+ * AM335x platform USB wrapper register overlay.
+ */
+struct am335x_usb_regs {
+	u32 revision;			/* 0x00 */
+	u32 reserved0[4];
+	u32 ctrl;			/* 0x14 */
+	u32 status;			/* 0x18 */
+	u32 reserved1[1];
+	u32 irqmstat;			/* 0x20 */
+	u32 irqeoi;			/* 0x24 */
+	u32 irqstatraw0;		/* 0x28 */
+	u32 irqstatraw1;		/* 0x2c */
+	u32 irqstat0;			/* 0x30 */
+	u32 irqstat1;			/* 0x34 */
+	u32 irqenableset0;		/* 0x38 */
+	u32 irqenableset1;		/* 0x3c */
+	u32 irqenableclr0;		/* 0x40 */
+	u32 irqenableclr1;		/* 0x44 */
+	u32 reserved2[10];
+	u32 txmode;			/* 0x70 */
+	u32 rxmode;			/* 0x74 */
+	u32 reserved3[2];
+	u32 genrndisep1;		/* 0x80	*/
+	u32 genrndisep2;		/* 0x84	*/
+	u32 genrndisep3;		/* 0x88	*/
+	u32 genrndisep4;		/* 0x8c	*/
+	u32 genrndisep5;		/* 0x90	*/
+	u32 genrndisep6;		/* 0x94	*/
+	u32 genrndisep7;		/* 0x98	*/
+	u32 genrndisep8;		/* 0x9c	*/
+	u32 genrndisep9;		/* 0xa0	*/
+	u32 genrndisep10;		/* 0xa4	*/
+	u32 genrndisep11;		/* 0xa8	*/
+	u32 genrndisep12;		/* 0xac	*/
+	u32 genrndisep13;		/* 0xb0	*/
+	u32 genrndisep14;		/* 0xb4	*/
+	u32 genrndisep15;		/* 0xb8	*/
+	u32 reserved4[5];
+	u32 autoreq;			/* 0xd0	*/
+	u32 srpfixtime;			/* 0xd4	*/
+	u32 tdown;			/* 0xd8	*/
+	u32 reserved5[1];
+	u32 utmi;			/* 0xe0	*/
+	u32 utmilb;			/* 0xe4	*/
+	u32 mode;			/* 0xe8	*/
+};
+
+#define am335x_usb_regs ((struct am335x_usb_regs *)AM335X_USB_OTG_BASE)
+
+/* USB 2.0 PHY Control */
+#define CM_PHY_PWRDN		(1 << 0)
+#define CM_PHY_OTG_PWRDN	(1 << 1)
+#define OTGVDET_EN		(1 << 19)
+#define OTGSESSENDEN		(1 << 20)
+
+/* USB CTRL REG FIELDS */
+#define SOFT_RESET_BIT		(1 << 0)
+
+#endif	/* __AM335X_USB_H__ */
diff --git a/include/usb.h b/include/usb.h
index 6da91e7..13f5434 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -141,7 +141,8 @@ struct usb_device {
 	defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \
 	defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI) || \
 	defined(CONFIG_USB_OMAP3) || defined(CONFIG_USB_DA8XX) || \
-	defined(CONFIG_USB_BLACKFIN) || defined(CONFIG_USB_AM35X)
+	defined(CONFIG_USB_BLACKFIN) || defined(CONFIG_USB_AM35X) || \
+	defined(CONFIG_USB_AM335X)
 
 int usb_lowlevel_init(void);
 int usb_lowlevel_stop(void);
-- 
1.7.0.4

[U-Boot] [PATCH v3 2/3] AM335x : Configs to add USB host support.

[Harman Sohanpal]

Adds required configs in config file for
am335x_evm to add support for usb host mode.
To enable USB device mode, add CONFIG_MUSB_UDC in
place of CONFIG_MUSB_HCD.
Tested using usbtty for device mode and
mass storage for host mode.

Signed-off-by: Harman Sohanpal <harman_sohanpal@ti.com>
---
Changes for v2:
	- #define cleanups.
Changes for v3:
	- remove configs for usb as keyboard in host mode.
	- removed extra test of CONFIG_USB_AM335X.
	- changed commit messgae.
	
 include/configs/am335x_evm.h |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index d0fbc88..a1e16cc 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -165,6 +165,38 @@
 #define CONFIG_SKIP_LOWLEVEL_INIT
 #endif
 
+
+/*
+ * USB configuration
+ * Enable CONFIG_MUSB_HCD for Host functionalities MSC
+ * Enable CONFIG_MUSB_UDC for Device functionalities.
+ */
+#define CONFIG_USB_AM335X
+#define CONFIG_MUSB_HCD
+
+#ifdef CONFIG_MUSB_HCD
+#define CONFIG_CMD_USB
+
+#define CONFIG_USB_STORAGE
+#define CONGIG_CMD_STORAGE
+#define CONFIG_CMD_FAT
+
+#endif /* CONFIG_MUSB_HCD */
+
+#ifdef CONFIG_MUSB_UDC
+/* USB device configuration */
+#ifndef CONFIG_SPL_BUILD
+#define CONFIG_USB_DEVICE
+#define CONFIG_USB_TTY
+#define CONFIG_SYS_CONSOLE_IS_IN_ENV
+#endif	/* CONFIG_SPL_BUILD */
+/* Change these to suit your needs */
+#define CONFIG_USBD_VENDORID            0x0451
+#define CONFIG_USBD_PRODUCTID           0x5678
+#define CONFIG_USBD_MANUFACTURER        "Texas Instruments"
+#define CONFIG_USBD_PRODUCT_NAME        "AM335xEVM"
+#endif /* CONFIG_MUSB_UDC */
+
 /* Unsupported features */
 #undef CONFIG_USE_IRQ
 
-- 
1.7.0.4

[U-Boot] [PATCH v3 3/3] musb_udc : Fix compile warning.

[Harman Sohanpal]

Fix the compile warning :
implicit declaration of musb_platform_init
when CONFIG_MUSB_UDC is defined.
The extern musb_platform_init was declared in musb_hcd.h
but no such extern function was declared for musb_udc.
So a common function has been declared in musb_core.h
which can be used for both host mode and device mode.

Signed-off-by: Harman Sohanpal <harman_sohanpal@ti.com>
---
Changes for v2:
	- none.
Changes for v3:
	- none.

 drivers/usb/musb/musb_core.h |    2 ++
 drivers/usb/musb/musb_hcd.h  |    3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index a8adcce..14253f0 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -360,6 +360,8 @@ extern void musb_start(void);
 extern void musb_configure_ep(const struct musb_epinfo *epinfo, u8 cnt);
 extern void write_fifo(u8 ep, u32 length, void *fifo_data);
 extern void read_fifo(u8 ep, u32 length, void *fifo_data);
+extern int musb_platform_init(void);
+extern void musb_platform_deinit(void);
 
 #if defined(CONFIG_USB_BLACKFIN)
 /* Every USB register is accessed as a 16-bit even if the value itself
diff --git a/drivers/usb/musb/musb_hcd.h b/drivers/usb/musb/musb_hcd.h
index dde7d37..5621f7e 100644
--- a/drivers/usb/musb/musb_hcd.h
+++ b/drivers/usb/musb/musb_hcd.h
@@ -105,8 +105,5 @@ extern unsigned char new[];
 #define RH_REQ_ERR		   -1
 #define RH_NACK			   0x00
 
-/* extern functions */
-extern int musb_platform_init(void);
-extern void musb_platform_deinit(void);
 
 #endif	/* __MUSB_HCD_H__ */
-- 
1.7.0.4

[U-Boot] [PATCH v3 1/3] AM335x : Add USB support for AM335x in u-boot

[Tom Rini]

On Tue, Jun 26, 2012 at 03:08:01PM +0530, Harman Sohanpal wrote:

> From: Gene Zarkhin <gene_zarkhin@bose.com>
> 
> Adds USB support in uboot for AM335x.
> By default the USB 1 module is enabled.
> The support for USB 0 can be enabled by changing the
> USB base address and the phy control register address
> in the header file am335x.h.
> USB 1 has a full size connector so acts in host mode and
> USB 0 has a mini connector so used in device mode.
> By default, the support is added for host mode hence USB 1
> has been enabled by default.
> 
> Signed-off-by: Gene Zarkhin <gene_zarkhin@bose.com>
> Signed-off-by: Harman Sohanpal <harman_sohanpal@ti.com>

What I was saying was that we want to make am335x.h do:
#define AM335X_USB0_BASE 0x47401000
#define AM335X_USB1_BASE 0x47401800

Then:
#ifdef CONFIG_AM335X_USB0
#define AM335X_USB_OTG_BASE AM335X_USB0_BASE
#elif defined(CONFIG_AM335X_USB1)
#define AM335X_USB_OTG_BASE AM335X_USB1_BASE
#endif
#define AM335X_USB_OTG_CORE_BASE (AM335X_USB_OTG_BASE + 0x400)

So that in boards.cfg we could add an entry for am335x_evm_usb1 and set
CONFIG_AM335X_USB1 dynamically.  Then in am335x_evm.h set either
MUSB_HCD or MUSB_UDC based on if CONFIG_AM335X_USB0 or
CONFIG_AM335X_USB1 is set (and have boards.cfg set CONFIG_AM335X_USB0
for 'am335x_evm).

Then you can test device mode on the evm :)  And how did you test device
mode on the beaglebone?  I couldn't since I don't have the rest set of
adapters here.

Also:

> +++ b/drivers/usb/musb/am335x.h
[snip]
> +/* Control Module Registers */
> +#define CM_REGISTERS			CTRL_BASE
> +#define USB_CTRL0_REG_OFFSET	(0x628)
> +#define USB_CTRL1_REG_OFFSET	(0x680)

Please get all indentation out to a consistent level

> +#define PRCM_IDLEST                             0x30000
> +#define DPLL_CLKDCOLDO_GATE_CTRL                0x300

And always use tabs :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120627/d05edaef/attachment.pgp>

[U-Boot] [PATCH v3 1/3] AM335x : Add USB support for AM335x in u-boot

[Sohanpal, Harman]

> -----Original Message-----
> From: Tom Rini [mailto:tom.rini at gmail.com] On Behalf Of Rini, Tom
> Sent: Wednesday, June 27, 2012 10:50 PM
> To: Sohanpal, Harman
> Cc: u-boot at lists.denx.de; Gene Zarkhin
> Subject: Re: [U-Boot] [PATCH v3 1/3] AM335x : Add USB support for AM335x
> in u-boot
> 
> On Tue, Jun 26, 2012 at 03:08:01PM +0530, Harman Sohanpal wrote:
> 
> > From: Gene Zarkhin <gene_zarkhin@bose.com>
> >
> > Adds USB support in uboot for AM335x.
> > By default the USB 1 module is enabled.
> > The support for USB 0 can be enabled by changing the
> > USB base address and the phy control register address
> > in the header file am335x.h.
> > USB 1 has a full size connector so acts in host mode and
> > USB 0 has a mini connector so used in device mode.
> > By default, the support is added for host mode hence USB 1
> > has been enabled by default.
> >
> > Signed-off-by: Gene Zarkhin <gene_zarkhin@bose.com>
> > Signed-off-by: Harman Sohanpal <harman_sohanpal@ti.com>
> 
> What I was saying was that we want to make am335x.h do:
> #define AM335X_USB0_BASE 0x47401000
> #define AM335X_USB1_BASE 0x47401800
> 
> Then:
> #ifdef CONFIG_AM335X_USB0
> #define AM335X_USB_OTG_BASE AM335X_USB0_BASE
> #elif defined(CONFIG_AM335X_USB1)
> #define AM335X_USB_OTG_BASE AM335X_USB1_BASE
> #endif
> #define AM335X_USB_OTG_CORE_BASE (AM335X_USB_OTG_BASE + 0x400)
> 
> So that in boards.cfg we could add an entry for am335x_evm_usb1 and set
> CONFIG_AM335X_USB1 dynamically.  Then in am335x_evm.h set either
> MUSB_HCD or MUSB_UDC based on if CONFIG_AM335X_USB0 or
> CONFIG_AM335X_USB1 is set (and have boards.cfg set CONFIG_AM335X_USB0
> for 'am335x_evm).
> 
> Then you can test device mode on the evm :)  And how did you test device
> mode on the beaglebone?  I couldn't since I don't have the rest set of
> adapters here.
> 
> Also:
> 
> > +++ b/drivers/usb/musb/am335x.h
> [snip]
> > +/* Control Module Registers */
> > +#define CM_REGISTERS			CTRL_BASE
> > +#define USB_CTRL0_REG_OFFSET	(0x628)
> > +#define USB_CTRL1_REG_OFFSET	(0x680)
> 
> Please get all indentation out to a consistent level
> 
> > +#define PRCM_IDLEST                             0x30000
> > +#define DPLL_CLKDCOLDO_GATE_CTRL                0x300
> 
> And always use tabs :)
> 
Hi Tom,
Thanks for the review.
I will send the patches doing 
all the modifications you have mentioned.
Also there was some confusion regarding testing in the device mode.
A new node /dev/ttyUSB0 was seen when beagle bone was connected to 
a linux machine, but it is not using USB as serial using usbtty.
It was using the USB to serial chip in the beagle bone. Sorry for that.
So I am still to confirm whether the device mode works with usbtty.
But previously I have tested the device mode on both evm 
and beagle bone using some local DFU patches and it worked fine.
Thanks,
Harman
 

[U-Boot] [PATCH v3 1/3] AM335x : Add USB support for AM335x in u-boot

[Ilya Yanok]

Hi,

Harman Sohanpal <harman_sohanpal <at> ti.com> writes:
> +/* Control Module Registers */
> +#define CM_REGISTERS			CTRL_BASE
> +#define USB_CTRL0_REG_OFFSET	(0x628)
> +#define USB_CTRL1_REG_OFFSET	(0x680)

USB_CTRL1 seems to be unused. Furthermore, both values seems to be incorrect:
according to the RM USB_CTRL0 offset should be 0x628 and USB_CTRL1 should be
0x628. I understand that's cause you use USB1 by default but that's kind of
misguiding. Could you please use the approach Tom suggested for base address
here too? Define USB_CRTL* to correct values but use USB_CONTROL_REG_OFFSET
(defined conditionally) in code?

Regards, Ilya.

[U-Boot] [PATCH v3 1/3] AM335x : Add USB support for AM335x in u-boot

[Harman Sohanpal]

On Sat, Jun 30, 2012 at 9:38 PM, Ilya Yanok
<ilya.yanok@cogentembedded.com> wrote:
> Hi,
>
> Harman Sohanpal <harman_sohanpal <at> ti.com> writes:
>> +/* Control Module Registers */
>> +#define CM_REGISTERS                 CTRL_BASE
>> +#define USB_CTRL0_REG_OFFSET (0x628)
>> +#define USB_CTRL1_REG_OFFSET (0x680)
Hi llya,
Thanks for your review.
But I believe you are talking of version 3 of the patches.
Kindly have a look at the version 4 I have sent.
I have used the same approach which Tom has mentioned
for control register offset also.
Thanks,
Harman
> USB_CTRL1 seems to be unused. Furthermore, both values seems to be incorrect:
> according to the RM USB_CTRL0 offset should be 0x628 and USB_CTRL1 should be
> 0x628. I understand that's cause you use USB1 by default but that's kind of
> misguiding. Could you please use the approach Tom suggested for base address
> here too? Define USB_CRTL* to correct values but use USB_CONTROL_REG_OFFSET
> (defined conditionally) in code?
>
> Regards, Ilya.
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 1/3] AM335x : Add USB support for AM335x in u-boot

[Ilya Yanok]

Hi,

On Sun, Jul 1, 2012 at 9:33 PM, Harman Sohanpal <harmansohanpal@gmail.com>wrote:

>
> Thanks for your review.
> But I believe you are talking of version 3 of the patches.
> Kindly have a look at the version 4 I have sent.
>

Yes, my bad. I've seen your fourth version just after sending my comment.


> I have used the same approach which Tom has mentioned
> for control register offset also.
>

Yes, I've seen. I still think that it's a bit misguiding to conditionally
define USB_CTRL0 but the general approach is ok.
Other than that I think it's good to have boards.cfg entries for both
options.

And how much have you tested these patches? I tried them on my beaglebone
and have issues both with host and device modes.

In device mode I tried to use CDC ACM. My Linux host sees the device and
/dev/ttyACM0 node shows up. I can put stdout in U-Boot to usbtty and this
works nice too. But once I try to reassign stdin also I got my picocom
hanged (only killable with kill -9).

I host mode I tried with two flashdisks (512MB and 4GB) and card-reader
(32MB SD and 8GB microSD cards). Only 512MB stick works as expected. With
others I got:
 1. 4GB flashdisk -- 0 device found, second "usb start" command seems to
wedge the board
 2. 8GB card --  scanning bus for storage devices... Device NOT ready
   Request Sense returned 00 00 00. Second "usb start" is lethal too.
 3. 32MB card -- scanning bus for storage devices... READ_CAP ERROR

Regards, Ilya.