[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

There is no need to use get_timer() and reset_timer() and there are build
breakages occuring because of them (specifically cfi_flash). Remove any
usage outside arch/ to fix build breakages and to prepare for complete
removal

Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
---
checkpatch complains about long lines and brace usage in the board specific
flash.c files - They are deprecated and not worth fixing for style

 board/BuS/EB+MCF-EV123/flash.c |   10 ++++++----
 board/cobra5272/flash.c        |   10 ++++++----
 board/idmr/flash.c             |   10 ++++++----
 drivers/block/mg_disk.c        |    1 -
 drivers/mtd/cfi_flash.c        |    2 --
 5 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/board/BuS/EB+MCF-EV123/flash.c b/board/BuS/EB+MCF-EV123/flash.c
index 3c36367..8b7f957 100644
--- a/board/BuS/EB+MCF-EV123/flash.c
+++ b/board/BuS/EB+MCF-EV123/flash.c
@@ -157,6 +157,7 @@ int amd_flash_erase_sector(flash_info_t * info, int sector)
 {
 	int state;
 	ulong result;
+	ulong start;

 	volatile u16 *addr =
 				(volatile u16 *) (info->start[sector]);
@@ -171,13 +172,13 @@ int amd_flash_erase_sector(flash_info_t * info, int sector)

 	/* wait until flash is ready */
 	state = 0;
-	set_timer (0);
+	start = get_timer(0);

 	do {
 		result = *addr;

 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 			MEM_FLASH_ADDR1 = CMD_READ_ARRAY;
 			state = ERR_TIMOUT;
 		}
@@ -267,6 +268,7 @@ volatile static int amd_write_word (flash_info_t * info, ulong dest, u16 data)
 	ulong result;
 	int cflag, iflag;
 	int state;
+	ulong start;

 	/*
 	 * Check if Flash is (sufficiently) erased
@@ -295,7 +297,7 @@ volatile static int amd_write_word (flash_info_t * info, ulong dest, u16 data)
 	*addr = data;

 	/* arm simple, non interrupt dependent timer */
-	set_timer (0);
+	start = get_timer(0);

 	/* wait until flash is ready */
 	state = 0;
@@ -303,7 +305,7 @@ volatile static int amd_write_word (flash_info_t * info, ulong dest, u16 data)
 		result = *addr;

 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 				state = ERR_TIMOUT;
 		}
 		if (!state && ((result & BIT_RDY_MASK) == (data & BIT_RDY_MASK)))
diff --git a/board/cobra5272/flash.c b/board/cobra5272/flash.c
index 33c9361..e8f02eb 100644
--- a/board/cobra5272/flash.c
+++ b/board/cobra5272/flash.c
@@ -147,6 +147,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 	int iflag, cflag, prot, sect;
 	int rc = ERR_OK;
 	int chip1;
+	ulong start;

 	/* first look for protection bits */

@@ -190,7 +191,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 		printf ("Erasing sector %2d ... ", sect);

 		/* arm simple, non interrupt dependent timer */
-		set_timer (0);
+		start = get_timer(0);

 		if (info->protect[sect] == 0) {	/* not protected */
 			volatile u16 *addr =
@@ -211,7 +212,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				result = *addr;

 				/* check timeout */
-				if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+				if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 					MEM_FLASH_ADDR1 = CMD_READ_ARRAY;
 					chip1 = TMO;
 					break;
@@ -264,6 +265,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	int rc = ERR_OK;
 	int cflag, iflag;
 	int chip1;
+	ulong start;

 	/*
 	 * Check if Flash is (sufficiently) erased
@@ -291,7 +293,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	*addr = data;

 	/* arm simple, non interrupt dependent timer */
-	set_timer (0);
+	start = get_timer(0);

 	/* wait until flash is ready */
 	chip1 = 0;
@@ -299,7 +301,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 		result = *addr;

 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 			chip1 = ERR | TMO;
 			break;
 		}
diff --git a/board/idmr/flash.c b/board/idmr/flash.c
index 57c9948..9f4ff2b 100644
--- a/board/idmr/flash.c
+++ b/board/idmr/flash.c
@@ -130,6 +130,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 	int iflag, prot, sect;
 	int rc = ERR_OK;
 	int chip1;
+	ulong start;

 	/* first look for protection bits */

@@ -170,7 +171,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 		printf ("Erasing sector %2d ... ", sect);

 		/* arm simple, non interrupt dependent timer */
-		set_timer (0);
+		start = get_timer(0);

 		if (info->protect[sect] == 0) {	/* not protected */
 			volatile u16 *addr =
@@ -191,7 +192,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				result = *addr;

 				/* check timeout */
-				if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
+				if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
 					MEM_FLASH_ADDR1 = CMD_READ_ARRAY;
 					chip1 = TMO;
 					break;
@@ -248,6 +249,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	int rc = ERR_OK;
 	int iflag;
 	int chip1;
+	ulong start;

 	/*
 	 * Check if Flash is (sufficiently) erased
@@ -272,7 +274,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	*addr = data;

 	/* arm simple, non interrupt dependent timer */
-	set_timer (0);
+	start = get_timer(0);

 	/* wait until flash is ready */
 	chip1 = 0;
@@ -280,7 +282,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 		result = *addr;

 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
 			chip1 = ERR | TMO;
 			break;
 		}
diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index b74307a..7609557 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -91,7 +91,6 @@ static unsigned int mg_wait (u32 expect, u32 msec)
 	u32 from, cur, err;

 	err = MG_ERR_NONE;
-	reset_timer();
 	from = get_timer(0);

 	status = readb(mg_base() + MG_REG_STATUS);
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 5788328..ea5e1f2 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -573,7 +573,6 @@ static int flash_status_check (flash_info_t * info, flash_sect_t sector,
 #endif

 	/* Wait for command completion */
-	reset_timer();
 	start = get_timer (0);
 	while (flash_is_busy (info, sector)) {
 		if (get_timer (start) > tout) {
@@ -660,7 +659,6 @@ static int flash_status_poll(flash_info_t *info, void *src, void *dst,
 #endif

 	/* Wait for command completion */
-	reset_timer();
 	start = get_timer(0);
 	while (1) {
 		switch (info->portwidth) {
--
1.7.5.2.317.g391b14

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Scott McNutt]

Hi Graeme,

Graeme Russ wrote:
> There is no need to use get_timer() and reset_timer() and there are build

I must have missed something WRT reset_timer() -- my apologies
if I'm covering old ground.

When the timestamp is incremented using an interrupt that occurs with
a period greater than 1 ms, we can get early timeouts. reset_timer()
solved the problem. What's the recommended approach for dealing with
this without reset_timer() ?

Regards,
--Scott

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

On 23/05/11 22:19, Scott McNutt wrote:
> Hi Graeme,
> 
> Graeme Russ wrote:
>> There is no need to use get_timer() and reset_timer() and there are build
> 
> I must have missed something WRT reset_timer() -- my apologies
> if I'm covering old ground.
> 
> When the timestamp is incremented using an interrupt that occurs with
> a period greater than 1 ms, we can get early timeouts. reset_timer()
> solved the problem. What's the recommended approach for dealing with
> this without reset_timer() ?
> 

There is an active thread on the timer API right now. Short answer - The
API is broken - Calling reset_timer() is not the right solution because:
 a) It breaks recursive or nested timing loops
 b) For some arches, udelay() has a side-effect as well

All this needs fixing

Regards,

Graeme

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Scott McNutt]

Dear Graeme,

Graeme Russ wrote:
> On 23/05/11 22:19, Scott McNutt wrote:
>> Hi Graeme,
>>
>> Graeme Russ wrote:
>>> There is no need to use get_timer() and reset_timer() and there are build
>> I must have missed something WRT reset_timer() -- my apologies
>> if I'm covering old ground.
>>
>> When the timestamp is incremented using an interrupt that occurs with
>> a period greater than 1 ms, we can get early timeouts. reset_timer()
>> solved the problem. What's the recommended approach for dealing with
>> this without reset_timer() ?
>>
> 
> There is an active thread on the timer API right now. Short answer - The
> API is broken - Calling reset_timer() is not the right solution because:
>  a) It breaks recursive or nested timing loops
>  b) For some arches, udelay() has a side-effect as well
> 
> All this needs fixing

Understood. However, removing reset_timer() from cfi_flash.c, will
result in early timeouts for certain boards/archs. Your patch removes
commit 22d6c8faac4e9fa43232b0cf4da427ec14d72ad3 (Thu Apr 1 2010).
I'd rather not break something that has been working for over a year
before I know how to make it work again. Fair enough?

Regards,
--Scott

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

On 24/05/11 04:29, Scott McNutt wrote:
> Hi Bill,
> 
> J. William Campbell wrote:
>> On 5/23/2011 6:12 AM, Scott McNutt wrote:
>>> Dear Graeme,
>>>
>>> Graeme Russ wrote:
>>>> On 23/05/11 22:19, Scott McNutt wrote:
>>>>> Hi Graeme,
>>>>>
>>>>> Graeme Russ wrote:
>>>>>> There is no need to use get_timer() and reset_timer() and there are
>>>>>> build
>>>>> I must have missed something WRT reset_timer() -- my apologies
>>>>> if I'm covering old ground.
>>>>>
>>>>> When the timestamp is incremented using an interrupt that occurs with
>>>>> a period greater than 1 ms, we can get early timeouts. reset_timer()
>>>>> solved the problem. What's the recommended approach for dealing with
>>>>> this without reset_timer() ?
>> Hi Scott,
>>           Are you saying that the interrupt frequency is greater than
>> 1000 times per second, or as I read it, the frequency is less than 1000
>> per second (period greater than 1 ms). If anything, that should make the
>> timer run slow, not fast.
>>  I wonder if it is a resolution issue. What are the typical delays in ms
>> you are using?
> 
> Some older nios2 implementations have _fixed_ 10 msec timers.
> Basically, the timestamp is incremented asynchronous to get_timer(0).
> So a  10 msec timeout can occur, for example, almost immediately if
> the timer isn't reset just prior to calling get_timer(0). There are
> more details in the comments for the following commits:
> 
> nios2: Reload timer count in reset_timer():
>   d8bc0a2889700ba063598de6d4e7d135360b537e
> 
> cfi_flash: reset timer in flash status check:
>   22d6c8faac4e9fa43232b0cf4da427ec14d72ad3
> 
> I'm totally in favor of cleaning this stuff up. It caused some
> headaches (and wasted time) about 13 months ago. My primary concern
> is to avoid breaking things that currently work for us nios2
> weenies ... at least for any length of time.
> 
> Things are a bit tight for me until next week or so. I'll probably
> come up for air around June 1st ... and I'll be glad to help out.
> 

Is there any reason why we cannot silently perform a reset_timer() any time
set_timer() is called with a parameter of 0?

Regards,

Graeme

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[J. William Campbell]

On 5/23/2011 1:10 PM, Graeme Russ wrote:
> On 24/05/11 04:29, Scott McNutt wrote:
>> Hi Bill,
>>
>> J. William Campbell wrote:
>>> On 5/23/2011 6:12 AM, Scott McNutt wrote:
>>>> Dear Graeme,
>>>>
>>>> Graeme Russ wrote:
>>>>> On 23/05/11 22:19, Scott McNutt wrote:
>>>>>> Hi Graeme,
>>>>>>
>>>>>> Graeme Russ wrote:
>>>>>>> There is no need to use get_timer() and reset_timer() and there are
>>>>>>> build
>>>>>> I must have missed something WRT reset_timer() -- my apologies
>>>>>> if I'm covering old ground.
>>>>>>
>>>>>> When the timestamp is incremented using an interrupt that occurs with
>>>>>> a period greater than 1 ms, we can get early timeouts. reset_timer()
>>>>>> solved the problem. What's the recommended approach for dealing with
>>>>>> this without reset_timer() ?
>>> Hi Scott,
>>>            Are you saying that the interrupt frequency is greater than
>>> 1000 times per second, or as I read it, the frequency is less than 1000
>>> per second (period greater than 1 ms). If anything, that should make the
>>> timer run slow, not fast.
>>>   I wonder if it is a resolution issue. What are the typical delays in ms
>>> you are using?
>> Some older nios2 implementations have _fixed_ 10 msec timers.
>> Basically, the timestamp is incremented asynchronous to get_timer(0).
>> So a  10 msec timeout can occur, for example, almost immediately if
>> the timer isn't reset just prior to calling get_timer(0). There are
>> more details in the comments for the following commits:
>>
>> nios2: Reload timer count in reset_timer():
>>    d8bc0a2889700ba063598de6d4e7d135360b537e
>>
>> cfi_flash: reset timer in flash status check:
>>    22d6c8faac4e9fa43232b0cf4da427ec14d72ad3
>>
>> I'm totally in favor of cleaning this stuff up. It caused some
>> headaches (and wasted time) about 13 months ago. My primary concern
>> is to avoid breaking things that currently work for us nios2
>> weenies ... at least for any length of time.
>>
>> Things are a bit tight for me until next week or so. I'll probably
>> come up for air around June 1st ... and I'll be glad to help out.
>>
> Is there any reason why we cannot silently perform a reset_timer() any time
> set_timer() is called with a parameter of 0?
Hi All,
      I assume you mean get_timer(0)?  In principle, you cannot do this 
because it could be inside another get_timer(0) loop that has already 
some time elapsed before you hit the inner get_timer(0). I think what 
needs to happen on the old NIOS with 10 ms resolution on the interrupt 
times is that all timer intervals must have 10 ms added and then rounded 
up to the nearest multiple of 10. Thus, if you wanted to wait for 1 
millisecond, you must use an argument of 20 ms to be sure you wait at 
all! If you use an argument of 10, it won't help because you could get 
an interrupt right away and exit. If these routines are nios2 specific, 
you could add a local reset_timer, but I assume they are generic. . Note 
that if these routines are not nios2 specific, is there any harm in 
waiting "too long"?

Best Regards,
Bill Campbell
> Regards,
>
> Graeme
>
>

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

On 24/05/11 06:49, J. William Campbell wrote:
> On 5/23/2011 1:10 PM, Graeme Russ wrote:
>> On 24/05/11 04:29, Scott McNutt wrote:
>>> Hi Bill,
>>>
>>> J. William Campbell wrote:
>>>> On 5/23/2011 6:12 AM, Scott McNutt wrote:
>>>>> Dear Graeme,
>>>>>
>>>>> Graeme Russ wrote:
>>>>>> On 23/05/11 22:19, Scott McNutt wrote:
>>>>>>> Hi Graeme,
>>>>>>>
>>>>>>> Graeme Russ wrote:
>>>>>>>> There is no need to use get_timer() and reset_timer() and there are
>>>>>>>> build
>>>>>>> I must have missed something WRT reset_timer() -- my apologies
>>>>>>> if I'm covering old ground.
>>>>>>>
>>>>>>> When the timestamp is incremented using an interrupt that occurs with
>>>>>>> a period greater than 1 ms, we can get early timeouts. reset_timer()
>>>>>>> solved the problem. What's the recommended approach for dealing with
>>>>>>> this without reset_timer() ?
>>>> Hi Scott,
>>>>            Are you saying that the interrupt frequency is greater than
>>>> 1000 times per second, or as I read it, the frequency is less than 1000
>>>> per second (period greater than 1 ms). If anything, that should make the
>>>> timer run slow, not fast.
>>>>   I wonder if it is a resolution issue. What are the typical delays in ms
>>>> you are using?
>>> Some older nios2 implementations have _fixed_ 10 msec timers.
>>> Basically, the timestamp is incremented asynchronous to get_timer(0).
>>> So a  10 msec timeout can occur, for example, almost immediately if
>>> the timer isn't reset just prior to calling get_timer(0). There are
>>> more details in the comments for the following commits:
>>>
>>> nios2: Reload timer count in reset_timer():
>>>    d8bc0a2889700ba063598de6d4e7d135360b537e
>>>
>>> cfi_flash: reset timer in flash status check:
>>>    22d6c8faac4e9fa43232b0cf4da427ec14d72ad3
>>>
>>> I'm totally in favor of cleaning this stuff up. It caused some
>>> headaches (and wasted time) about 13 months ago. My primary concern
>>> is to avoid breaking things that currently work for us nios2
>>> weenies ... at least for any length of time.
>>>
>>> Things are a bit tight for me until next week or so. I'll probably
>>> come up for air around June 1st ... and I'll be glad to help out.
>>>
>> Is there any reason why we cannot silently perform a reset_timer() any time
>> set_timer() is called with a parameter of 0?
> Hi All,
>      I assume you mean get_timer(0)?  In principle, you cannot do this

Yes - it's early, no coffee yet ;)
> because it could be inside another get_timer(0) loop that has already some
> time elapsed before you hit the inner get_timer(0). I think what needs to

Correct, but that is what is already happening for ALL arches in cfi due to
the reset_timer() before get_timer(0) - I am suggesting sandboxing the
problem to NIOS until we sort out the timer API properly

> happen on the old NIOS with 10 ms resolution on the interrupt times is that
> all timer intervals must have 10 ms added and then rounded up to the
> nearest multiple of 10. Thus, if you wanted to wait for 1 millisecond, you
> must use an argument of 20 ms to be sure you wait at all! If you use an
> argument of 10, it won't help because you could get an interrupt right away
> and exit. If these routines are nios2 specific, you could add a local
> reset_timer, but I assume they are generic. . Note that if these routines
> are not nios2 specific, is there any harm in waiting "too long"?

Well, we have no control over the argument in cfi driver (unless you plan
to put #ifdef NIOS all over the place)

Maybe we could round up the parameter inside get_timer() itself?

Regards,

Graeme

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[J. William Campbell]

On 5/23/2011 2:02 PM, Graeme Russ wrote:
> On 24/05/11 06:49, J. William Campbell wrote:
>> On 5/23/2011 1:10 PM, Graeme Russ wrote:
>>> On 24/05/11 04:29, Scott McNutt wrote:
>>>> Hi Bill,
>>>>
>>>> J. William Campbell wrote:
>>>>> On 5/23/2011 6:12 AM, Scott McNutt wrote:
>>>>>> Dear Graeme,
>>>>>>
>>>>>> Graeme Russ wrote:
>>>>>>> On 23/05/11 22:19, Scott McNutt wrote:
>>>>>>>> Hi Graeme,
>>>>>>>>
>>>>>>>> Graeme Russ wrote:
>>>>>>>>> There is no need to use get_timer() and reset_timer() and there are
>>>>>>>>> build
>>>>>>>> I must have missed something WRT reset_timer() -- my apologies
>>>>>>>> if I'm covering old ground.
>>>>>>>>
>>>>>>>> When the timestamp is incremented using an interrupt that occurs with
>>>>>>>> a period greater than 1 ms, we can get early timeouts. reset_timer()
>>>>>>>> solved the problem. What's the recommended approach for dealing with
>>>>>>>> this without reset_timer() ?
>>>>> Hi Scott,
>>>>>             Are you saying that the interrupt frequency is greater than
>>>>> 1000 times per second, or as I read it, the frequency is less than 1000
>>>>> per second (period greater than 1 ms). If anything, that should make the
>>>>> timer run slow, not fast.
>>>>>    I wonder if it is a resolution issue. What are the typical delays in ms
>>>>> you are using?
>>>> Some older nios2 implementations have _fixed_ 10 msec timers.
>>>> Basically, the timestamp is incremented asynchronous to get_timer(0).
>>>> So a  10 msec timeout can occur, for example, almost immediately if
>>>> the timer isn't reset just prior to calling get_timer(0). There are
>>>> more details in the comments for the following commits:
>>>>
>>>> nios2: Reload timer count in reset_timer():
>>>>     d8bc0a2889700ba063598de6d4e7d135360b537e
>>>>
>>>> cfi_flash: reset timer in flash status check:
>>>>     22d6c8faac4e9fa43232b0cf4da427ec14d72ad3
>>>>
>>>> I'm totally in favor of cleaning this stuff up. It caused some
>>>> headaches (and wasted time) about 13 months ago. My primary concern
>>>> is to avoid breaking things that currently work for us nios2
>>>> weenies ... at least for any length of time.
>>>>
>>>> Things are a bit tight for me until next week or so. I'll probably
>>>> come up for air around June 1st ... and I'll be glad to help out.
>>>>
>>> Is there any reason why we cannot silently perform a reset_timer() any time
>>> set_timer() is called with a parameter of 0?
>> Hi All,
>>       I assume you mean get_timer(0)?  In principle, you cannot do this
> Yes - it's early, no coffee yet ;)
>> because it could be inside another get_timer(0) loop that has already some
>> time elapsed before you hit the inner get_timer(0). I think what needs to
> Correct, but that is what is already happening for ALL arches in cfi due to
> the reset_timer() before get_timer(0) - I am suggesting sandboxing the
> problem to NIOS until we sort out the timer API properly
>
>> happen on the old NIOS with 10 ms resolution on the interrupt times is that
>> all timer intervals must have 10 ms added and then rounded up to the
>> nearest multiple of 10. Thus, if you wanted to wait for 1 millisecond, you
>> must use an argument of 20 ms to be sure you wait at all! If you use an
>> argument of 10, it won't help because you could get an interrupt right away
>> and exit. If these routines are nios2 specific, you could add a local
>> reset_timer, but I assume they are generic. . Note that if these routines
>> are not nios2 specific, is there any harm in waiting "too long"?
> Well, we have no control over the argument in cfi driver (unless you plan
> to put #ifdef NIOS all over the place)
>
> Maybe we could round up the parameter inside get_timer() itself?
Hi All,
        That would probably be the best way to go for now. It might slow 
things down a bit though, if these delays are all desired to be "short", 
like 1 ms. We would expand the 1 ms delay to 15 ms (average) while the 
current (illegal) solution would expand a 1 ms delay to 10 ms always. It 
is worth trying I think. It is also true that any other delays in the 
program will suffer from the 10 ms resolution problem, so your idea is I 
think a good one.

Best Regards,
Bill Campbell
> Regards,
>
> Graeme
>
>
>

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

On Tue, May 24, 2011 at 7:02 AM, Graeme Russ <graeme.russ@gmail.com> wrote:

[snip]
>
> Well, we have no control over the argument in cfi driver (unless you plan
> to put #ifdef NIOS all over the place)
>
> Maybe we could round up the parameter inside get_timer() itself?

Wow, what was I on! - Oh, thats right, no coffee ;)

The parameter to get_timer() is not a timeout, it is a reference epoch

So I think adding reset_timer() to the NIOS get_timer() when the parameter
is zero is the only option

Regards,

Graeme

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[J. William Campbell]

On 5/23/2011 10:13 PM, Graeme Russ wrote:
> On Tue, May 24, 2011 at 7:02 AM, Graeme Russ<graeme.russ@gmail.com>  wrote:
>
> [snip]
>> Well, we have no control over the argument in cfi driver (unless you plan
>> to put #ifdef NIOS all over the place)
>>
>> Maybe we could round up the parameter inside get_timer() itself?
> Wow, what was I on! - Oh, thats right, no coffee ;)
>
> The parameter to get_timer() is not a timeout, it is a reference epoch
Hi Graeme,
        No, you were not on drugs.  If base != 0 then { if (timer - base 
< 20) return 0;  return timer - base -10} return timer.  That will make 
the subsequent comparison do what you wanted. Differences of less than 
20 are unreliable because of quantization, so we return
0. If the difference was > 20, return a conservative number, the 
difference -10. If the input base was 0, return the timer, the user is 
just trying to read it.

Best Regards
Bill Campbell
> So I think adding reset_timer() to the NIOS get_timer() when the parameter
> is zero is the only option
>
> Regards,
>
> Graeme
>
>

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDAC866.1050508@comcast.net> you wrote:
>
> > Is there any reason why we cannot silently perform a reset_timer() any time
> > set_timer() is called with a parameter of 0?
> Hi All,
>       I assume you mean get_timer(0)?  In principle, you cannot do this 
> because it could be inside another get_timer(0) loop that has already 
> some time elapsed before you hit the inner get_timer(0). I think what 
> needs to happen on the old NIOS with 10 ms resolution on the interrupt 
> times is that all timer intervals must have 10 ms added and then rounded 
> up to the nearest multiple of 10. Thus, if you wanted to wait for 1 
> millisecond, you must use an argument of 20 ms to be sure you wait at 
> all! If you use an argument of 10, it won't help because you could get 
> an interrupt right away and exit. If these routines are nios2 specific, 
> you could add a local reset_timer, but I assume they are generic. . Note 
> that if these routines are not nios2 specific, is there any harm in 
> waiting "too long"?

I think in the context of this rework get_timer() should be changed to
take no argumen. Actually noe is needed, and if used as is now it can
only cuse harm.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Hindsight is an exact science.

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

Hi Wolfgang,

On Tue, May 24, 2011 at 7:53 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear "J. William Campbell",
>
> In message <4DDAC866.1050508@comcast.net> you wrote:
>>
>> > Is there any reason why we cannot silently perform a reset_timer() any time
>> > set_timer() is called with a parameter of 0?
>> Hi All,
>> ? ? ? I assume you mean get_timer(0)? ?In principle, you cannot do this
>> because it could be inside another get_timer(0) loop that has already
>> some time elapsed before you hit the inner get_timer(0). I think what
>> needs to happen on the old NIOS with 10 ms resolution on the interrupt
>> times is that all timer intervals must have 10 ms added and then rounded
>> up to the nearest multiple of 10. Thus, if you wanted to wait for 1
>> millisecond, you must use an argument of 20 ms to be sure you wait at
>> all! If you use an argument of 10, it won't help because you could get
>> an interrupt right away and exit. If these routines are nios2 specific,
>> you could add a local reset_timer, but I assume they are generic. . Note
>> that if these routines are not nios2 specific, is there any harm in
>> waiting "too long"?
>
> I think in the context of this rework get_timer() should be changed to
> take no argumen. Actually noe is needed, and if used as is now it can
> only cuse harm.
>

I think this might break stand alone applications - I am sure get_timer()
is exported. If not, sure, we can remove the argument, but that will be
a huge patch. If we do, we should change it from get_timer() to get_ms()
while we are at it

Regards,

Graeme

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Mike Frysinger]

On Monday, May 23, 2011 18:44:33 Graeme Russ wrote:
> I think this might break stand alone applications - I am sure get_timer()
> is exported.

the export list has a rev associated with it.  see XF_VERSION in 
include/exports.h.  u-boot will detect the mismatch at runtime and bail 
forcing people to rebuild their stand alone apps.  we've done this in the past 
(6 times even!) and no one has complained (at least with any visibility on the 
mailing list).  i dont have a problem with it.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110523/0daec81d/attachment.pgp 

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Jens Scharsig]

Am 2011-05-23 13:54, schrieb Graeme Russ:
> There is no need to use get_timer() and reset_timer() and there are build
> breakages occuring because of them (specifically cfi_flash). Remove any
> usage outside arch/ to fix build breakages and to prepare for complete
> removal
> 
> Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
> ---
> checkpatch complains about long lines and brace usage in the board specific
> flash.c files - They are deprecated and not worth fixing for style
> 
>  board/BuS/EB+MCF-EV123/flash.c |   10 ++++++----

Ack, for EB+MCF-EV123 board

Best regards

Jens Scharsig

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Wolfgang Denk]

