From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marek.vasut@gmail.com>
Date: Sun, 25 Sep 2011 19:34:00 +0200
Subject: [U-Boot] [PATCH v4 4/4] Add USB support for Efika
In-Reply-To: <1316971511-5667-4-git-send-email-fermata7@gmail.com>
References: <1316312368-13006-1-git-send-email-fermata7@gmail.com>
	<1316971511-5667-1-git-send-email-fermata7@gmail.com>
	<1316971511-5667-4-git-send-email-fermata7@gmail.com>
Message-ID: <201109251934.00630.marek.vasut@gmail.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 Sunday, September 25, 2011 07:25:10 PM Jana Rapava wrote:
> This commit adds USB support for EfikaMX and EfikaSB.
> 
> Signed-off-by: Jana Rapava <fermata7@gmail.com>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Remy Bohmer <linux@bohmer.net>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
> Changes for v2:
>       - changed to proper patch
> Changes for v3:
>       - merged other USB patches from u-boot-pxa/efikasb
>       - offset-based access changed to struct-based access
>       - use {clrset,clr,set}bits_le32() calls
>       - CodingStyle and naming cleanup
> Changes for v4:
>        - split into patchset
>        - CodingStyle and naming cleanup
>        - remove endless loops
>        - silence compiler warnings
> 

Dear Jana Rapava,

[...]
> +
> +u32 ulpi_wait(u32 ulpi_bit, const char *operation, struct usb_ehci *ehci)

Put struct usb_ehci * at the first place.

> +{
> +	int timeout = ULPI_TIMEOUT;
> +	u32 tmp;

Better separation of chunks of code with newlines will help readability. Please 
fix globally.

> +	while (--timeout) {
> +		tmp = readl(&ehci->ulpi_viewpoint);
> +		if (!(tmp & ulpi_bit))
> +			break;
> +		WATCHDOG_RESET();
> +	}
> +	if (!timeout) {
> +		printf("ULPI %s timed out\n", operation);
> +		return 0;
> +	}
> +	return tmp;
> +}
> +
> +void ulpi_write(u8 *reg, u32 value, struct usb_ehci *ehci)

ulpi_write(ehci, reg, value), please fix globally and in similar functions.

Also, change u8 *reg to u8 reg, why are you passing a pointer?

[...]

> +
> +int ehci_hcd_init(void)
> +{
> +	struct usb_ehci *ehci;
> +	struct usb_control_regs *mx5_usb_control_regs;
> +	struct ulpi_regs *ulpi;
> +
> +	/* Init iMX51 EHCI */
> +	efika_usb_phy_reset();
> +	efika_usb_hub_reset();
> +	efika_usb_enable_devices();
> +
> +	mx5_usb_control_regs = (struct usb_control_regs *)(OTG_BASE_ADDR +
> +		 MX51_CTRL_REGS);

rename to MX5_CTRL_REGS_OFFSET.

> +	control_regs_setup(mx5_usb_control_regs);
> +
> +	ulpi = (struct ulpi_regs *)0;
> +	/* Init EHCI core */
> +	ehci = (struct usb_ehci *)(OTG_BASE_ADDR +
> +		(MX51_REGISTER_LAYOUT_LENGTH * CONFIG_MXC_USB_PORT));
> +	hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
> +	hcor = (struct ehci_hcor *)((uint32_t) hccr +
> +			HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
> +	setbits_le32(&ehci->usbmode, CM_HOST);
> +	setbits_le32(&ehci->control, USB_EN);
> +
> +	switch (CONFIG_MXC_USB_PORT) {
> +	case 0:
> +		ehci0_init(ehci);
> +		break;
> +	case 1:
> +		ehci1_init(ehci, ulpi);
> +		break;
> +#ifdef	MACH_EFIKASB
> +	case 2:
> +		ehci2_init(ehci, ulpi);
> +		break;
> +#endif
> +	};
> +
> +	/* EfikaMX USB has issues ... */
> +	udelay(10000);
> +
> +

One newline is enough.

> +	return 0;
> +}
> +

Cheers