diff mbox series

[U-Boot,v6,1/2] x86: Add TSC-specific timer functions

Message ID 269698c303c3da454b34f149d4a3562fc57091f2.1523570597.git.ivan.gorinov@intel.com
State Deferred
Delegated to: Bin Meng
Headers show
Series timer: Add High Precision Event Timers (HPET) support | expand

Commit Message

Ivan Gorinov April 12, 2018, 10:12 p.m. UTC
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.

Add tsc_rate_mhz() function and use it in the code that specifically
needs to get TSC rate regardless of currently selected early timer.

Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
---
 arch/x86/cpu/coreboot/timestamp.c |  2 +-
 arch/x86/cpu/quark/mrc_util.c     | 13 ++++++-------
 arch/x86/include/asm/u-boot-x86.h |  2 +-
 drivers/timer/tsc_timer.c         | 33 ++++++++++++++++++++++++---------
 4 files changed, 32 insertions(+), 18 deletions(-)

Comments

Andy Shevchenko April 20, 2018, 12:25 p.m. UTC | #1
On Thu, 2018-04-12 at 15:12 -0700, Ivan Gorinov 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.
> 
> Add tsc_rate_mhz() function and use it in the code that specifically
> needs to get TSC rate regardless of currently selected early timer.


>  void delay_n(uint32_t ns)
>  {
>  	/* 1000 MHz clock has 1ns period --> no conversion required
> */
> -	uint64_t final_tsc = rdtsc();
> +	uint64_t start_tsc = rdtsc();
> +	uint64_t ticks;
>  
> -	final_tsc += ((get_tbclk_mhz() * ns) / 1000);
> -
> -	while (rdtsc() < final_tsc)
> -		;
> +	ticks = (tsc_rate_mhz() * ns) / 1000;

> +	while (rdtsc() - start_tsc < ticks);

I would rather preserve existing style.

>  }
 
>  /* Delay number of microseconds */
> -void delay_u(uint32_t ms)
> +void delay_u(uint32_t us)
>  {
>  	/* 64-bit math is not an option, just use loops */
> -	while (ms--)
> +	while (us--)
>  		delay_n(1000);
>  }