Dear Jens Scharsig,

In message <4DDA53B7.7020804@bus-elektronik.de> you wrote:
> Am 2011-05-23 13:54, schrieb Graeme Russ:
> > There is no need to use get_timer() and reset_timer() and there are build
> > breakages occuring because of them (specifically cfi_flash). Remove any
> > usage outside arch/ to fix build breakages and to prepare for complete
> > removal
> > 
> > Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
> > ---
> > checkpatch complains about long lines and brace usage in the board specific
> > flash.c files - They are deprecated and not worth fixing for style
> > 
> >  board/BuS/EB+MCF-EV123/flash.c |   10 ++++++----
> 
> Ack, for EB+MCF-EV123 board

Can you please send a formal Acked-by: message?


Note that this is NOT nitpicking:  Patchwork will automatically add
such correct Acked-by: lines to the patch, so we don't have to track
thse manually - this saves a LOT of time to the maintainers.


Thanks.

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Why waste negative entropy on comments, when you could use the  same
entropy to create bugs instead?"                        - Steve Elias

[U-Boot] Formal acks and patchwork (was: [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/)

[Albert ARIBAUD]

Le 23/05/2011 15:22, Wolfgang Denk a ?crit :

> Can you please send a formal Acked-by: message?
>
> Note that this is NOT nitpicking:  Patchwork will automatically add
> such correct Acked-by: lines to the patch, so we don't have to track
> thse manually - this saves a LOT of time to the maintainers.

One question: where is the information presented in patchwork, apart 
from coloring the ack line when displaying the patch discussion thread? 
I cannot find e.g. a summary list of all acks to a given patch.

> Thanks.
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] Formal acks and patchwork (was: [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/)

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4DDA69A1.1070009@aribaud.net> you wrote:
> 
> > Note that this is NOT nitpicking:  Patchwork will automatically add
> > such correct Acked-by: lines to the patch, so we don't have to track
> > thse manually - this saves a LOT of time to the maintainers.
> 
> One question: where is the information presented in patchwork, apart 
> from coloring the ack line when displaying the patch discussion thread? 
> I cannot find e.g. a summary list of all acks to a given patch.

PW automatically inserts this into the patch if you click on the
"Download: mbox" link, or is you access the patch through pwclient.

For example, instead of applying a patch directly from my mailbox I
use this file only to get the hash value for the PW entry, and then
use pwclient to apply it and to update it's state:

        HASH=$(pwparser.py --hash <$PATCH)
        if [ -z "$HASH" ]
        then
                echo "ERROR: $PATCH - no such entry in PatchWork" >&2
                exit 1
        fi

        if pwclient apply -h $HASH
        then
                pwclient update -s 'Accepted' -h $HASH
        fi

This is extremely convenient as it automatically takes care of both
the Acks and the state change.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Each honest calling, each walk of life, has its own  elite,  its  own
aristocracy based on excellence of performance. - James Bryant Conant

[U-Boot] Formal acks and patchwork

[Albert ARIBAUD]

Le 23/05/2011 20:44, Wolfgang Denk a ?crit :
> Dear Albert ARIBAUD,
>
> In message<4DDA69A1.1070009@aribaud.net>  you wrote:
>>
>>> Note that this is NOT nitpicking:  Patchwork will automatically add
>>> such correct Acked-by: lines to the patch, so we don't have to track
>>> thse manually - this saves a LOT of time to the maintainers.
>>
>> One question: where is the information presented in patchwork, apart
>> from coloring the ack line when displaying the patch discussion thread?
>> I cannot find e.g. a summary list of all acks to a given patch.
>
> PW automatically inserts this into the patch if you click on the
> "Download: mbox" link, or is you access the patch through pwclient.
>
> For example, instead of applying a patch directly from my mailbox I
> use this file only to get the hash value for the PW entry, and then
> use pwclient to apply it and to update it's state:
>
>          HASH=$(pwparser.py --hash<$PATCH)
>          if [ -z "$HASH" ]
>          then
>                  echo "ERROR: $PATCH - no such entry in PatchWork">&2
>                  exit 1
>          fi
>
>          if pwclient apply -h $HASH
>          then
>                  pwclient update -s 'Accepted' -h $HASH
>          fi
>
> This is extremely convenient as it automatically takes care of both
> the Acks and the state change.

Thanks. Looks like this could be handy for many others. Maybe it should 
be 'backported' into pwclient itself as a new command.

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] Formal acks and patchwork

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4DDAB341.20006@aribaud.net> you wrote:
>
> > For example, instead of applying a patch directly from my mailbox I
> > use this file only to get the hash value for the PW entry, and then
> > use pwclient to apply it and to update it's state:
> >
> >          HASH=$(pwparser.py --hash<$PATCH)
> >          if [ -z "$HASH" ]
> >          then
> >                  echo "ERROR: $PATCH - no such entry in PatchWork">&2
> >                  exit 1
> >          fi
> >
> >          if pwclient apply -h $HASH
> >          then
> >                  pwclient update -s 'Accepted' -h $HASH
> >          fi
> >
> > This is extremely convenient as it automatically takes care of both
> > the Acks and the state change.
> 
> Thanks. Looks like this could be handy for many others. Maybe it should 
> be 'backported' into pwclient itself as a new command.

