diff mbox

[U-Boot,v3,2/7] ARM: OMAP5: USB: Add OMAP5 common USB EHCI information

Message ID 1374092167-27645-3-git-send-email-dmurphy@ti.com
State Awaiting Upstream
Delegated to: Marek Vasut
Headers show

Commit Message

Dan Murphy July 17, 2013, 8:16 p.m. UTC 
* Enable the OMAP5 EHCI host clocks
* Add OMAP5 EHCI register definitions
* Add OMAP5 ES2 host revision

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
v3 - Updated per comments - http://patchwork.ozlabs.org/patch/258230/

 arch/arm/cpu/armv7/omap5/hw_data.c      |   17 ++++++++++++
 arch/arm/include/asm/arch-omap5/clock.h |    6 +++++
 arch/arm/include/asm/arch-omap5/ehci.h  |   43 +++++++++++++++++++++++++++++++
 arch/arm/include/asm/ehci-omap.h        |    1 +
 drivers/usb/host/ehci-omap.c            |    2 +-
 5 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/asm/arch-omap5/ehci.h

Comments

Roger Quadros July 18, 2013, 7:10 a.m. UTC | #1 
Dan,

On 07/17/2013 11:16 PM, Dan Murphy wrote:
> * Enable the OMAP5 EHCI host clocks
> * Add OMAP5 EHCI register definitions
> * Add OMAP5 ES2 host revision
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> v3 - Updated per comments - http://patchwork.ozlabs.org/patch/258230/
> 
>  arch/arm/cpu/armv7/omap5/hw_data.c      |   17 ++++++++++++
>  arch/arm/include/asm/arch-omap5/clock.h |    6 +++++
>  arch/arm/include/asm/arch-omap5/ehci.h  |   43 +++++++++++++++++++++++++++++++
>  arch/arm/include/asm/ehci-omap.h        |    1 +
>  drivers/usb/host/ehci-omap.c            |    2 +-
>  5 files changed, 68 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/include/asm/arch-omap5/ehci.h
> 
> diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c
> index 07b1108..b1be748 100644
> --- a/arch/arm/cpu/armv7/omap5/hw_data.c
> +++ b/arch/arm/cpu/armv7/omap5/hw_data.c
> @@ -414,6 +414,10 @@ void enable_basic_clocks(void)
>  		(*prcm)->cm_l4per_gpio6_clkctrl,
>  		(*prcm)->cm_l4per_gpio7_clkctrl,
>  		(*prcm)->cm_l4per_gpio8_clkctrl,
> +#ifdef CONFIG_USB_EHCI_OMAP
> +		(*prcm)->cm_clksel_usb_60mhz,
> +		(*prcm)->cm_l3init_hsusbtll_clkctrl,
> +#endif
>  		0
>  	};
>  
> @@ -425,6 +429,9 @@ void enable_basic_clocks(void)
>  		(*prcm)->cm_wkup_wdtimer2_clkctrl,
>  		(*prcm)->cm_l4per_uart3_clkctrl,
>  		(*prcm)->cm_l4per_i2c1_clkctrl,
> +#ifdef CONFIG_USB_EHCI_OMAP
> +		(*prcm)->cm_l3init_hsusbhost_clkctrl,
> +#endif
>  		0
>  	};
>  
> @@ -448,6 +455,16 @@ void enable_basic_clocks(void)
>  	setbits_le32((*prcm)->cm_wkup_gptimer1_clkctrl,
>  			GPTIMER1_CLKCTRL_CLKSEL_MASK);
>  
> +#ifdef CONFIG_USB_EHCI
> +	/* Enable port 2 and 3 clocks*/
> +	setbits_le32((*prcm)->cm_l3init_hsusbhost_clkctrl,
> +			USB_HOST_HS_CLKCTRL_MASK);

