AT91 Fix: return value of get_tbclk
Subsystems (2)
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
* Fix: return value of get_tbclk
* this fixes issue with prematurely restart/retry, if BOOT_RETRY_TIMEOUT is used
---
the V3 supports the actually file structure.
the original patch http://lists.denx.de/pipermail/u-boot/2010-April/069415.html
use the old directory /cpu/arch structure.
arch/arm/cpu/arm926ejs/at91/timer.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
arch/arm/cpu/arm926ejs/at91/timer.c +1 -4
@@ -138,8 +138,5 @@ ulong get_timer(ulong base)*/ ulong get_tbclk (void ) { -ulong tbclk; - - tbclk =CONFIG_SYS_HZ ; -return tbclk; +return timer_freq; }
Signature · 1 line
1.7.1
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Quoted 19 lines · Jens Scharsig
* Fix: return value of
get_tbclk
* this fixes issue with prematurely restart/retry, ifBOOT_RETRY_TIMEOUTis usedSigned-off-by Jens Scharsig <[email protected]>---
the V3 supports the actually file structure.
the original patch http://lists.denx.de/pipermail/u-boot/2010-April/069415.html
use the old directory /cpu/arch structure.arch/arm/cpu/arm926ejs/at91/timer.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)arch/arm/cpu/arm926ejs/at91/timer.c +0
-0
arch/arm/cpu/arm926ejs/at91/timer.c ulong get_timer(ulong base) · line 142
Quoted 9 lines
arch/arm/cpu/arm926ejs/at91/timer.c +1
-4@@ -138,8 +138,5 @@ ulong get_timer(ulong base)*/ ulong get_tbclk (void ) { -ulong tbclk; - - tbclk =CONFIG_SYS_HZ ; -return tbclk; +return timer_freq; }
Dear Jens,
I think the timer.c is more broken that just that return value:
example 1:
/*
* timer without interrupts
*/
unsigned long long get_ticks(void)
{
at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE;
ulong now = readl(&pit->piir);
if (now >= lastinc) /* normal mode (non roll) */
/* move stamp forward with absolut diff ticks */
timestamp += (now - lastinc);
else /* we have rollover of incrementer */
timestamp += (0xFFFFFFFF - lastinc) + now;
lastinc = now;
return timestamp;
}observation: timestamp, lastinc, now are all ulong.
timestamp += (0xFFFFFFFF - lastinc) + now;
is the same as
timestamp += (now - lastinc) - 1;so in case of a "rollover" timestamp is incremented just by one less. I think the if is superfluous. ulong will handle the rollover automatically
example 2:
void __udelay(unsigned long usec)
{
unsigned long long tmp;
ulong tmo;
tmo = usec_to_tick(usec);
tmp = get_ticks() + tmo; /* get current timestamp */
while (get_ticks() < tmp) /* loop till event */
/*NOP*/;
}observation: tmp being unsigned long long, get_ticks returning the unsigned long timestamp, tmp being a sum of two ulongs, the while might NEVER end. In practice that is very unlikely, however the code should be corrected.
I was going to rework that timer sooner or later to address all those issues, but you are welcome to go ahead, too. Just we should avoid double work :)
Greetings, Reinhard
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Dear Reinhard
Quoted 2 lines
I think the
timer.cis more broken that just that return value:
You are right, but the patch fixes only the single small problem.
Quoted 19 lines
example 1:
/*
* timer without interrupts
*/
unsigned long longget_ticks(void){ at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE; ulong now = readl(&pit->piir); if (now >= lastinc) /* normal mode (non roll) */ /* move stamp forward with absolut diff ticks */ timestamp += (now - lastinc); else /* we have rollover of incrementer */ timestamp += (0xFFFFFFFF - lastinc) + now; lastinc = now; return timestamp; }
Quoted 6 lines
observation: timestamp, lastinc, now are all ulong.
timestamp += (0xFFFFFFFF - lastinc) + now; is the same as timestamp += (now - lastinc) - 1;so in case of a "rollover" timestamp is incremented just by one less. I think the if is superfluous. ulong will handle the rollover automatically
But this is only true, if we are using ulong variables. I think the idea behind this code is use unsigned long long for variables. (especially timestamp)
Quoted 14 lines
example 2:
void__udelay(unsigned long usec){ unsigned long long tmp; ulong tmo; tmo = usec_to_tick(usec); tmp = get_ticks() + tmo; /* get current timestamp */ while (get_ticks() < tmp) /* loop till event */ /*NOP*/; }
Quoted 4 lines
observation: tmp being unsigned long long,
get_ticksreturning the unsigned long timestamp, tmp being a sum of two ulongs, the while might NEVER end. In practice that is very unlikely, however the code should be corrected.
Right, but this isn't only a problem of AT91 arch. Is should be fixed global. The code is correct, if get_ticks returns real long long values.
Quoted 4 lines
I was going to rework that timer sooner or later to address all those issues, but you are welcome to go ahead, too. Just we should avoid double work :)
I have no time enough at the moment to do that.
Quoted 2 lines
Greetings, Reinhard
Best regards
Jens Scharsig
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
In reply to Jens ScharsigHello,
Am Samstag, 7. August 2010, 19:49:42 schrieb Jens Scharsig:
Quoted 3 lines
* Fix: return value of
get_tbclk
* this fixes issue with prematurely restart/retry, ifBOOT_RETRY_TIMEOUT
is used
This issue also arises, if you use CONFIG_AUTOBOOT_KEYED & friends
Quoted 15 lines
Signed-off-by Jens Scharsig <[email protected]>---
the V3 supports the actually file structure.
the original patch
http://lists.denx.de/pipermail/u-boot/2010-April/069415.html use the old directory /cpu/arch structure.arch/arm/cpu/arm926ejs/at91/timer.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/arm926ejs/at91/timer.cindex d21eebf..8efc34b 100644
arch/arm/cpu/arm926ejs/at91/timer.c ulong get_timer(ulong base) · line 142
Quoted 9 lines
arch/arm/cpu/arm926ejs/at91/timer.c +1
-4@@ -138,8 +138,5 @@ ulong get_timer(ulong base)*/ ulong get_tbclk (void ) { -ulong tbclk; - - tbclk =CONFIG_SYS_HZ ; -return tbclk; +return timer_freq; }
Best regards,
Alexander
Xu
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Hi Alex,
Quoted 7 lines
-----Original Message-----
From: Alexander Stein [mailto:alexander.stein at systec-electronic.com]
Sent: Monday, August 09, 2010 2:35 PM
To: u-boot at lists.denx.de
Cc: Jens Scharsig; Xu, Hong
Subject: Re: [U-Boot] [PATCH V3] AT91 Fix: return value ofget_tbclk
Quoted 9 lines
Hello,
Am Samstag, 7. August 2010, 19:49:42 schrieb Jens Scharsig:
Quoted 3 lines
* Fix: return value of
get_tbclk
* this fixes issue with prematurely restart/retry, ifBOOT_RETRY_TIMEOUTis usedThis issue also arises, if you use
CONFIG_AUTOBOOT_KEYED& friends
I tried both BOOT_RETRY_TIMEOUT and CONFIG_AUTOBOOT_KEYED & CONFIG_AUTOBOOT_STOP_STR, Both work well on AT91SAM9260EK
Which board did you tried?
BR,
Eric
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Hello Eric,
Quoted 6 lines · On Wednesday 18 August 2010, 09:17:26 you wrote:
Quoted 6 lines
Hello,
Am Samstag, 7. August 2010, 19:49:42 schrieb Jens Scharsig:
Quoted 3 lines
* Fix: return value of
get_tbclk
* this fixes issue with prematurely restart/retry, if
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
BOOT_RETRY_TIMEOUTis usedThis issue also arises, if you use
CONFIG_AUTOBOOT_KEYED& friendsI tried both
BOOT_RETRY_TIMEOUTandCONFIG_AUTOBOOT_KEYED&CONFIG_AUTOBOOT_STOP_STR, Both work well on AT91SAM9260EKWhich board did you tried?
That's some time ago and I don't remember which u-boot version that was. I think i came across that problem on the AT91SAM9G20EK and our own SAM9G20 board.
Best regards,
Alexander
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Hi,
this patch also fixed BOOTDELAY on AT91:
* Fix: return value of get_tbclk
* this fixes issue with prematurely restart/retry, if BOOT_RETRY_TIMEOUT is used
--- cpu/arm926ejs/at91/timer.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
cpu/arm926ejs/at91/timer.c +1 -4
@@ -138,8 +138,5 @@ ulong get_timer(ulong base)*/ ulong get_tbclk (void ) { -ulong tbclk; - - tbclk =CONFIG_SYS_HZ ; -return tbclk; +return timer_freq; }
-- 1.6.0.2
Applied to u-boot-atmel/next
Thanks,
Reinhard
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Am 2010-08-31 09:36, schrieb Reinhard Meyer:
Quoted 9 lines
Hi,
}
-- 1.6.0.2Applied to u-boot-atmel/next
Thanks,Reinhard
What is the reason for the new branch/Custodian Tree u-boot-atmel.
Thanks for more details
Jens
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Quoted 11 lines · Jens Scharsig
Am 2010-08-31 09:36, schrieb Reinhard Meyer:
Quoted 9 lines
Hi,
}
-- 1.6.0.2Applied to u-boot-atmel/next
Thanks,Reinhard
What is the reason for the new branch/Custodian Tree u-boot-atmel.
Hello,
one reason is that AVR32 and AT91 share alot in the peripheral building blocks, so many drivers are both for AVR32 and AT91.
I will try to make sure changes to drivers in that area are not breaking either architecture.
On that behalf I'd like a briefing why there is
at91_emac AND macb?
macb works for AVR32AP700x and (at least) for AT91SAM9260/9G20/9XE.
What is the deal with at91_emac? Is it required for some AT91's because macb does not work there?
Does it provide better "results" than macb?
Best Regards
Reinhard
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Am 2010-09-01 09:30, schrieb Reinhard Meyer:
Quoted 5 lines
at91_emacAND macb?macb works for AVR32AP700x and (at least) for AT91SAM9260/9G20/9XE.
Quoted 7 lines
What is the deal with
at91_emac? Is it required for some AT91's because macb does not work there?Does it provide better "results" than macb?
Best Regards
Reinhard
Hello,
I think the separation has historical reasons.
For AT91RM and AT91SAM existed separate drivers and separate arch. I remeber in older datascheets the ethernet inteface also marked with MACB. In my opinion, the at91_emac the latest drivers (NET_MULTI api SoC access via c structurs).
But I dont know the exact differences are not of single use devices. Please ask Xu, Hong
Best regards
Jens
Xu
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
Hi,
Quoted 29 lines
-----Original Message-----
From: Jens Scharsig [mailto:js_at_ng at scharsoft.de]
Sent: Thursday, September 02, 2010 1:37 AM
To: Reinhard Meyer
Cc: Xu, Hong; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH V3] AT91 Fix: return value ofget_tbclkAm 2010-09-01 09:30, schrieb Reinhard Meyer:
Quoted 5 lines
at91_emacAND macb?macb works for AVR32AP700x and (at least) for AT91SAM9260/9G20/9XE.
Quoted 7 lines
What is the deal with
at91_emac? Is it required for some AT91's because macb does not work there?Does it provide better "results" than macb?
Best Regards
ReinhardHello,
I think the separation has historical reasons.
For AT91RM and AT91SAM existed separate drivers and separate arch. I remeber in older datascheets the ethernet inteface also marked with MACB.
In my opinion, theat91_emacthe latest drivers (NET_MULTIapi SoC access via c structurs).
Quoted 2 lines
But I dont know the exact differences are not of single use devices. Please ask Xu, Hong
I believe there must be historic reasons. I'm not familiar with AT91RM series. But from the mainline code,
$ grep CONFIG_DRIVER_AT91EMAC include/configs/* | wc -l 11 $ grep CONFIG_MACB include/configs/* | wc -l 19
Many boards are dependent on both. And currently AT91SAM always uses CONFIG_MACB, I don't see any change in the near future.
BR,
Eric
PATCH v3 1/1 AT91 Fix: return value of get_tbclk
In reply to Jens ScharsigHi,
Am 01.09.2010 19:37, schrieb Jens Scharsig:
Quoted 6 lines
Am 2010-09-01 09:30, schrieb Reinhard Meyer:
Quoted 5 lines
at91_emacAND macb?macb works for AVR32AP700x and (at least) for AT91SAM9260/9G20/9XE.
Quoted 8 lines
Quoted 4 lines
What is the deal with
at91_emac? Is it required for some AT91's because macb does not work there?Does it provide better "results" than macb?
I think the separation has historical reasons.
For AT91RM and AT91SAM existed separate drivers and separate arch. I remeber in older datascheets the ethernet inteface also marked with
MACB.
Quoted 2 lines
In my opinion, the
at91_emacthe latest drivers (NET_MULTIapi SoC access via c structurs).
currently at91_emac is used for AT91RM9200 devices cause this register footprint differs to the one of MACB used in SAM9/AVR32 devices. Both subsystems use similar namings but some register flags have different offsets. MACB additionally seems to have more features than EMAC (namely WOL register, some more flags in status register, ...).
But I think it is possible to merge both drivers cause except for the different register footprint they both seems to work the same way. I already have some improvements for at91_emac on stack and can look for possibilities merging both.
regards
Andreas Bie?mann