Note that there is a caveat of this approach.  Or call it bug in PW.
We frequently see patch series, which get posted in several versions.
Quite often not all patches are changed, so it happens that for
example patch 3/8 is the same in all versions 2, 3 and 4 of the patch
series.  That means all versions of this patch have the same hash.
Usually I apply the latest version, and want to change the status of
this one, but the 'pwclient -h $HASH' will always reference the
_oldest_ version of the patch.  So watchout when you see it changing
the state from "superseded" to "applied" - usually this means it
changed the wrong version.  [I reported this on the PW ML, but no
response yet.]

I should mention that I have hacked pwclient a bit to use "git am"
instead of plain "patch", and to report the status change:

@@ -110,7 +110,7 @@
                         (os.path.basename(sys.argv[0])))
     sys.stderr.write("Where <action> is one of:\n")
     sys.stderr.write(
-"""        apply <ID>    : Apply a patch (in the current dir, using -p1)
+"""        apply <ID>    : Apply a patch (using 'git-am -3 -u --whitespace=strip')
         get <ID>      : Download a patch and save it locally
         projects      : List all projects
         states        : Show list of potential patch states
@@ -260,7 +260,7 @@
     print "Description: %s" % patch['name']
     s = rpc.patch_get_mbox(patch_id)
     if len(s) > 0:
-        proc = subprocess.Popen(['patch', '-p1'], stdin = subprocess.PIPE)
+        proc = subprocess.Popen(['git', 'am', '-3', '-u', '--whitespace=strip'], stdin = subprocess.PIPE)
         proc.communicate(s)
     else:
         sys.stderr.write("Error: No patch content found\n")
@@ -280,6 +280,8 @@
         if state_id == 0:
             sys.stderr.write("Error: No State found matching %s*\n" % state)
             sys.exit(1)
+       old_id = patch['state']
+        sys.stderr.write("## commit: %s  state: %s ==> %s\n" % (commit, old_id, state))
         params['state'] = state_id
 
     if commit:


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The only perfect science is hind-sight.

[U-Boot] Formal acks and patchwork

[Albert ARIBAUD]

Le 23/05/2011 21:42, Wolfgang Denk a ?crit :
> Dear Albert ARIBAUD,
>
> In message<4DDAB341.20006@aribaud.net>  you wrote:
>>
>>> For example, instead of applying a patch directly from my mailbox I
>>> use this file only to get the hash value for the PW entry, and then
>>> use pwclient to apply it and to update it's state:
>>>
>>>           HASH=$(pwparser.py --hash<$PATCH)
>>>           if [ -z "$HASH" ]
>>>           then
>>>                   echo "ERROR: $PATCH - no such entry in PatchWork">&2
>>>                   exit 1
>>>           fi
>>>
>>>           if pwclient apply -h $HASH
>>>           then
>>>                   pwclient update -s 'Accepted' -h $HASH
>>>           fi
>>>
>>> This is extremely convenient as it automatically takes care of both
>>> the Acks and the state change.
>>
>> Thanks. Looks like this could be handy for many others. Maybe it should
>> be 'backported' into pwclient itself as a new command.
>
> Note that there is a caveat of this approach.  Or call it bug in PW.
> We frequently see patch series, which get posted in several versions.
> Quite often not all patches are changed, so it happens that for
> example patch 3/8 is the same in all versions 2, 3 and 4 of the patch
> series.  That means all versions of this patch have the same hash.
> Usually I apply the latest version, and want to change the status of
> this one, but the 'pwclient -h $HASH' will always reference the
> _oldest_ version of the patch.  So watchout when you see it changing
> the state from "superseded" to "applied" - usually this means it
> changed the wrong version.  [I reported this on the PW ML, but no
> response yet.]
>
> I should mention that I have hacked pwclient a bit to use "git am"
> instead of plain "patch", and to report the status change:
>
> @@ -110,7 +110,7 @@
>                           (os.path.basename(sys.argv[0])))
>       sys.stderr.write("Where<action>  is one of:\n")
>       sys.stderr.write(
> -"""        apply<ID>     : Apply a patch (in the current dir, using -p1)
> +"""        apply<ID>     : Apply a patch (using 'git-am -3 -u --whitespace=strip')
>           get<ID>       : Download a patch and save it locally
>           projects      : List all projects
>           states        : Show list of potential patch states
> @@ -260,7 +260,7 @@
>       print "Description: %s" % patch['name']
>       s = rpc.patch_get_mbox(patch_id)
>       if len(s)>  0:
> -        proc = subprocess.Popen(['patch', '-p1'], stdin = subprocess.PIPE)
> +        proc = subprocess.Popen(['git', 'am', '-3', '-u', '--whitespace=strip'], stdin = subprocess.PIPE)
>           proc.communicate(s)
>       else:
>           sys.stderr.write("Error: No patch content found\n")
> @@ -280,6 +280,8 @@
>           if state_id == 0:
>               sys.stderr.write("Error: No State found matching %s*\n" % state)
>               sys.exit(1)
> +       old_id = patch['state']
> +        sys.stderr.write("## commit: %s  state: %s ==>  %s\n" % (commit, old_id, state))
>           params['state'] = state_id
>
>       if commit:

I personally only added a "pwclient am" command to do a git am, the same 
way as "pwclient apply" does a git apply.

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Jens Scharsig]

