linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: TWD: fix the clock calculation for TWD
@ 2011-01-20  6:33 Chao Xie
  2011-01-20 14:08 ` Russell King - ARM Linux
  0 siblings, 1 reply; 6+ messages in thread
From: Chao Xie @ 2011-01-20  6:33 UTC (permalink / raw)
  To: linux-arm-kernel

The original code will do count * (HZ /5). It will make the twd
timer rate decreased if HZ can not be excatly divided. For
example HZ=128.

Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
 arch/arm/kernel/smp_twd.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 35882fb..ca6405b 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -111,7 +111,8 @@ static void __cpuinit twd_calibrate_rate(void)
 
 		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
 
-		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
+		twd_timer_rate = ((unsigned long)(0xFFFFFFFFU - count))
+			* HZ / 5;
 
 		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
 			(twd_timer_rate / 100000) % 100);
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] ARM: TWD: fix the clock calculation for TWD
  2011-01-20  6:33 [PATCH] ARM: TWD: fix the clock calculation for TWD Chao Xie
@ 2011-01-20 14:08 ` Russell King - ARM Linux
  2011-01-20 14:44   ` Jamie Iles
  0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2011-01-20 14:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 20, 2011 at 02:33:57PM +0800, Chao Xie wrote:
> The original code will do count * (HZ /5). It will make the twd
> timer rate decreased if HZ can not be excatly divided. For
> example HZ=128.
> 
> Signed-off-by: Chao Xie <chao.xie@marvell.com>
> ---
>  arch/arm/kernel/smp_twd.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index 35882fb..ca6405b 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -111,7 +111,8 @@ static void __cpuinit twd_calibrate_rate(void)
>  
>  		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
>  
> -		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
> +		twd_timer_rate = ((unsigned long)(0xFFFFFFFFU - count))
> +			* HZ / 5;
>  
>  		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
>  			(twd_timer_rate / 100000) % 100);

I don't think this patch has any effect what so ever, so I just tried:

#define HZ      128
unsigned long twd_timer_rate;
void calc1(unsigned long count)
{
        twd_timer_rate = (0xffffffffU - count) * (HZ / 5);
}
void calc2(unsigned long count)
{
        twd_timer_rate = ((unsigned long)(0xffffffffU - count)) * (HZ / 5);
}

calc1:
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	@ link register save eliminated.
	mvn	r0, r0
	add	r0, r0, r0, asl #2
	ldr	r3, .L2
	add	r0, r0, r0, asl #2
	@ lr needed for prologue
	str	r0, [r3, #0]
	mov	pc, lr
calc2:
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	@ link register save eliminated.
	mvn	r0, r0
	add	r0, r0, r0, asl #2
	ldr	r3, .L5
	add	r0, r0, r0, asl #2
	@ lr needed for prologue
	str	r0, [r3, #0]
	mov	pc, lr

The code looks identical to me, so the patch appears to have no effect.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: TWD: fix the clock calculation for TWD
  2011-01-20 14:08 ` Russell King - ARM Linux
@ 2011-01-20 14:44   ` Jamie Iles
  2011-01-20 16:38     ` Russell King - ARM Linux
  0 siblings, 1 reply; 6+ messages in thread
From: Jamie Iles @ 2011-01-20 14:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 20, 2011 at 02:08:11PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 20, 2011 at 02:33:57PM +0800, Chao Xie wrote:
> > The original code will do count * (HZ /5). It will make the twd
> > timer rate decreased if HZ can not be excatly divided. For
> > example HZ=128.
> > 
> > Signed-off-by: Chao Xie <chao.xie@marvell.com>
> > ---
> >  arch/arm/kernel/smp_twd.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> > index 35882fb..ca6405b 100644
> > --- a/arch/arm/kernel/smp_twd.c
> > +++ b/arch/arm/kernel/smp_twd.c
> > @@ -111,7 +111,8 @@ static void __cpuinit twd_calibrate_rate(void)
> >  
> >  		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
> >  
> > -		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
> > +		twd_timer_rate = ((unsigned long)(0xFFFFFFFFU - count))
> > +			* HZ / 5;
> >  
> >  		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
> >  			(twd_timer_rate / 100000) % 100);
> 
> I don't think this patch has any effect what so ever, so I just tried:
> 
> #define HZ      128
> unsigned long twd_timer_rate;
> void calc1(unsigned long count)
> {
>         twd_timer_rate = (0xffffffffU - count) * (HZ / 5);
> }
> void calc2(unsigned long count)
> {
>         twd_timer_rate = ((unsigned long)(0xffffffffU - count)) * (HZ / 5);
> }

Hi Russell,

In Chao's patch the parentheses around (HZ / 5) have been removed so its 
now dowing the multiplication before the division (if I remember the 
precedences correctly!).  I don't think the cast to unsigned long is 
needed though.

Jamie

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: TWD: fix the clock calculation for TWD
  2011-01-20 14:44   ` Jamie Iles
@ 2011-01-20 16:38     ` Russell King - ARM Linux
  2011-01-20 18:10       ` Jamie Iles
  0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2011-01-20 16:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 20, 2011 at 02:44:09PM +0000, Jamie Iles wrote:
> On Thu, Jan 20, 2011 at 02:08:11PM +0000, Russell King - ARM Linux wrote:
> > On Thu, Jan 20, 2011 at 02:33:57PM +0800, Chao Xie wrote:
> > > The original code will do count * (HZ /5). It will make the twd
> > > timer rate decreased if HZ can not be excatly divided. For
> > > example HZ=128.
> > > 
> > > Signed-off-by: Chao Xie <chao.xie@marvell.com>
> > > ---
> > >  arch/arm/kernel/smp_twd.c |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> > > index 35882fb..ca6405b 100644
> > > --- a/arch/arm/kernel/smp_twd.c
> > > +++ b/arch/arm/kernel/smp_twd.c
> > > @@ -111,7 +111,8 @@ static void __cpuinit twd_calibrate_rate(void)
> > >  
> > >  		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
> > >  
> > > -		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
> > > +		twd_timer_rate = ((unsigned long)(0xFFFFFFFFU - count))
> > > +			* HZ / 5;
> > >  
> > >  		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
> > >  			(twd_timer_rate / 100000) % 100);
> > 
> > I don't think this patch has any effect what so ever, so I just tried:
> > 
> > #define HZ      128
> > unsigned long twd_timer_rate;
> > void calc1(unsigned long count)
> > {
> >         twd_timer_rate = (0xffffffffU - count) * (HZ / 5);
> > }
> > void calc2(unsigned long count)
> > {
> >         twd_timer_rate = ((unsigned long)(0xffffffffU - count)) * (HZ / 5);
> > }
> 
> Hi Russell,
> 
> In Chao's patch the parentheses around (HZ / 5) have been removed so its 
> now dowing the multiplication before the division (if I remember the 
> precedences correctly!).  I don't think the cast to unsigned long is 
> needed though.

Hmm.  That means the maximum twd timer rate we can support is about
860MHz.  Is that enough?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: TWD: fix the clock calculation for TWD
  2011-01-20 16:38     ` Russell King - ARM Linux
@ 2011-01-20 18:10       ` Jamie Iles
  2011-01-24  1:35         ` Chao Xie
  0 siblings, 1 reply; 6+ messages in thread
From: Jamie Iles @ 2011-01-20 18:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 20, 2011 at 04:38:36PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 20, 2011 at 02:44:09PM +0000, Jamie Iles wrote:
> > On Thu, Jan 20, 2011 at 02:08:11PM +0000, Russell King - ARM Linux wrote:
> > > On Thu, Jan 20, 2011 at 02:33:57PM +0800, Chao Xie wrote:
> > > > The original code will do count * (HZ /5). It will make the twd
> > > > timer rate decreased if HZ can not be excatly divided. For
> > > > example HZ=128.
> > > > 
> > > > Signed-off-by: Chao Xie <chao.xie@marvell.com>
> > > > ---
> > > >  arch/arm/kernel/smp_twd.c |    3 ++-
> > > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> > > > index 35882fb..ca6405b 100644
> > > > --- a/arch/arm/kernel/smp_twd.c
> > > > +++ b/arch/arm/kernel/smp_twd.c
> > > > @@ -111,7 +111,8 @@ static void __cpuinit twd_calibrate_rate(void)
> > > >  
> > > >  		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
> > > >  
> > > > -		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
> > > > +		twd_timer_rate = ((unsigned long)(0xFFFFFFFFU - count))
> > > > +			* HZ / 5;
> > > >  
> > > >  		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
> > > >  			(twd_timer_rate / 100000) % 100);
> > > 
> > > I don't think this patch has any effect what so ever, so I just tried:
> > > 
> > > #define HZ      128
> > > unsigned long twd_timer_rate;
> > > void calc1(unsigned long count)
> > > {
> > >         twd_timer_rate = (0xffffffffU - count) * (HZ / 5);
> > > }
> > > void calc2(unsigned long count)
> > > {
> > >         twd_timer_rate = ((unsigned long)(0xffffffffU - count)) * (HZ / 5);
> > > }
> > 
> > Hi Russell,
> > 
> > In Chao's patch the parentheses around (HZ / 5) have been removed so its 
> > now dowing the multiplication before the division (if I remember the 
> > precedences correctly!).  I don't think the cast to unsigned long is 
> > needed though.
> 
> Hmm.  That means the maximum twd timer rate we can support is about
> 860MHz.  Is that enough?

I'm not familiar with any ARM SMP systems so I wouldn't like to commit 
to that!  If the answer is no, then I guess could we do something like 
the (untested in a kernel) patch below to get a few more bits and we 
should then be able to support rates of up to ~4294MHz.

Jamie

