| Message ID | 1446732171-7439-5-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Bin, On 2015年11月05日 22:02, Bin Meng wrote: > There are timers with a 64-bit counter value but current timer > uclass driver assumes a 32-bit one. Introduce a device tree > property "counter-64bit", and modify timer_get_count() in the > timer uclass driver to handle the 32-bit/64-bit conversion > automatically. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- Thanks a lot. I tested patches [1-4] on nios2 boards with "ut time" and "sleep". It works fine. I would suggest that we do not need the additional "counter-64bit" property by requiring the timer always return 64 bits count. We can provide an inline func to do the 32/64 conversion for all 32 bits timers. Best regards, Thomas
Hi Thomas, On Fri, Nov 6, 2015 at 3:14 PM, Thomas Chou <thomas@wytron.com.tw> wrote: > Hi Bin, > > On 2015年11月05日 22:02, Bin Meng wrote: >> >> There are timers with a 64-bit counter value but current timer >> uclass driver assumes a 32-bit one. Introduce a device tree >> property "counter-64bit", and modify timer_get_count() in the >> timer uclass driver to handle the 32-bit/64-bit conversion >> automatically. >> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> --- > > > Thanks a lot. I tested patches [1-4] on nios2 boards with "ut time" and > "sleep". It works fine. > > I would suggest that we do not need the additional "counter-64bit" property > by requiring the timer always return 64 bits count. We can provide an inline > func to do the 32/64 conversion for all 32 bits timers. > Do you mean we move the following codes: + /* increment tbh if tbl has rolled over */ + if (now < gd->timebase_l) + gd->timebase_h++; + gd->timebase_l = now; + *count = ((u64)gd->timebase_h << 32) | gd->timebase_l; to an inline function, to have 32-bit timer driver to call this inline function in their get_count() op? This looks duplicate to me. Regards, Bin
Hi Bin, On 2015年11月06日 22:35, Bin Meng wrote: > Hi Thomas, > > On Fri, Nov 6, 2015 at 3:14 PM, Thomas Chou <thomas@wytron.com.tw> wrote: >> Hi Bin, >> >> On 2015年11月05日 22:02, Bin Meng wrote: >>> >>> There are timers with a 64-bit counter value but current timer >>> uclass driver assumes a 32-bit one. Introduce a device tree >>> property "counter-64bit", and modify timer_get_count() in the >>> timer uclass driver to handle the 32-bit/64-bit conversion >>> automatically. >>> >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> --- >> >> >> Thanks a lot. I tested patches [1-4] on nios2 boards with "ut time" and >> "sleep". It works fine. >> >> I would suggest that we do not need the additional "counter-64bit" property >> by requiring the timer always return 64 bits count. We can provide an inline >> func to do the 32/64 conversion for all 32 bits timers. >> > > Do you mean we move the following codes: > > + /* increment tbh if tbl has rolled over */ > + if (now < gd->timebase_l) > + gd->timebase_h++; > + gd->timebase_l = now; > + *count = ((u64)gd->timebase_h << 32) | gd->timebase_l; > > to an inline function, to have 32-bit timer driver to call this inline > function in their get_count() op? > > This looks duplicate to me. Yes. If we want to support 64 bits count, it should be the driver's responsibility to return 64 bits count. It would be more natural than adding a property to tell the uclass to to the conversion. The "duplicate" you mentioned could be an alternative to the "counter-64bit" addition. I believe 64 bits hardware counter will become more popular in the future. Best regards, Thomas
Hi Thomas, On Sat, Nov 7, 2015 at 6:40 AM, Thomas Chou <thomas@wytron.com.tw> wrote: > Hi Bin, > > > On 2015年11月06日 22:35, Bin Meng wrote: >> >> Hi Thomas, >> >> On Fri, Nov 6, 2015 at 3:14 PM, Thomas Chou <thomas@wytron.com.tw> wrote: >>> >>> Hi Bin, >>> >>> On 2015年11月05日 22:02, Bin Meng wrote: >>>> >>>> >>>> There are timers with a 64-bit counter value but current timer >>>> uclass driver assumes a 32-bit one. Introduce a device tree >>>> property "counter-64bit", and modify timer_get_count() in the >>>> timer uclass driver to handle the 32-bit/64-bit conversion >>>> automatically. >>>> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>> --- >>> >>> >>> >>> Thanks a lot. I tested patches [1-4] on nios2 boards with "ut time" and >>> "sleep". It works fine. >>> >>> I would suggest that we do not need the additional "counter-64bit" >>> property >>> by requiring the timer always return 64 bits count. We can provide an >>> inline >>> func to do the 32/64 conversion for all 32 bits timers. >>> >> >> Do you mean we move the following codes: >> >> + /* increment tbh if tbl has rolled over */ >> + if (now < gd->timebase_l) >> + gd->timebase_h++; >> + gd->timebase_l = now; >> + *count = ((u64)gd->timebase_h << 32) | gd->timebase_l; >> >> to an inline function, to have 32-bit timer driver to call this inline >> function in their get_count() op? >> >> This looks duplicate to me. > > > Yes. If we want to support 64 bits count, it should be the driver's > responsibility to return 64 bits count. It would be more natural than adding > a property to tell the uclass to to the conversion. The "duplicate" you > mentioned could be an alternative to the "counter-64bit" addition. I believe > 64 bits hardware counter will become more popular in the future. > OK, will do in v2 and rebase on top of sandbox timer patches. Regards, Bin
diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c index e4d0301..b363b19 100644 --- a/drivers/timer/altera_timer.c +++ b/drivers/timer/altera_timer.c @@ -34,7 +34,7 @@ struct altera_timer_platdata { #define ALTERA_TIMER_START (1 << 2) /* Start timer */ #define ALTERA_TIMER_STOP (1 << 3) /* Stop timer */ -static int altera_timer_get_count(struct udevice *dev, unsigned long *count) +static int altera_timer_get_count(struct udevice *dev, u64 *count) { struct altera_timer_platdata *plat = dev->platdata; struct altera_timer_regs *const regs = plat->regs; diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index 0218591..8451b40 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -13,21 +13,39 @@ DECLARE_GLOBAL_DATA_PTR; /* * Implement a timer uclass to work with lib/time.c. The timer is usually - * a 32 bits free-running up counter. The get_rate() method is used to get + * a 32/64 bits free-running up counter. The get_rate() method is used to get * the input clock frequency of the timer. The get_count() method is used - * to get the current 32 bits count value. If the hardware is counting down, + * to get the current 32/64 bits count value. If the hardware is counting down, * the value should be inversed inside the method. There may be no real * tick, and no timer interrupt. */ -int timer_get_count(struct udevice *dev, unsigned long *count) +int timer_get_count(struct udevice *dev, u64 *count) { const struct timer_ops *ops = device_get_ops(dev); + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); + u32 now; + int ret; if (!ops->get_count) return -ENOSYS; - return ops->get_count(dev, count); + if (uc_priv->counter_64) { + return ops->get_count(dev, count); + } else { + ret = ops->get_count(dev, count); + if (ret) + return ret; + + now = *count; + /* increment tbh if tbl has rolled over */ + if (now < gd->timebase_l) + gd->timebase_h++; + gd->timebase_l = now; + *count = ((u64)gd->timebase_h << 32) | gd->timebase_l; + + return 0; + } } unsigned long timer_get_rate(struct udevice *dev) @@ -43,6 +61,8 @@ static int timer_pre_probe(struct udevice *dev) uc_priv->clock_rate = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock-frequency", 0); + uc_priv->counter_64 = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, + "counter-64bit"); return 0; } diff --git a/include/timer.h b/include/timer.h index ed5c685..4e061ed 100644 --- a/include/timer.h +++ b/include/timer.h @@ -14,7 +14,7 @@ * @count: pointer that returns the current timer count * @return: 0 if OK, -ve on error */ -int timer_get_count(struct udevice *dev, unsigned long *count); +int timer_get_count(struct udevice *dev, u64 *count); /* * Get the timer input clock frequency @@ -38,16 +38,18 @@ struct timer_ops { * @count: pointer that returns the current timer count * @return: 0 if OK, -ve on error */ - int (*get_count)(struct udevice *dev, unsigned long *count); + int (*get_count)(struct udevice *dev, u64 *count); }; /* * struct timer_dev_priv - information about a device used by the uclass * * @clock_rate: the timer input clock frequency + * @counter_64: the timer counter is a 64-bit value or not */ struct timer_dev_priv { unsigned long clock_rate; + bool counter_64; }; #endif /* _TIMER_H_ */ diff --git a/lib/time.c b/lib/time.c index b001745..f37a662 100644 --- a/lib/time.c +++ b/lib/time.c @@ -69,9 +69,9 @@ ulong notrace get_tbclk(void) return timer_get_rate(gd->timer); } -unsigned long notrace timer_read_counter(void) +uint64_t notrace get_ticks(void) { - unsigned long count; + u64 count; int ret; ret = dm_timer_init(); @@ -84,7 +84,8 @@ unsigned long notrace timer_read_counter(void) return count; } -#endif /* CONFIG_TIMER */ + +#else /* !CONFIG_TIMER */ uint64_t __weak notrace get_ticks(void) { @@ -97,6 +98,8 @@ uint64_t __weak notrace get_ticks(void) return ((uint64_t)gd->timebase_h << 32) | gd->timebase_l; } +#endif /* CONFIG_TIMER */ + /* Returns time in milliseconds */ static uint64_t notrace tick_to_time(uint64_t tick) {
There are timers with a 64-bit counter value but current timer uclass driver assumes a 32-bit one. Introduce a device tree property "counter-64bit", and modify timer_get_count() in the timer uclass driver to handle the 32-bit/64-bit conversion automatically. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- drivers/timer/altera_timer.c | 2 +- drivers/timer/timer-uclass.c | 28 ++++++++++++++++++++++++---- include/timer.h | 6 ++++-- lib/time.c | 9 ++++++--- 4 files changed, 35 insertions(+), 10 deletions(-)