Am 2011-05-23 13:54, schrieb Graeme Russ:
> There is no need to use get_timer() and reset_timer() and there are build
> breakages occuring because of them (specifically cfi_flash). Remove any
> usage outside arch/ to fix build breakages and to prepare for complete
> removal
> 
> Signed-off-by: Graeme Russ <graeme.russ@gmail.com>

for EB+MCF-EV123 board

Acked-by: Jens Scharsig <esw@bus-elektronik.de>

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

On 24/05/11 00:00, Andrew Dyer wrote:
> On Mon, May 23, 2011 at 06:54, Graeme Russ <graeme.russ@gmail.com> wrote:
>> There is no need to use get_timer() and reset_timer() and there are build
> 
> shouldn't the above say set_timer() instead of get_timer()?
> 

Yes, thanks

Regards,

Graeme

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

There is no need to use set_timer() and reset_timer() and there are build
breakages occuring because of them (specifically cfi_flash). Remove any
usage outside arch/ to fix build breakages and to prepare for complete
removal

The Nios2 timer implentation updates every 10ms (increases in 10ms steps)
and required a reset_timer() call before timing operations. For Nios2, call
reset_timer() when get_timer() is called with a parameter value of 0

Signed-off-by: Graeme Russ <graeme.russ@gmail.com>