For consistency, maybe you should enable all 3 port clocks
since you are enabling all 3 TLL clocks below?
> +
> +	/* Enable all 3 usb host ports tll clocks*/
> +	setbits_le32((*prcm)->cm_l3init_hsusbtll_clkctrl,
> +			USB_TLL_HS_CLKCTRL_MASK);
> +#endif
> +
>  	do_enable_clocks(clk_domains_essential,
>  			 clk_modules_hw_auto_essential,
>  			 clk_modules_explicit_en_essential,
> diff --git a/arch/arm/include/asm/arch-omap5/clock.h b/arch/arm/include/asm/arch-omap5/clock.h
> index 4d2765d..3a58337 100644
> --- a/arch/arm/include/asm/arch-omap5/clock.h
> +++ b/arch/arm/include/asm/arch-omap5/clock.h
> @@ -165,6 +165,12 @@
>  /* CM_L3INIT_USBPHY_CLKCTRL */
>  #define USBPHY_CLKCTRL_OPTFCLKEN_PHY_48M_MASK	8
>  
> +/* CM_L3INIT_USB_HOST_HS_CLKCTRL */
> +#define USB_HOST_HS_CLKCTRL_MASK		0x56C0
> +
> +/* CM_L3INIT_USB_TLL_HS_CLKCTRL */
> +#define USB_TLL_HS_CLKCTRL_MASK			0x700
> +
>  /* CM_MPU_MPU_CLKCTRL */
>  #define MPU_CLKCTRL_CLKSEL_EMIF_DIV_MODE_SHIFT	24
>  #define MPU_CLKCTRL_CLKSEL_EMIF_DIV_MODE_MASK	(3 << 24)
> diff --git a/arch/arm/include/asm/arch-omap5/ehci.h b/arch/arm/include/asm/arch-omap5/ehci.h
> new file mode 100644
> index 0000000..3921e4a
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-omap5/ehci.h
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com*
> + * Author: Govindraj R <govindraj.raja@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _EHCI_H
> +#define _EHCI_H
> +
> +#define OMAP_EHCI_BASE				(OMAP54XX_L4_CORE_BASE + 0x64C00)
> +#define OMAP_UHH_BASE				(OMAP54XX_L4_CORE_BASE + 0x64000)
> +#define OMAP_USBTLL_BASE			(OMAP54XX_L4_CORE_BASE + 0x62000)
> +
> +/* TLL Register Set */
> +#define OMAP_USBTLL_SYSCONFIG_SIDLEMODE		(1 << 3)
> +#define OMAP_USBTLL_SYSCONFIG_ENAWAKEUP		(1 << 2)
> +#define OMAP_USBTLL_SYSCONFIG_SOFTRESET		(1 << 1)
> +#define OMAP_USBTLL_SYSCONFIG_CACTIVITY		(1 << 8)
> +#define OMAP_USBTLL_SYSSTATUS_RESETDONE		1
> +
> +#define OMAP_UHH_SYSCONFIG_SOFTRESET		1
> +#define OMAP_UHH_SYSSTATUS_EHCI_RESETDONE	(1 << 2)
> +#define OMAP_UHH_SYSCONFIG_NOIDLE		(1 << 2)
> +#define OMAP_UHH_SYSCONFIG_NOSTDBY		(1 << 4)
> +
> +#define OMAP_UHH_SYSCONFIG_VAL	(OMAP_UHH_SYSCONFIG_NOIDLE | \
> +					OMAP_UHH_SYSCONFIG_NOSTDBY)
> +
> +#endif /* _EHCI_H */
> diff --git a/arch/arm/include/asm/ehci-omap.h b/arch/arm/include/asm/ehci-omap.h
> index 77e8170..0b09e9d 100644
> --- a/arch/arm/include/asm/ehci-omap.h
> +++ b/arch/arm/include/asm/ehci-omap.h
> @@ -42,6 +42,7 @@ enum usbhs_omap_port_mode {
>  /* Values of UHH_REVISION - Note: these are not given in the TRM */
>  #define OMAP_USBHS_REV1					0x00000010 /* OMAP3 */
>  #define OMAP_USBHS_REV2					0x50700100 /* OMAP4 */
> +#define OMAP_USBHS_REV2_1				0x50700101 /* OMAP5 */
>  
>  /* UHH Register Set */
>  #define OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN		(1 << 2)
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 086c697..17f2214 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -208,7 +208,7 @@ int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata,
>  			clrbits_le32(&reg, OMAP_UHH_HOSTCONFIG_ULPI_P3_BYPASS);
>  		else
>  			setbits_le32(&reg, OMAP_UHH_HOSTCONFIG_ULPI_P3_BYPASS);
> -	} else if (rev == OMAP_USBHS_REV2) {
> +	} else if ((rev == OMAP_USBHS_REV2) || (rev == OMAP_USBHS_REV2_1)) {
>  		clrsetbits_le32(&reg, (OMAP_P1_MODE_CLEAR | OMAP_P2_MODE_CLEAR),
>  					OMAP4_UHH_HOSTCONFIG_APP_START_CLK);
>  
As OMAP5 has 3 ports, are we missing OMAP_P3_MODE_CLEAR here? Better to put REV2_1
in a separate else {}?

Also, REV2 is for OMAP4 which has only 2 ports so I don't understand why this is in
the code for REV2.
                if (is_ehci_hsic_mode(usbhs_pdata->port_mode[2]))
                        setbits_le32(&reg, OMAP_P3_MODE_HSIC);

That could be moved into REV2_1 as well.

A little below in the file I see this.

omap_ehci_phy_reset(0, 10);

This function takes care only of PHY1 and PHY2 but not PHY3. Could this be the reason why
the PORT3 (i.e. LAN chip) is not working?

Alternatively that function should be called there only be called if we are in PHY mode.
i.e. is_ehci_phy_mode(), as it seems to release the PHY reset.

cheers,
-roger

cheers,
-roger
Dan Murphy July 18, 2013, 5:48 p.m. UTC | #2 
Dear Roger

On 07/18/2013 02:10 AM, Roger Quadros wrote:
> Dan,
>
> On 07/17/2013 11:16 PM, Dan Murphy wrote:
>> * Enable the OMAP5 EHCI host clocks
>> * Add OMAP5 EHCI register definitions
>> * Add OMAP5 ES2 host revision
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>> v3 - Updated per comments - http://patchwork.ozlabs.org/patch/258230/
>>
>>  arch/arm/cpu/armv7/omap5/hw_data.c      |   17 ++++++++++++
>>  arch/arm/include/asm/arch-omap5/clock.h |    6 +++++
>>  arch/arm/include/asm/arch-omap5/ehci.h  |   43 +++++++++++++++++++++++++++++++
>>  arch/arm/include/asm/ehci-omap.h        |    1 +
>>  drivers/usb/host/ehci-omap.c            |    2 +-
>>  5 files changed, 68 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/arm/include/asm/arch-omap5/ehci.h
>>
>> diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c
>> index 07b1108..b1be748 100644
>> --- a/arch/arm/cpu/armv7/omap5/hw_data.c
>> +++ b/arch/arm/cpu/armv7/omap5/hw_data.c
>> @@ -414,6 +414,10 @@ void enable_basic_clocks(void)
>>  		(*prcm)->cm_l4per_gpio6_clkctrl,
>>  		(*prcm)->cm_l4per_gpio7_clkctrl,
>>  		(*prcm)->cm_l4per_gpio8_clkctrl,
>> +#ifdef CONFIG_USB_EHCI_OMAP
>> +		(*prcm)->cm_clksel_usb_60mhz,
>> +		(*prcm)->cm_l3init_hsusbtll_clkctrl,
>> +#endif
>>  		0
>>  	};
>>  
>> @@ -425,6 +429,9 @@ void enable_basic_clocks(void)
>>  		(*prcm)->cm_wkup_wdtimer2_clkctrl,
>>  		(*prcm)->cm_l4per_uart3_clkctrl,
>>  		(*prcm)->cm_l4per_i2c1_clkctrl,
>> +#ifdef CONFIG_USB_EHCI_OMAP
>> +		(*prcm)->cm_l3init_hsusbhost_clkctrl,
>> +#endif
>>  		0
>>  	};
>>  
>> @@ -448,6 +455,16 @@ void enable_basic_clocks(void)
>>  	setbits_le32((*prcm)->cm_wkup_gptimer1_clkctrl,
>>  			GPTIMER1_CLKCTRL_CLKSEL_MASK);
>>  
>> +#ifdef CONFIG_USB_EHCI
>> +	/* Enable port 2 and 3 clocks*/
>> +	setbits_le32((*prcm)->cm_l3init_hsusbhost_clkctrl,
>> +			USB_HOST_HS_CLKCTRL_MASK);
> For consistency, maybe you should enable all 3 port clocks
> since you are enabling all 3 TLL clocks below?
So I thought about this we should enable all 3 clocks since it is a common file and disable the unused clocks in the board file.

