* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
@ 2010-08-07 17:49 Jens Scharsig
2010-08-07 18:07 ` Reinhard Meyer
2010-08-09 6:34 ` Alexander Stein
0 siblings, 2 replies; 12+ messages in thread
From: Jens Scharsig @ 2010-08-07 17:49 UTC (permalink / raw)
To: u-boot
* Fix: return value of get_tbclk
* this fixes issue with prematurely restart/retry, if BOOT_RETRY_TIMEOUT is used
Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
---
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(-)
diff --git a/arch/arm/cpu/arm926ejs/at91/timer.c b/arch/arm/cpu/arm926ejs/at91/timer.c
index d21eebf..8efc34b 100644
--- a/arch/arm/cpu/arm926ejs/at91/timer.c
+++ b/arch/arm/cpu/arm926ejs/at91/timer.c
@@ -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.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-07 17:49 [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk Jens Scharsig
@ 2010-08-07 18:07 ` Reinhard Meyer
2010-08-08 10:52 ` Jens Scharsig
2010-08-09 6:34 ` Alexander Stein
1 sibling, 1 reply; 12+ messages in thread
From: Reinhard Meyer @ 2010-08-07 18:07 UTC (permalink / raw)
To: u-boot
Jens Scharsig wrote:
> * Fix: return value of get_tbclk
> * this fixes issue with prematurely restart/retry, if BOOT_RETRY_TIMEOUT is used
>
>
> Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
> ---
> 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(-)
>
> diff --git a/arch/arm/cpu/arm926ejs/at91/timer.c b/arch/arm/cpu/arm926ejs/at91/timer.c
> index d21eebf..8efc34b 100644
> --- a/arch/arm/cpu/arm926ejs/at91/timer.c
> +++ b/arch/arm/cpu/arm926ejs/at91/timer.c
> @@ -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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-07 18:07 ` Reinhard Meyer
@ 2010-08-08 10:52 ` Jens Scharsig
0 siblings, 0 replies; 12+ messages in thread
From: Jens Scharsig @ 2010-08-08 10:52 UTC (permalink / raw)
To: u-boot
Dear Reinhard
> I think the timer.c is more broken that just that return value:
>
You are right, but the patch fixes only the single small problem.
> 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
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)
>
> 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.
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.
>
> 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.
>
> Greetings, Reinhard
Best regards
Jens Scharsig
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-07 17:49 [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk Jens Scharsig
2010-08-07 18:07 ` Reinhard Meyer
@ 2010-08-09 6:34 ` Alexander Stein
2010-08-18 7:17 ` Xu, Hong
1 sibling, 1 reply; 12+ messages in thread
From: Alexander Stein @ 2010-08-09 6:34 UTC (permalink / raw)
To: u-boot
Hello,
Am Samstag, 7. August 2010, 19:49:42 schrieb Jens Scharsig:
> * Fix: return value of get_tbclk
> * this fixes issue with prematurely restart/retry, if BOOT_RETRY_TIMEOUT
> is used
This issue also arises, if you use CONFIG_AUTOBOOT_KEYED & friends
> Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
> ---
> 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(-)
>
> diff --git a/arch/arm/cpu/arm926ejs/at91/timer.c
> b/arch/arm/cpu/arm926ejs/at91/timer.c index d21eebf..8efc34b 100644
> --- a/arch/arm/cpu/arm926ejs/at91/timer.c
> +++ b/arch/arm/cpu/arm926ejs/at91/timer.c
> @@ -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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-09 6:34 ` Alexander Stein
@ 2010-08-18 7:17 ` Xu, Hong
2010-08-30 6:52 ` Alexander Stein
0 siblings, 1 reply; 12+ messages in thread
From: Xu, Hong @ 2010-08-18 7:17 UTC (permalink / raw)
To: u-boot
Hi Alex,
> -----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 of get_tbclk
>
> Hello,
>
> Am Samstag, 7. August 2010, 19:49:42 schrieb Jens Scharsig:
> > * Fix: return value of get_tbclk
> > * this fixes issue with prematurely restart/retry, if
> > BOOT_RETRY_TIMEOUT is used
>
> This 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-18 7:17 ` Xu, Hong
@ 2010-08-30 6:52 ` Alexander Stein
2010-08-31 7:36 ` Reinhard Meyer
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Stein @ 2010-08-30 6:52 UTC (permalink / raw)
To: u-boot
Hello Eric,
On Wednesday 18 August 2010, 09:17:26 you wrote:
> > Hello,
> >
> > Am Samstag, 7. August 2010, 19:49:42 schrieb Jens Scharsig:
> > > * Fix: return value of get_tbclk
> > > * this fixes issue with prematurely restart/retry, if
> > >
> > > BOOT_RETRY_TIMEOUT is used
> >
> > This 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?
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-30 6:52 ` Alexander Stein
@ 2010-08-31 7:36 ` Reinhard Meyer
2010-08-31 16:22 ` Jens Scharsig
0 siblings, 1 reply; 12+ messages in thread
From: Reinhard Meyer @ 2010-08-31 7:36 UTC (permalink / raw)
To: u-boot
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
Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
---
cpu/arm926ejs/at91/timer.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/cpu/arm926ejs/at91/timer.c b/cpu/arm926ejs/at91/timer.c
index d21eebf..8efc34b 100644
--- a/cpu/arm926ejs/at91/timer.c
+++ b/cpu/arm926ejs/at91/timer.c
@@ -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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-31 7:36 ` Reinhard Meyer
@ 2010-08-31 16:22 ` Jens Scharsig
2010-09-01 7:30 ` Reinhard Meyer
0 siblings, 1 reply; 12+ messages in thread
From: Jens Scharsig @ 2010-08-31 16:22 UTC (permalink / raw)
To: u-boot
Am 2010-08-31 09:36, schrieb Reinhard Meyer:
> Hi,
> }
> -- 1.6.0.2
>
> Applied 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-08-31 16:22 ` Jens Scharsig
@ 2010-09-01 7:30 ` Reinhard Meyer
2010-09-01 17:37 ` Jens Scharsig
0 siblings, 1 reply; 12+ messages in thread
From: Reinhard Meyer @ 2010-09-01 7:30 UTC (permalink / raw)
To: u-boot
Jens Scharsig schrieb:
> Am 2010-08-31 09:36, schrieb Reinhard Meyer:
>> Hi,
>> }
>> -- 1.6.0.2
>>
>> Applied 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-09-01 7:30 ` Reinhard Meyer
@ 2010-09-01 17:37 ` Jens Scharsig
2010-09-02 1:55 ` Xu, Hong
2010-09-02 6:58 ` Andreas Bießmann
0 siblings, 2 replies; 12+ messages in thread
From: Jens Scharsig @ 2010-09-01 17:37 UTC (permalink / raw)
To: u-boot
Am 2010-09-01 09:30, schrieb Reinhard Meyer:
>
> 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
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-09-01 17:37 ` Jens Scharsig
@ 2010-09-02 1:55 ` Xu, Hong
2010-09-02 6:58 ` Andreas Bießmann
1 sibling, 0 replies; 12+ messages in thread
From: Xu, Hong @ 2010-09-02 1:55 UTC (permalink / raw)
To: u-boot
Hi,
> -----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 of get_tbclk
>
> Am 2010-09-01 09:30, schrieb Reinhard Meyer:
> >
> > 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
> 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
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
2010-09-01 17:37 ` Jens Scharsig
2010-09-02 1:55 ` Xu, Hong
@ 2010-09-02 6:58 ` Andreas Bießmann
1 sibling, 0 replies; 12+ messages in thread
From: Andreas Bießmann @ 2010-09-02 6:58 UTC (permalink / raw)
To: u-boot
Hi,
Am 01.09.2010 19:37, schrieb Jens Scharsig:
> Am 2010-09-01 09:30, schrieb Reinhard Meyer:
>>
>> 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?
> 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).
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-09-02 6:58 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-07 17:49 [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk Jens Scharsig
2010-08-07 18:07 ` Reinhard Meyer
2010-08-08 10:52 ` Jens Scharsig
2010-08-09 6:34 ` Alexander Stein
2010-08-18 7:17 ` Xu, Hong
2010-08-30 6:52 ` Alexander Stein
2010-08-31 7:36 ` Reinhard Meyer
2010-08-31 16:22 ` Jens Scharsig
2010-09-01 7:30 ` Reinhard Meyer
2010-09-01 17:37 ` Jens Scharsig
2010-09-02 1:55 ` Xu, Hong
2010-09-02 6:58 ` Andreas Bießmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.