---
checkpatch complains about long lines and brace usage in the board specific
flash.c files - They are deprecated and not worth fixing for style

Changes since V1:
 - Fix typo in commit message
 - Add reset_timer() to Nios2 get_timer(0)
---
 arch/nios2/cpu/interrupts.c    |    4 ++++
 board/BuS/EB+MCF-EV123/flash.c |   10 ++++++----
 board/cobra5272/flash.c        |   10 ++++++----
 board/idmr/flash.c             |   10 ++++++----
 drivers/block/mg_disk.c        |    1 -
 drivers/mtd/cfi_flash.c        |    2 --
 6 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/arch/nios2/cpu/interrupts.c b/arch/nios2/cpu/interrupts.c
index 63acfa9..4e027f1 100644
--- a/arch/nios2/cpu/interrupts.c
+++ b/arch/nios2/cpu/interrupts.c
@@ -95,6 +95,10 @@ void reset_timer (void)
 ulong get_timer (ulong base)
 {
 	WATCHDOG_RESET ();
+
+	if (base == 0)
+		reset_timer();
+
 	return (timestamp - base);
 }
 
diff --git a/board/BuS/EB+MCF-EV123/flash.c b/board/BuS/EB+MCF-EV123/flash.c
index 3c36367..8b7f957 100644
--- a/board/BuS/EB+MCF-EV123/flash.c
+++ b/board/BuS/EB+MCF-EV123/flash.c
@@ -157,6 +157,7 @@ int amd_flash_erase_sector(flash_info_t * info, int sector)
 {
 	int state;
 	ulong result;
+	ulong start;
 
 	volatile u16 *addr =
 				(volatile u16 *) (info->start[sector]);
@@ -171,13 +172,13 @@ int amd_flash_erase_sector(flash_info_t * info, int sector)
 
 	/* wait until flash is ready */
 	state = 0;
-	set_timer (0);
+	start = get_timer(0);
 
 	do {
 		result = *addr;
 
 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 			MEM_FLASH_ADDR1 = CMD_READ_ARRAY;
 			state = ERR_TIMOUT;
 		}
@@ -267,6 +268,7 @@ volatile static int amd_write_word (flash_info_t * info, ulong dest, u16 data)
 	ulong result;
 	int cflag, iflag;
 	int state;
+	ulong start;
 
 	/*
 	 * Check if Flash is (sufficiently) erased
@@ -295,7 +297,7 @@ volatile static int amd_write_word (flash_info_t * info, ulong dest, u16 data)
 	*addr = data;
 
 	/* arm simple, non interrupt dependent timer */
-	set_timer (0);
+	start = get_timer(0);
 
 	/* wait until flash is ready */
 	state = 0;
@@ -303,7 +305,7 @@ volatile static int amd_write_word (flash_info_t * info, ulong dest, u16 data)
 		result = *addr;
 
 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 				state = ERR_TIMOUT;
 		}
 		if (!state && ((result & BIT_RDY_MASK) == (data & BIT_RDY_MASK)))