This is a separate change.
Ivan Gorinov April 20, 2018, 6 p.m. UTC | #2
On Fri, Apr 20, 2018 at 06:25:08AM -0600, Andy Shevchenko 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.
> > 
> > Add tsc_rate_mhz() function and use it in the code that specifically
> > needs to get TSC rate regardless of currently selected early timer.
> 
> 
> >  void delay_n(uint32_t ns)
> >  {
> >  	/* 1000 MHz clock has 1ns period --> no conversion required
> > */
> > -	uint64_t final_tsc = rdtsc();
> > +	uint64_t start_tsc = rdtsc();
> > +	uint64_t ticks;
> >  
> > -	final_tsc += ((get_tbclk_mhz() * ns) / 1000);
> > -
> > -	while (rdtsc() < final_tsc)
> > -		;
> > +	ticks = (tsc_rate_mhz() * ns) / 1000;
> 
> > +	while (rdtsc() - start_tsc < ticks);
> 
> I would rather preserve existing style.

OK. Existing style does not correctly handle overflow,
but for a 64-bit counter it's a very unlikely event.
Bin Meng April 23, 2018, 7:38 a.m. UTC | #3
Hi Ivan,

On Fri, Apr 13, 2018 at 6:12 AM, Ivan Gorinov <ivan.gorinov@intel.com> 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?

> Add tsc_rate_mhz() function and use it in the code that specifically
> needs to get TSC rate regardless of currently selected early timer.
>
> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
> ---
>  arch/x86/cpu/coreboot/timestamp.c |  2 +-
>  arch/x86/cpu/quark/mrc_util.c     | 13 ++++++-------
>  arch/x86/include/asm/u-boot-x86.h |  2 +-
>  drivers/timer/tsc_timer.c         | 33 ++++++++++++++++++++++++---------
>  4 files changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/cpu/coreboot/timestamp.c b/arch/x86/cpu/coreboot/timestamp.c
> index b382795..05bb214 100644
> --- a/arch/x86/cpu/coreboot/timestamp.c
> +++ b/arch/x86/cpu/coreboot/timestamp.c
> @@ -78,7 +78,7 @@ int timestamp_add_to_bootstage(void)
>                 if (name) {
>                         bootstage_add_record(0, name, BOOTSTAGEF_ALLOC,
>                                              tse->entry_stamp /
> -                                                       get_tbclk_mhz());
> +                                                       tsc_rate_mhz());
>                 }
>         }
>
> diff --git a/arch/x86/cpu/quark/mrc_util.c b/arch/x86/cpu/quark/mrc_util.c
> index fac2d72..4794395 100644
> --- a/arch/x86/cpu/quark/mrc_util.c
> +++ b/arch/x86/cpu/quark/mrc_util.c
> @@ -57,19 +57,18 @@ void mrc_post_code(uint8_t major, uint8_t minor)
>  void delay_n(uint32_t ns)
>  {
>         /* 1000 MHz clock has 1ns period --> no conversion required */
> -       uint64_t final_tsc = rdtsc();
> +       uint64_t start_tsc = rdtsc();
> +       uint64_t ticks;
>
> -       final_tsc += ((get_tbclk_mhz() * ns) / 1000);
> -
> -       while (rdtsc() < final_tsc)
> -               ;
> +       ticks = (tsc_rate_mhz() * ns) / 1000;
> +       while (rdtsc() - start_tsc < ticks);
>  }
>
>  /* Delay number of microseconds */
> -void delay_u(uint32_t ms)
> +void delay_u(uint32_t us)
>  {
>         /* 64-bit math is not an option, just use loops */
> -       while (ms--)
> +       while (us--)
>                 delay_n(1000);
>  }
>
> diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
> index 187fe5f..de68120 100644
> --- a/arch/x86/include/asm/u-boot-x86.h
> +++ b/arch/x86/include/asm/u-boot-x86.h
> @@ -29,7 +29,7 @@ int cleanup_before_linux(void);
>  void timer_isr(void *);
>  typedef void (timer_fnc_t) (void);
>  int register_timer_isr (timer_fnc_t *isr_func);
> -unsigned long get_tbclk_mhz(void);
> +unsigned long tsc_rate_mhz(void);
>  void timer_set_base(uint64_t base);
>  int i8254_init(void);
>
> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
> index 9296de6..98cbf12 100644
> --- a/drivers/timer/tsc_timer.c
> +++ b/drivers/timer/tsc_timer.c
> @@ -277,15 +277,12 @@ success:
>         return delta / 1000;
>  }
>
> -/* Get the speed of the TSC timer in MHz */
> -unsigned notrace long get_tbclk_mhz(void)
> -{
> -       return get_tbclk() / 1000000;
> -}
> -
>  static ulong get_ms_timer(void)
>  {
> -       return (get_ticks() * 1000) / get_tbclk();
> +       if (gd->arch.clock_rate == 0)
> +               return 0;
> +
> +       return (rdtsc() * 1000) / gd->arch.clock_rate;
>  }
>
>  ulong get_timer(ulong base)
> @@ -295,7 +292,10 @@ ulong get_timer(ulong base)
>
>  ulong notrace timer_get_us(void)
>  {
> -       return get_ticks() / get_tbclk_mhz();
> +       if (gd->arch.clock_rate == 0)
> +               return 0;
> +
> +       return (rdtsc() * 1000000) / gd->arch.clock_rate;
>  }
>
>  ulong timer_get_boot_us(void)
> @@ -308,7 +308,7 @@ void __udelay(unsigned long usec)
>         u64 now = get_ticks();
>         u64 stop;
>
> -       stop = now + usec * get_tbclk_mhz();
> +       stop = now + ((u64)usec * gd->arch.clock_rate) / 1000000;
>
>         while ((int64_t)(stop - get_ticks()) > 0)
>  #if defined(CONFIG_QEMU) && defined(CONFIG_SMP)
> @@ -355,6 +355,14 @@ static void tsc_timer_ensure_setup(void)
>         }
>  }
>
> +/* Get the speed of the TSC timer in MHz */
> +unsigned notrace long tsc_rate_mhz(void)
> +{
> +       tsc_timer_ensure_setup();
> +
> +       return gd->arch.clock_rate / 1000000;
> +}
> +
>  static int tsc_timer_probe(struct udevice *dev)
>  {
>         struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> @@ -365,6 +373,13 @@ static int tsc_timer_probe(struct udevice *dev)
>         return 0;
>  }
>
> +int timer_init(void)
> +{
> +       tsc_timer_ensure_setup();
> +
> +       return 0;
> +}
> +
>  unsigned long notrace timer_early_get_rate(void)
>  {
>         tsc_timer_ensure_setup();
> --

Regards,
Bin
Bin Meng April 23, 2018, 7:53 a.m. UTC | #4
Hi Ivan,

On Mon, Apr 23, 2018 at 3:38 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Ivan,
>
> On Fri, Apr 13, 2018 at 6:12 AM, Ivan Gorinov <ivan.gorinov@intel.com> 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?
>

Further request on TSC timer driver clean up, in order to make HPET
work with either TSC or HPET being the U-Boot (early) timer, we
should:

- Remove get_tbclk_mhz() in tsc_timer.c
- Remove get_timer() in tsc_timer.c
- Remove timer_get_us() in tsc_timer.c
- Remove timer_get_boot_us() in tsc_timer.c
- Move __udelay() implementation in tsc_timer.c to
arch/x86/cpu/qemu/qemu.c, as this is the QEMU specific support. For
other platforms, use the default one provided in lib/time.c

Regards,
Bin
Andy Shevchenko April 23, 2018, 8:22 a.m. UTC | #5
On Fri, 2018-04-20 at 11:00 -0700, Ivan Gorinov wrote:
> On Fri, Apr 20, 2018 at 06:25:08AM -0600, Andy Shevchenko wrote:

> > > -	while (rdtsc() < final_tsc)
> > > -		;

> > > +	while (rdtsc() - start_tsc < ticks);
> > 
> > I would rather preserve existing style.
> 
> OK. Existing style does not correctly handle overflow,
> but for a 64-bit counter it's a very unlikely event.

You didn't get me. I'm just talking about style, not about
functionality.

See above for lines I left in this reply.
Ivan Gorinov April 23, 2018, 11:56 p.m. UTC | #6
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.

> > Add tsc_rate_mhz() function and use it in the code that specifically
> > needs to get TSC rate regardless of currently selected early timer.
> >
> > Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
> > ---
> >  arch/x86/cpu/coreboot/timestamp.c |  2 +-
> >  arch/x86/cpu/quark/mrc_util.c     | 13 ++++++-------
> >  arch/x86/include/asm/u-boot-x86.h |  2 +-
> >  drivers/timer/tsc_timer.c         | 33 ++++++++++++++++++++++++---------
> >  4 files changed, 32 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/x86/cpu/coreboot/timestamp.c b/arch/x86/cpu/coreboot/timestamp.c
> > index b382795..05bb214 100644
> > --- a/arch/x86/cpu/coreboot/timestamp.c
> > +++ b/arch/x86/cpu/coreboot/timestamp.c
> > @@ -78,7 +78,7 @@ int timestamp_add_to_bootstage(void)
> >                 if (name) {
> >                         bootstage_add_record(0, name, BOOTSTAGEF_ALLOC,
> >                                              tse->entry_stamp /
> > -                                                       get_tbclk_mhz());
> > +                                                       tsc_rate_mhz());
> >                 }
> >         }
> >
> > diff --git a/arch/x86/cpu/quark/mrc_util.c b/arch/x86/cpu/quark/mrc_util.c
> > index fac2d72..4794395 100644
> > --- a/arch/x86/cpu/quark/mrc_util.c
> > +++ b/arch/x86/cpu/quark/mrc_util.c
> > @@ -57,19 +57,18 @@ void mrc_post_code(uint8_t major, uint8_t minor)
> >  void delay_n(uint32_t ns)
> >  {
> >         /* 1000 MHz clock has 1ns period --> no conversion required */
> > -       uint64_t final_tsc = rdtsc();
> > +       uint64_t start_tsc = rdtsc();
> > +       uint64_t ticks;
> >
> > -       final_tsc += ((get_tbclk_mhz() * ns) / 1000);
> > -
> > -       while (rdtsc() < final_tsc)
> > -               ;
> > +       ticks = (tsc_rate_mhz() * ns) / 1000;
> > +       while (rdtsc() - start_tsc < ticks);
> >  }
> >
> >  /* Delay number of microseconds */
> > -void delay_u(uint32_t ms)
> > +void delay_u(uint32_t us)
> >  {
> >         /* 64-bit math is not an option, just use loops */
> > -       while (ms--)
> > +       while (us--)
> >                 delay_n(1000);
> >  }
> >
> > diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
> > index 187fe5f..de68120 100644
> > --- a/arch/x86/include/asm/u-boot-x86.h
> > +++ b/arch/x86/include/asm/u-boot-x86.h
> > @@ -29,7 +29,7 @@ int cleanup_before_linux(void);
> >  void timer_isr(void *);
> >  typedef void (timer_fnc_t) (void);
> >  int register_timer_isr (timer_fnc_t *isr_func);
> > -unsigned long get_tbclk_mhz(void);
> > +unsigned long tsc_rate_mhz(void);
> >  void timer_set_base(uint64_t base);
> >  int i8254_init(void);
> >
> > diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
> > index 9296de6..98cbf12 100644
> > --- a/drivers/timer/tsc_timer.c
> > +++ b/drivers/timer/tsc_timer.c
> > @@ -277,15 +277,12 @@ success:
> >         return delta / 1000;
> >  }
> >
> > -/* Get the speed of the TSC timer in MHz */
> > -unsigned notrace long get_tbclk_mhz(void)
> > -{
> > -       return get_tbclk() / 1000000;
> > -}
> > -
> >  static ulong get_ms_timer(void)
> >  {
> > -       return (get_ticks() * 1000) / get_tbclk();
> > +       if (gd->arch.clock_rate == 0)
> > +               return 0;
> > +
> > +       return (rdtsc() * 1000) / gd->arch.clock_rate;
> >  }
> >
> >  ulong get_timer(ulong base)
> > @@ -295,7 +292,10 @@ ulong get_timer(ulong base)
> >
> >  ulong notrace timer_get_us(void)
> >  {
> > -       return get_ticks() / get_tbclk_mhz();
> > +       if (gd->arch.clock_rate == 0)
> > +               return 0;
> > +
> > +       return (rdtsc() * 1000000) / gd->arch.clock_rate;
> >  }
> >
> >  ulong timer_get_boot_us(void)
> > @@ -308,7 +308,7 @@ void __udelay(unsigned long usec)
> >         u64 now = get_ticks();
> >         u64 stop;
> >
> > -       stop = now + usec * get_tbclk_mhz();
> > +       stop = now + ((u64)usec * gd->arch.clock_rate) / 1000000;
> >
> >         while ((int64_t)(stop - get_ticks()) > 0)
> >  #if defined(CONFIG_QEMU) && defined(CONFIG_SMP)
> > @@ -355,6 +355,14 @@ static void tsc_timer_ensure_setup(void)
> >         }
> >  }
> >
> > +/* Get the speed of the TSC timer in MHz */
> > +unsigned notrace long tsc_rate_mhz(void)
> > +{
> > +       tsc_timer_ensure_setup();
> > +
> > +       return gd->arch.clock_rate / 1000000;
> > +}
> > +
> >  static int tsc_timer_probe(struct udevice *dev)
> >  {
> >         struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> > @@ -365,6 +373,13 @@ static int tsc_timer_probe(struct udevice *dev)
> >         return 0;
> >  }
> >
> > +int timer_init(void)
> > +{
> > +       tsc_timer_ensure_setup();
> > +
> > +       return 0;
> > +}
> > +
> >  unsigned long notrace timer_early_get_rate(void)
> >  {
> >         tsc_timer_ensure_setup();
> > --
Bin Meng April 24, 2018, 8:41 a.m. UTC | #7
Hi Ivan,

On Tue, Apr 24, 2018 at 7:56 AM, Ivan Gorinov <ivan.gorinov@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.

Another driver that explicitly calls rdtsc() is hw_watchdog_reset() in
watchdog/tangier_wdt.c driver. We need fix that too.

Regards,
Bin
Bin Meng April 26, 2018, 3:42 a.m. UTC | #8
Hi Ivan,

On Tue, Apr 24, 2018 at 4:41 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Ivan,
>
> On Tue, Apr 24, 2018 at 7:56 AM, Ivan Gorinov <ivan.gorinov@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.

Regards,
Bin
Simon Glass April 30, 2018, 11:13 p.m. UTC | #9
Hi Bin,

On 25 April 2018 at 21:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Ivan,
>
> On Tue, Apr 24, 2018 at 4:41 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Ivan,
>>
>> On Tue, Apr 24, 2018 at 7:56 AM, Ivan Gorinov <ivan.gorinov@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
diff mbox series

Patch

diff --git a/arch/x86/cpu/coreboot/timestamp.c b/arch/x86/cpu/coreboot/timestamp.c
index b382795..05bb214 100644
--- a/arch/x86/cpu/coreboot/timestamp.c
+++ b/arch/x86/cpu/coreboot/timestamp.c
@@ -78,7 +78,7 @@  int timestamp_add_to_bootstage(void)
 		if (name) {
 			bootstage_add_record(0, name, BOOTSTAGEF_ALLOC,
 					     tse->entry_stamp /
-							get_tbclk_mhz());
+							tsc_rate_mhz());
 		}
 	}
 
