[U-Boot] [PATCH] imx-common: timer: fix 32-bit overflow

[Stefano Babic]

On 11/03/2013 07:44, Dirk Behme wrote:
> On 04.03.2013 15:16, Dirk Behme wrote:
>> From: Knut Wohlrab <knut.wohlrab at de.bosch.com>
>>
>> The i.MX6 common timer uses the 32-bit variable tbl (time base lower)
>> to record the overflow of the 32-bit counter. I.e. if the counter
>> overflows, the variable tbl does overflow, too.
>>
>> To capture this overflow, use the variable tbu (time base upper), too.
>> Return the combined value of tbl and tbu.
>>
>> lastinc is unused then, remove it.
>>
>> Signed-off-by: Knut Wohlrab <knut.wohlrab at de.bosch.com>
>> Signed-off-by: Dirk Behme <dirk.behme at de.bosch.com>
>> ---
>> Note: This replaces the patch
>>       http://patchwork.ozlabs.org/patch/224646/
>>
>>  arch/arm/imx-common/timer.c |   26 +++++++-------------------
>>  1 file changed, 7 insertions(+), 19 deletions(-)
>>
>> Index: freescale-u-boot-imx.git/arch/arm/imx-common/timer.c
>> ===================================================================
>> --- freescale-u-boot-imx.git.orig/arch/arm/imx-common/timer.c
>> +++ freescale-u-boot-imx.git/arch/arm/imx-common/timer.c
>> @@ -48,9 +48,6 @@ static struct mxc_gpt *cur_gpt = (struct
>>  
>>  DECLARE_GLOBAL_DATA_PTR;
>>  
>> -#define timestamp (gd->arch.tbl)
>> -#define lastinc (gd->arch.lastinc)
>> -
>>  static inline unsigned long long tick_to_time(unsigned long long tick)
>>  {
>>      tick *= CONFIG_SYS_HZ;
>> @@ -70,7 +67,6 @@ static inline unsigned long long us_to_t
>>  int timer_init(void)
>>  {
>>      int i;
>> -    ulong val;
>>  
>>      /* setup GP Timer 1 */
>>      __raw_writel(GPTCR_SWR, &cur_gpt->control);
>> @@ -85,9 +81,8 @@ int timer_init(void)
>>      i = __raw_readl(&cur_gpt->control);
>>      __raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, &cur_gpt->control);
>>  
>> -    val = __raw_readl(&cur_gpt->counter);
>> -    lastinc = val / (MXC_CLK32 / CONFIG_SYS_HZ);
>> -    timestamp = 0;
>> +    gd->arch.tbl = __raw_readl(&cur_gpt->counter);
>> +    gd->arch.tbu = 0;
>>  
>>      return 0;
>>  }
>> @@ -96,18 +91,11 @@ unsigned long long get_ticks(void)
>>  {
>>      ulong now = __raw_readl(&cur_gpt->counter); /* current tick value */
>>  
>> -    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;
>> +    /* increment tbu if tbl has rolled over */
>> +    if (now < gd->arch.tbl)
>> +        gd->arch.tbu++;
>> +    gd->arch.tbl = now;
>> +    return (((unsigned long long)gd->arch.tbu) << 32) | gd->arch.tbl;
>>  }
>>  
>>  ulong get_timer_masked(void)
>>
> 
> Any comments on this?
> 
> Many thanks


Hi Dirk,

no thanks to fix this coherently with other SOCs. Checking your patch I
get convinced that we have a lot of duplicated code (get_ticks() for
example) in most SOCs, but let it for a further improvement.

Acked-by: Stefano Babic <sbabic at denx.de>

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================