Hi Bin, On 25 April 2018 at 21:42, Bin Meng <bmeng...@gmail.com> wrote: > Hi Ivan, > > On Tue, Apr 24, 2018 at 4:41 PM, Bin Meng <bmeng...@gmail.com> wrote: >> Hi Ivan, >> >> On Tue, Apr 24, 2018 at 7:56 AM, Ivan Gorinov <ivan.gori...@intel.com> wrote: >>> Hi Bin, >>> >>> On Mon, Apr 23, 2018 at 01:38:05AM -0600, Bin Meng wrote: >>>> > Coreboot timestamp functions and Quark memory reference code use >>>> > get_tbclk() to get TSC frequency. This will not work if another >>>> > early timer is selected. >>>> >>>> Thanks for working on this. But get_tbclk() is one API provided by the >>>> timer library. The get_tbclk_mhz() is something that is implemented by >>>> the TSC timer driver, so can we get rid of the get_tbclk_mhz() and use >>>> the get_tbclk() instead in coreboot/timestamp.c and quark/mrc_util.c? >>> >>> The Coreboot timestamp code and Quark MRC specifically use rdtsc(). >>> We can replace it with timer_early_get_count() or provide a function >>> to get the TSC frequency even when another early timer is selected. >>> >> >> Good catch. Yes, we should fix coreboot timestamp code and Quark MRC >> codes to not explicitly call rdtsc. >> > > Further checking the coreboot timestamp codes, I think we may have to > leave the coreboot timestamp codes as it is now. > > We have the codes blow: > void timestamp_add_now(enum timestamp_id id) > { > timestamp_add(id, rdtsc()); > } > > We cannot replace rdtsc() with timer_early_get_count(), because this > timestamp_add_now() is called both before and after DM initialization. > If the HPET is selected as the early timer and TSC is selected as the > normal timer, the timestamp numbers are meaningless to compare against > each other. > >> Another driver that explicitly calls rdtsc() is hw_watchdog_reset() in >> watchdog/tangier_wdt.c driver. We need fix that too. >> > > Simon, another issue is the bootstage support. So far the > timer_get_boot_us() is not implemented by DM timer APIs. > timer_get_boot_us() is implemented per timer driver if > CONFIG_SYS_TIMER_COUNTER is not defined. Note CONFIG_SYS_TIMER_COUNTER > is non-DM stuff. That means the bootstage support is bounded by a > specific timer driver, instead of a generic library. To me this > overall timer support is somehow fragmentary.
Yes I agree. I'm open to ideas and patches :-) Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot