[U-Boot-Users] Resubmit : [PATCH] Correct ARM Versatile Timer Initialization

[U-Boot-Users] Resubmit : [PATCH] Correct ARM Versatile Timer Initialization

[Gururaja Hebbar K R]

- According to ARM Dual-Timer Module (SP804) TRM (ARM DDI0271), 
 -- Timer Value Register @ TIMER Base + 4 is Read-only. 
 -- Prescale Value (Bits 3-2 of TIMER Control register) 
	can only be one of 00,01,10. 11 is undefined.

Signed-off-by: Gururaja Hebbar <gururajakr@sanyo.co.in>
---
 cpu/arm926ejs/versatile/timer.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/cpu/arm926ejs/versatile/timer.c b/cpu/arm926ejs/versatile/timer.c
index 32872d2..9659b67 100644
--- a/cpu/arm926ejs/versatile/timer.c
+++ b/cpu/arm926ejs/versatile/timer.c
@@ -50,8 +50,7 @@ static ulong lastdec;
 int timer_init (void)
 {
 	*(volatile ulong *)(CFG_TIMERBASE + 0) = CFG_TIMER_RELOAD;	/* TimerLoad */
-	*(volatile ulong *)(CFG_TIMERBASE + 4) = CFG_TIMER_RELOAD;	/* TimerValue */
-	*(volatile ulong *)(CFG_TIMERBASE + 8) = 0x8C;
+	*(volatile ulong *)(CFG_TIMERBASE + 8) = 0x80;
 
 	/* init the timestamp and lastdec value */
 	reset_timer_masked();
-- 
1.5.6.4

[U-Boot-Users] Resubmit : [PATCH] Correct ARM Versatile Timer Initialization

[Jean-Christophe PLAGNIOL-VILLARD]

On 06:30 Mon 04 Aug     , Gururaja Hebbar K R wrote:
> - According to ARM Dual-Timer Module (SP804) TRM (ARM DDI0271), 
>  -- Timer Value Register @ TIMER Base + 4 is Read-only. 
>  -- Prescale Value (Bits 3-2 of TIMER Control register) 
> 	can only be one of 00,01,10. 11 is undefined.
> 
> Signed-off-by: Gururaja Hebbar <gururajakr@sanyo.co.in>
> ---
>  cpu/arm926ejs/versatile/timer.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/cpu/arm926ejs/versatile/timer.c b/cpu/arm926ejs/versatile/timer.c
> index 32872d2..9659b67 100644
> --- a/cpu/arm926ejs/versatile/timer.c
> +++ b/cpu/arm926ejs/versatile/timer.c
> @@ -50,8 +50,7 @@ static ulong lastdec;
>  int timer_init (void)
>  {
>  	*(volatile ulong *)(CFG_TIMERBASE + 0) = CFG_TIMER_RELOAD;	/* TimerLoad */
> -	*(volatile ulong *)(CFG_TIMERBASE + 4) = CFG_TIMER_RELOAD;	/* TimerValue */
> -	*(volatile ulong *)(CFG_TIMERBASE + 8) = 0x8C;
> +	*(volatile ulong *)(CFG_TIMERBASE + 8) = 0x80;

according to datasheet for the register TimerXControl

we are supposed to not modify the bits [31:8] and [4]

so we are suppose to read the register and modify only the others
register.

Best Regards,
J.

[U-Boot] Resubmit : [PATCH] Correct ARM Versatile Timer Initialization

[Gururaja Hebbar K R]

 
Hi,

> according to datasheet for the register TimerXControl
> 
> we are supposed to not modify the bits [31:8] and [4]
> 
> so we are suppose to read the register and modify only the 
> others register.

Does this mean i need to resend the earlier patch. Writing 00 to these
bits ( 31:8 & 4) is undefined. 

So i think this will not make any problems. 

anyone could you please update me about the status of this patch. if not
accessible, i will change & resend

Regards
Gururaja

[U-Boot] Resubmit : [PATCH] Correct ARM Versatile Timer Initialization

[Peter Pearse]

 

> -----Original Message-----
> From: Gururaja Hebbar K R [mailto:gururajakr at sanyo.co.in] 
> Sent: 12 August 2008 03:57
> To: Jean-Christophe PLAGNIOL-VILLARD
> Cc: u-boot at lists.denx.de; wd at denx.de; Peter Pearse
> Subject: RE: Resubmit : [PATCH] Correct ARM Versatile Timer 
> Initialization
> 
>  
> Hi,
> 
> > according to datasheet for the register TimerXControl
> > 
> > we are supposed to not modify the bits [31:8] and [4]
> > 
> > so we are suppose to read the register and modify only the others 
> > register.
> 
> Does this mean i need to resend the earlier patch. Writing 00 
> to these bits ( 31:8 & 4) is undefined. 

Generally, in ARM terminology, one should avoid writing values where writing
a value or bit is undefined.
"Undefined" implies, not that there is no result, but that the outcome is
not defined by the specification.
Hence it is good practice to read/change/write registers with undefined
bits, or bits where writing is undefined.
This is especially important with bits defined as such in ARM TRMs since
different ARM customers may implement
the ARM IP in different ways i.e writing a value to such bits may have
different results in different implementations
and or versions of the hardware.

> 
> So i think this will not make any problems. 
> 
> anyone could you please update me about the status of this 
> patch. if not accessible, i will change & resend
> 
> Regards
> Gururaja
> 

[U-Boot] Resubmit : [PATCH] Correct ARM Versatile Timer Initialization

[Jean-Christophe PLAGNIOL-VILLARD]

On 08:23 Tue 12 Aug     , Peter Pearse wrote:
>  
> 
> > -----Original Message-----
> > From: Gururaja Hebbar K R [mailto:gururajakr at sanyo.co.in] 
> > Sent: 12 August 2008 03:57
> > To: Jean-Christophe PLAGNIOL-VILLARD
> > Cc: u-boot at lists.denx.de; wd at denx.de; Peter Pearse
> > Subject: RE: Resubmit : [PATCH] Correct ARM Versatile Timer 
> > Initialization
> > 
> >  
> > Hi,
> > 
> > > according to datasheet for the register TimerXControl
> > > 
> > > we are supposed to not modify the bits [31:8] and [4]
> > > 
> > > so we are suppose to read the register and modify only the others 
> > > register.
     s/register/bits/
> > 
> > Does this mean i need to resend the earlier patch. Writing 00 
> > to these bits ( 31:8 & 4) is undefined. 
> 
> Generally, in ARM terminology, one should avoid writing values where writing
> a value or bit is undefined.

Not only in ARM.

> "Undefined" implies, not that there is no result, but that the outcome is
> not defined by the specification.
> Hence it is good practice to read/change/write registers with undefined
> bits, or bits where writing is undefined.

That exaclty what I mean

> This is especially important with bits defined as such in ARM TRMs since
> different ARM customers may implement
> the ARM IP in different ways i.e writing a value to such bits may have
> different results in different implementations
> and or versions of the hardware.

Thanks Peter for the clarification,

Best Regards,
J.

[U-Boot] Resubmit : [PATCH] Correct ARM Versatile TimerInitialization

[Gururaja Hebbar K R]

Hi< 

> > > Hi,
> > > 
> > > > according to datasheet for the register TimerXControl
> > > > we are supposed to not modify the bits [31:8] and [4]
> > > > so we are suppose to read the register and modify only 
> the others registers/register/bits/
> > > 
> > > Does this mean i need to resend the earlier patch. Writing 00 to 
> > > these bits ( 31:8 & 4) is undefined.
> > 
> > Generally, in ARM terminology, one should avoid writing 
> values where writing a value or bit is undefined.
> 
> Not only in ARM.
> 
> > "Undefined" implies, not that there is no result, but that 
> the outcome is not defined by the specification.
> > Hence it is good practice to read/change/write registers with 
> > undefined bits, or bits where writing is undefined.
> 
> That exaclty what I mean
> 
> > This is especially important with bits defined as such in ARM TRMs 
> > since different ARM customers may implement the ARM IP in different 
> > ways i.e writing a value to such bits may have different results in 
> > different implementations and or versions of the hardware.
> 
> Thanks Peter for the clarification,


I checked the source code & found that the original code itself modifies
the Timer Control Register directly.

Also, after going through the Timer Module i found that,

For free-running Mode of the timer, Writing to Timer_Load register has
no effect.

The TRM Says,

"If the timer is operating in Free-Running Mode, it continues to
decrement from its maximum value"

So i think the writing to timer_load register is also unnecessary in
this case. 

Kindly correct me if i am wrong.

Also Kindly let me know, if i need to change anything in the patch.
Else i resend the patch once again

regards
Gururaja

[U-Boot] Resubmit : [PATCH] Correct ARM Versatile TimerInitialization

[Gururaja Hebbar K R]

Hi,

Today I download u-boot-1.3.4 and checked drivers\rtc directory with u-boot-1.3.3\drivers\rtc .

 There is change in the files that this patch touches. So I think the same patch can be applied to u-boot-1.3.4 also.

 

Kindly update me if any changes is needed

 

Regards

Gururaja