| Message ID | 1362387637-32334-1-git-send-email-dirk.behme@de.bosch.com |
|---|---|
| State | Superseded |
| Headers | show |
Dear Dirk Behme, In message <1362387637-32334-1-git-send-email-dirk.behme@de.bosch.com> you wrote: > From: Dirk Behme <dirk.behme@de.bosch.com> > > Several ARM timer implementations use gd->arch.tbl to record the > absolute tick count of 32-bit counters, including timer overflows. > For example arch/arm/imx-common/timer.c does: > > ulong lastinc; > ulong now = counter value; > if (no overflow) { > ... > } else { /* counter overflow */ > gd->arch.tbl += (0xFFFFFFFF - lastinc) + now; > } > lastinc = now; > > As we use a 32-bit counter and the two ulong (32-bit) variables 'lastinc' > and 'now' here, gd->arch.tbl should be long long (64-bit) to not overflow > at the same time, too. I think this is wrong. "tbl" means "time base, lower 32 bits" and is complemented by "tbu" (time base, upper 32 bits) to form a 64 bit time base counter. If you need the full 64 bit precision, then please either maintain the carry manually, or use a proper union or similar. Best regards, Wolfgang Denk
Dear Wolfgang, On 04.03.2013 12:10, Wolfgang Denk wrote: > Dear Dirk Behme, > > In message <1362387637-32334-1-git-send-email-dirk.behme@de.bosch.com> you wrote: >> From: Dirk Behme <dirk.behme@de.bosch.com> >> >> Several ARM timer implementations use gd->arch.tbl to record the >> absolute tick count of 32-bit counters, including timer overflows. >> For example arch/arm/imx-common/timer.c does: >> >> ulong lastinc; >> ulong now = counter value; >> if (no overflow) { >> ... >> } else { /* counter overflow */ >> gd->arch.tbl += (0xFFFFFFFF - lastinc) + now; >> } >> lastinc = now; >> >> As we use a 32-bit counter and the two ulong (32-bit) variables 'lastinc' >> and 'now' here, gd->arch.tbl should be long long (64-bit) to not overflow >> at the same time, too. > > I think this is wrong. > > "tbl" means "time base, lower 32 bits" and is complemented by "tbu" > (time base, upper 32 bits) to form a 64 bit time base counter. > > If you need the full 64 bit precision, then please either maintain the > carry manually, or use a proper union or similar. Many thanks for this explanation! This patch is obsolete now, replaced by http://patchwork.ozlabs.org/patch/224740/ Thanks Dirk
Dear Dirk, In message <5134ADB7.2000106@de.bosch.com> you wrote: > > Many thanks for this explanation! You are welcome... > This patch is obsolete now, replaced by > > http://patchwork.ozlabs.org/patch/224740/ Thanks, looks good to me. I hesitate to add an Acked-by: or even Reviewed-by: as this timer stuff on ARM is a bit of black magic to me... Best regards, Wolfgang Denk
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 37ac0da..1099af9 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -41,7 +41,7 @@ struct arch_global_data { /* "static data" needed by most of timer.c on ARM platforms */ unsigned long timer_rate_hz; unsigned long tbu; - unsigned long tbl; + unsigned long long tbl; unsigned long lastinc; unsigned long long timer_reset_value; #ifdef CONFIG_IXP425