Or we should enable the specific port clocks during ehci_hcd_init in the board file only since it is a board implementation detail.

Thoughts?
>> +
>> +	/* Enable all 3 usb host ports tll clocks*/
>> +	setbits_le32((*prcm)->cm_l3init_hsusbtll_clkctrl,
>> +			USB_TLL_HS_CLKCTRL_MASK);
>> +#endif
>> +
>>  	do_enable_clocks(clk_domains_essential,
>>  			 clk_modules_hw_auto_essential,
>>  			 clk_modules_explicit_en_essential,
>> diff --git a/arch/arm/include/asm/arch-omap5/clock.h b/arch/arm/include/asm/arch-omap5/clock.h
>> index 4d2765d..3a58337 100644
>> --- a/arch/arm/include/asm/arch-omap5/clock.h
>> +++ b/arch/arm/include/asm/arch-omap5/clock.h
>> @@ -165,6 +165,12 @@
>>  /* CM_L3INIT_USBPHY_CLKCTRL */
>>  #define USBPHY_CLKCTRL_OPTFCLKEN_PHY_48M_MASK	8
>>  
>> +/* CM_L3INIT_USB_HOST_HS_CLKCTRL */
>> +#define USB_HOST_HS_CLKCTRL_MASK		0x56C0
>> +
>> +/* CM_L3INIT_USB_TLL_HS_CLKCTRL */
>> +#define USB_TLL_HS_CLKCTRL_MASK			0x700
>> +
>>  /* CM_MPU_MPU_CLKCTRL */
>>  #define MPU_CLKCTRL_CLKSEL_EMIF_DIV_MODE_SHIFT	24
>>  #define MPU_CLKCTRL_CLKSEL_EMIF_DIV_MODE_MASK	(3 << 24)
>> diff --git a/arch/arm/include/asm/arch-omap5/ehci.h b/arch/arm/include/asm/arch-omap5/ehci.h
>> new file mode 100644
>> index 0000000..3921e4a
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-omap5/ehci.h
>> @@ -0,0 +1,43 @@
>> +/*
>> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com*
>> + * Author: Govindraj R <govindraj.raja@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#ifndef _EHCI_H
>> +#define _EHCI_H
>> +
>> +#define OMAP_EHCI_BASE				(OMAP54XX_L4_CORE_BASE + 0x64C00)
>> +#define OMAP_UHH_BASE				(OMAP54XX_L4_CORE_BASE + 0x64000)
>> +#define OMAP_USBTLL_BASE			(OMAP54XX_L4_CORE_BASE + 0x62000)
>> +
>> +/* TLL Register Set */
>> +#define OMAP_USBTLL_SYSCONFIG_SIDLEMODE		(1 << 3)
>> +#define OMAP_USBTLL_SYSCONFIG_ENAWAKEUP		(1 << 2)
>> +#define OMAP_USBTLL_SYSCONFIG_SOFTRESET		(1 << 1)
>> +#define OMAP_USBTLL_SYSCONFIG_CACTIVITY		(1 << 8)
>> +#define OMAP_USBTLL_SYSSTATUS_RESETDONE		1
>> +
>> +#define OMAP_UHH_SYSCONFIG_SOFTRESET		1
>> +#define OMAP_UHH_SYSSTATUS_EHCI_RESETDONE	(1 << 2)
>> +#define OMAP_UHH_SYSCONFIG_NOIDLE		(1 << 2)
>> +#define OMAP_UHH_SYSCONFIG_NOSTDBY		(1 << 4)
>> +
>> +#define OMAP_UHH_SYSCONFIG_VAL	(OMAP_UHH_SYSCONFIG_NOIDLE | \
>> +					OMAP_UHH_SYSCONFIG_NOSTDBY)
>> +
>> +#endif /* _EHCI_H */
>> diff --git a/arch/arm/include/asm/ehci-omap.h b/arch/arm/include/asm/ehci-omap.h
>> index 77e8170..0b09e9d 100644
>> --- a/arch/arm/include/asm/ehci-omap.h
>> +++ b/arch/arm/include/asm/ehci-omap.h
>> @@ -42,6 +42,7 @@ enum usbhs_omap_port_mode {
>>  /* Values of UHH_REVISION - Note: these are not given in the TRM */
>>  #define OMAP_USBHS_REV1					0x00000010 /* OMAP3 */
>>  #define OMAP_USBHS_REV2					0x50700100 /* OMAP4 */
>> +#define OMAP_USBHS_REV2_1				0x50700101 /* OMAP5 */
>>  
>>  /* UHH Register Set */
>>  #define OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN		(1 << 2)
>> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
>> index 086c697..17f2214 100644
>> --- a/drivers/usb/host/ehci-omap.c
>> +++ b/drivers/usb/host/ehci-omap.c
>> @@ -208,7 +208,7 @@ int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata,
>>  			clrbits_le32(&reg, OMAP_UHH_HOSTCONFIG_ULPI_P3_BYPASS);
>>  		else
>>  			setbits_le32(&reg, OMAP_UHH_HOSTCONFIG_ULPI_P3_BYPASS);
>> -	} else if (rev == OMAP_USBHS_REV2) {
>> +	} else if ((rev == OMAP_USBHS_REV2) || (rev == OMAP_USBHS_REV2_1)) {
>>  		clrsetbits_le32(&reg, (OMAP_P1_MODE_CLEAR | OMAP_P2_MODE_CLEAR),
>>  					OMAP4_UHH_HOSTCONFIG_APP_START_CLK);
>>  
> As OMAP5 has 3 ports, are we missing OMAP_P3_MODE_CLEAR here? Better to put REV2_1
> in a separate else {}?
>
> Also, REV2 is for OMAP4 which has only 2 ports so I don't understand why this is in
> the code for REV2.
>                 if (is_ehci_hsic_mode(usbhs_pdata->port_mode[2]))
>                         setbits_le32(&reg, OMAP_P3_MODE_HSIC);
>
> That could be moved into REV2_1 as well.
OK will add else case for rev 2.1.
> A little below in the file I see this.
>
> omap_ehci_phy_reset(0, 10);
>
> This function takes care only of PHY1 and PHY2 but not PHY3. Could this be the reason why
> the PORT3 (i.e. LAN chip) is not working?
No that is not the reason.  The GPIO would be reset based on PHY1 value in this patch series.
But I should technically add the PHY3 GPIO as well.
> Alternatively that function should be called there only be called if we are in PHY mode.
> i.e. is_ehci_phy_mode(), as it seems to release the PHY reset.
>
> cheers,
> -roger
>
> cheers,
> -roger
Roger Quadros July 19, 2013, 9:30 a.m. UTC | #3 
On 07/18/2013 08:48 PM, Dan Murphy wrote:
> Dear Roger
> 
> On 07/18/2013 02:10 AM, Roger Quadros wrote:
>> Dan,
>>
>> On 07/17/2013 11:16 PM, Dan Murphy wrote:
>>> * Enable the OMAP5 EHCI host clocks
>>> * Add OMAP5 EHCI register definitions
>>> * Add OMAP5 ES2 host revision
>>>
>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> ---
>>> v3 - Updated per comments - http://patchwork.ozlabs.org/patch/258230/
>>>
>>>  arch/arm/cpu/armv7/omap5/hw_data.c      |   17 ++++++++++++
>>>  arch/arm/include/asm/arch-omap5/clock.h |    6 +++++
>>>  arch/arm/include/asm/arch-omap5/ehci.h  |   43 +++++++++++++++++++++++++++++++
>>>  arch/arm/include/asm/ehci-omap.h        |    1 +
>>>  drivers/usb/host/ehci-omap.c            |    2 +-
>>>  5 files changed, 68 insertions(+), 1 deletion(-)
>>>  create mode 100644 arch/arm/include/asm/arch-omap5/ehci.h
>>>
>>> diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c
>>> index 07b1108..b1be748 100644
>>> --- a/arch/arm/cpu/armv7/omap5/hw_data.c
>>> +++ b/arch/arm/cpu/armv7/omap5/hw_data.c
>>> @@ -414,6 +414,10 @@ void enable_basic_clocks(void)
>>>  		(*prcm)->cm_l4per_gpio6_clkctrl,
>>>  		(*prcm)->cm_l4per_gpio7_clkctrl,
>>>  		(*prcm)->cm_l4per_gpio8_clkctrl,
>>> +#ifdef CONFIG_USB_EHCI_OMAP
>>> +		(*prcm)->cm_clksel_usb_60mhz,
>>> +		(*prcm)->cm_l3init_hsusbtll_clkctrl,
>>> +#endif
>>>  		0
>>>  	};
>>>  
>>> @@ -425,6 +429,9 @@ void enable_basic_clocks(void)
>>>  		(*prcm)->cm_wkup_wdtimer2_clkctrl,
>>>  		(*prcm)->cm_l4per_uart3_clkctrl,
>>>  		(*prcm)->cm_l4per_i2c1_clkctrl,
>>> +#ifdef CONFIG_USB_EHCI_OMAP
>>> +		(*prcm)->cm_l3init_hsusbhost_clkctrl,
>>> +#endif
>>>  		0
>>>  	};
>>>  
>>> @@ -448,6 +455,16 @@ void enable_basic_clocks(void)
>>>  	setbits_le32((*prcm)->cm_wkup_gptimer1_clkctrl,
>>>  			GPTIMER1_CLKCTRL_CLKSEL_MASK);
>>>  
>>> +#ifdef CONFIG_USB_EHCI
>>> +	/* Enable port 2 and 3 clocks*/
>>> +	setbits_le32((*prcm)->cm_l3init_hsusbhost_clkctrl,
>>> +			USB_HOST_HS_CLKCTRL_MASK);
>> For consistency, maybe you should enable all 3 port clocks
>> since you are enabling all 3 TLL clocks below?
> So I thought about this we should enable all 3 clocks since it is a common file and disable the unused clocks in the board file.
> 
> Or we should enable the specific port clocks during ehci_hcd_init in the board file only since it is a board implementation detail.
> 
> Thoughts?

IMHO enabling the clocks based on board is best.

cheers,
-roger
Tom Rini July 22, 2013, 3:14 p.m. UTC | #4 
On Fri, Jul 19, 2013 at 12:30:54PM +0300, Roger Quadros wrote:
> On 07/18/2013 08:48 PM, Dan Murphy wrote:
> > Dear Roger
> > 
> > On 07/18/2013 02:10 AM, Roger Quadros wrote:
> >> Dan,
> >>
> >> On 07/17/2013 11:16 PM, Dan Murphy wrote:
> >>> * Enable the OMAP5 EHCI host clocks
> >>> * Add OMAP5 EHCI register definitions
> >>> * Add OMAP5 ES2 host revision
> >>>
> >>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> >>> ---
> >>> v3 - Updated per comments - http://patchwork.ozlabs.org/patch/258230/
> >>>
> >>>  arch/arm/cpu/armv7/omap5/hw_data.c      |   17 ++++++++++++
> >>>  arch/arm/include/asm/arch-omap5/clock.h |    6 +++++
> >>>  arch/arm/include/asm/arch-omap5/ehci.h  |   43 +++++++++++++++++++++++++++++++
> >>>  arch/arm/include/asm/ehci-omap.h        |    1 +
> >>>  drivers/usb/host/ehci-omap.c            |    2 +-
> >>>  5 files changed, 68 insertions(+), 1 deletion(-)
> >>>  create mode 100644 arch/arm/include/asm/arch-omap5/ehci.h
> >>>
> >>> diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c
> >>> index 07b1108..b1be748 100644
> >>> --- a/arch/arm/cpu/armv7/omap5/hw_data.c
> >>> +++ b/arch/arm/cpu/armv7/omap5/hw_data.c
> >>> @@ -414,6 +414,10 @@ void enable_basic_clocks(void)
> >>>  		(*prcm)->cm_l4per_gpio6_clkctrl,
> >>>  		(*prcm)->cm_l4per_gpio7_clkctrl,
> >>>  		(*prcm)->cm_l4per_gpio8_clkctrl,
> >>> +#ifdef CONFIG_USB_EHCI_OMAP
> >>> +		(*prcm)->cm_clksel_usb_60mhz,
> >>> +		(*prcm)->cm_l3init_hsusbtll_clkctrl,
> >>> +#endif
> >>>  		0
> >>>  	};
> >>>  
> >>> @@ -425,6 +429,9 @@ void enable_basic_clocks(void)
> >>>  		(*prcm)->cm_wkup_wdtimer2_clkctrl,
> >>>  		(*prcm)->cm_l4per_uart3_clkctrl,
> >>>  		(*prcm)->cm_l4per_i2c1_clkctrl,
> >>> +#ifdef CONFIG_USB_EHCI_OMAP
> >>> +		(*prcm)->cm_l3init_hsusbhost_clkctrl,
> >>> +#endif
> >>>  		0
> >>>  	};
> >>>  
> >>> @@ -448,6 +455,16 @@ void enable_basic_clocks(void)
> >>>  	setbits_le32((*prcm)->cm_wkup_gptimer1_clkctrl,
> >>>  			GPTIMER1_CLKCTRL_CLKSEL_MASK);
> >>>  
> >>> +#ifdef CONFIG_USB_EHCI
> >>> +	/* Enable port 2 and 3 clocks*/
> >>> +	setbits_le32((*prcm)->cm_l3init_hsusbhost_clkctrl,
> >>> +			USB_HOST_HS_CLKCTRL_MASK);
> >> For consistency, maybe you should enable all 3 port clocks
> >> since you are enabling all 3 TLL clocks below?
> > So I thought about this we should enable all 3 clocks since it is a common file and disable the unused clocks in the board file.
> > 
> > Or we should enable the specific port clocks during ehci_hcd_init in the board file only since it is a board implementation detail.
> > 
> > Thoughts?
> 
> IMHO enabling the clocks based on board is best.