8<----

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index fd91566..334ad95 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -89,6 +89,8 @@ static void __cpuinit twd_calibrate_rate(void)
 	 * the timer ticks
 	 */
 	if (twd_timer_rate == 0) {
+		u64 rate64;
+
 		printk(KERN_INFO "Calibrating local timer... ");
 
 		/* Wait for a tick to start */
@@ -111,7 +113,9 @@ static void __cpuinit twd_calibrate_rate(void)
 
 		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
 
-		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
+		rate64 = (0xFFFFFFFFLLU - count) * (u64)HZ;
+		do_div(rate64, 5);
+		twd_timer_rate = rate64;
 
 		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
 			(twd_timer_rate / 1000000) % 100);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] ARM: TWD: fix the clock calculation for TWD
  2011-01-20 18:10       ` Jamie Iles
@ 2011-01-24  1:35         ` Chao Xie
  0 siblings, 0 replies; 6+ messages in thread
From: Chao Xie @ 2011-01-24  1:35 UTC (permalink / raw)
  To: linux-arm-kernel

Your patch is better. 64bit is enough, I think.

-----Original Message-----
From: Jamie Iles [mailto:jamie at jamieiles.com] 
Sent: Friday, January 21, 2011 2:11 AM
To: Russell King - ARM Linux
Cc: Jamie Iles; Chao Xie; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] ARM: TWD: fix the clock calculation for TWD

On Thu, Jan 20, 2011 at 04:38:36PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 20, 2011 at 02:44:09PM +0000, Jamie Iles wrote:
> > On Thu, Jan 20, 2011 at 02:08:11PM +0000, Russell King - ARM Linux wrote:
> > > On Thu, Jan 20, 2011 at 02:33:57PM +0800, Chao Xie wrote:
> > > > The original code will do count * (HZ /5). It will make the twd
> > > > timer rate decreased if HZ can not be excatly divided. For
> > > > example HZ=128.
> > > > 
> > > > Signed-off-by: Chao Xie <chao.xie@marvell.com>
> > > > ---
> > > >  arch/arm/kernel/smp_twd.c |    3 ++-
> > > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> > > > index 35882fb..ca6405b 100644
> > > > --- a/arch/arm/kernel/smp_twd.c
> > > > +++ b/arch/arm/kernel/smp_twd.c
> > > > @@ -111,7 +111,8 @@ static void __cpuinit twd_calibrate_rate(void)
> > > >  
> > > >  		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
> > > >  
> > > > -		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
> > > > +		twd_timer_rate = ((unsigned long)(0xFFFFFFFFU - count))
> > > > +			* HZ / 5;
> > > >  
> > > >  		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
> > > >  			(twd_timer_rate / 100000) % 100);
> > > 
> > > I don't think this patch has any effect what so ever, so I just tried:
> > > 
> > > #define HZ      128
> > > unsigned long twd_timer_rate;
> > > void calc1(unsigned long count)
> > > {
> > >         twd_timer_rate = (0xffffffffU - count) * (HZ / 5);
> > > }
> > > void calc2(unsigned long count)
> > > {
> > >         twd_timer_rate = ((unsigned long)(0xffffffffU - count)) * (HZ / 5);
> > > }
> > 
> > Hi Russell,
> > 
> > In Chao's patch the parentheses around (HZ / 5) have been removed so its 
> > now dowing the multiplication before the division (if I remember the 
> > precedences correctly!).  I don't think the cast to unsigned long is 
> > needed though.
> 
> Hmm.  That means the maximum twd timer rate we can support is about
> 860MHz.  Is that enough?

I'm not familiar with any ARM SMP systems so I wouldn't like to commit 
to that!  If the answer is no, then I guess could we do something like 
the (untested in a kernel) patch below to get a few more bits and we 
should then be able to support rates of up to ~4294MHz.

Jamie

8<----

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index fd91566..334ad95 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -89,6 +89,8 @@ static void __cpuinit twd_calibrate_rate(void)
 	 * the timer ticks
 	 */
 	if (twd_timer_rate == 0) {
+		u64 rate64;
+
 		printk(KERN_INFO "Calibrating local timer... ");
 
 		/* Wait for a tick to start */
@@ -111,7 +113,9 @@ static void __cpuinit twd_calibrate_rate(void)
 
 		count = __raw_readl(twd_base + TWD_TIMER_COUNTER);
 
-		twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5);
+		rate64 = (0xFFFFFFFFLLU - count) * (u64)HZ;
+		do_div(rate64, 5);
+		twd_timer_rate = rate64;
 
 		printk("%lu.%02luMHz.\n", twd_timer_rate / 1000000,
 			(twd_timer_rate / 1000000) % 100);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-01-24  1:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-20  6:33 [PATCH] ARM: TWD: fix the clock calculation for TWD Chao Xie
2011-01-20 14:08 ` Russell King - ARM Linux
2011-01-20 14:44   ` Jamie Iles
2011-01-20 16:38     ` Russell King - ARM Linux
2011-01-20 18:10       ` Jamie Iles
2011-01-24  1:35         ` Chao Xie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).