diff --git a/board/cobra5272/flash.c b/board/cobra5272/flash.c
index 33c9361..e8f02eb 100644
--- a/board/cobra5272/flash.c
+++ b/board/cobra5272/flash.c
@@ -147,6 +147,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 	int iflag, cflag, prot, sect;
 	int rc = ERR_OK;
 	int chip1;
+	ulong start;
 
 	/* first look for protection bits */
 
@@ -190,7 +191,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 		printf ("Erasing sector %2d ... ", sect);
 
 		/* arm simple, non interrupt dependent timer */
-		set_timer (0);
+		start = get_timer(0);
 
 		if (info->protect[sect] == 0) {	/* not protected */
 			volatile u16 *addr =
@@ -211,7 +212,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				result = *addr;
 
 				/* check timeout */
-				if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+				if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 					MEM_FLASH_ADDR1 = CMD_READ_ARRAY;
 					chip1 = TMO;
 					break;
@@ -264,6 +265,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	int rc = ERR_OK;
 	int cflag, iflag;
 	int chip1;
+	ulong start;
 
 	/*
 	 * Check if Flash is (sufficiently) erased
@@ -291,7 +293,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	*addr = data;
 
 	/* arm simple, non interrupt dependent timer */
-	set_timer (0);
+	start = get_timer(0);
 
 	/* wait until flash is ready */
 	chip1 = 0;
@@ -299,7 +301,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 		result = *addr;
 
 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT) {
 			chip1 = ERR | TMO;
 			break;
 		}