diff --git a/arch/x86/cpu/quark/mrc_util.c b/arch/x86/cpu/quark/mrc_util.c
index fac2d72..4794395 100644
--- a/arch/x86/cpu/quark/mrc_util.c
+++ b/arch/x86/cpu/quark/mrc_util.c
@@ -57,19 +57,18 @@  void mrc_post_code(uint8_t major, uint8_t minor)
 void delay_n(uint32_t ns)
 {
 	/* 1000 MHz clock has 1ns period --> no conversion required */
-	uint64_t final_tsc = rdtsc();
+	uint64_t start_tsc = rdtsc();
+	uint64_t ticks;
 
-	final_tsc += ((get_tbclk_mhz() * ns) / 1000);
-
-	while (rdtsc() < final_tsc)
-		;
+	ticks = (tsc_rate_mhz() * ns) / 1000;
+	while (rdtsc() - start_tsc < ticks);
 }
 
 /* Delay number of microseconds */
-void delay_u(uint32_t ms)
+void delay_u(uint32_t us)
 {
 	/* 64-bit math is not an option, just use loops */
-	while (ms--)
+	while (us--)
 		delay_n(1000);
 }
 
diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
index 187fe5f..de68120 100644
--- a/arch/x86/include/asm/u-boot-x86.h
+++ b/arch/x86/include/asm/u-boot-x86.h
@@ -29,7 +29,7 @@  int cleanup_before_linux(void);
 void timer_isr(void *);
 typedef void (timer_fnc_t) (void);
 int register_timer_isr (timer_fnc_t *isr_func);