Agree.  We only enable what we need, not enable and then disable as
someone will forget to disable (or not know they need to).  Even if this
means adding a define or 3 for which ports we do have.
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c
index 07b1108..b1be748 100644
--- a/arch/arm/cpu/armv7/omap5/hw_data.c
+++ b/arch/arm/cpu/armv7/omap5/hw_data.c
@@ -414,6 +414,10 @@  void enable_basic_clocks(void)
 		(*prcm)->cm_l4per_gpio6_clkctrl,
 		(*prcm)->cm_l4per_gpio7_clkctrl,
 		(*prcm)->cm_l4per_gpio8_clkctrl,
+#ifdef CONFIG_USB_EHCI_OMAP
+		(*prcm)->cm_clksel_usb_60mhz,
+		(*prcm)->cm_l3init_hsusbtll_clkctrl,
+#endif
 		0
 	};
 
@@ -425,6 +429,9 @@  void enable_basic_clocks(void)
 		(*prcm)->cm_wkup_wdtimer2_clkctrl,
 		(*prcm)->cm_l4per_uart3_clkctrl,
 		(*prcm)->cm_l4per_i2c1_clkctrl,
+#ifdef CONFIG_USB_EHCI_OMAP
+		(*prcm)->cm_l3init_hsusbhost_clkctrl,
+#endif
 		0
 	};
 
