From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nick Thompson <nick.thompson@gefanuc.com>
Date: Thu, 12 Nov 2009 16:26:26 +0000
Subject: [U-Boot] [PATCH 2/3] DA8xx: Add MUSB host support
In-Reply-To: <1258040357-15800-2-git-send-email-ajay.gupta@ti.com>
References: <1258040357-15800-1-git-send-email-ajay.gupta@ti.com>
	<1258040357-15800-2-git-send-email-ajay.gupta@ti.com>
Message-ID: <4AFC3732.40000@gefanuc.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.