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

[Kumar Gala]

On Aug 8, 2011, at 3:15 PM, Remy Bohmer wrote:

> 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.

Not really sure how to do that.  The patch is a bit more messy than the actual code.

- k