[U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS

[U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS

[Ajay Kumar Gupta]

USB0_DRVVBUS pinmux configuration is required for USB functinality
in uboot.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
Signed-off-by: Swaminathan S <swami.iyer@ti.com>
---
This patch set is created against Nick Thompson's latest patch set
(v5) on DA8xx support and another patch from him on Davinci pinmux.
[Davinci: add a pin multiplexer configuration API]

 board/davinci/da830evm/da830evm.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/board/davinci/da830evm/da830evm.c b/board/davinci/da830evm/da830evm.c
index bb8cc3c..7cf6013 100644
--- a/board/davinci/da830evm/da830evm.c
+++ b/board/davinci/da830evm/da830evm.c
@@ -65,6 +65,11 @@ const struct pinmux_config i2c_pins[] = {
 	{ pinmux[9], 2, 4 }
 };
 
+/* USB0_DRVVBUS pin muxer settings */
+const struct pinmux_config usb_pins[] = {
+	{ pinmux[9], 1, 1 }
+};
+
 int board_init(void)
 {
 #ifndef CONFIG_USE_IRQ
@@ -118,6 +123,9 @@ int board_init(void)
 	if (davinci_configure_pin_mux(i2c_pins, ARRAY_SIZE(i2c_pins)) != 0)
 		return 1;
 
+	if (davinci_configure_pin_mux(usb_pins, ARRAY_SIZE(usb_pins)) != 0)
+		return 1;
+
 	/* enable the console UART */
 	writel((DAVINCI_UART_PWREMU_MGMT_FREE | DAVINCI_UART_PWREMU_MGMT_URRST |
 		DAVINCI_UART_PWREMU_MGMT_UTRST),
-- 
1.6.2.4

[U-Boot] [PATCH 2/3] DA8xx: Add MUSB host support

[Ajay Kumar Gupta]

Tested USB host functionality on DA830 EVM.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
Signed-off-by: Swaminathan S <swami.iyer@ti.com>
---
 drivers/usb/musb/Makefile |    1 +
 drivers/usb/musb/da8xx.c  |  143 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/da8xx.h  |   83 ++++++++++++++++++++++++++
 include/usb.h             |    3 +-
 4 files changed, 229 insertions(+), 1 deletions(-)
 create mode 100644 drivers/usb/musb/da8xx.c
 create mode 100644 drivers/usb/musb/da8xx.h

diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
index 09e0a5f..eb4d8fd 100644
--- a/drivers/usb/musb/Makefile
+++ b/drivers/usb/musb/Makefile
@@ -27,6 +27,7 @@ LIB	:= $(obj)libusb_musb.a
 
 COBJS-$(CONFIG_MUSB_HCD) += musb_hcd.o musb_core.o
 COBJS-$(CONFIG_USB_DAVINCI) += davinci.o
+COBJS-$(CONFIG_USB_DA8XX) += da8xx.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
new file mode 100644
index 0000000..9e4ba70
--- /dev/null
+++ b/drivers/usb/musb/da8xx.c
@@ -0,0 +1,143 @@
+/*
+ * da8xx.c - TI's DA8xx platform specific usb wrapper functions.
+ *
+ * Author: Ajay Kumar Gupta <ajay.gupta@ti.com>
+ *
+ * Based on drivers/usb/musb/davinci.c
+ *
+ * Copyright (c) 2009 Texas Instruments Incorporated
+ *
+ * This package is free software;  you can redistribute it and/or
+ * modify it under the terms of the license found in the file
+ * named COPYING that should have accompanied this file.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ */
+#include <common.h>
+
+#ifdef CONFIG_USB_DA8XX
+
+#include "da8xx.h"
+
+/* MUSB platform configuration */
+struct musb_config musb_cfg = {
+	(struct	musb_regs *)DA8XX_USB_OTG_CORE_BASE,
+	DA8XX_USB_OTG_TIMEOUT,
+	0
+};
+
+/*
+ * This function enables VBUS by driving the GPIO Bank4 Pin 15 high.
+ */
+static void enable_vbus(void)
+{
+	u32 value;
+
+	/* configure GPIO bank4 pin 15 in output direction */
+	value = readl(DAVINCI_GPIO_BASE + BANK4_REG_DIR_ADDR);
+	writel((value & (~USB_VBUS_GPIO)),
+			(DAVINCI_GPIO_BASE + BANK4_REG_DIR_ADDR));
+
+	/* set GPIO bank4 pin 15 high to drive VBUS */
+	value = readl(DAVINCI_GPIO_BASE + BANK4_REG_SET_ADDR);
+	writel((value | USB_VBUS_GPIO),
+			(DAVINCI_GPIO_BASE + BANK4_REG_SET_ADDR));
+}
+
+/*
+ * Enable the usb0 phy. This initialization procedure is explained in
+ * the DA8xx USB user guide document.
+ */
+static u8 phy_on(void)
+{
+	u32 timeout;
+	u32 cfgchip2;
+
+	cfgchip2 = readl(DAVINCI_BOOTCFG_BASE + CFGCHIP2);
+
+	cfgchip2 &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN |
+			CFGCHIP2_OTGMODE | CFGCHIP2_REFFREQ);
+	cfgchip2 |= CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN | CFGCHIP2_PHY_PLLON |
+			CFGCHIP2_REFFREQ_24MHZ;
+
+	writel(cfgchip2, (DAVINCI_BOOTCFG_BASE + CFGCHIP2));
+
+	/* wait until the usb phy pll locks */
+	timeout = musb_cfg.timeout;
+	while (timeout--)
+		if (readl(DAVINCI_BOOTCFG_BASE + CFGCHIP2) & CFGCHIP2_PHYCLKGD)
+			return 1;
+
+	/* USB phy was not turned on */
+	return 0;
+}
+
+/*
+ * Disable the usb phy
+ */
+static void phy_off(void)
+{
+	u32 cfgchip2;
+
+	/*
+	 * Power down the on-chip PHY.
+	 */
+	cfgchip2 = readl(DAVINCI_BOOTCFG_BASE + CFGCHIP2);
+
+	cfgchip2 &= ~CFGCHIP2_PHY_PLLON;
+	cfgchip2 |= CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN;
+	writel(cfgchip2, (DAVINCI_BOOTCFG_BASE + CFGCHIP2));
+}
+
+/*
+ * This function performs DA8xx platform specific initialization for usb0.
+ */
+int musb_platform_init(void)
+{
+	u32  revision;
+
+	/* enable psc for usb2.0 */
+	lpsc_on(33);
+
+	/* enable usb vbus */
+	enable_vbus();
+
+	/* reset the controller */
+	writel(0x1, (DA8XX_USB_OTG_BASE + DA8XX_USB_CTRL_REG));
+	udelay(5000);
+
+	/* start the on-chip usb phy and its pll */
+	if (phy_on() == 0)
+		return -1;
+
+	/* Returns zero if e.g. not clocked */
+	revision = readl(DA8XX_USB_OTG_BASE + DA8XX_USB_VERSION_REG);
+	if (revision == 0)
+		return -1;
+
+	/* Disable all interrupts */
+	writel((DA8XX_USB_USBINT_MASK | DA8XX_USB_TXINT_MASK
+			| DA8XX_USB_RXINT_MASK),
+		(DA8XX_USB_OTG_BASE + DA8XX_USB_INT_MASK_SET_REG));
+	return 0;
+}
+
+/*
+ * This function performs DA8xx platform specific deinitialization for usb0.
+ */
+void musb_platform_deinit(void)
+{
+	/* Turn of the phy */
+	phy_off();
+
+	/* flush any interrupts */
+	writel((DA8XX_USB_USBINT_MASK | DA8XX_USB_TXINT_MASK |
+			DA8XX_USB_RXINT_MASK),
+			(DA8XX_USB_OTG_BASE + DA8XX_USB_INT_MASK_CLR_REG));
+	writel(0, (DA8XX_USB_OTG_BASE + DA8XX_USB_EOI_REG));
+}
+
+#endif /* CONFIG_USB_DA8XX */
diff --git a/drivers/usb/musb/da8xx.h b/drivers/usb/musb/da8xx.h
new file mode 100644
index 0000000..9441777
--- /dev/null
+++ b/drivers/usb/musb/da8xx.h
@@ -0,0 +1,83 @@
+/*
+ * da8xx.h -- TI's DA8xx platform specific usb wrapper definitions.
+ *
+ * Author: Ajay Kumar Gupta <ajay.gupta@ti.com>
+ *
+ * Based on drivers/usb/musb/davinci.h
+ *
+ * Copyright (c) 2009 Texas Instruments Incorporated
+ *
+ * This package is free software;  you can redistribute it and/or
+ * modify it under the terms of the license found in the file
+ * named COPYING that should have accompanied this file.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ */
+#ifndef __DA8XX_MUSB_H__
+#define __DA8XX_MUSB_H__
+
+#include "musb_core.h"
+
+/* extern functions */
+extern void lpsc_on(unsigned int id);
+
+/* Base address of da8xx usb0 wrapper */
+#define DA8XX_USB_OTG_BASE  0x01E00000
+
+/* Base address of da8xx musb core */
+#define DA8XX_USB_OTG_CORE_BASE (DA8XX_USB_OTG_BASE + 0x400)
+
+/* Timeout for DA8xx usb module */
+#define DA8XX_USB_OTG_TIMEOUT 0x3FFFFFF
+
+/* For now include usb OTG module registers here */
+#define DA8XX_USB_VERSION_REG			0x00
+#define DA8XX_USB_CTRL_REG			0x04
+#define DA8XX_USB_INT_MASK_SET_REG		0x30
+#define DA8XX_USB_INT_MASK_CLR_REG		0x34
+#define DA8XX_USB_EOI_REG			0x3c
+
+#define DA8XX_USB_TX_ENDPTS_MASK  0x1f		  /* ep0 + 4 tx */
+#define DA8XX_USB_RX_ENDPTS_MASK  0x1e		  /* 4 rx */
+#define DA8XX_USB_TXINT_SHIFT	  0
+#define DA8XX_USB_RXINT_SHIFT	  8
+
+#define DA8XX_USB_USBINT_MASK	  0x01ff0000  /* 8 Mentor, DRVVBUS */
+#define DA8XX_USB_TXINT_MASK \
+		(DA8XX_USB_TX_ENDPTS_MASK << DA8XX_USB_TXINT_SHIFT)
+#define DA8XX_USB_RXINT_MASK \
+		(DA8XX_USB_RX_ENDPTS_MASK << DA8XX_USB_RXINT_SHIFT)
+
+/* DA8xx CFGCHIP2 (USB 2.0 PHY Control) register bits */
+#define CFGCHIP2_PHYCLKGD	(1 << 17)
+#define CFGCHIP2_VBUSSENSE	(1 << 16)
+#define CFGCHIP2_RESET		(1 << 15)
+#define CFGCHIP2_OTGMODE	(3 << 13)
+#define CFGCHIP2_NO_OVERRIDE	(0 << 13)
+#define CFGCHIP2_FORCE_HOST	(1 << 13)
+#define CFGCHIP2_FORCE_DEVICE 	(2 << 13)
+#define CFGCHIP2_FORCE_HOST_VBUS_LOW (3 << 13)
+#define CFGCHIP2_USB1PHYCLKMUX	(1 << 12)
+#define CFGCHIP2_USB2PHYCLKMUX	(1 << 11)
+#define CFGCHIP2_PHYPWRDN	(1 << 10)
+#define CFGCHIP2_OTGPWRDN	(1 << 9)
+#define CFGCHIP2_DATPOL 	(1 << 8)
+#define CFGCHIP2_USB1SUSPENDM	(1 << 7)
+#define CFGCHIP2_PHY_PLLON	(1 << 6)	/* override PLL suspend */
+#define CFGCHIP2_SESENDEN	(1 << 5)	/* Vsess_end comparator */
+#define CFGCHIP2_VBDTCTEN	(1 << 4)	/* Vbus comparator */
+#define CFGCHIP2_REFFREQ	(0xf << 0)
+#define CFGCHIP2_REFFREQ_12MHZ	(1 << 0)
+#define CFGCHIP2_REFFREQ_24MHZ	(2 << 0)
+#define CFGCHIP2_REFFREQ_48MHZ	(3 << 0)
+
+#define BANK4_REG_DIR_ADDR	0x60
+#define BANK4_REG_SET_ADDR	0x68
+#define USB_VBUS_GPIO		(1 << 15)
+
+#define CFGCHIP2		0x184
+#endif	/* __DA8XX_MUSB_H__ */
+
diff --git a/include/usb.h b/include/usb.h
index 7c47098..7c56ace 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -183,7 +183,8 @@ struct usb_device {
 #if defined(CONFIG_USB_UHCI) || defined(CONFIG_USB_OHCI) || \
 	defined(CONFIG_USB_EHCI) || defined(CONFIG_USB_OHCI_NEW) || \
 	defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \
-	defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI)
+	defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI) || \
+	defined(CONFIG_USB_DA8XX)
 
 int usb_lowlevel_init(void);
 int usb_lowlevel_stop(void);
-- 
1.6.2.4

[U-Boot] [PATCH 3/3] DA830: Add usb config

[Ajay Kumar Gupta]

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
Signed-off-by: Swaminathan S <swami.iyer@ti.com>
---
 include/configs/da830evm.h |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/include/configs/da830evm.h b/include/configs/da830evm.h
index 38e2ce1..52e6473 100644
--- a/include/configs/da830evm.h
+++ b/include/configs/da830evm.h
@@ -149,6 +149,11 @@
 #define CONFIG_SYS_ENV_SPI_MAX_HZ	CONFIG_SF_DEFAULT_SPEED
 #endif
 
+/*==============================*/
+/* USB  configuration           */
+/*==============================*/
+#define CONFIG_USB_DA8XX	/* Platform hookup to MUSB controller */
+#define CONFIG_MUSB_HCD
 
 /*
  * U-Boot general configuration
@@ -240,4 +245,34 @@
 #define CONFIG_DOS_PARTITION	/* include support for FAT/storage*/
 #endif
 
+#ifdef CONFIG_USB_DA8XX
+
+#ifdef CONFIG_MUSB_HCD
+#define CONFIG_CMD_USB		/* inclue support for usb       */
+
+#define CONFIG_USB_STORAGE	/* MSC class support */
+#define CONFIG_CMD_STORAGE	/* inclue support for usb       */
+#define CONFIG_CMD_FAT		/* inclue support for FAT/storage*/
+#define CONFIG_DOS_PARTITION	/* inclue support for FAT/storage*/
+
+#ifdef CONFIG_USB_KEYBOARD
+#define CONFIG_SYS_USB_EVENT_POLL
+#define CONFIG_PREBOOT "usb start"
+#endif /* CONFIG_USB_KEYBOARD */
+
+#endif /* CONFIG_MUSB_HCD */
+
+#ifdef CONFIG_MUSB_UDC
+/* USB device configuration */
+#define CONFIG_USB_DEVICE		1
+#define CONFIG_USB_TTY			1
+#define CONFIG_SYS_CONSOLE_IS_IN_ENV	1
+/* 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	"DA830EVM"
+#endif /* CONFIG_MUSB_UDC */
+
+#endif /* CONFIG_USB_DA8XX */
 #endif /* __CONFIG_H */
-- 
1.6.2.4

[U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS

[Tom]

Ajay Kumar Gupta wrote:
> USB0_DRVVBUS pinmux configuration is required for USB functinality
> in uboot.
> 
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> ---
> This patch set is created against Nick Thompson's latest patch set
> (v5) on DA8xx support and another patch from him on Davinci pinmux.
> [Davinci: add a pin multiplexer configuration API]

It is difficult to review code that is dependent on other outstanding
patches.

The base DA8xx support will be in arm-ti branch shortly.
Please rebase these patches when this happens and resubmit.

Tom


> 
>  board/davinci/da830evm/da830evm.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/board/davinci/da830evm/da830evm.c b/board/davinci/da830evm/da830evm.c
> index bb8cc3c..7cf6013 100644
> --- a/board/davinci/da830evm/da830evm.c
> +++ b/board/davinci/da830evm/da830evm.c
> @@ -65,6 +65,11 @@ const struct pinmux_config i2c_pins[] = {
>  	{ pinmux[9], 2, 4 }
>  };
>  
> +/* USB0_DRVVBUS pin muxer settings */
> +const struct pinmux_config usb_pins[] = {
> +	{ pinmux[9], 1, 1 }
> +};
> +
>  int board_init(void)
>  {
>  #ifndef CONFIG_USE_IRQ
> @@ -118,6 +123,9 @@ int board_init(void)
>  	if (davinci_configure_pin_mux(i2c_pins, ARRAY_SIZE(i2c_pins)) != 0)
>  		return 1;
>  
> +	if (davinci_configure_pin_mux(usb_pins, ARRAY_SIZE(usb_pins)) != 0)
> +		return 1;
> +
>  	/* enable the console UART */
>  	writel((DAVINCI_UART_PWREMU_MGMT_FREE | DAVINCI_UART_PWREMU_MGMT_URRST |
>  		DAVINCI_UART_PWREMU_MGMT_UTRST),

[U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS

[Paulraj, Sandeep]

> Ajay Kumar Gupta wrote:
> > USB0_DRVVBUS pinmux configuration is required for USB functinality
> > in uboot.
> >
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> > Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> > ---
> > This patch set is created against Nick Thompson's latest patch set
> > (v5) on DA8xx support and another patch from him on Davinci pinmux.
> > [Davinci: add a pin multiplexer configuration API]
> 
> It is difficult to review code that is dependent on other outstanding
> patches.
> 
> The base DA8xx support will be in arm-ti branch shortly.
> Please rebase these patches when this happens and resubmit.
> 
> Tom

I think he might have added it locally and then made his patches.
So he might not have to rebase.

Without Nick's patches da830evm.c would not even exist :-)

> 
> >
> >  board/davinci/da830evm/da830evm.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/board/davinci/da830evm/da830evm.c
> b/board/davinci/da830evm/da830evm.c
> > index bb8cc3c..7cf6013 100644
> > --- a/board/davinci/da830evm/da830evm.c
> > +++ b/board/davinci/da830evm/da830evm.c
> > @@ -65,6 +65,11 @@ const struct pinmux_config i2c_pins[] = {
> >  	{ pinmux[9], 2, 4 }
> >  };
> >
> > +/* USB0_DRVVBUS pin muxer settings */
> > +const struct pinmux_config usb_pins[] = {
> > +	{ pinmux[9], 1, 1 }
> > +};
> > +
> >  int board_init(void)
> >  {
> >  #ifndef CONFIG_USE_IRQ
> > @@ -118,6 +123,9 @@ int board_init(void)
> >  	if (davinci_configure_pin_mux(i2c_pins, ARRAY_SIZE(i2c_pins)) != 0)
> >  		return 1;
> >
> > +	if (davinci_configure_pin_mux(usb_pins, ARRAY_SIZE(usb_pins)) != 0)
> > +		return 1;
> > +
> >  	/* enable the console UART */
> >  	writel((DAVINCI_UART_PWREMU_MGMT_FREE |
> DAVINCI_UART_PWREMU_MGMT_URRST |
> >  		DAVINCI_UART_PWREMU_MGMT_UTRST),
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS

[Tom]

Paulraj, Sandeep wrote:
> 
>> Ajay Kumar Gupta wrote:
>>> USB0_DRVVBUS pinmux configuration is required for USB functinality
>>> in uboot.
>>>
>>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
>>> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
>>> ---
>>> This patch set is created against Nick Thompson's latest patch set
>>> (v5) on DA8xx support and another patch from him on Davinci pinmux.
>>> [Davinci: add a pin multiplexer configuration API]
>> It is difficult to review code that is dependent on other outstanding
>> patches.
>>
>> The base DA8xx support will be in arm-ti branch shortly.
>> Please rebase these patches when this happens and resubmit.
>>
>> Tom
> 
> I think he might have added it locally and then made his patches.
> So he might not have to rebase.
> 
> Without Nick's patches da830evm.c would not even exist :-)
> 
> 

My issue is not with how development continues, it is with how patches
are reviewed.  To really review this patchset someone would need to
apply the pin mux patch, the the 8xx patch set, then this set.
Reviewing patches against outstanding patches does not scale and should be
avoided.

Tom

[U-Boot] [PATCH 2/3] DA8xx: Add MUSB host support

[Nick Thompson]

On 12/11/09 15:39, Ajay Kumar Gupta wrote:
> Tested USB host functionality on DA830 EVM.
> 
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> ---
>  drivers/usb/musb/Makefile |    1 +
>  drivers/usb/musb/da8xx.c  |  143 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/musb/da8xx.h  |   83 ++++++++++++++++++++++++++
>  include/usb.h             |    3 +-
>  4 files changed, 229 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/usb/musb/da8xx.c
>  create mode 100644 drivers/usb/musb/da8xx.h

> +static void enable_vbus(void)
> +{
> +	u32 value;
> +
> +	/* configure GPIO bank4 pin 15 in output direction */
> +	value = readl(DAVINCI_GPIO_BASE + BANK4_REG_DIR_ADDR);
> +	writel((value & (~USB_VBUS_GPIO)),
> +			(DAVINCI_GPIO_BASE + BANK4_REG_DIR_ADDR));

In general you should be using C structure pointers for all readl/writel
accessors in new code. #defines are no longer considered acceptable, but occur
through this patch. In case there is a compatibility issue with existing code,
I think the legacy form is acceptable.

Indentation is also wrong here (and elsewhere) I believe. Using underscores to
illustrate spaces, this should be:

	writel((value | USB_VBUS_GPIO),
	_______(DAVINCI_GPIO_BASE + BANK4_REG_SET_ADDR));

This is a mix of tabs (8) and spaces (<= 7). Indentation should use tabs only,
alignment uses spaces as required.

> +static u8 phy_on(void)
> +{
> +	u32 timeout;
> +	u32 cfgchip2;
> +
> +	cfgchip2 = readl(DAVINCI_BOOTCFG_BASE + CFGCHIP2);

C structures again, but in this case the initial da8xx patches provide the C
structure and pointer already, so you can already replace this with:

	cfgchip2 = readl(&davinci_syscfg_regs->cfgchip2);

Please take a look at hardware.h after the initial da8xx patches are in.

Thanks,
Nick.

[U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS

[Paulraj, Sandeep]

> 
> Paulraj, Sandeep wrote:
> >
> >> Ajay Kumar Gupta wrote:
> >>> USB0_DRVVBUS pinmux configuration is required for USB functinality
> >>> in uboot.
> >>>
> >>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> >>> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> >>> ---
> >>> This patch set is created against Nick Thompson's latest patch set
> >>> (v5) on DA8xx support and another patch from him on Davinci pinmux.
> >>> [Davinci: add a pin multiplexer configuration API]
> >> It is difficult to review code that is dependent on other outstanding
> >> patches.
> >>
> >> The base DA8xx support will be in arm-ti branch shortly.
> >> Please rebase these patches when this happens and resubmit.
> >>
> >> Tom
> >
> > I think he might have added it locally and then made his patches.
> > So he might not have to rebase.
> >
> > Without Nick's patches da830evm.c would not even exist :-)
> >
> >
> 
> My issue is not with how development continues, it is with how patches
> are reviewed.  To really review this patchset someone would need to
> apply the pin mux patch, the the 8xx patch set, then this set.
> Reviewing patches against outstanding patches does not scale and should be
> avoided.
> 
> Tom

Not any more :-)

Take a look at http://git.denx.de/?p=u-boot/u-boot-ti.git;a=shortlog;h=refs/heads/master

I have tested some other DaVincs SOCs as well after adding these patches and they seem to be working fine as of now.

Lets continue the review process for the USB support.

Thanks,
Sandeep

[U-Boot] [PATCH 2/3] DA8xx: Add MUSB host support

[Paulraj, Sandeep]

> On 12/11/09 15:39, Ajay Kumar Gupta wrote:
> > Tested USB host functionality on DA830 EVM.
> >
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> > Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> > ---
> >  drivers/usb/musb/Makefile |    1 +
> >  drivers/usb/musb/da8xx.c  |  143
> +++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/usb/musb/da8xx.h  |   83 ++++++++++++++++++++++++++
> >  include/usb.h             |    3 +-
> >  4 files changed, 229 insertions(+), 1 deletions(-)
> >  create mode 100644 drivers/usb/musb/da8xx.c
> >  create mode 100644 drivers/usb/musb/da8xx.h
> 
> > +static void enable_vbus(void)
> > +{
> > +	u32 value;
> > +
> > +	/* configure GPIO bank4 pin 15 in output direction */
> > +	value = readl(DAVINCI_GPIO_BASE + BANK4_REG_DIR_ADDR);
> > +	writel((value & (~USB_VBUS_GPIO)),
> > +			(DAVINCI_GPIO_BASE + BANK4_REG_DIR_ADDR));

And there is now a gpio_defs.h in /include/asm-arm/arch-davinci

So please use that.

Ideally a GPIO driver would be nice but it is not yet part of u-boot.

> 
> In general you should be using C structure pointers for all readl/writel
> accessors in new code. #defines are no longer considered acceptable, but
> occur
> through this patch. In case there is a compatibility issue with existing
> code,
> I think the legacy form is acceptable.
> 
> Indentation is also wrong here (and elsewhere) I believe. Using
> underscores to
> illustrate spaces, this should be:
> 
> 	writel((value | USB_VBUS_GPIO),
> 	_______(DAVINCI_GPIO_BASE + BANK4_REG_SET_ADDR));
> 
> This is a mix of tabs (8) and spaces (<= 7). Indentation should use tabs
> only,
> alignment uses spaces as required.
> 
> > +static u8 phy_on(void)
> > +{
> > +	u32 timeout;
> > +	u32 cfgchip2;
> > +
> > +	cfgchip2 = readl(DAVINCI_BOOTCFG_BASE + CFGCHIP2);
> 
> C structures again, but in this case the initial da8xx patches provide the
> C
> structure and pointer already, so you can already replace this with:
> 
> 	cfgchip2 = readl(&davinci_syscfg_regs->cfgchip2);
> 
> Please take a look at hardware.h after the initial da8xx patches are in.
> 
> Thanks,
> Nick.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/3] DA830: Add usb config

[Nick Thompson]

On 12/11/09 15:39, Ajay Kumar Gupta wrote:
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> ---
>  include/configs/da830evm.h |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/include/configs/da830evm.h b/include/configs/da830evm.h
> index 38e2ce1..52e6473 100644
> --- a/include/configs/da830evm.h
> +++ b/include/configs/da830evm.h
> @@ -149,6 +149,11 @@
>  #define CONFIG_SYS_ENV_SPI_MAX_HZ	CONFIG_SF_DEFAULT_SPEED
>  #endif
>  
> +/*==============================*/
> +/* USB  configuration           */
> +/*==============================*/

Block comments should be in this format:

/*
 * USB  configuration
 */

Thanks,
Nick.

[U-Boot] [PATCH 3/3] DA830: Add usb config

[Gupta, Ajay Kumar]

> -----Original Message-----
> From: Nick Thompson [mailto:nick.thompson at gefanuc.com]
> Sent: Thursday, November 12, 2009 10:25 PM
> To: Gupta, Ajay Kumar
> Cc: u-boot at lists.denx.de; Subbrathnam, Swaminathan
> Subject: Re: [PATCH 3/3] DA830: Add usb config
> 
> On 12/11/09 15:39, Ajay Kumar Gupta wrote:
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> > Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> > ---
> >  include/configs/da830evm.h |   35 +++++++++++++++++++++++++++++++++++
> >  1 files changed, 35 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/configs/da830evm.h b/include/configs/da830evm.h
> > index 38e2ce1..52e6473 100644
> > --- a/include/configs/da830evm.h
> > +++ b/include/configs/da830evm.h
> > @@ -149,6 +149,11 @@
> >  #define CONFIG_SYS_ENV_SPI_MAX_HZ	CONFIG_SF_DEFAULT_SPEED
> >  #endif
> >
> > +/*==============================*/
> > +/* USB  configuration           */
> > +/*==============================*/
> 
> Block comments should be in this format:
> 
> /*
>  * USB  configuration
>  */

Thanks, I will update and post v2.

Regards,
Ajay
> 
> Thanks,
> Nick.

[U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS

[Gupta, Ajay Kumar]

> -----Original Message-----
> From: Paulraj, Sandeep
> Sent: Thursday, November 12, 2009 10:00 PM
> To: Tom; Nick Thompson
> Cc: Gupta, Ajay Kumar; u-boot at lists.denx.de; Subbrathnam, Swaminathan
> Subject: RE: [U-Boot] [PATCH 1/3] DA830: Add pinmux for USB0_DRVVBUS
> 
> 
> 
> >
> > Paulraj, Sandeep wrote:
> > >
> > >> Ajay Kumar Gupta wrote:
> > >>> USB0_DRVVBUS pinmux configuration is required for USB functinality
> > >>> in uboot.
> > >>>
> > >>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> > >>> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> > >>> ---
> > >>> This patch set is created against Nick Thompson's latest patch set
> > >>> (v5) on DA8xx support and another patch from him on Davinci pinmux.
> > >>> [Davinci: add a pin multiplexer configuration API]
> > >> It is difficult to review code that is dependent on other outstanding
> > >> patches.
> > >>
> > >> The base DA8xx support will be in arm-ti branch shortly.
> > >> Please rebase these patches when this happens and resubmit.
> > >>
> > >> Tom
> > >
> > > I think he might have added it locally and then made his patches.
> > > So he might not have to rebase.
> > >
> > > Without Nick's patches da830evm.c would not even exist :-)
> > >
> > >
> >
> > My issue is not with how development continues, it is with how patches
> > are reviewed.  To really review this patchset someone would need to
> > apply the pin mux patch, the the 8xx patch set, then this set.
> > Reviewing patches against outstanding patches does not scale and should
> be
> > avoided.
> >
> > Tom
> 
> Not any more :-)
> 
> Take a look at http://git.denx.de/?p=u-boot/u-boot-
> ti.git;a=shortlog;h=refs/heads/master
> 
> I have tested some other DaVincs SOCs as well after adding these patches
> and they seem to be working fine as of now.
> 
> Lets continue the review process for the USB support.

I had actually applied all dependent patches and then created the USB patches and tested. So it removes the rebase issue.

Regards,
Ajay
> 
> Thanks,
> Sandeep