diff mbox

[U-Boot,v1,(WIP),16/16,Timer] Replace get_timer() usage in arch/

Message ID 1309261269-4363-17-git-send-email-graeme.russ@gmail.com
State RFC
Headers show

Commit Message

Graeme Russ June 28, 2011, 11:41 a.m. UTC
Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
---
 arch/blackfin/cpu/jtag-console.c           |    4 ++--
 arch/microblaze/lib/time.c                 |    4 ++--
 arch/nios2/cpu/epcs.c                      |   12 ++++++------
 arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c |    8 ++++----
 arch/powerpc/cpu/mpc8260/ether_fcc.c       |   12 ++++++------
 5 files changed, 20 insertions(+), 20 deletions(-)

Comments

Simon Glass June 29, 2011, 4:45 a.m. UTC | #1
Hi Graeme,

On Tue, Jun 28, 2011 at 4:41 AM, Graeme Russ <graeme.russ@gmail.com> wrote:
>
> Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
> ---
>  arch/blackfin/cpu/jtag-console.c           |    4 ++--
>  arch/microblaze/lib/time.c                 |    4 ++--
>  arch/nios2/cpu/epcs.c                      |   12 ++++++------
>  arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c |    8 ++++----
>  arch/powerpc/cpu/mpc8260/ether_fcc.c       |   12 ++++++------
>  5 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/arch/blackfin/cpu/jtag-console.c b/arch/blackfin/cpu/jtag-console.c
> index e0f2975..97ff1de 100644
> --- a/arch/blackfin/cpu/jtag-console.c
> +++ b/arch/blackfin/cpu/jtag-console.c
> @@ -48,11 +48,11 @@ static inline uint32_t bfin_read_emudat(void)
>  static bool jtag_write_emudat(uint32_t emudat)
>  {
>        static bool overflowed = false;
> -       ulong timeout = get_timer(0) + CONFIG_JTAG_CONSOLE_TIMEOUT;
> +       ulong start = time_now_ms();
>        while (bfin_read_DBGSTAT() & 0x1) {
>                if (overflowed)
>                        return overflowed;
> -               if (timeout < get_timer(0))
> +               if (time_since_ms(start) >= CONFIG_JTAG_CONSOLE_TIMEOUT)
>                        overflowed = true;
>        }
>        overflowed = false;

Here I think I have found a use of future time. It is true what they
say (or should say) that there is every kind of timeout in U-Boot.

Regards,
Simon
Graeme Russ June 29, 2011, 4:51 a.m. UTC | #2
Hi Simon,

On Wed, Jun 29, 2011 at 2:45 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Graeme,
>
> On Tue, Jun 28, 2011 at 4:41 AM, Graeme Russ <graeme.russ@gmail.com> wrote:
>>
>> Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
>> ---
>>  arch/blackfin/cpu/jtag-console.c           |    4 ++--
>>  arch/microblaze/lib/time.c                 |    4 ++--
>>  arch/nios2/cpu/epcs.c                      |   12 ++++++------
>>  arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c |    8 ++++----
>>  arch/powerpc/cpu/mpc8260/ether_fcc.c       |   12 ++++++------
>>  5 files changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/blackfin/cpu/jtag-console.c b/arch/blackfin/cpu/jtag-console.c
>> index e0f2975..97ff1de 100644
>> --- a/arch/blackfin/cpu/jtag-console.c
>> +++ b/arch/blackfin/cpu/jtag-console.c
>> @@ -48,11 +48,11 @@ static inline uint32_t bfin_read_emudat(void)
>>  static bool jtag_write_emudat(uint32_t emudat)
>>  {
>>        static bool overflowed = false;
>> -       ulong timeout = get_timer(0) + CONFIG_JTAG_CONSOLE_TIMEOUT;
>> +       ulong start = time_now_ms();
>>        while (bfin_read_DBGSTAT() & 0x1) {
>>                if (overflowed)
>>                        return overflowed;
>> -               if (timeout < get_timer(0))
>> +               if (time_since_ms(start) >= CONFIG_JTAG_CONSOLE_TIMEOUT)
>>                        overflowed = true;
>>        }
>>        overflowed = false;
>
> Here I think I have found a use of future time. It is true what they
> say (or should say) that there is every kind of timeout in U-Boot.

I personally think that this particular use-case of the timer API is ugly,
but I was not out to change any symantics, just do a blind translation
from the old API to the new API

Hopefully, this will highlight a few dodgy use cases (which I am willing
to apply fixes for as and when others suggest them)

Regards,

Graeme
Mike Frysinger June 29, 2011, 5:15 a.m. UTC | #3
On Wednesday, June 29, 2011 00:51:50 Graeme Russ wrote:
> On Wed, Jun 29, 2011 at 2:45 PM, Simon Glass wrote:
> > On Tue, Jun 28, 2011 at 4:41 AM, Graeme Russ wrote:
> >> --- a/arch/blackfin/cpu/jtag-console.c
> >> +++ b/arch/blackfin/cpu/jtag-console.c
> >>
> >> -       ulong timeout = get_timer(0) + CONFIG_JTAG_CONSOLE_TIMEOUT;
> >> +       ulong start = time_now_ms();
> >>        while (bfin_read_DBGSTAT() & 0x1) {
> >>                if (overflowed)
> >>                        return overflowed;
> >> -               if (timeout < get_timer(0))
> >> +               if (time_since_ms(start) >= CONFIG_JTAG_CONSOLE_TIMEOUT)
> >>                        overflowed = true;
> >>        }
> > 
> > Here I think I have found a use of future time. It is true what they
> > say (or should say) that there is every kind of timeout in U-Boot.
> 
> I personally think that this particular use-case of the timer API is ugly,
> but I was not out to change any symantics, just do a blind translation
> from the old API to the new API
> 
> Hopefully, this will highlight a few dodgy use cases (which I am willing
> to apply fixes for as and when others suggest them)

i wrote this long before i really understood the timer api.  after all, even 
now, there is 0 documentation on the API in the u-boot tree.

should be easy to change the two lines to use the API as designed.
-mike
Mike Frysinger June 29, 2011, 5:26 a.m. UTC | #4
On Wednesday, June 29, 2011 01:15:53 Mike Frysinger wrote:
> i wrote this long before i really understood the timer api.  after all,
> even now, there is 0 documentation on the API in the u-boot tree.

unless i missed something, even this patchset doesnt include a new 
doc/README.timer (or similar) file, nor does the header include any comments 
above the func prototypes, nor comments above the func definitions in 
lib/time.c ...
-mike
Graeme Russ June 29, 2011, 5:29 a.m. UTC | #5
Hi Mike,

On Wed, Jun 29, 2011 at 3:26 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday, June 29, 2011 01:15:53 Mike Frysinger wrote:
>> i wrote this long before i really understood the timer api.  after all,
>> even now, there is 0 documentation on the API in the u-boot tree.
>
> unless i missed something, even this patchset doesnt include a new
> doc/README.timer (or similar) file, nor does the header include any comments
> above the func prototypes, nor comments above the func definitions in
> lib/time.c ...

Correct - I am going on holidays on Friday and really needed to get
something in the pipeline for discussion - The wiki has a detailed writeup
of how the API works - I'll translate that into a README and code comments

Regards,

Graeme
diff mbox

Patch

diff --git a/arch/blackfin/cpu/jtag-console.c b/arch/blackfin/cpu/jtag-console.c
index e0f2975..97ff1de 100644
--- a/arch/blackfin/cpu/jtag-console.c
+++ b/arch/blackfin/cpu/jtag-console.c
@@ -48,11 +48,11 @@  static inline uint32_t bfin_read_emudat(void)
 static bool jtag_write_emudat(uint32_t emudat)
 {
 	static bool overflowed = false;
-	ulong timeout = get_timer(0) + CONFIG_JTAG_CONSOLE_TIMEOUT;
+	ulong start = time_now_ms();
 	while (bfin_read_DBGSTAT() & 0x1) {
 		if (overflowed)
 			return overflowed;
-		if (timeout < get_timer(0))
+		if (time_since_ms(start) >= CONFIG_JTAG_CONSOLE_TIMEOUT)
 			overflowed = true;
 	}
 	overflowed = false;
diff --git a/arch/microblaze/lib/time.c b/arch/microblaze/lib/time.c
index da016a0..e5ed3bf 100644
--- a/arch/microblaze/lib/time.c
+++ b/arch/microblaze/lib/time.c
@@ -30,8 +30,8 @@ 
 void __udelay (unsigned long usec)
 {
 	int i;
-	i = get_timer (0);
-	while ((get_timer (0) - i) < (usec / 1000)) ;
+	i = time_now_ms();
+	while (time_since_ms(i) < (usec / 1000)) ;
 }
 #else
 void __udelay (unsigned long usec)
diff --git a/arch/nios2/cpu/epcs.c b/arch/nios2/cpu/epcs.c
index 2369431..90e5b99 100644
--- a/arch/nios2/cpu/epcs.c
+++ b/arch/nios2/cpu/epcs.c
@@ -88,9 +88,9 @@  static int epcs_cs (int assert)
 		writel (tmp | NIOS_SPI_SSO, &epcs->control);
 	} else {
 		/* Let all bits shift out */
-		start = get_timer (0);
+		start = time_now_ms();
 		while ((readl (&epcs->status) & NIOS_SPI_TMT) == 0)
-			if (get_timer (start) > EPCS_TIMEOUT)
+			if (time_since_ms(start) > EPCS_TIMEOUT)
 				return (-1);
 		tmp = readl (&epcs->control);
 		writel (tmp & ~NIOS_SPI_SSO, &epcs->control);
@@ -102,9 +102,9 @@  static int epcs_tx (unsigned char c)
 {
 	ulong start;
 
-	start = get_timer (0);
+	start = time_now_ms();
 	while ((readl (&epcs->status) & NIOS_SPI_TRDY) == 0)
-		if (get_timer (start) > EPCS_TIMEOUT)
+		if (time_since_ms(start) > EPCS_TIMEOUT)
 			return (-1);
 	writel (c, &epcs->txdata);
 	return (0);
@@ -114,9 +114,9 @@  static int epcs_rx (void)
 {
 	ulong start;
 
-	start = get_timer (0);
+	start = time_now_ms();
 	while ((readl (&epcs->status) & NIOS_SPI_RRDY) == 0)
-		if (get_timer (start) > EPCS_TIMEOUT)
+		if (time_since_ms(start) > EPCS_TIMEOUT)
 			return (-1);
 	return (readl (&epcs->rxdata));
 }
diff --git a/arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c b/arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c
index 637ae4c..7822811 100644
--- a/arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c
+++ b/arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c
@@ -97,10 +97,10 @@  void i2c_init (int speed, int slaveadd)
 
 static __inline__ int i2c_wait4bus (void)
 {
-	ulong timeval = get_timer (0);
+	ulong timeval = time_now_ms();
 
 	while (readl (I2CCSR) & MPC107_CSR_MBB)
-		if (get_timer (timeval) > TIMEOUT)
+		if (time_since_ms(timeval) > TIMEOUT)
 			return -1;
 
 	return 0;
@@ -109,7 +109,7 @@  static __inline__ int i2c_wait4bus (void)
 static __inline__ int i2c_wait (int write)
 {
 	u32 csr;
-	ulong timeval = get_timer (0);
+	ulong timeval = time_now_ms();
 
 	do {
 		csr = readl (I2CCSR);
@@ -141,7 +141,7 @@  static __inline__ int i2c_wait (int write)
 		}
 
 		return 0;
-	} while (get_timer (timeval) < TIMEOUT);
+	} while (time_since_ms(timeval) < TIMEOUT);
 
 #ifdef I2CDBG
 	printf ("i2c_wait: timed out\n");
diff --git a/arch/powerpc/cpu/mpc8260/ether_fcc.c b/arch/powerpc/cpu/mpc8260/ether_fcc.c
index c82958d..89703d9 100644
--- a/arch/powerpc/cpu/mpc8260/ether_fcc.c
+++ b/arch/powerpc/cpu/mpc8260/ether_fcc.c
@@ -887,7 +887,7 @@  eth_loopback_test (void)
 	 */
 
 	clear_ctrlc ();
-	runtime = get_timer (0);
+	runtime = time_now_ms();
 
 	do {
 		nclosed = 0;
@@ -931,7 +931,7 @@  eth_loopback_test (void)
 						__asm__ __volatile__ ("eieio");
 					} while (cp->cp_cpcr & CPM_CR_FLG);
 
-					ecp->clstime = get_timer (0);
+					ecp->clstime = time_now_ms();
 					ecp->state = Closing;
 				}
 				/* fall through ... */
@@ -990,7 +990,7 @@  eth_loopback_test (void)
 					}
 
 					if (ecp->state == Closing)
-						ecp->clstime = get_timer (0);
+						ecp->clstime = time_now_ms();
 
 					/* make it ready again */
 					bdp->cbd_sc |= BD_ENET_TX_READY;
@@ -1092,7 +1092,7 @@  eth_loopback_test (void)
 					}
 
 					if (ecp->state == Closing)
-					    ecp->clstime = get_timer (0);
+					    ecp->clstime = time_now_ms();
 
 					/* make it empty again */
 					bdp->cbd_sc |= BD_ENET_RX_EMPTY;
@@ -1106,7 +1106,7 @@  eth_loopback_test (void)
 				 * waited long enough
 				 */
 
-				if (get_timer (ecp->clstime) >= ELBT_CLSWAIT) {
+				if (time_since_ms(ecp->clstime) >= ELBT_CLSWAIT) {
 					/* write GFMR: disable tx/rx */
 					fcp->fcc_gfmr &= \
 						~(FCC_GFMR_ENT | FCC_GFMR_ENR);
@@ -1123,7 +1123,7 @@  eth_loopback_test (void)
 
 	} while (nclosed < (FCC_END_LOOP - FCC_START_LOOP + 1));
 
-	runtime = get_timer (runtime);
+	runtime = time_max_since_ms(runtime);
 	if (runtime <= ELBT_CLSWAIT) {
 		printf ("Whoops! somehow elapsed time (%ld) is wrong (<= %d)\n",
 			runtime, ELBT_CLSWAIT);