-unsigned long get_tbclk_mhz(void);
+unsigned long tsc_rate_mhz(void);
 void timer_set_base(uint64_t base);
 int i8254_init(void);
 
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
index 9296de6..98cbf12 100644
--- a/drivers/timer/tsc_timer.c
+++ b/drivers/timer/tsc_timer.c
@@ -277,15 +277,12 @@  success:
 	return delta / 1000;
 }
 
-/* Get the speed of the TSC timer in MHz */
-unsigned notrace long get_tbclk_mhz(void)
-{
-	return get_tbclk() / 1000000;
-}
-
 static ulong get_ms_timer(void)
 {
-	return (get_ticks() * 1000) / get_tbclk();
+	if (gd->arch.clock_rate == 0)
+		return 0;
+
+	return (rdtsc() * 1000) / gd->arch.clock_rate;
 }
 
 ulong get_timer(ulong base)
@@ -295,7 +292,10 @@  ulong get_timer(ulong base)
 
 ulong notrace timer_get_us(void)
 {
-	return get_ticks() / get_tbclk_mhz();
+	if (gd->arch.clock_rate == 0)
+		return 0;
+
+	return (rdtsc() * 1000000) / gd->arch.clock_rate;
 }
 
 ulong timer_get_boot_us(void)
@@ -308,7 +308,7 @@  void __udelay(unsigned long usec)
 	u64 now = get_ticks();
 	u64 stop;
 
-	stop = now + usec * get_tbclk_mhz();
+	stop = now + ((u64)usec * gd->arch.clock_rate) / 1000000;
 
 	while ((int64_t)(stop - get_ticks()) > 0)
 #if defined(CONFIG_QEMU) && defined(CONFIG_SMP)
@@ -355,6 +355,14 @@  static void tsc_timer_ensure_setup(void)
 	}
 }
 
+/* Get the speed of the TSC timer in MHz */
+unsigned notrace long tsc_rate_mhz(void)
+{
+	tsc_timer_ensure_setup();
+
+	return gd->arch.clock_rate / 1000000;
+}
+
 static int tsc_timer_probe(struct udevice *dev)
 {
 	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
@@ -365,6 +373,13 @@  static int tsc_timer_probe(struct udevice *dev)
 	return 0;
 }
 
+int timer_init(void)
+{
+       tsc_timer_ensure_setup();
+
+       return 0;
+}
+
 unsigned long notrace timer_early_get_rate(void)
 {
 	tsc_timer_ensure_setup();