[U-Boot] [PATCH v2] davinci timer.c: Remove volatiles and memory mapped structures

[U-Boot] [PATCH v2] davinci timer.c: Remove volatiles and memory mapped structures

[Nick Thompson]

Remove volatiles and memory mapped structure accesses and replace with
readl and writel macro usage.

Signed-off-by: Nick Thompson <nick.thompson@gefanuc.com>
---
This patch was originally part of the da830 support patch, but this
effort is now being integrated into davinci. As a result, these
changes would be have been lost, as no change is required for da830.
The changes where request to be kept available however, so here they
are.

 cpu/arm926ejs/davinci/timer.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/cpu/arm926ejs/davinci/timer.c b/cpu/arm926ejs/davinci/timer.c
index 80751ad..f705a6e 100644
--- a/cpu/arm926ejs/davinci/timer.c
+++ b/cpu/arm926ejs/davinci/timer.c
@@ -38,8 +38,9 @@
  */
 
 #include <common.h>
+#include <asm/io.h>
 
-typedef volatile struct {
+typedef struct {
 	u_int32_t	pid12;
 	u_int32_t	emumgt;
 	u_int32_t	na1;
@@ -53,7 +54,7 @@ typedef volatile struct {
 	u_int32_t	wdtcr;
 } davinci_timer;
 
-davinci_timer		*timer = (davinci_timer *)CONFIG_SYS_TIMERBASE;
+static davinci_timer * const timer = (davinci_timer *)CONFIG_SYS_TIMERBASE;
 
 #define TIMER_LOAD_VAL	(CONFIG_SYS_HZ_CLOCK / CONFIG_SYS_HZ)
 #define TIM_CLK_DIV	16
@@ -64,30 +65,30 @@ static ulong lastinc;
 int timer_init(void)
 {
 	/* We are using timer34 in unchained 32-bit mode, full speed */
-	timer->tcr = 0x0;
-	timer->tgcr = 0x0;
-	timer->tgcr = 0x06 | ((TIM_CLK_DIV - 1) << 8);
-	timer->tim34 = 0x0;
-	timer->prd34 = TIMER_LOAD_VAL;
+	writel(0x0, &timer->tcr);
+	writel(0x0, &timer->tgcr);
+	writel(0x06 | ((TIM_CLK_DIV - 1) << 8), &timer->tgcr);
+	writel(0x0, &timer->tim34);
+	writel(TIMER_LOAD_VAL, &timer->prd34);
 	lastinc = 0;
 	timestamp = 0;
-	timer->tcr = 2 << 22;
+	writel(2 << 22, &timer->tcr);
 
 	return(0);
 }
 
 void reset_timer(void)
 {
-	timer->tcr = 0x0;
-	timer->tim34 = 0;
+	writel(0x0, &timer->tcr);
+	writel(0x0, &timer->tim34);
 	lastinc = 0;
 	timestamp = 0;
-	timer->tcr = 2 << 22;
+	writel(2 << 22, &timer->tcr);
 }
 
 static ulong get_timer_raw(void)
 {
-	ulong now = timer->tim34;
+	ulong now = readl(&timer->tim34);
 
 	if (now >= lastinc) {
 		/* normal mode */

[U-Boot] [PATCH v2] davinci timer.c: Remove volatiles and memory mapped structures

[Paulraj, Sandeep]

> 
> Remove volatiles and memory mapped structure accesses and replace with
> readl and writel macro usage.
> 
> Signed-off-by: Nick Thompson <nick.thompson@gefanuc.com>
> ---
> This patch was originally part of the da830 support patch, but this
> effort is now being integrated into davinci. As a result, these
> changes would be have been lost, as no change is required for da830.
> The changes where request to be kept available however, so here they
> are.
> 
>  cpu/arm926ejs/davinci/timer.c |   29 +++++++++++++++++------------
>  1 files changed, 17 insertions(+), 12 deletions(-)

I started the process of integrating into my branch and doing some quick tests on other DM parts.
But, I got 2 checkpatch warnings while running checkpatch.
One of them I believe is unavoidable because U-boot does not have a linux/io.h

The other deals with using a typedef. 


> 
> diff --git a/cpu/arm926ejs/davinci/timer.c b/cpu/arm926ejs/davinci/timer.c
> index 80751ad..f705a6e 100644
> --- a/cpu/arm926ejs/davinci/timer.c
> +++ b/cpu/arm926ejs/davinci/timer.c
> @@ -38,8 +38,9 @@
>   */
> 
>  #include <common.h>
> +#include <asm/io.h>
> 
> -typedef volatile struct {
> +typedef struct {
>  	u_int32_t	pid12;
>  	u_int32_t	emumgt;
>  	u_int32_t	na1;
> @@ -53,7 +54,7 @@ typedef volatile struct {
>  	u_int32_t	wdtcr;
>  } davinci_timer;
> 
> -davinci_timer		*timer = (davinci_timer *)CONFIG_SYS_TIMERBASE;
> +static davinci_timer * const timer = (davinci_timer
> *)CONFIG_SYS_TIMERBASE;
> 
>  #define TIMER_LOAD_VAL	(CONFIG_SYS_HZ_CLOCK / CONFIG_SYS_HZ)
>  #define TIM_CLK_DIV	16
> @@ -64,30 +65,30 @@ static ulong lastinc;
>  int timer_init(void)
>  {
>  	/* We are using timer34 in unchained 32-bit mode, full speed */
> -	timer->tcr = 0x0;
> -	timer->tgcr = 0x0;
> -	timer->tgcr = 0x06 | ((TIM_CLK_DIV - 1) << 8);
> -	timer->tim34 = 0x0;
> -	timer->prd34 = TIMER_LOAD_VAL;
> +	writel(0x0, &timer->tcr);
> +	writel(0x0, &timer->tgcr);
> +	writel(0x06 | ((TIM_CLK_DIV - 1) << 8), &timer->tgcr);
> +	writel(0x0, &timer->tim34);
> +	writel(TIMER_LOAD_VAL, &timer->prd34);
>  	lastinc = 0;
>  	timestamp = 0;
> -	timer->tcr = 2 << 22;
> +	writel(2 << 22, &timer->tcr);
> 
>  	return(0);
>  }
> 
>  void reset_timer(void)
>  {
> -	timer->tcr = 0x0;
> -	timer->tim34 = 0;
> +	writel(0x0, &timer->tcr);
> +	writel(0x0, &timer->tim34);
>  	lastinc = 0;
>  	timestamp = 0;
> -	timer->tcr = 2 << 22;
> +	writel(2 << 22, &timer->tcr);
>  }
> 
>  static ulong get_timer_raw(void)
>  {
> -	ulong now = timer->tim34;
> +	ulong now = readl(&timer->tim34);
> 
>  	if (now >= lastinc) {
>  		/* normal mode */

[U-Boot] [PATCH v2] davinci timer.c: Remove volatiles and memory mapped structures

[Nick Thompson]

Paulraj, Sandeep wrote:
> 
>> Remove volatiles and memory mapped structure accesses and replace with
>> readl and writel macro usage.
>>
>> Signed-off-by: Nick Thompson <nick.thompson@gefanuc.com>
>> ---
>> This patch was originally part of the da830 support patch, but this
>> effort is now being integrated into davinci. As a result, these
>> changes would be have been lost, as no change is required for da830.
>> The changes where request to be kept available however, so here they
>> are.
>>
>>  cpu/arm926ejs/davinci/timer.c |   29 +++++++++++++++++------------
>>  1 files changed, 17 insertions(+), 12 deletions(-)
> 
> I started the process of integrating into my branch and doing some quick tests on other DM parts.
> But, I got 2 checkpatch warnings while running checkpatch.
> One of them I believe is unavoidable because U-boot does not have a linux/io.h
> 
> The other deals with using a typedef. 

I only get one warning with 0.09 of checkpatch. It warns about not adding new typedefs.
The typedef is already there though - I only removed the volatile. No excuse really.

In this case I expect it is fine to just to use a struct instead. Do you want me
to make that change?

> 
> 
>> diff --git a/cpu/arm926ejs/davinci/timer.c b/cpu/arm926ejs/davinci/timer.c
>> index 80751ad..f705a6e 100644
>> --- a/cpu/arm926ejs/davinci/timer.c
>> +++ b/cpu/arm926ejs/davinci/timer.c
>> @@ -38,8 +38,9 @@
>>   */
>>
>>  #include <common.h>
>> +#include <asm/io.h>
>>
>> -typedef volatile struct {
>> +typedef struct {
>>  	u_int32_t	pid12;
>>  	u_int32_t	emumgt;
>>  	u_int32_t	na1;
>> @@ -53,7 +54,7 @@ typedef volatile struct {
>>  	u_int32_t	wdtcr;
>>  } davinci_timer;
>>
>> -davinci_timer		*timer = (davinci_timer *)CONFIG_SYS_TIMERBASE;
>> +static davinci_timer * const timer = (davinci_timer
>> *)CONFIG_SYS_TIMERBASE;
>>
>>  #define TIMER_LOAD_VAL	(CONFIG_SYS_HZ_CLOCK / CONFIG_SYS_HZ)
>>  #define TIM_CLK_DIV	16
>> @@ -64,30 +65,30 @@ static ulong lastinc;
>>  int timer_init(void)
>>  {
>>  	/* We are using timer34 in unchained 32-bit mode, full speed */
>> -	timer->tcr = 0x0;
>> -	timer->tgcr = 0x0;
>> -	timer->tgcr = 0x06 | ((TIM_CLK_DIV - 1) << 8);
>> -	timer->tim34 = 0x0;
>> -	timer->prd34 = TIMER_LOAD_VAL;
>> +	writel(0x0, &timer->tcr);
>> +	writel(0x0, &timer->tgcr);
>> +	writel(0x06 | ((TIM_CLK_DIV - 1) << 8), &timer->tgcr);
>> +	writel(0x0, &timer->tim34);
>> +	writel(TIMER_LOAD_VAL, &timer->prd34);
>>  	lastinc = 0;
>>  	timestamp = 0;
>> -	timer->tcr = 2 << 22;
>> +	writel(2 << 22, &timer->tcr);
>>
>>  	return(0);
>>  }
>>
>>  void reset_timer(void)
>>  {
>> -	timer->tcr = 0x0;
>> -	timer->tim34 = 0;
>> +	writel(0x0, &timer->tcr);
>> +	writel(0x0, &timer->tim34);
>>  	lastinc = 0;
>>  	timestamp = 0;
>> -	timer->tcr = 2 << 22;
>> +	writel(2 << 22, &timer->tcr);
>>  }
>>
>>  static ulong get_timer_raw(void)
>>  {
>> -	ulong now = timer->tim34;
>> +	ulong now = readl(&timer->tim34);
>>
>>  	if (now >= lastinc) {
>>  		/* normal mode */
> 

[U-Boot] [PATCH v2] davinci timer.c: Remove volatiles and memory mapped structures

[Paulraj, Sandeep]

> 
> Paulraj, Sandeep wrote:
> >
> >> Remove volatiles and memory mapped structure accesses and replace with
> >> readl and writel macro usage.
> >>
> >> Signed-off-by: Nick Thompson <nick.thompson@gefanuc.com>
> >> ---
> >> This patch was originally part of the da830 support patch, but this
> >> effort is now being integrated into davinci. As a result, these
> >> changes would be have been lost, as no change is required for da830.
> >> The changes where request to be kept available however, so here they
> >> are.
> >>
> >>  cpu/arm926ejs/davinci/timer.c |   29 +++++++++++++++++------------
> >>  1 files changed, 17 insertions(+), 12 deletions(-)
> >
> > I started the process of integrating into my branch and doing some quick
> tests on other DM parts.
> > But, I got 2 checkpatch warnings while running checkpatch.
> > One of them I believe is unavoidable because U-boot does not have a
> linux/io.h
> >
> > The other deals with using a typedef.
> 
> I only get one warning with 0.09 of checkpatch. It warns about not adding
> new typedefs.
> The typedef is already there though - I only removed the volatile. No
> excuse really.
I know and I understand this is precisely what gets on the nerves of everybody who submits updates to existing pieces of code. They end up cleaning stuff which they did not introduce.
> 
> In this case I expect it is fine to just to use a struct instead. Do you
> want me
> to make that change?
It's a simple change. Yes please make the change and then I can get on with the process of applying the patch.

Thanks,
Sandeep