[PATCH 6/9] ARM: OMAP2+: Fix external clock support for dmtimers

[PATCH 6/9] ARM: OMAP2+: Fix external clock support for dmtimers

[Jon Hunter]

From: Jon Hunter <jon-hunter@ti.com>

Currently, the dmtimer determines whether an timer can support an external
clock source (sys_altclk) for driving the timer by the IP version. Only
OMAP24xx devices can support an external clock source, but the IP version
between OMAP24xx and OMAP3xxx is common and so this incorrectly indicates
that OMAP3 devices can use an external clock source.

Rather than use the IP version, use the OMAP_TIMER_HAS_ALTCLK flag added
to the HWMOD timer device attributes. By doing this, this allows us to
eliminate the "timer_ip_version" variable passed as part of the platform data.

We can also remove the timer IP version from the HWMOD data because the dmtimer
driver uses the TIDR register to determine the IP version.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |    1 -
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |    2 --
 arch/arm/mach-omap2/timer.c                        |    3 +--
 arch/arm/plat-omap/include/plat/dmtimer.h          |    7 -------
 4 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
index 4ecb2e8..de8442c 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
@@ -68,7 +68,6 @@ static struct omap_hwmod_class_sysconfig omap2xxx_timer_sysc = {
 struct omap_hwmod_class omap2xxx_timer_hwmod_class = {
 	.name	= "timer",
 	.sysc	= &omap2xxx_timer_sysc,
-	.rev	= OMAP_TIMER_IP_VERSION_1,
 };
 
 /*
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 7b33094..0ea53bc 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -129,7 +129,6 @@ static struct omap_hwmod_class_sysconfig omap3xxx_timer_1ms_sysc = {
 static struct omap_hwmod_class omap3xxx_timer_1ms_hwmod_class = {
 	.name = "timer",
 	.sysc = &omap3xxx_timer_1ms_sysc,
-	.rev = OMAP_TIMER_IP_VERSION_1,
 };
 
 static struct omap_hwmod_class_sysconfig omap3xxx_timer_sysc = {
@@ -145,7 +144,6 @@ static struct omap_hwmod_class_sysconfig omap3xxx_timer_sysc = {
 static struct omap_hwmod_class omap3xxx_timer_hwmod_class = {
 	.name = "timer",
 	.sysc = &omap3xxx_timer_sysc,
-	.rev =  OMAP_TIMER_IP_VERSION_1,
 };
 
 /* secure timers dev attribute */
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index d8a5dc3..cfc5781 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -418,7 +418,7 @@ static int omap2_dm_timer_set_src(struct platform_device *pdev, int source)
 		break;
 
 	case OMAP_TIMER_SRC_EXT_CLK:
-		if (pdata->timer_ip_version == OMAP_TIMER_IP_VERSION_1) {
+		if (pdata->timer_capability & OMAP_TIMER_HAS_ALTCLK) {
 			parent_name = "alt_ck";
 			break;
 		}
@@ -498,7 +498,6 @@ static int __init omap_timer_init(struct omap_hwmod *oh, void *unused)
 	sscanf(oh->name, "timer%2d", &id);
 
 	pdata->set_timer_src = omap2_dm_timer_set_src;
-	pdata->timer_ip_version = oh->class->rev;
 
 	if (timer_dev_attr)
 		pdata->timer_capability = timer_dev_attr->timer_capability;
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 2ce1e4a..2c5c155 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -55,12 +55,6 @@
 #define OMAP_TIMER_TRIGGER_OVERFLOW		0x01
 #define OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE	0x02
 
-/*
- * IP revision identifier so that Highlander IP
- * in OMAP4 can be distinguished.
- */
-#define OMAP_TIMER_IP_VERSION_1                        0x1
-
 /* timer capabilities used in hwmod database */
 #define OMAP_TIMER_SECURE				0x80000000
 #define OMAP_TIMER_ALWON				0x40000000
@@ -97,7 +91,6 @@ struct timer_regs {
 
 struct dmtimer_platform_data {
 	int (*set_timer_src)(struct platform_device *pdev, int source);
-	int timer_ip_version;
 	u32 needs_manual_reset:1;
 	bool loses_context;
 
-- 
1.7.5.4

Re: [PATCH 6/9] ARM: OMAP2+: Fix external clock support for dmtimers

[Cousson, Benoit]

Hi Jon,

On 5/16/2012 1:35 AM, Jon Hunter wrote:
> From: Jon Hunter<jon-hunter@ti.com>
>
> Currently, the dmtimer determines whether an timer can support an external
> clock source (sys_altclk) for driving the timer by the IP version. Only
> OMAP24xx devices can support an external clock source, but the IP version
> between OMAP24xx and OMAP3xxx is common and so this incorrectly indicates
> that OMAP3 devices can use an external clock source.
>
> Rather than use the IP version, use the OMAP_TIMER_HAS_ALTCLK flag added
> to the HWMOD timer device attributes. By doing this, this allows us to
> eliminate the "timer_ip_version" variable passed as part of the platform data.

I do not think this is the right way to handle that. The timer IP itself 
does have only one input clock.
This is the mux before that clock that will have several inputs 
depending on the SoC revision.
So this is purely PRCM stuff and has nothing to do with the timer IP itself.

The OMAP_TIMER_HAS_ALTCLK is thus not a timer IP information and cannot 
be stored inside timer hwmod.

In fact, if the alt clock is there the "alt_clk" alias will be there and 
thus you can use the clk_get(dev, "alt_clk") to figure out if the clock 
is there or not.


Regards,
Benoit


> We can also remove the timer IP version from the HWMOD data because the dmtimer
> driver uses the TIDR register to determine the IP version.
>
> Signed-off-by: Jon Hunter<jon-hunter@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |    1 -
>   arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |    2 --
>   arch/arm/mach-omap2/timer.c                        |    3 +--
>   arch/arm/plat-omap/include/plat/dmtimer.h          |    7 -------
>   4 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
> index 4ecb2e8..de8442c 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
> @@ -68,7 +68,6 @@ static struct omap_hwmod_class_sysconfig omap2xxx_timer_sysc = {
>   struct omap_hwmod_class omap2xxx_timer_hwmod_class = {
>   	.name	= "timer",
>   	.sysc	=&omap2xxx_timer_sysc,
> -	.rev	= OMAP_TIMER_IP_VERSION_1,
>   };
>
>   /*
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> index 7b33094..0ea53bc 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -129,7 +129,6 @@ static struct omap_hwmod_class_sysconfig omap3xxx_timer_1ms_sysc = {
>   static struct omap_hwmod_class omap3xxx_timer_1ms_hwmod_class = {
>   	.name = "timer",
>   	.sysc =&omap3xxx_timer_1ms_sysc,
> -	.rev = OMAP_TIMER_IP_VERSION_1,
>   };
>
>   static struct omap_hwmod_class_sysconfig omap3xxx_timer_sysc = {
> @@ -145,7 +144,6 @@ static struct omap_hwmod_class_sysconfig omap3xxx_timer_sysc = {
>   static struct omap_hwmod_class omap3xxx_timer_hwmod_class = {
>   	.name = "timer",
>   	.sysc =&omap3xxx_timer_sysc,
> -	.rev =  OMAP_TIMER_IP_VERSION_1,
>   };
>
>   /* secure timers dev attribute */
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index d8a5dc3..cfc5781 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -418,7 +418,7 @@ static int omap2_dm_timer_set_src(struct platform_device *pdev, int source)
>   		break;
>
>   	case OMAP_TIMER_SRC_EXT_CLK:
> -		if (pdata->timer_ip_version == OMAP_TIMER_IP_VERSION_1) {
> +		if (pdata->timer_capability&  OMAP_TIMER_HAS_ALTCLK) {
>   			parent_name = "alt_ck";
>   			break;
>   		}
> @@ -498,7 +498,6 @@ static int __init omap_timer_init(struct omap_hwmod *oh, void *unused)
>   	sscanf(oh->name, "timer%2d",&id);
>
>   	pdata->set_timer_src = omap2_dm_timer_set_src;
> -	pdata->timer_ip_version = oh->class->rev;
>
>   	if (timer_dev_attr)
>   		pdata->timer_capability = timer_dev_attr->timer_capability;
> diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
> index 2ce1e4a..2c5c155 100644
> --- a/arch/arm/plat-omap/include/plat/dmtimer.h
> +++ b/arch/arm/plat-omap/include/plat/dmtimer.h
> @@ -55,12 +55,6 @@
>   #define OMAP_TIMER_TRIGGER_OVERFLOW		0x01
>   #define OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE	0x02
>
> -/*
> - * IP revision identifier so that Highlander IP
> - * in OMAP4 can be distinguished.
> - */
> -#define OMAP_TIMER_IP_VERSION_1                        0x1
> -
>   /* timer capabilities used in hwmod database */
>   #define OMAP_TIMER_SECURE				0x80000000
>   #define OMAP_TIMER_ALWON				0x40000000
> @@ -97,7 +91,6 @@ struct timer_regs {
>
>   struct dmtimer_platform_data {
>   	int (*set_timer_src)(struct platform_device *pdev, int source);
> -	int timer_ip_version;
>   	u32 needs_manual_reset:1;
>   	bool loses_context;
>

Re: [PATCH 6/9] ARM: OMAP2+: Fix external clock support for dmtimers

[Jon Hunter]

Hi Benoit,

On 05/21/2012 11:58 AM, Cousson, Benoit wrote:
> Hi Jon,
> 
> On 5/16/2012 1:35 AM, Jon Hunter wrote:
>> From: Jon Hunter<jon-hunter@ti.com>
>>
>> Currently, the dmtimer determines whether an timer can support an
>> external
>> clock source (sys_altclk) for driving the timer by the IP version. Only
>> OMAP24xx devices can support an external clock source, but the IP version
>> between OMAP24xx and OMAP3xxx is common and so this incorrectly indicates
>> that OMAP3 devices can use an external clock source.
>>
>> Rather than use the IP version, use the OMAP_TIMER_HAS_ALTCLK flag added
>> to the HWMOD timer device attributes. By doing this, this allows us to
>> eliminate the "timer_ip_version" variable passed as part of the
>> platform data.
> 
> I do not think this is the right way to handle that. The timer IP itself
> does have only one input clock.
> This is the mux before that clock that will have several inputs
> depending on the SoC revision.
> So this is purely PRCM stuff and has nothing to do with the timer IP
> itself.
> 
> The OMAP_TIMER_HAS_ALTCLK is thus not a timer IP information and cannot
> be stored inside timer hwmod.

Ok, understood.

> In fact, if the alt clock is there the "alt_clk" alias will be there and
> thus you can use the clk_get(dev, "alt_clk") to figure out if the clock
> is there or not.

Ok, I can do this and did think about it, but then wondered why it had
been done this way in the first place? However, I prefer this approach
too as it simplifies the code :-)

So I modify how this is handled.

Cheers
Jon

Re: [PATCH 6/9] ARM: OMAP2+: Fix external clock support for dmtimers

[Cousson, Benoit]

On 5/22/2012 5:04 PM, Jon Hunter wrote:

...

>> In fact, if the alt clock is there the "alt_clk" alias will be there and
>> thus you can use the clk_get(dev, "alt_clk") to figure out if the clock
>> is there or not.
>
> Ok, I can do this and did think about it, but then wondered why it had
> been done this way in the first place?

Yeah, I don't know but I guess this is probably some OMAP1 legacy stuff...

Benoit