@@ -448,6 +455,16 @@  void enable_basic_clocks(void)
 	setbits_le32((*prcm)->cm_wkup_gptimer1_clkctrl,
 			GPTIMER1_CLKCTRL_CLKSEL_MASK);
 
+#ifdef CONFIG_USB_EHCI
+	/* Enable port 2 and 3 clocks*/
+	setbits_le32((*prcm)->cm_l3init_hsusbhost_clkctrl,
+			USB_HOST_HS_CLKCTRL_MASK);
+
+	/* Enable all 3 usb host ports tll clocks*/
+	setbits_le32((*prcm)->cm_l3init_hsusbtll_clkctrl,
+			USB_TLL_HS_CLKCTRL_MASK);
+#endif
+
 	do_enable_clocks(clk_domains_essential,
 			 clk_modules_hw_auto_essential,
 			 clk_modules_explicit_en_essential,
diff --git a/arch/arm/include/asm/arch-omap5/clock.h b/arch/arm/include/asm/arch-omap5/clock.h
index 4d2765d..3a58337 100644
--- a/arch/arm/include/asm/arch-omap5/clock.h
+++ b/arch/arm/include/asm/arch-omap5/clock.h
@@ -165,6 +165,12 @@ 
 /* CM_L3INIT_USBPHY_CLKCTRL */
 #define USBPHY_CLKCTRL_OPTFCLKEN_PHY_48M_MASK	8
 
+/* CM_L3INIT_USB_HOST_HS_CLKCTRL */
+#define USB_HOST_HS_CLKCTRL_MASK		0x56C0
+
+/* CM_L3INIT_USB_TLL_HS_CLKCTRL */
+#define USB_TLL_HS_CLKCTRL_MASK			0x700
+
 /* CM_MPU_MPU_CLKCTRL */
 #define MPU_CLKCTRL_CLKSEL_EMIF_DIV_MODE_SHIFT	24
 #define MPU_CLKCTRL_CLKSEL_EMIF_DIV_MODE_MASK	(3 << 24)
diff --git a/arch/arm/include/asm/arch-omap5/ehci.h b/arch/arm/include/asm/arch-omap5/ehci.h
new file mode 100644
index 0000000..3921e4a
--- /dev/null
+++ b/arch/arm/include/asm/arch-omap5/ehci.h
@@ -0,0 +1,43 @@ 
+/*
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com*
+ * Author: Govindraj R <govindraj.raja@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _EHCI_H
+#define _EHCI_H
+
+#define OMAP_EHCI_BASE				(OMAP54XX_L4_CORE_BASE + 0x64C00)
+#define OMAP_UHH_BASE				(OMAP54XX_L4_CORE_BASE + 0x64000)
+#define OMAP_USBTLL_BASE			(OMAP54XX_L4_CORE_BASE + 0x62000)
+
+/* TLL Register Set */
+#define OMAP_USBTLL_SYSCONFIG_SIDLEMODE		(1 << 3)
+#define OMAP_USBTLL_SYSCONFIG_ENAWAKEUP		(1 << 2)
+#define OMAP_USBTLL_SYSCONFIG_SOFTRESET		(1 << 1)
+#define OMAP_USBTLL_SYSCONFIG_CACTIVITY		(1 << 8)
+#define OMAP_USBTLL_SYSSTATUS_RESETDONE		1
+
+#define OMAP_UHH_SYSCONFIG_SOFTRESET		1
+#define OMAP_UHH_SYSSTATUS_EHCI_RESETDONE	(1 << 2)
+#define OMAP_UHH_SYSCONFIG_NOIDLE		(1 << 2)
+#define OMAP_UHH_SYSCONFIG_NOSTDBY		(1 << 4)
+
+#define OMAP_UHH_SYSCONFIG_VAL	(OMAP_UHH_SYSCONFIG_NOIDLE | \
+					OMAP_UHH_SYSCONFIG_NOSTDBY)
+
+#endif /* _EHCI_H */
diff --git a/arch/arm/include/asm/ehci-omap.h b/arch/arm/include/asm/ehci-omap.h
index 77e8170..0b09e9d 100644
--- a/arch/arm/include/asm/ehci-omap.h
+++ b/arch/arm/include/asm/ehci-omap.h
@@ -42,6 +42,7 @@  enum usbhs_omap_port_mode {
 /* Values of UHH_REVISION - Note: these are not given in the TRM */
 #define OMAP_USBHS_REV1					0x00000010 /* OMAP3 */
 #define OMAP_USBHS_REV2					0x50700100 /* OMAP4 */
+#define OMAP_USBHS_REV2_1				0x50700101 /* OMAP5 */
 
 /* UHH Register Set */
 #define OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN		(1 << 2)
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 086c697..17f2214 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -208,7 +208,7 @@  int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata,
 			clrbits_le32(&reg, OMAP_UHH_HOSTCONFIG_ULPI_P3_BYPASS);
 		else
 			setbits_le32(&reg, OMAP_UHH_HOSTCONFIG_ULPI_P3_BYPASS);
-	} else if (rev == OMAP_USBHS_REV2) {
+	} else if ((rev == OMAP_USBHS_REV2) || (rev == OMAP_USBHS_REV2_1)) {
 		clrsetbits_le32(&reg, (OMAP_P1_MODE_CLEAR | OMAP_P2_MODE_CLEAR),
 					OMAP4_UHH_HOSTCONFIG_APP_START_CLK);