[U-Boot] [PATCH] imx: usb: ehci-mx6: add usb support for imx7d soc

[Marek Vasut <marex@denx.de>]

On Friday, July 31, 2015 at 12:34:00 AM, Adrian Alonso wrote:
> Extend ehci-mx6 usb driver to support imx7d usb

Hi Adrian,

this is not all the patch does and I know you can do better, really.
If you want to do accessor cleanups, please put them into separate
patch. If you also add kerneldoc for functions, please add that to
a separate patch as well. If you add new features, that should be
the last patch(es) in the series.

> Signed-off-by: Adrian Alonso <aalonso@freescale.com>
> ---
>  drivers/usb/host/Makefile   |   1 +
>  drivers/usb/host/ehci-mx6.c | 125
> ++++++++++++++++++++++++++++++++------------ 2 files changed, 92
> insertions(+), 34 deletions(-)

[...]

> +/*
> + * board_ehci_power - enables/disables usb vbus voltage
> + * @port:      usb otg port
> + * @on:        on/off vbus voltage
> + *
> + * Enables/disables supply vbus voltage for usb otg port.
> + * Machine board file overrides board_ehci_power
> + *
> + * Return: 0 Success
> + */
>  int __weak board_ehci_power(int port, int on)
>  {
>  	return 0;
> @@ -228,9 +270,14 @@ int __weak board_ehci_power(int port, int on)
>  int ehci_hcd_init(int index, enum usb_init_type init,
>  		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
>  {
> +#if defined(CONFIG_MX6)

A helper variable like "u32 controller_spacing = 0x200" or 0x10000
based on the ifdef would make the ifdef much smaller.

>  	enum usb_init_type type;
>  	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
>  		(0x200 * index));
> +#elif defined(CONFIG_MX7)
> +	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
> +		(0x10000 * index));

And then use it here to have just one struct usb_ehci *ehci ... 

> +#endif
> 
>  	if (index > 3)
>  		return -EINVAL;
> @@ -240,24 +287,34 @@ int ehci_hcd_init(int index, enum usb_init_type init,
>  	/* Do board specific initialization */
>  	board_ehci_hcd_init(index);
> 
> +#if defined(CONFIG_MX6)
>  	usb_power_config(index);

Maybe you can implement empty functions for MX7 here instead of the ifdeffery ?

> +#endif
>  	usb_oc_config(index);
> +#if defined(CONFIG_MX6)
>  	usb_internal_phy_clock_gate(index, 1);
>  	usb_phy_enable(index, ehci);
>  	type = board_usb_phy_mode(index);
> +#endif
> 
>  	*hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
>  	*hcor = (struct ehci_hcor *)((uint32_t)*hccr +
>  			HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
> 
> +#if defined(CONFIG_MX6)
>  	if ((type == init) || (type == USB_INIT_DEVICE))
>  		board_ehci_power(index, (type == USB_INIT_DEVICE) ? 0 : 1);
>  	if (type != init)
>  		return -ENODEV;
>  	if (type == USB_INIT_DEVICE)
>  		return 0;

Can't this code be used for MX7 as well ? Why not ?

> +#elif defined(CONFIG_MX7)
> +	board_ehci_power(index, (init == USB_INIT_DEVICE) ? 0 : 1);
> +	if (init == USB_INIT_DEVICE)
> +		return 0;
> +#endif
>  	setbits_le32(&ehci->usbmode, CM_HOST);
> -	__raw_writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc);
> +	writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc);
>  	setbits_le32(&ehci->portsc, USB_EN);
> 
>  	mdelay(10);