[U-Boot] [PATCH 1/2] powerpc/85xx: Add ULPI and UTMI USB Phy support for P1010/P1014

[Remy Bohmer]

Hi,

2011/7/1 Kumar Gala <galak at kernel.crashing.org>:
> From: Ramneek Mehresh <ramneek.mehresh at freescale.com>
>
> Add UTMI and ULPI PHY support for USB controller on qoriq series of
> processors with internal UTMI PHY implemented, for example P1010/P1014
>  - Use both getenv() and hwconfig to get USB phy type till getenv()
>   is depricated
>  - Introduce CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY to specify if soc
>   has internal UTMI phy
>
> Signed-off-by: Ramneek Mehresh <ramneek.mehresh at freescale.com>
> CC: Remy Bohmer <linux at bohmer.net>
> Signed-off-by: Kumar Gala <galak at kernel.crashing.org>
> ---
>  arch/powerpc/include/asm/config_mpc85xx.h |    2 +
>  drivers/usb/host/ehci-fsl.c               |   37 ++++++++++++++++++++++++++--
>  2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
> index 04ca989..d9d04e7 100644
> --- a/arch/powerpc/include/asm/config_mpc85xx.h
> +++ b/arch/powerpc/include/asm/config_mpc85xx.h
> @@ -97,6 +97,7 @@
>  #define CONFIG_NUM_DDR_CONTROLLERS     1
>  #define CONFIG_SYS_CCSRBAR_DEFAULT     0xff700000
>  #define CONFIG_SYS_FSL_PCIE_COMPAT     "fsl,qoriq-pcie-v2.2"
> +#define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
>
>  /* P1011 is single core version of P1020 */
>  #elif defined(CONFIG_P1011)
> @@ -141,6 +142,7 @@
>  #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
>  #define CONFIG_NUM_DDR_CONTROLLERS     1
>  #define CONFIG_SYS_CCSRBAR_DEFAULT     0xff700000
> +#define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
>
>  /* P1015 is single core version of P1024 */
>  #elif defined(CONFIG_P1015)
> diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
> index 6e0043a..66b7da5 100644
> --- a/drivers/usb/host/ehci-fsl.c
> +++ b/drivers/usb/host/ehci-fsl.c
> @@ -1,5 +1,5 @@
>  /*
> - * (C) Copyright 2009 Freescale Semiconductor, Inc.
> + * (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
>  *
>  * (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
>  *
> @@ -26,6 +26,7 @@
>  #include <usb.h>
>  #include <asm/io.h>
>  #include <usb/ehci-fsl.h>
> +#include <hwconfig.h>
>
>  #include "ehci.h"
>  #include "ehci-core.h"
> @@ -39,6 +40,11 @@
>  int ehci_hcd_init(void)
>  {
>        struct usb_ehci *ehci;
> +       char usb_phy[5];
> +       const char *phy_type = NULL;
> +       size_t len;
> +
> +       usb_phy[0] = '\0';
>
>        ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;
>        hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
> @@ -52,10 +58,35 @@ int ehci_hcd_init(void)
>        out_be32(&ehci->snoop2, 0x80000000 | SNOOP_SIZE_2GB);
>
>        /* Init phy */
> -       if (!strcmp(getenv("usb_phy_type"), "utmi"))
> -               out_le32(&(hcor->or_portsc[0]), PORT_PTS_UTMI);
> +       if (hwconfig_sub("usb1", "phy_type"))
> +               phy_type = hwconfig_subarg("usb1", "phy_type", &len);
>        else
> +               phy_type = getenv("usb_phy_type");

Please insert an empty line here for readability.

> +       if (!phy_type) {
> +#ifdef CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY
> +               /* if none specified assume internal UTMI */
> +               strcpy(usb_phy, "utmi");
> +               phy_type = usb_phy;
> +#else
> +               printf("WARNING: USB phy type not defined !!\n");
> +               return -1;
> +#endif
> +               }

Alignment is messy due to this patch. Please fix.

> +       if (!strcmp(phy_type, "utmi")) {
> +#if defined(CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY)
> +               setbits_be32(&ehci->control, PHY_CLK_SEL_UTMI);
> +               setbits_be32(&ehci->control, UTMI_PHY_EN);
> +               udelay(1000); /* delay required for PHY Clk to appear */
> +#endif
> +               out_le32(&(hcor->or_portsc[0]), PORT_PTS_UTMI);
> +       } else {
> +#if defined(CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY)
> +               clrbits_be32(&ehci->control, UTMI_PHY_EN);
> +               setbits_be32(&ehci->control, PHY_CLK_SEL_ULPI);
> +               udelay(1000); /* delay required for PHY Clk to appear */
> +#endif
>                out_le32(&(hcor->or_portsc[0]), PORT_PTS_ULPI);
> +       }

The ifdef-ery makes the code somewhat messy as well. Can it be written
somewhat else?
Apart from that and the style issues, no remarks or concerns.

Kind regards,

Remy