[U-Boot] [PATCH v2 05/14] mx51: Fix USB PHY clocks

[Igor Grinberg]

Hi Benoît,

please, see a minor #ifdef comment below

On 09/27/12 22:21, Benoît Thébaudeau wrote:
> The i.MX51 has a single USB PHY clock, while the i.MX53 has two. These 3 clocks
> have different clock gate control bit-fields.
> 
> The existing code was correct only for i.MX53, so this patch fixes the i.MX51
> use case.
> 
> Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Jana Rapava <fermata7 at gmail.com>
> Cc: Wolfgang Grandegger <wg at denx.de>
> Cc: Igor Grinberg <grinberg at compulab.co.il>
> ---
> This patch supersedes http://patchwork.ozlabs.org/patch/177403/ .
> Changes for v2:
>  - Split patch into 3 parts (the 3, 4 and 5 from this v2 series).
>  - Merge the various set_usb_phy*_clk() functions (they were identical).
> 
>  .../arch/arm/cpu/armv7/mx5/clock.c                 |   20 +++++++++++++-------
>  .../arch/arm/include/asm/arch-mx5/clock.h          |    7 ++++++-
>  .../drivers/usb/host/ehci-mx5.c                    |    6 +++++-
>  3 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git u-boot-imx-e1eb75b.orig/arch/arm/cpu/armv7/mx5/clock.c u-boot-imx-e1eb75b/arch/arm/cpu/armv7/mx5/clock.c
> index df7e5cd..f727cfa 100644
> --- u-boot-imx-e1eb75b.orig/arch/arm/cpu/armv7/mx5/clock.c
> +++ u-boot-imx-e1eb75b/arch/arm/cpu/armv7/mx5/clock.c
> @@ -126,11 +126,21 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num)
>  }
>  #endif
>  
> -void set_usb_phy1_clk(void)
> +void set_usb_phy_clk(void)
>  {
>  	clrbits_le32(&mxc_ccm->cscmr1, MXC_CCM_CSCMR1_USB_PHY_CLK_SEL);
>  }
>  
> +#if defined(CONFIG_MX51)
> +void enable_usb_phy_clk(unsigned char enable)

I think the same name as the MX53 case will do better, see explanation below.

> +{
> +	unsigned int cg = enable ? MXC_CCM_CCGR_CG_ON : MXC_CCM_CCGR_CG_OFF;
> +
> +	clrsetbits_le32(&mxc_ccm->CCGR2,
> +			MXC_CCM_CCGR2_USB_PHY(MXC_CCM_CCGR_CG_MASK),
> +			MXC_CCM_CCGR2_USB_PHY(cg));
> +}

I would add here (again explanation below):
void enable_usb_phy2_clk(unsigned char enable)
{
	/* MX51 has a single USB PHY clock, so do nothing here */
}

> +#elif defined(CONFIG_MX53)
>  void enable_usb_phy1_clk(unsigned char enable)
>  {
>  	unsigned int cg = enable ? MXC_CCM_CCGR_CG_ON : MXC_CCM_CCGR_CG_OFF;
> @@ -140,11 +150,6 @@ void enable_usb_phy1_clk(unsigned char enable)
>  			MXC_CCM_CCGR4_USB_PHY1(cg));
>  }
>  
> -void set_usb_phy2_clk(void)
> -{
> -	clrbits_le32(&mxc_ccm->cscmr1, MXC_CCM_CSCMR1_USB_PHY_CLK_SEL);
> -}
> -
>  void enable_usb_phy2_clk(unsigned char enable)
>  {
>  	unsigned int cg = enable ? MXC_CCM_CCGR_CG_ON : MXC_CCM_CCGR_CG_OFF;
> @@ -153,6 +158,7 @@ void enable_usb_phy2_clk(unsigned char enable)
>  			MXC_CCM_CCGR4_USB_PHY2(MXC_CCM_CCGR_CG_MASK),
>  			MXC_CCM_CCGR4_USB_PHY2(cg));
>  }
> +#endif
>  
>  /*
>   * Calculate the frequency of PLLn.
> @@ -803,7 +809,7 @@ void mxc_set_sata_internal_clock(void)
>  	u32 *tmp_base =
>  		(u32 *)(IIM_BASE_ADDR + 0x180c);
>  
> -	set_usb_phy1_clk();
> +	set_usb_phy_clk();
>  
>  	clrsetbits_le32(tmp_base, 0x6, 0x4);
>  }
> diff --git u-boot-imx-e1eb75b.orig/arch/arm/include/asm/arch-mx5/clock.h u-boot-imx-e1eb75b/arch/arm/include/asm/arch-mx5/clock.h
> index 55e3b51..e4ca417 100644
> --- u-boot-imx-e1eb75b.orig/arch/arm/include/asm/arch-mx5/clock.h
> +++ u-boot-imx-e1eb75b/arch/arm/include/asm/arch-mx5/clock.h
> @@ -56,8 +56,13 @@ u32 imx_get_uartclk(void);
>  u32 imx_get_fecclk(void);
>  unsigned int mxc_get_clock(enum mxc_clock clk);
>  int mxc_set_clock(u32 ref, u32 freq, u32 clk_type);
> -void set_usb_phy2_clk(void);
> +void set_usb_phy_clk(void);
> +#if defined(CONFIG_MX51)
> +void enable_usb_phy_clk(unsigned char enable);
> +#elif defined(CONFIG_MX53)
> +void enable_usb_phy1_clk(unsigned char enable);
>  void enable_usb_phy2_clk(unsigned char enable);
> +#endif

If you follow my suggestion above, then here you will only have:
 -void set_usb_phy2_clk(void);
 +void set_usb_phy_clk(void);
 +void enable_usb_phy1_clk(unsigned char enable);

and no complicating with #ifdefs needed.

>  void set_usboh3_clk(void);
>  void enable_usboh3_clk(unsigned char enable);
>  void mxc_set_sata_internal_clock(void);
> diff --git u-boot-imx-e1eb75b.orig/drivers/usb/host/ehci-mx5.c u-boot-imx-e1eb75b/drivers/usb/host/ehci-mx5.c
> index 58cdcbe..5bf89f0 100644
> --- u-boot-imx-e1eb75b.orig/drivers/usb/host/ehci-mx5.c
> +++ u-boot-imx-e1eb75b/drivers/usb/host/ehci-mx5.c
> @@ -221,8 +221,12 @@ int ehci_hcd_init(void)
>  
>  	set_usboh3_clk();
>  	enable_usboh3_clk(1);
> -	set_usb_phy2_clk();
> +	set_usb_phy_clk();
> +#if defined(CONFIG_MX51) && CONFIG_MXC_USB_PORT == 0
> +	enable_usb_phy_clk(1);
> +#elif defined(CONFIG_MX53)
>  	enable_usb_phy2_clk(1);
> +#endif

same here, it should be much cleaner.

>  	mdelay(1);
>  
>  	/* Do board specific initialization */
> 

-- 
Regards,
Igor.