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 <aalo...@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);
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