diff --git a/board/idmr/flash.c b/board/idmr/flash.c
index 57c9948..9f4ff2b 100644
--- a/board/idmr/flash.c
+++ b/board/idmr/flash.c
@@ -130,6 +130,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 	int iflag, prot, sect;
 	int rc = ERR_OK;
 	int chip1;
+	ulong start;
 
 	/* first look for protection bits */
 
@@ -170,7 +171,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 		printf ("Erasing sector %2d ... ", sect);
 
 		/* arm simple, non interrupt dependent timer */
-		set_timer (0);
+		start = get_timer(0);
 
 		if (info->protect[sect] == 0) {	/* not protected */
 			volatile u16 *addr =
@@ -191,7 +192,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				result = *addr;
 
 				/* check timeout */
-				if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
+				if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
 					MEM_FLASH_ADDR1 = CMD_READ_ARRAY;
 					chip1 = TMO;
 					break;
@@ -248,6 +249,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	int rc = ERR_OK;
 	int iflag;
 	int chip1;
+	ulong start;
 
 	/*
 	 * Check if Flash is (sufficiently) erased
@@ -272,7 +274,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 	*addr = data;
 
 	/* arm simple, non interrupt dependent timer */
-	set_timer (0);
+	start = get_timer(0);
 
 	/* wait until flash is ready */
 	chip1 = 0;
@@ -280,7 +282,7 @@ static int write_word (flash_info_t * info, ulong dest, ulong data)
 		result = *addr;
 
 		/* check timeout */
-		if (get_timer (0) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
+		if (get_timer(start) > CONFIG_SYS_FLASH_ERASE_TOUT * CONFIG_SYS_HZ / 1000) {
 			chip1 = ERR | TMO;
 			break;
 		}
diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index b74307a..7609557 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -91,7 +91,6 @@ static unsigned int mg_wait (u32 expect, u32 msec)
 	u32 from, cur, err;
 
 	err = MG_ERR_NONE;
-	reset_timer();
 	from = get_timer(0);
 
 	status = readb(mg_base() + MG_REG_STATUS);
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 5788328..ea5e1f2 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -573,7 +573,6 @@ static int flash_status_check (flash_info_t * info, flash_sect_t sector,
 #endif
 
 	/* Wait for command completion */
-	reset_timer();
 	start = get_timer (0);
 	while (flash_is_busy (info, sector)) {
 		if (get_timer (start) > tout) {
@@ -660,7 +659,6 @@ static int flash_status_poll(flash_info_t *info, void *src, void *dst,
 #endif
 
 	/* Wait for command completion */
-	reset_timer();
 	start = get_timer(0);
 	while (1) {
 		switch (info->portwidth) {
-- 
1.7.5.2.317.g391b14

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

On 24/05/11 20:54, Graeme Russ wrote:
> There is no need to use set_timer() and reset_timer() and there are build
> breakages occuring because of them (specifically cfi_flash). Remove any
> usage outside arch/ to fix build breakages and to prepare for complete
> removal
> 
> The Nios2 timer implentation updates every 10ms (increases in 10ms steps)
> and required a reset_timer() call before timing operations. For Nios2, call
> reset_timer() when get_timer() is called with a parameter value of 0
> 
> Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
> 
> ---
> checkpatch complains about long lines and brace usage in the board specific
> flash.c files - They are deprecated and not worth fixing for style
> 
> Changes since V1:
>  - Fix typo in commit message
>  - Add reset_timer() to Nios2 get_timer(0)
> ---
>  arch/nios2/cpu/interrupts.c    |    4 ++++
>  board/BuS/EB+MCF-EV123/flash.c |   10 ++++++----
>  board/cobra5272/flash.c        |   10 ++++++----
>  board/idmr/flash.c             |   10 ++++++----
>  drivers/block/mg_disk.c        |    1 -
>  drivers/mtd/cfi_flash.c        |    2 --
>  6 files changed, 22 insertions(+), 15 deletions(-)

Argh - Sorry about the subject - It should be [PATCH v2]

Do I need to resend or will that make matters worse?

Regards,

Graeme

[U-Boot] [PATCH] [Timer]Remove calls to [get, reset]_timer outside arch/

[Graeme Russ]

On 24/05/11 20:54, Graeme Russ wrote:
> There is no need to use set_timer() and reset_timer() and there are build
> breakages occuring because of them (specifically cfi_flash). Remove any
> usage outside arch/ to fix build breakages and to prepare for complete
> removal
> 
> The Nios2 timer implentation updates every 10ms (increases in 10ms steps)
> and required a reset_timer() call before timing operations. For Nios2, call
> reset_timer() when get_timer() is called with a parameter value of 0

That's not going to work either - start = get_timer(0) will reset the timer
and set start to zero so get_timer(start) will continually reset the timer
and return zero :(

Stay tuned for v3 :)

Regards,

Graeme