[U-Boot] [PATCH v3 00/11] dm: timer: x86: 64-bit counter support and tsc timer dm conversion

[U-Boot] [PATCH v3 00/11] dm: timer: x86: 64-bit counter support and tsc timer dm conversion

[Bin Meng]

This series enhances timer uclass driver to support 64-bit counter
value, and convert tsc timer to driver model to be used by all x86
boards.

As a result of dm conversion, the TSC_CALIBRATION_BYPASS Kconfig
option is no longer needed, and the TSC frequency can be specified
in the board device tree.

This v2 is rebased on top of u-boot-dm/master, to resolve conflicts
with Altera timer updates and the new Sandbox timer driver.

Changes in v3:
- Update commit message to reflect the v2 changes (ie: there is
  no "counter-64bit" property)
- Really remove "counter-64bit" property from tsc_timer.dtsi

Changes in v2:
- Rebase on u-boot-dm/master
- Change 'Timer' to 'timer' in the sandbox timer Kconfig
- New patch to use device tree to pass the clock frequency
- Do not use "counter-64bit" property, instead create an inline
  function for 32-bit timer driver to construct a 64-bit timer value.
- Remove "counter-64bit" property

Bin Meng (11):
  dm: timer: Fix several nits
  dm: timer: Implement pre_probe()
  timer: altera: Remove the codes to get clock frequency
  timer: sandbox: Use device tree to pass the clock frequency
  dm: timer: Support 64-bit counter
  x86: Reomve MIN_PORT80_KCLOCKS_DELAY
  x86: tsc: Use notrace from <linux/compiler.h>
  x86: tsc: Add driver model timer support
  x86: Convert to use driver model timer
  x86: tsc: Remove legacy timer codes
  x86: tsc: Move tsc_timer.c to drivers/timer

 arch/sandbox/dts/sandbox.dts                |   1 +
 arch/x86/Kconfig                            |  20 ------
 arch/x86/cpu/baytrail/valleyview.c          |   3 -
 arch/x86/cpu/coreboot/timestamp.c           |  22 ------
 arch/x86/cpu/cpu.c                          |  18 -----
 arch/x86/cpu/efi/efi.c                      |   4 --
 arch/x86/cpu/ivybridge/cpu.c                |   1 -
 arch/x86/cpu/qemu/Kconfig                   |   1 -
 arch/x86/cpu/qemu/qemu.c                    |   3 -
 arch/x86/cpu/quark/Kconfig                  |   5 --
 arch/x86/cpu/quark/quark.c                  |   3 -
 arch/x86/cpu/queensbay/tnc.c                |   3 -
 arch/x86/dts/bayleybay.dts                  |   1 +
 arch/x86/dts/broadwell_som-6896.dts         |   1 +
 arch/x86/dts/chromebook_link.dts            |   1 +
 arch/x86/dts/chromebox_panther.dts          |   1 +
 arch/x86/dts/crownbay.dts                   |   1 +
 arch/x86/dts/efi.dts                        |   5 ++
 arch/x86/dts/galileo.dts                    |   5 ++
 arch/x86/dts/minnowmax.dts                  |   1 +
 arch/x86/dts/qemu-x86_i440fx.dts            |   5 ++
 arch/x86/dts/qemu-x86_q35.dts               |   5 ++
 arch/x86/dts/tsc_timer.dtsi                 |   6 ++
 arch/x86/include/asm/global_data.h          |   3 -
 arch/x86/lib/Makefile                       |   1 -
 configs/bayleybay_defconfig                 |   1 +
 configs/chromebook_link_defconfig           |   2 +-
 configs/chromebox_panther_defconfig         |   2 +-
 configs/coreboot-x86_defconfig              |   3 +-
 configs/crownbay_defconfig                  |   1 +
 configs/efi-x86_defconfig                   |   2 +-
 configs/galileo_defconfig                   |   1 +
 configs/minnowmax_defconfig                 |   1 +
 configs/qemu-x86_defconfig                  |   1 +
 drivers/timer/Kconfig                       |  19 +++--
 drivers/timer/Makefile                      |   1 +
 drivers/timer/altera_timer.c                |  10 +--
 drivers/timer/sandbox_timer.c               |   6 +-
 drivers/timer/timer-uclass.c                |  19 +++--
 {arch/x86/lib => drivers/timer}/tsc_timer.c | 104 ++++++++++++++++------------
 include/configs/x86-common.h                |   2 -
 include/timer.h                             |  34 ++++++---
 lib/time.c                                  |   9 ++-
 43 files changed, 164 insertions(+), 174 deletions(-)
 create mode 100644 arch/x86/dts/tsc_timer.dtsi
 rename {arch/x86/lib => drivers/timer}/tsc_timer.c (87%)

-- 
1.8.2.1

[U-Boot] [PATCH v3 01/11] dm: timer: Fix several nits

[Bin Meng]

This changes 'Timer' to 'timer' at several places.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Thomas Chou <thomas@wytron.com.tw>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

Changes in v3: None
Changes in v2:
- Rebase on u-boot-dm/master
- Change 'Timer' to 'timer' in the sandbox timer Kconfig

 drivers/timer/Kconfig        | 12 ++++++------
 drivers/timer/timer-uclass.c |  4 ++--
 include/timer.h              | 11 ++++++-----
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index 601e493..029af64 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -1,23 +1,23 @@
 menu "Timer Support"
 
 config TIMER
-	bool "Enable Driver Model for Timer drivers"
+	bool "Enable driver model for timer drivers"
 	depends on DM
 	help
-	  Enable driver model for Timer access. It uses the same API as
-	  lib/time.c. But now implemented by the uclass. The first timer
+	  Enable driver model for timer access. It uses the same API as
+	  lib/time.c, but now implemented by the uclass. The first timer
 	  will be used. The timer is usually a 32 bits free-running up
 	  counter. There may be no real tick, and no timer interrupt.
 
 config ALTERA_TIMER
-	bool "Altera Timer support"
+	bool "Altera timer support"
 	depends on TIMER
 	help
-	  Select this to enable an timer for Altera devices. Please find
+	  Select this to enable a timer for Altera devices. Please find
 	  details on the "Embedded Peripherals IP User Guide" of Altera.
 
 config SANDBOX_TIMER
-	bool "Sandbox Timer support"
+	bool "Sandbox timer support"
 	depends on SANDBOX && TIMER
 	help
 	  Select this to enable an emulated timer for sandbox. It gets
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index 12aee5b..82c6897 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -10,10 +10,10 @@
 #include <timer.h>
 
 /*
- * Implement a Timer uclass to work with lib/time.c. The timer is usually
+ * 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
  * the input clock frequency of the timer. The get_count() method is used
- * get the current 32 bits count value. If the hardware is counting down,
+ * to get the current 32 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.
  */
diff --git a/include/timer.h b/include/timer.h
index cdf385d..ed5c685 100644
--- a/include/timer.h
+++ b/include/timer.h
@@ -10,30 +10,31 @@
 /*
  * Get the current timer count
  *
- * @dev: The Timer device
+ * @dev: The timer device
  * @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);
+
 /*
  * Get the timer input clock frequency
  *
- * @dev: The Timer device
+ * @dev: The timer device
  * @return: the timer input clock frequency
  */
 unsigned long timer_get_rate(struct udevice *dev);
 
 /*
- * struct timer_ops - Driver model Timer operations
+ * struct timer_ops - Driver model timer operations
  *
- * The uclass interface is implemented by all Timer devices which use
+ * The uclass interface is implemented by all timer devices which use
  * driver model.
  */
 struct timer_ops {
 	/*
 	 * Get the current timer count
 	 *
-	 * @dev: The Timer device
+	 * @dev: The timer device
 	 * @count: pointer that returns the current timer count
 	 * @return: 0 if OK, -ve on error
 	 */
-- 
1.8.2.1

[U-Boot] [PATCH v3 01/11] dm: timer: Fix several nits

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 02/11] dm: timer: Implement pre_probe()

[Bin Meng]

Every timer device needs to have a valid clock frequency and it
can be specified in the device tree. Use pre_probe() to get this
in the timer uclass driver.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Thomas Chou <thomas@wytron.com.tw>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 drivers/timer/timer-uclass.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index 82c6897..0218591 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -9,6 +9,8 @@
 #include <errno.h>
 #include <timer.h>
 
+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
@@ -35,8 +37,19 @@ unsigned long timer_get_rate(struct udevice *dev)
 	return uc_priv->clock_rate;
 }
 
+static int timer_pre_probe(struct udevice *dev)
+{
+	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+	uc_priv->clock_rate = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+					     "clock-frequency", 0);
+
+	return 0;
+}
+
 UCLASS_DRIVER(timer) = {
 	.id		= UCLASS_TIMER,
 	.name		= "timer",
+	.pre_probe	= timer_pre_probe,
 	.per_device_auto_alloc_size = sizeof(struct timer_dev_priv),
 };
-- 
1.8.2.1

[U-Boot] [PATCH v3 02/11] dm: timer: Implement pre_probe()

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 03/11] timer: altera: Remove the codes to get clock frequency

[Bin Meng]

Since we have timer uclass to get clock frequency for us, remove
the custom version in the altera timer driver.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Thomas Chou <thomas@wytron.com.tw>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 drivers/timer/altera_timer.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
index 46a598a..6fe24f2 100644
--- a/drivers/timer/altera_timer.c
+++ b/drivers/timer/altera_timer.c
@@ -32,7 +32,6 @@ struct altera_timer_regs {
 
 struct altera_timer_platdata {
 	struct altera_timer_regs *regs;
-	unsigned long clock_rate;
 };
 
 static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
@@ -54,12 +53,9 @@ static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
 
 static int altera_timer_probe(struct udevice *dev)
 {
-	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
 	struct altera_timer_platdata *plat = dev->platdata;
 	struct altera_timer_regs *const regs = plat->regs;
 
-	uc_priv->clock_rate = plat->clock_rate;
-
 	writel(0, &regs->status);
 	writel(0, &regs->control);
 	writel(ALTERA_TIMER_STOP, &regs->control);
@@ -77,8 +73,6 @@ static int altera_timer_ofdata_to_platdata(struct udevice *dev)
 
 	plat->regs = ioremap(dev_get_addr(dev),
 		sizeof(struct altera_timer_regs));
-	plat->clock_rate = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
-		"clock-frequency", 0);
 
 	return 0;
 }
-- 
1.8.2.1

[U-Boot] [PATCH v3 03/11] timer: altera: Remove the codes to get clock frequency

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 04/11] timer: sandbox: Use device tree to pass the clock frequency

[Bin Meng]

We should use device tree to pass the clock frequency of the timer
instead of hardcoded in the driver codes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v3: None
Changes in v2:
- New patch to use device tree to pass the clock frequency

 arch/sandbox/dts/sandbox.dts  | 1 +
 drivers/timer/sandbox_timer.c | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index 720ef93..d2addb4 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -179,6 +179,7 @@
 
 	timer {
 		compatible = "sandbox,timer";
+		clock-frequency = <1000000>;
 	};
 
 	tpm {
diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
index 38de763..4b20af2 100644
--- a/drivers/timer/sandbox_timer.c
+++ b/drivers/timer/sandbox_timer.c
@@ -27,10 +27,6 @@ static int sandbox_timer_get_count(struct udevice *dev, unsigned long *count)
 
 static int sandbox_timer_probe(struct udevice *dev)
 {
-	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
-
-	uc_priv->clock_rate = 1000000;
-
 	return 0;
 }
 
-- 
1.8.2.1

[U-Boot] [PATCH v3 04/11] timer: sandbox: Use device tree to pass the clock frequency

[Simon Glass]

On 13 November 2015 at 01:11, Bin Meng <bmeng.cn@gmail.com> wrote:
> We should use device tree to pass the clock frequency of the timer
> instead of hardcoded in the driver codes.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v3: None
> Changes in v2:
> - New patch to use device tree to pass the clock frequency
>
>  arch/sandbox/dts/sandbox.dts  | 1 +
>  drivers/timer/sandbox_timer.c | 4 ----
>  2 files changed, 1 insertion(+), 4 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Bin Meng]

There are timers with a 64-bit counter value but current timer
uclass driver assumes a 32-bit one. Modify timer_get_count()
to ask timer driver to always return a 64-bit counter value,
and provide an inline helper function timer_conv_64() to handle
the 32-bit/64-bit conversion automatically.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v3:
- Update commit message to reflect the v2 changes (ie: there is
  no "counter-64bit" property)

Changes in v2:
- Do not use "counter-64bit" property, instead create an inline
  function for 32-bit timer driver to construct a 64-bit timer value.

 drivers/timer/altera_timer.c  |  4 ++--
 drivers/timer/sandbox_timer.c |  2 +-
 drivers/timer/timer-uclass.c  |  8 +++-----
 include/timer.h               | 23 ++++++++++++++++++++---
 lib/time.c                    |  9 ++++++---
 5 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
index 6fe24f2..a7ed3cc 100644
--- a/drivers/timer/altera_timer.c
+++ b/drivers/timer/altera_timer.c
@@ -34,7 +34,7 @@ struct altera_timer_platdata {
 	struct altera_timer_regs *regs;
 };
 
-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;
@@ -46,7 +46,7 @@ static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
 	/* Read timer value */
 	val = readl(&regs->snapl) & 0xffff;
 	val |= (readl(&regs->snaph) & 0xffff) << 16;
-	*count = ~val;
+	*count = timer_conv_64(~val);
 
 	return 0;
 }
diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
index 4b20af2..00a9944 100644
--- a/drivers/timer/sandbox_timer.c
+++ b/drivers/timer/sandbox_timer.c
@@ -18,7 +18,7 @@ void sandbox_timer_add_offset(unsigned long offset)
 	sandbox_timer_offset += offset;
 }
 
-static int sandbox_timer_get_count(struct udevice *dev, unsigned long *count)
+static int sandbox_timer_get_count(struct udevice *dev, u64 *count)
 {
 	*count = os_get_nsec() / 1000 + sandbox_timer_offset * 1000;
 
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index 0218591..1ef0012 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -9,18 +9,16 @@
 #include <errno.h>
 #include <timer.h>
 
-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 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);
 
diff --git a/include/timer.h b/include/timer.h
index ed5c685..4eed504 100644
--- a/include/timer.h
+++ b/include/timer.h
@@ -7,6 +7,23 @@
 #ifndef _TIMER_H_
 #define _TIMER_H_
 
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * timer_conv_64 - convert 32-bit counter value to 64-bit
+ *
+ * @count: 32-bit counter value
+ * @return: 64-bit counter value
+ */
+static inline u64 timer_conv_64(u32 count)
+{
+	/* increment tbh if tbl has rolled over */
+	if (count < gd->timebase_l)
+		gd->timebase_h++;
+	gd->timebase_l = count;
+	return ((u64)gd->timebase_h << 32) | gd->timebase_l;
+}
+
 /*
  * Get the current timer count
  *
@@ -14,7 +31,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
@@ -35,10 +52,10 @@ struct timer_ops {
 	 * Get the current timer count
 	 *
 	 * @dev: The timer device
-	 * @count: pointer that returns the current timer count
+	 * @count: pointer that returns the current 64-bit 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);
 };
 
 /*
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)
 {
-- 
1.8.2.1

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Hi Bin,

On 13 November 2015 at 01:11, Bin Meng <bmeng.cn@gmail.com> wrote:
> There are timers with a 64-bit counter value but current timer
> uclass driver assumes a 32-bit one. Modify timer_get_count()
> to ask timer driver to always return a 64-bit counter value,
> and provide an inline helper function timer_conv_64() to handle
> the 32-bit/64-bit conversion automatically.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v3:
> - Update commit message to reflect the v2 changes (ie: there is
>   no "counter-64bit" property)
>
> Changes in v2:
> - Do not use "counter-64bit" property, instead create an inline
>   function for 32-bit timer driver to construct a 64-bit timer value.
>
>  drivers/timer/altera_timer.c  |  4 ++--
>  drivers/timer/sandbox_timer.c |  2 +-
>  drivers/timer/timer-uclass.c  |  8 +++-----
>  include/timer.h               | 23 ++++++++++++++++++++---
>  lib/time.c                    |  9 ++++++---
>  5 files changed, 32 insertions(+), 14 deletions(-)

Is there a binding file for this timer somewhere? If both altera and
sandbox share this property, should we add a generic binding? It
doesn't look like Linux has one though.

>
> diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
> index 6fe24f2..a7ed3cc 100644
> --- a/drivers/timer/altera_timer.c
> +++ b/drivers/timer/altera_timer.c
> @@ -34,7 +34,7 @@ struct altera_timer_platdata {
>         struct altera_timer_regs *regs;
>  };
>
> -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;
> @@ -46,7 +46,7 @@ static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
>         /* Read timer value */
>         val = readl(&regs->snapl) & 0xffff;
>         val |= (readl(&regs->snaph) & 0xffff) << 16;
> -       *count = ~val;
> +       *count = timer_conv_64(~val);
>
>         return 0;
>  }
> diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
> index 4b20af2..00a9944 100644
> --- a/drivers/timer/sandbox_timer.c
> +++ b/drivers/timer/sandbox_timer.c
> @@ -18,7 +18,7 @@ void sandbox_timer_add_offset(unsigned long offset)
>         sandbox_timer_offset += offset;
>  }
>
> -static int sandbox_timer_get_count(struct udevice *dev, unsigned long *count)
> +static int sandbox_timer_get_count(struct udevice *dev, u64 *count)
>  {
>         *count = os_get_nsec() / 1000 + sandbox_timer_offset * 1000;
>
> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
> index 0218591..1ef0012 100644
> --- a/drivers/timer/timer-uclass.c
> +++ b/drivers/timer/timer-uclass.c
> @@ -9,18 +9,16 @@
>  #include <errno.h>
>  #include <timer.h>
>
> -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 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);
>
> diff --git a/include/timer.h b/include/timer.h
> index ed5c685..4eed504 100644
> --- a/include/timer.h
> +++ b/include/timer.h
> @@ -7,6 +7,23 @@
>  #ifndef _TIMER_H_
>  #define _TIMER_H_
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * timer_conv_64 - convert 32-bit counter value to 64-bit
> + *
> + * @count: 32-bit counter value
> + * @return: 64-bit counter value
> + */
> +static inline u64 timer_conv_64(u32 count)
> +{
> +       /* increment tbh if tbl has rolled over */
> +       if (count < gd->timebase_l)
> +               gd->timebase_h++;
> +       gd->timebase_l = count;
> +       return ((u64)gd->timebase_h << 32) | gd->timebase_l;
> +}
> +
>  /*
>   * Get the current timer count
>   *
> @@ -14,7 +31,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
> @@ -35,10 +52,10 @@ struct timer_ops {
>          * Get the current timer count
>          *
>          * @dev: The timer device
> -        * @count: pointer that returns the current timer count
> +        * @count: pointer that returns the current 64-bit 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);

Why do we need to change the API to 64-bit?

My only concern is that we are pretty happy with the 32-bit timer and
I'm not sure it should change. At present unsigned long can be 32-bit
on 32-bit machines.

>  };
>
>  /*
> 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)
>  {
> --
> 1.8.2.1
>

Regards,
Simon

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Bin Meng]

Hi Simon,

On Sat, Nov 14, 2015 at 10:04 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 13 November 2015 at 01:11, Bin Meng <bmeng.cn@gmail.com> wrote:
>> There are timers with a 64-bit counter value but current timer
>> uclass driver assumes a 32-bit one. Modify timer_get_count()
>> to ask timer driver to always return a 64-bit counter value,
>> and provide an inline helper function timer_conv_64() to handle
>> the 32-bit/64-bit conversion automatically.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>
>> ---
>>
>> Changes in v3:
>> - Update commit message to reflect the v2 changes (ie: there is
>>   no "counter-64bit" property)
>>
>> Changes in v2:
>> - Do not use "counter-64bit" property, instead create an inline
>>   function for 32-bit timer driver to construct a 64-bit timer value.
>>
>>  drivers/timer/altera_timer.c  |  4 ++--
>>  drivers/timer/sandbox_timer.c |  2 +-
>>  drivers/timer/timer-uclass.c  |  8 +++-----
>>  include/timer.h               | 23 ++++++++++++++++++++---
>>  lib/time.c                    |  9 ++++++---
>>  5 files changed, 32 insertions(+), 14 deletions(-)
>
> Is there a binding file for this timer somewhere? If both altera and
> sandbox share this property, should we add a generic binding? It
> doesn't look like Linux has one though.
>
>>
>> diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
>> index 6fe24f2..a7ed3cc 100644
>> --- a/drivers/timer/altera_timer.c
>> +++ b/drivers/timer/altera_timer.c
>> @@ -34,7 +34,7 @@ struct altera_timer_platdata {
>>         struct altera_timer_regs *regs;
>>  };
>>
>> -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;
>> @@ -46,7 +46,7 @@ static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
>>         /* Read timer value */
>>         val = readl(&regs->snapl) & 0xffff;
>>         val |= (readl(&regs->snaph) & 0xffff) << 16;
>> -       *count = ~val;
>> +       *count = timer_conv_64(~val);
>>
>>         return 0;
>>  }
>> diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
>> index 4b20af2..00a9944 100644
>> --- a/drivers/timer/sandbox_timer.c
>> +++ b/drivers/timer/sandbox_timer.c
>> @@ -18,7 +18,7 @@ void sandbox_timer_add_offset(unsigned long offset)
>>         sandbox_timer_offset += offset;
>>  }
>>
>> -static int sandbox_timer_get_count(struct udevice *dev, unsigned long *count)
>> +static int sandbox_timer_get_count(struct udevice *dev, u64 *count)
>>  {
>>         *count = os_get_nsec() / 1000 + sandbox_timer_offset * 1000;
>>
>> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
>> index 0218591..1ef0012 100644
>> --- a/drivers/timer/timer-uclass.c
>> +++ b/drivers/timer/timer-uclass.c
>> @@ -9,18 +9,16 @@
>>  #include <errno.h>
>>  #include <timer.h>
>>
>> -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 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);
>>
>> diff --git a/include/timer.h b/include/timer.h
>> index ed5c685..4eed504 100644
>> --- a/include/timer.h
>> +++ b/include/timer.h
>> @@ -7,6 +7,23 @@
>>  #ifndef _TIMER_H_
>>  #define _TIMER_H_
>>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +/*
>> + * timer_conv_64 - convert 32-bit counter value to 64-bit
>> + *
>> + * @count: 32-bit counter value
>> + * @return: 64-bit counter value
>> + */
>> +static inline u64 timer_conv_64(u32 count)
>> +{
>> +       /* increment tbh if tbl has rolled over */
>> +       if (count < gd->timebase_l)
>> +               gd->timebase_h++;
>> +       gd->timebase_l = count;
>> +       return ((u64)gd->timebase_h << 32) | gd->timebase_l;
>> +}
>> +
>>  /*
>>   * Get the current timer count
>>   *
>> @@ -14,7 +31,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
>> @@ -35,10 +52,10 @@ struct timer_ops {
>>          * Get the current timer count
>>          *
>>          * @dev: The timer device
>> -        * @count: pointer that returns the current timer count
>> +        * @count: pointer that returns the current 64-bit 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);
>
> Why do we need to change the API to 64-bit?

IMHO, this API should be created to be accept a 64-bit value in the
first place. As you see the U-Boot time APIs in lib/time.c like
get_ticks() has been using 64-bit value.

>
> My only concern is that we are pretty happy with the 32-bit timer and
> I'm not sure it should change. At present unsigned long can be 32-bit
> on 32-bit machines.

32-bit timer has to be converted to 64-bit value as get_ticks()
requires 64-bit. As for 'unsigned long can be 32-bit on 32-bit
machines', I think we should explicitly declare its width as this is
hardware limitation (32-bit timer can only produce 32-bit value, even
if it is on a 64-bit machine, where long on 64-bit means 64-bit, which
is wrong for this 32-bit timer hardware).

>
>>  };
>>
>>  /*
>> 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)
>>  {
>> --

Regards,
Bin

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Hi Bin,

On 15 November 2015 at 19:19, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Sat, Nov 14, 2015 at 10:04 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Bin,
>>
>> On 13 November 2015 at 01:11, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> There are timers with a 64-bit counter value but current timer
>>> uclass driver assumes a 32-bit one. Modify timer_get_count()
>>> to ask timer driver to always return a 64-bit counter value,
>>> and provide an inline helper function timer_conv_64() to handle
>>> the 32-bit/64-bit conversion automatically.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>
>>> ---
>>>
>>> Changes in v3:
>>> - Update commit message to reflect the v2 changes (ie: there is
>>>   no "counter-64bit" property)
>>>
>>> Changes in v2:
>>> - Do not use "counter-64bit" property, instead create an inline
>>>   function for 32-bit timer driver to construct a 64-bit timer value.
>>>
>>>  drivers/timer/altera_timer.c  |  4 ++--
>>>  drivers/timer/sandbox_timer.c |  2 +-
>>>  drivers/timer/timer-uclass.c  |  8 +++-----
>>>  include/timer.h               | 23 ++++++++++++++++++++---
>>>  lib/time.c                    |  9 ++++++---
>>>  5 files changed, 32 insertions(+), 14 deletions(-)
>>
>> Is there a binding file for this timer somewhere? If both altera and
>> sandbox share this property, should we add a generic binding? It
>> doesn't look like Linux has one though.
>>
>>>
>>> diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
>>> index 6fe24f2..a7ed3cc 100644
>>> --- a/drivers/timer/altera_timer.c
>>> +++ b/drivers/timer/altera_timer.c
>>> @@ -34,7 +34,7 @@ struct altera_timer_platdata {
>>>         struct altera_timer_regs *regs;
>>>  };
>>>
>>> -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;
>>> @@ -46,7 +46,7 @@ static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
>>>         /* Read timer value */
>>>         val = readl(&regs->snapl) & 0xffff;
>>>         val |= (readl(&regs->snaph) & 0xffff) << 16;
>>> -       *count = ~val;
>>> +       *count = timer_conv_64(~val);
>>>
>>>         return 0;
>>>  }
>>> diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
>>> index 4b20af2..00a9944 100644
>>> --- a/drivers/timer/sandbox_timer.c
>>> +++ b/drivers/timer/sandbox_timer.c
>>> @@ -18,7 +18,7 @@ void sandbox_timer_add_offset(unsigned long offset)
>>>         sandbox_timer_offset += offset;
>>>  }
>>>
>>> -static int sandbox_timer_get_count(struct udevice *dev, unsigned long *count)
>>> +static int sandbox_timer_get_count(struct udevice *dev, u64 *count)
>>>  {
>>>         *count = os_get_nsec() / 1000 + sandbox_timer_offset * 1000;
>>>
>>> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
>>> index 0218591..1ef0012 100644
>>> --- a/drivers/timer/timer-uclass.c
>>> +++ b/drivers/timer/timer-uclass.c
>>> @@ -9,18 +9,16 @@
>>>  #include <errno.h>
>>>  #include <timer.h>
>>>
>>> -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 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);
>>>
>>> diff --git a/include/timer.h b/include/timer.h
>>> index ed5c685..4eed504 100644
>>> --- a/include/timer.h
>>> +++ b/include/timer.h
>>> @@ -7,6 +7,23 @@
>>>  #ifndef _TIMER_H_
>>>  #define _TIMER_H_
>>>
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +/*
>>> + * timer_conv_64 - convert 32-bit counter value to 64-bit
>>> + *
>>> + * @count: 32-bit counter value
>>> + * @return: 64-bit counter value
>>> + */
>>> +static inline u64 timer_conv_64(u32 count)
>>> +{
>>> +       /* increment tbh if tbl has rolled over */
>>> +       if (count < gd->timebase_l)
>>> +               gd->timebase_h++;
>>> +       gd->timebase_l = count;
>>> +       return ((u64)gd->timebase_h << 32) | gd->timebase_l;
>>> +}
>>> +
>>>  /*
>>>   * Get the current timer count
>>>   *
>>> @@ -14,7 +31,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
>>> @@ -35,10 +52,10 @@ struct timer_ops {
>>>          * Get the current timer count
>>>          *
>>>          * @dev: The timer device
>>> -        * @count: pointer that returns the current timer count
>>> +        * @count: pointer that returns the current 64-bit 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);
>>
>> Why do we need to change the API to 64-bit?
>
> IMHO, this API should be created to be accept a 64-bit value in the
> first place. As you see the U-Boot time APIs in lib/time.c like
> get_ticks() has been using 64-bit value.

Right, but the point of this timer is for time delays. Making everyone
one of those 64-bit on a 32-bit machine does not seem like a win to
me.

>
>>
>> My only concern is that we are pretty happy with the 32-bit timer and
>> I'm not sure it should change. At present unsigned long can be 32-bit
>> on 32-bit machines.
>
> 32-bit timer has to be converted to 64-bit value as get_ticks()
> requires 64-bit. As for 'unsigned long can be 32-bit on 32-bit
> machines', I think we should explicitly declare its width as this is
> hardware limitation (32-bit timer can only produce 32-bit value, even
> if it is on a 64-bit machine, where long on 64-bit means 64-bit, which
> is wrong for this 32-bit timer hardware).

This is a bit messy, but for now I think we should follow what
get_timer() does. It would be worth getting more input on the list I
think.

>
>>
>>>  };
>>>
>>>  /*
>>> 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)
>>>  {
>>> --
>
> Regards,
> Bin

Regards,
Simon

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Thomas Chou]

Hi Simon,

On 2015?11?21? 05:10, Simon Glass wrote:
>>>> @@ -35,10 +52,10 @@ struct timer_ops {
>>>>           * Get the current timer count
>>>>           *
>>>>           * @dev: The timer device
>>>> -        * @count: pointer that returns the current timer count
>>>> +        * @count: pointer that returns the current 64-bit 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);
>>>
>>> Why do we need to change the API to 64-bit?
>>
>> IMHO, this API should be created to be accept a 64-bit value in the
>> first place. As you see the U-Boot time APIs in lib/time.c like
>> get_ticks() has been using 64-bit value.
>
> Right, but the point of this timer is for time delays. Making everyone
> one of those 64-bit on a 32-bit machine does not seem like a win to
> me.
>
>>
>>>
>>> My only concern is that we are pretty happy with the 32-bit timer and
>>> I'm not sure it should change. At present unsigned long can be 32-bit
>>> on 32-bit machines.
>>
>> 32-bit timer has to be converted to 64-bit value as get_ticks()
>> requires 64-bit. As for 'unsigned long can be 32-bit on 32-bit
>> machines', I think we should explicitly declare its width as this is
>> hardware limitation (32-bit timer can only produce 32-bit value, even
>> if it is on a 64-bit machine, where long on 64-bit means 64-bit, which
>> is wrong for this 32-bit timer hardware).
>
> This is a bit messy, but for now I think we should follow what
> get_timer() does. It would be worth getting more input on the list I
> think.
>

I would agree with Bin that the API should have been created as 64 bits. 
I considered this option before, because most part of lib/time.c uses 64 
bits. It was only that I didn't have much confidence to change so many 
global code (without buildman). I realized this mistake when I wrote the 
sandbox timer. It is wasteful to truncate the 64 bits count to 32 bits, 
and then reconstruct 32 bits to 64 bits. This is what Bin wanted to 
resolve. There is no overhead for 32 bits timer, as the patch only shift 
the 64 bits conversion from lib/time.c to driver. Yet for 64 bits timer, 
this is a good saving.

Best regards,
Thomas

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Hi,

On 20 November 2015 at 17:41, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Simon,
>
>
> On 2015?11?21? 05:10, Simon Glass wrote:
>>>>>
>>>>> @@ -35,10 +52,10 @@ struct timer_ops {
>>>>>           * Get the current timer count
>>>>>           *
>>>>>           * @dev: The timer device
>>>>> -        * @count: pointer that returns the current timer count
>>>>> +        * @count: pointer that returns the current 64-bit 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);
>>>>
>>>>
>>>> Why do we need to change the API to 64-bit?
>>>
>>>
>>> IMHO, this API should be created to be accept a 64-bit value in the
>>> first place. As you see the U-Boot time APIs in lib/time.c like
>>> get_ticks() has been using 64-bit value.
>>
>>
>> Right, but the point of this timer is for time delays. Making everyone
>> one of those 64-bit on a 32-bit machine does not seem like a win to
>> me.
>>
>>>
>>>>
>>>> My only concern is that we are pretty happy with the 32-bit timer and
>>>> I'm not sure it should change. At present unsigned long can be 32-bit
>>>> on 32-bit machines.
>>>
>>>
>>> 32-bit timer has to be converted to 64-bit value as get_ticks()
>>> requires 64-bit. As for 'unsigned long can be 32-bit on 32-bit
>>> machines', I think we should explicitly declare its width as this is
>>> hardware limitation (32-bit timer can only produce 32-bit value, even
>>> if it is on a 64-bit machine, where long on 64-bit means 64-bit, which
>>> is wrong for this 32-bit timer hardware).
>>
>>
>> This is a bit messy, but for now I think we should follow what
>> get_timer() does. It would be worth getting more input on the list I
>> think.
>>
>
> I would agree with Bin that the API should have been created as 64 bits. I
> considered this option before, because most part of lib/time.c uses 64 bits.
> It was only that I didn't have much confidence to change so many global code
> (without buildman). I realized this mistake when I wrote the sandbox timer.
> It is wasteful to truncate the 64 bits count to 32 bits, and then
> reconstruct 32 bits to 64 bits. This is what Bin wanted to resolve. There is
> no overhead for 32 bits timer, as the patch only shift the 64 bits
> conversion from lib/time.c to driver. Yet for 64 bits timer, this is a good
> saving.

OK thank you both for your explanation. This makes sense to me now.

Acked-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Hi,

On 22 November 2015 at 09:21, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 20 November 2015 at 17:41, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Hi Simon,
>>
>>
>> On 2015?11?21? 05:10, Simon Glass wrote:
>>>>>>
>>>>>> @@ -35,10 +52,10 @@ struct timer_ops {
>>>>>>           * Get the current timer count
>>>>>>           *
>>>>>>           * @dev: The timer device
>>>>>> -        * @count: pointer that returns the current timer count
>>>>>> +        * @count: pointer that returns the current 64-bit 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);
>>>>>
>>>>>
>>>>> Why do we need to change the API to 64-bit?
>>>>
>>>>
>>>> IMHO, this API should be created to be accept a 64-bit value in the
>>>> first place. As you see the U-Boot time APIs in lib/time.c like
>>>> get_ticks() has been using 64-bit value.
>>>
>>>
>>> Right, but the point of this timer is for time delays. Making everyone
>>> one of those 64-bit on a 32-bit machine does not seem like a win to
>>> me.
>>>
>>>>
>>>>>
>>>>> My only concern is that we are pretty happy with the 32-bit timer and
>>>>> I'm not sure it should change. At present unsigned long can be 32-bit
>>>>> on 32-bit machines.
>>>>
>>>>
>>>> 32-bit timer has to be converted to 64-bit value as get_ticks()
>>>> requires 64-bit. As for 'unsigned long can be 32-bit on 32-bit
>>>> machines', I think we should explicitly declare its width as this is
>>>> hardware limitation (32-bit timer can only produce 32-bit value, even
>>>> if it is on a 64-bit machine, where long on 64-bit means 64-bit, which
>>>> is wrong for this 32-bit timer hardware).
>>>
>>>
>>> This is a bit messy, but for now I think we should follow what
>>> get_timer() does. It would be worth getting more input on the list I
>>> think.
>>>
>>
>> I would agree with Bin that the API should have been created as 64 bits. I
>> considered this option before, because most part of lib/time.c uses 64 bits.
>> It was only that I didn't have much confidence to change so many global code
>> (without buildman). I realized this mistake when I wrote the sandbox timer.
>> It is wasteful to truncate the 64 bits count to 32 bits, and then
>> reconstruct 32 bits to 64 bits. This is what Bin wanted to resolve. There is
>> no overhead for 32 bits timer, as the patch only shift the 64 bits
>> conversion from lib/time.c to driver. Yet for 64 bits timer, this is a good
>> saving.
>
> OK thank you both for your explanation. This makes sense to me now.
>
> Acked-by: Simon Glass <sjg@chromium.org>

Unfortunately this causes build errors for a few avr32 boards - e.g.
grasshopper.

I think the problem is the gd declaration in the timer.h header file.
I don't think that is a good idea. Can we move it to the C file?

Regards,
Simon

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Thomas Chou]

Hi Simon,

On 2015?11?24? 18:09, Simon Glass wrote:
>
> Unfortunately this causes build errors for a few avr32 boards - e.g.
> grasshopper.
>
> I think the problem is the gd declaration in the timer.h header file.
> I don't think that is a good idea. Can we move it to the C file?

If you meant, "warning: input is not relaxable", it is fixed after 
commit f300dccde433 ("i2c, avr32: fix compiler warning "input is not 
relaxable""). The u-boot-dm/testing branch does not have such warning.

Best regards,
Thomas

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Hi Thomas,

On 24 November 2015 at 07:01, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Simon,
>
> On 2015?11?24? 18:09, Simon Glass wrote:
>>
>>
>> Unfortunately this causes build errors for a few avr32 boards - e.g.
>> grasshopper.
>>
>> I think the problem is the gd declaration in the timer.h header file.
>> I don't think that is a good idea. Can we move it to the C file?
>
>
> If you meant, "warning: input is not relaxable", it is fixed after commit
> f300dccde433 ("i2c, avr32: fix compiler warning "input is not relaxable"").
> The u-boot-dm/testing branch does not have such warning.

Please try u-boot-dm/testing. This seems to be a different problem:

     avr32:  +   grasshopper
+lib/time.c:20: warning: register used for two global register variables

We should not declare the global_data pointer in a header file.

Bin, are you able to take a look? Perhaps the function that needs it
should move to the C file?

Regards,
Simon

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Hi,

On 24 November 2015 at 11:23, Simon Glass <sjg@chromium.org> wrote:
> Hi Thomas,
>
> On 24 November 2015 at 07:01, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Hi Simon,
>>
>> On 2015?11?24? 18:09, Simon Glass wrote:
>>>
>>>
>>> Unfortunately this causes build errors for a few avr32 boards - e.g.
>>> grasshopper.
>>>
>>> I think the problem is the gd declaration in the timer.h header file.
>>> I don't think that is a good idea. Can we move it to the C file?
>>
>>
>> If you meant, "warning: input is not relaxable", it is fixed after commit
>> f300dccde433 ("i2c, avr32: fix compiler warning "input is not relaxable"").
>> The u-boot-dm/testing branch does not have such warning.
>
> Please try u-boot-dm/testing. This seems to be a different problem:
>
>      avr32:  +   grasshopper
> +lib/time.c:20: warning: register used for two global register variables
>
> We should not declare the global_data pointer in a header file.
>
> Bin, are you able to take a look? Perhaps the function that needs it
> should move to the C file?

I've had a crack at this and will send an updated patch.

Regards,
Simon

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Thomas Chou]

Hi Simon,

On 2015?11?25? 02:23, Simon Glass wrote:
>
> Please try u-boot-dm/testing. This seems to be a different problem:
>
>       avr32:  +   grasshopper
> +lib/time.c:20: warning: register used for two global register variables
>

I am using the avr32 toolchain from kernel.org, and couldn't see the 
problem.
   gcc version 4.2.4-atmel.1.1.3.avr32linux.1

Would you please offer the link to download the avr32 toolchain you used?

Best regards,
Thomas

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Bin Meng]

On Wed, Nov 25, 2015 at 11:20 AM, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Simon,
>
> On 2015?11?25? 02:23, Simon Glass wrote:
>>
>>
>> Please try u-boot-dm/testing. This seems to be a different problem:
>>
>>       avr32:  +   grasshopper
>> +lib/time.c:20: warning: register used for two global register variables
>>
>
> I am using the avr32 toolchain from kernel.org, and couldn't see the
> problem.
>   gcc version 4.2.4-atmel.1.1.3.avr32linux.1

For me, I cannot reproduce the build error too with buildman. I am
using the same toolchain as Thomas.
>
> Would you please offer the link to download the avr32 toolchain you used?
>

Regards,
Bin

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

Hi,

On 24 November 2015 at 23:44, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Nov 25, 2015 at 11:20 AM, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Hi Simon,
>>
>> On 2015?11?25? 02:23, Simon Glass wrote:
>>>
>>>
>>> Please try u-boot-dm/testing. This seems to be a different problem:
>>>
>>>       avr32:  +   grasshopper
>>> +lib/time.c:20: warning: register used for two global register variables
>>>
>>
>> I am using the avr32 toolchain from kernel.org, and couldn't see the
>> problem.
>>   gcc version 4.2.4-atmel.1.1.3.avr32linux.1
>
> For me, I cannot reproduce the build error too with buildman. I am
> using the same toolchain as Thomas.
>>
>> Would you please offer the link to download the avr32 toolchain you used?

It is avr32-buildroot-linux-uclibc-gcc

Target: avr32-buildroot-linux-uclibc
Configured with:
/home/sjg/c/buildroot/buildroot-2013.02/output/toolchain/gcc-4.2.2-avr32-2.1.5/configure
--prefix=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr
--build=x86_64-unknown-linux-gnu --host=x86_64-unknown-linux-gnu
--target=avr32-buildroot-linux-uclibc --enable-languages=c
--with-sysroot=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr/avr32-buildroot-linux-uclibc/sysroot
--with-build-time-tools=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr/avr32-buildroot-linux-uclibc/bin
--disable-__cxa_atexit --enable-target-optspace --disable-libquadmath
--disable-libgomp --with-gnu-ld --disable-libssp --disable-multilib
--disable-tls --enable-shared
--with-gmp=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr
--with-mpfr=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr
--disable-nls --enable-threads --disable-largefile
--disable-libmudflap
Thread model: posix
gcc version 4.2.2-atmel.1.0.8

So fairly old.

I'm not sure if I built it or downloaded it - it was a while ago.

So perhaps this is a toolchain problem. But still I don't think we
want to declare the global_data pointer in a header file.

Regards,
Simon

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

On 25 November 2015 at 09:51, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 24 November 2015 at 23:44, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On Wed, Nov 25, 2015 at 11:20 AM, Thomas Chou <thomas@wytron.com.tw> wrote:
>>> Hi Simon,
>>>
>>> On 2015?11?25? 02:23, Simon Glass wrote:
>>>>
>>>>
>>>> Please try u-boot-dm/testing. This seems to be a different problem:
>>>>
>>>>       avr32:  +   grasshopper
>>>> +lib/time.c:20: warning: register used for two global register variables
>>>>
>>>
>>> I am using the avr32 toolchain from kernel.org, and couldn't see the
>>> problem.
>>>   gcc version 4.2.4-atmel.1.1.3.avr32linux.1
>>
>> For me, I cannot reproduce the build error too with buildman. I am
>> using the same toolchain as Thomas.
>>>
>>> Would you please offer the link to download the avr32 toolchain you used?
>
> It is avr32-buildroot-linux-uclibc-gcc
>
> Target: avr32-buildroot-linux-uclibc
> Configured with:
> /home/sjg/c/buildroot/buildroot-2013.02/output/toolchain/gcc-4.2.2-avr32-2.1.5/configure
> --prefix=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr
> --build=x86_64-unknown-linux-gnu --host=x86_64-unknown-linux-gnu
> --target=avr32-buildroot-linux-uclibc --enable-languages=c
> --with-sysroot=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr/avr32-buildroot-linux-uclibc/sysroot
> --with-build-time-tools=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr/avr32-buildroot-linux-uclibc/bin
> --disable-__cxa_atexit --enable-target-optspace --disable-libquadmath
> --disable-libgomp --with-gnu-ld --disable-libssp --disable-multilib
> --disable-tls --enable-shared
> --with-gmp=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr
> --with-mpfr=/home/sjg/c/buildroot/buildroot-2013.02/output/host/usr
> --disable-nls --enable-threads --disable-largefile
> --disable-libmudflap
> Thread model: posix
> gcc version 4.2.2-atmel.1.0.8
>
> So fairly old.
>
> I'm not sure if I built it or downloaded it - it was a while ago.
>
> So perhaps this is a toolchain problem. But still I don't think we
> want to declare the global_data pointer in a header file.

Applied to u-boot-dm, replacing the earlier v2 patch.

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Bin Meng]

Hi Simon,

On Sat, Nov 14, 2015 at 10:04 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 13 November 2015 at 01:11, Bin Meng <bmeng.cn@gmail.com> wrote:
>> There are timers with a 64-bit counter value but current timer
>> uclass driver assumes a 32-bit one. Modify timer_get_count()
>> to ask timer driver to always return a 64-bit counter value,
>> and provide an inline helper function timer_conv_64() to handle
>> the 32-bit/64-bit conversion automatically.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>
>> ---
>>
>> Changes in v3:
>> - Update commit message to reflect the v2 changes (ie: there is
>>   no "counter-64bit" property)
>>
>> Changes in v2:
>> - Do not use "counter-64bit" property, instead create an inline
>>   function for 32-bit timer driver to construct a 64-bit timer value.
>>
>>  drivers/timer/altera_timer.c  |  4 ++--
>>  drivers/timer/sandbox_timer.c |  2 +-
>>  drivers/timer/timer-uclass.c  |  8 +++-----
>>  include/timer.h               | 23 ++++++++++++++++++++---
>>  lib/time.c                    |  9 ++++++---
>>  5 files changed, 32 insertions(+), 14 deletions(-)
>
> Is there a binding file for this timer somewhere? If both altera and
> sandbox share this property, should we add a generic binding? It
> doesn't look like Linux has one though.
>

Missed this comment in my previous post.

I cannot find one too. But this is quite natural as without a
frequency the timer does not count :) I believe 'clock-frequency'
might be dynamically calculated on some platforms like ARM SoCs, which
is similar to the ns16550 serial clock source frequency (we've had
such discussion before).

[snip]

Regards,
Bin

[U-Boot] [PATCH v3 06/11] x86: Reomve MIN_PORT80_KCLOCKS_DELAY

[Bin Meng]

This is not referenced anywhere. Remove it, as well as
tsc_base_kclocks and tsc_prev in the global data.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/x86/cpu/cpu.c                 | 18 ------------------
 arch/x86/include/asm/global_data.h |  2 --
 2 files changed, 20 deletions(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 812c5e4..1707993 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -641,24 +641,6 @@ int cpu_jump_to_64bit(ulong setup_base, ulong target)
 
 void show_boot_progress(int val)
 {
-#if MIN_PORT80_KCLOCKS_DELAY
-	/*
-	 * Scale the time counter reading to avoid using 64 bit arithmetics.
-	 * Can't use get_timer() here becuase it could be not yet
-	 * initialized or even implemented.
-	 */
-	if (!gd->arch.tsc_prev) {
-		gd->arch.tsc_base_kclocks = rdtsc() / 1000;
-		gd->arch.tsc_prev = 0;
-	} else {
-		uint32_t now;
-
-		do {
-			now = rdtsc() / 1000 - gd->arch.tsc_base_kclocks;
-		} while (now < (gd->arch.tsc_prev + MIN_PORT80_KCLOCKS_DELAY));
-		gd->arch.tsc_prev = now;
-	}
-#endif
 	outb(val, POST_PORT);
 }
 
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
index 35148ab..5966b7c 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -54,8 +54,6 @@ struct arch_global_data {
 	uint8_t x86_mask;
 	uint32_t x86_device;
 	uint64_t tsc_base;		/* Initial value returned by rdtsc() */
-	uint32_t tsc_base_kclocks;	/* Initial tsc as a kclocks value */
-	uint32_t tsc_prev;		/* For show_boot_progress() */
 	uint32_t tsc_mhz;		/* TSC frequency in MHz */
 	void *new_fdt;			/* Relocated FDT */
 	uint32_t bist;			/* Built-in self test value */
-- 
1.8.2.1

[U-Boot] [PATCH v3 06/11] x86: Reomve MIN_PORT80_KCLOCKS_DELAY

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 07/11] x86: tsc: Use notrace from <linux/compiler.h>

[Bin Meng]

Replace __attribute__((no_instrument_function)) with notrace from
<linux/compiler.h>.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/x86/lib/tsc_timer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/lib/tsc_timer.c b/arch/x86/lib/tsc_timer.c
index e02b918..5a962ce 100644
--- a/arch/x86/lib/tsc_timer.c
+++ b/arch/x86/lib/tsc_timer.c
@@ -288,7 +288,7 @@ void timer_set_base(u64 base)
  * restart. This yields a free running counter guaranteed to take almost 6
  * years to wrap around even at 100GHz clock rate.
  */
-u64 __attribute__((no_instrument_function)) get_ticks(void)
+u64 notrace get_ticks(void)
 {
 	u64 now_tick = rdtsc();
 
@@ -299,7 +299,7 @@ u64 __attribute__((no_instrument_function)) get_ticks(void)
 }
 
 /* Get the speed of the TSC timer in MHz */
-unsigned __attribute__((no_instrument_function)) long get_tbclk_mhz(void)
+unsigned notrace long get_tbclk_mhz(void)
 {
 	unsigned long fast_calibrate;
 
@@ -337,7 +337,7 @@ ulong get_timer(ulong base)
 	return get_ms_timer() - base;
 }
 
-ulong __attribute__((no_instrument_function)) timer_get_us(void)
+ulong notrace timer_get_us(void)
 {
 	return get_ticks() / get_tbclk_mhz();
 }
-- 
1.8.2.1

[U-Boot] [PATCH v3 07/11] x86: tsc: Use notrace from <linux/compiler.h>

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 08/11] x86: tsc: Add driver model timer support

[Bin Meng]

This adds driver model timer support to x86 tsc timer driver.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/x86/lib/tsc_timer.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/arch/x86/lib/tsc_timer.c b/arch/x86/lib/tsc_timer.c
index 5a962ce..4a95959 100644
--- a/arch/x86/lib/tsc_timer.c
+++ b/arch/x86/lib/tsc_timer.c
@@ -8,7 +8,9 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <malloc.h>
+#include <timer.h>
 #include <asm/io.h>
 #include <asm/i8254.h>
 #include <asm/ibmpc.h>
@@ -278,6 +280,7 @@ success:
 	return delta / 1000;
 }
 
+#ifndef CONFIG_TIMER
 void timer_set_base(u64 base)
 {
 	gd->arch.tsc_base = base;
@@ -297,10 +300,14 @@ u64 notrace get_ticks(void)
 		panic("No tick base available");
 	return now_tick - gd->arch.tsc_base;
 }
+#endif /* CONFIG_TIMER */
 
 /* Get the speed of the TSC timer in MHz */
 unsigned notrace long get_tbclk_mhz(void)
 {
+#ifdef CONFIG_TIMER
+	return get_tbclk() / 1000000;
+#else
 	unsigned long fast_calibrate;
 
 	if (gd->arch.tsc_mhz)
@@ -320,12 +327,15 @@ unsigned notrace long get_tbclk_mhz(void)
 
 	gd->arch.tsc_mhz = fast_calibrate;
 	return fast_calibrate;
+#endif
 }
 
+#ifndef CONFIG_TIMER
 unsigned long get_tbclk(void)
 {
 	return get_tbclk_mhz() * 1000 * 1000;
 }
+#endif
 
 static ulong get_ms_timer(void)
 {
@@ -375,3 +385,58 @@ int timer_init(void)
 
 	return 0;
 }
+
+#ifdef CONFIG_TIMER
+static int tsc_timer_get_count(struct udevice *dev, u64 *count)
+{
+	u64 now_tick = rdtsc();
+
+	*count = now_tick - gd->arch.tsc_base;
+
+	return 0;
+}
+
+static int tsc_timer_probe(struct udevice *dev)
+{
+	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+	gd->arch.tsc_base = rdtsc();
+
+	/*
+	 * If there is no clock frequency specified in the device tree,
+	 * calibrate it by ourselves.
+	 */
+	if (!uc_priv->clock_rate) {
+		unsigned long fast_calibrate;
+
+		fast_calibrate = try_msr_calibrate_tsc();
+		if (!fast_calibrate) {
+			fast_calibrate = quick_pit_calibrate();
+			if (!fast_calibrate)
+				panic("TSC frequency is ZERO");
+		}
+
+		uc_priv->clock_rate = fast_calibrate * 1000000;
+	}
+
+	return 0;
+}
+
+static const struct timer_ops tsc_timer_ops = {
+	.get_count = tsc_timer_get_count,
+};
+
+static const struct udevice_id tsc_timer_ids[] = {
+	{ .compatible = "x86,tsc-timer", },
+	{ }
+};
+
+U_BOOT_DRIVER(tsc_timer) = {
+	.name	= "tsc_timer",
+	.id	= UCLASS_TIMER,
+	.of_match = tsc_timer_ids,
+	.probe = tsc_timer_probe,
+	.ops	= &tsc_timer_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
+#endif /* CONFIG_TIMER */
-- 
1.8.2.1

[U-Boot] [PATCH v3 08/11] x86: tsc: Add driver model timer support

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 09/11] x86: Convert to use driver model timer

[Bin Meng]

Convert all x86 boards to use driver model tsc timer.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>

---

Changes in v3:
- Really remove "counter-64bit" property from tsc_timer.dtsi

Changes in v2:
- Remove "counter-64bit" property

 arch/x86/cpu/baytrail/valleyview.c  |  3 ---
 arch/x86/cpu/coreboot/timestamp.c   | 22 ----------------------
 arch/x86/cpu/efi/efi.c              |  4 ----
 arch/x86/cpu/ivybridge/cpu.c        |  1 -
 arch/x86/cpu/qemu/qemu.c            |  3 ---
 arch/x86/cpu/quark/quark.c          |  3 ---
 arch/x86/cpu/queensbay/tnc.c        |  3 ---
 arch/x86/dts/bayleybay.dts          |  1 +
 arch/x86/dts/broadwell_som-6896.dts |  1 +
 arch/x86/dts/chromebook_link.dts    |  1 +
 arch/x86/dts/chromebox_panther.dts  |  1 +
 arch/x86/dts/crownbay.dts           |  1 +
 arch/x86/dts/efi.dts                |  5 +++++
 arch/x86/dts/galileo.dts            |  5 +++++
 arch/x86/dts/minnowmax.dts          |  1 +
 arch/x86/dts/qemu-x86_i440fx.dts    |  5 +++++
 arch/x86/dts/qemu-x86_q35.dts       |  5 +++++
 arch/x86/dts/tsc_timer.dtsi         |  6 ++++++
 configs/bayleybay_defconfig         |  1 +
 configs/chromebook_link_defconfig   |  2 +-
 configs/chromebox_panther_defconfig |  2 +-
 configs/coreboot-x86_defconfig      |  2 +-
 configs/crownbay_defconfig          |  1 +
 configs/efi-x86_defconfig           |  1 +
 configs/galileo_defconfig           |  1 +
 configs/minnowmax_defconfig         |  1 +
 configs/qemu-x86_defconfig          |  1 +
 27 files changed, 41 insertions(+), 42 deletions(-)
 create mode 100644 arch/x86/dts/tsc_timer.dtsi

diff --git a/arch/x86/cpu/baytrail/valleyview.c b/arch/x86/cpu/baytrail/valleyview.c
index a009c14..9b30451 100644
--- a/arch/x86/cpu/baytrail/valleyview.c
+++ b/arch/x86/cpu/baytrail/valleyview.c
@@ -28,9 +28,6 @@ int arch_cpu_init(void)
 	int ret;
 
 	post_code(POST_CPU_INIT);
-#ifdef CONFIG_SYS_X86_TSC_TIMER
-	timer_set_base(rdtsc());
-#endif
 
 	ret = x86_cpu_init_f();
 	if (ret)
diff --git a/arch/x86/cpu/coreboot/timestamp.c b/arch/x86/cpu/coreboot/timestamp.c
index 0edee6b..b382795 100644
--- a/arch/x86/cpu/coreboot/timestamp.c
+++ b/arch/x86/cpu/coreboot/timestamp.c
@@ -27,28 +27,6 @@ static struct timestamp_table *ts_table  __attribute__((section(".data")));
 
 void timestamp_init(void)
 {
-#ifdef CONFIG_SYS_X86_TSC_TIMER
-	uint64_t base_time;
-#endif
-
-	ts_table = lib_sysinfo.tstamp_table;
-#ifdef CONFIG_SYS_X86_TSC_TIMER
-	/*
-	 * If coreboot is built with CONFIG_COLLECT_TIMESTAMPS, use the value
-	 * of base_time in coreboot's timestamp table as our timer base,
-	 * otherwise TSC counter value will be used.
-	 *
-	 * Sometimes even coreboot is built with CONFIG_COLLECT_TIMESTAMPS,
-	 * the value of base_time in the timestamp table is still zero, so
-	 * we must exclude this case too (this is currently seen on booting
-	 * coreboot in qemu)
-	 */
-	if (ts_table && ts_table->base_time)
-		base_time = ts_table->base_time;
-	else
-		base_time = rdtsc();
-	timer_set_base(base_time);
-#endif
 	timestamp_add_now(TS_U_BOOT_INITTED);
 }
 
diff --git a/arch/x86/cpu/efi/efi.c b/arch/x86/cpu/efi/efi.c
index 75ba0d4..993ab8d 100644
--- a/arch/x86/cpu/efi/efi.c
+++ b/arch/x86/cpu/efi/efi.c
@@ -10,10 +10,6 @@
 
 int arch_cpu_init(void)
 {
-#ifdef CONFIG_SYS_X86_TSC_TIMER
-	timer_set_base(rdtsc());
-#endif
-
 	return 0;
 }
 
diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c
index 0e6512c..0387444 100644
--- a/arch/x86/cpu/ivybridge/cpu.c
+++ b/arch/x86/cpu/ivybridge/cpu.c
@@ -118,7 +118,6 @@ static void set_spi_speed(void)
 int arch_cpu_init(void)
 {
 	post_code(POST_CPU_INIT);
-	timer_set_base(rdtsc());
 
 	return x86_cpu_init_f();
 }
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
index 84fb082..1f93f72 100644
--- a/arch/x86/cpu/qemu/qemu.c
+++ b/arch/x86/cpu/qemu/qemu.c
@@ -64,9 +64,6 @@ int arch_cpu_init(void)
 	int ret;
 
 	post_code(POST_CPU_INIT);
-#ifdef CONFIG_SYS_X86_TSC_TIMER
-	timer_set_base(rdtsc());
-#endif
 
 	ret = x86_cpu_init_f();
 	if (ret)
diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c
index f737e19..c2bf497 100644
--- a/arch/x86/cpu/quark/quark.c
+++ b/arch/x86/cpu/quark/quark.c
@@ -233,9 +233,6 @@ int arch_cpu_init(void)
 	int ret;
 
 	post_code(POST_CPU_INIT);
-#ifdef CONFIG_SYS_X86_TSC_TIMER
-	timer_set_base(rdtsc());
-#endif
 
 	ret = x86_cpu_init_f();
 	if (ret)
diff --git a/arch/x86/cpu/queensbay/tnc.c b/arch/x86/cpu/queensbay/tnc.c
index 933d189..fb81919 100644
--- a/arch/x86/cpu/queensbay/tnc.c
+++ b/arch/x86/cpu/queensbay/tnc.c
@@ -52,9 +52,6 @@ int arch_cpu_init(void)
 	int ret;
 
 	post_code(POST_CPU_INIT);
-#ifdef CONFIG_SYS_X86_TSC_TIMER
-	timer_set_base(rdtsc());
-#endif
 
 	ret = x86_cpu_init_f();
 	if (ret)
diff --git a/arch/x86/dts/bayleybay.dts b/arch/x86/dts/bayleybay.dts
index 52d0999..f8901c3 100644
--- a/arch/x86/dts/bayleybay.dts
+++ b/arch/x86/dts/bayleybay.dts
@@ -12,6 +12,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "Intel Bayley Bay";
diff --git a/arch/x86/dts/broadwell_som-6896.dts b/arch/x86/dts/broadwell_som-6896.dts
index a6b5d0f..194f0eb 100644
--- a/arch/x86/dts/broadwell_som-6896.dts
+++ b/arch/x86/dts/broadwell_som-6896.dts
@@ -3,6 +3,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "Advantech SOM-6896";
diff --git a/arch/x86/dts/chromebook_link.dts b/arch/x86/dts/chromebook_link.dts
index f27263a..d3e0758 100644
--- a/arch/x86/dts/chromebook_link.dts
+++ b/arch/x86/dts/chromebook_link.dts
@@ -3,6 +3,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "Google Link";
diff --git a/arch/x86/dts/chromebox_panther.dts b/arch/x86/dts/chromebox_panther.dts
index 61e8f2f..4e2b517 100644
--- a/arch/x86/dts/chromebox_panther.dts
+++ b/arch/x86/dts/chromebox_panther.dts
@@ -3,6 +3,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "Google Panther";
diff --git a/arch/x86/dts/crownbay.dts b/arch/x86/dts/crownbay.dts
index 3e354c4..c0a640d 100644
--- a/arch/x86/dts/crownbay.dts
+++ b/arch/x86/dts/crownbay.dts
@@ -11,6 +11,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "Intel Crown Bay";
diff --git a/arch/x86/dts/efi.dts b/arch/x86/dts/efi.dts
index 1f50428..6cd8116 100644
--- a/arch/x86/dts/efi.dts
+++ b/arch/x86/dts/efi.dts
@@ -7,6 +7,7 @@
 /dts-v1/;
 
 /include/ "skeleton.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "EFI";
@@ -16,6 +17,10 @@
 		stdout-path = &serial;
 	};
 
+	tsc-timer {
+		clock-frequency = <1000000000>;
+	};
+
 	serial: serial {
 		compatible = "efi,uart";
 	};
diff --git a/arch/x86/dts/galileo.dts b/arch/x86/dts/galileo.dts
index b49b1f5..2342de7 100644
--- a/arch/x86/dts/galileo.dts
+++ b/arch/x86/dts/galileo.dts
@@ -11,6 +11,7 @@
 
 /include/ "skeleton.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "Intel Galileo";
@@ -28,6 +29,10 @@
 		stdout-path = &pciuart0;
 	};
 
+	tsc-timer {
+		clock-frequency = <400000000>;
+	};
+
 	mrc {
 		compatible = "intel,quark-mrc";
 		flags = <MRC_FLAG_SCRAMBLE_EN>;
diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts
index b03f987..bbfd6d4 100644
--- a/arch/x86/dts/minnowmax.dts
+++ b/arch/x86/dts/minnowmax.dts
@@ -12,6 +12,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "Intel Minnowboard Max";
diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts
index fc74cd0..d3158a3 100644
--- a/arch/x86/dts/qemu-x86_i440fx.dts
+++ b/arch/x86/dts/qemu-x86_i440fx.dts
@@ -11,6 +11,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "QEMU x86 (I440FX)";
@@ -43,6 +44,10 @@
 		};
 	};
 
+	tsc-timer {
+		clock-frequency = <1000000000>;
+	};
+
 	pci {
 		compatible = "pci-x86";
 		#address-cells = <3>;
diff --git a/arch/x86/dts/qemu-x86_q35.dts b/arch/x86/dts/qemu-x86_q35.dts
index 7f16971..5716df2 100644
--- a/arch/x86/dts/qemu-x86_q35.dts
+++ b/arch/x86/dts/qemu-x86_q35.dts
@@ -21,6 +21,7 @@
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "tsc_timer.dtsi"
 
 / {
 	model = "QEMU x86 (Q35)";
@@ -54,6 +55,10 @@
 		};
 	};
 
+	tsc-timer {
+		clock-frequency = <1000000000>;
+	};
+
 	pci {
 		compatible = "pci-x86";
 		#address-cells = <3>;
diff --git a/arch/x86/dts/tsc_timer.dtsi b/arch/x86/dts/tsc_timer.dtsi
new file mode 100644
index 0000000..4f5021d
--- /dev/null
+++ b/arch/x86/dts/tsc_timer.dtsi
@@ -0,0 +1,6 @@
+/ {
+	tsc-timer {
+		compatible = "x86,tsc-timer";
+		u-boot,dm-pre-reloc;
+	};
+};
diff --git a/configs/bayleybay_defconfig b/configs/bayleybay_defconfig
index fc40da8..e9abbd0 100644
--- a/configs/bayleybay_defconfig
+++ b/configs/bayleybay_defconfig
@@ -24,6 +24,7 @@ CONFIG_DM_ETH=y
 CONFIG_E1000=y
 CONFIG_DM_PCI=y
 CONFIG_DM_RTC=y
+CONFIG_TIMER=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_VIDEO_VESA=y
diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig
index 0b75781..c20762b 100644
--- a/configs/chromebook_link_defconfig
+++ b/configs/chromebook_link_defconfig
@@ -27,7 +27,7 @@ CONFIG_DEBUG_UART_NS16550=y
 CONFIG_DEBUG_UART_BASE=0x3f8
 CONFIG_DEBUG_UART_CLOCK=1843200
 CONFIG_DEBUG_UART_BOARD_INIT=y
-CONFIG_DM_TPM=y
+CONFIG_TIMER=y
 CONFIG_TPM_TIS_LPC=y
 CONFIG_VIDEO_VESA=y
 CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
diff --git a/configs/chromebox_panther_defconfig b/configs/chromebox_panther_defconfig
index 663aab0..fca6e53 100644
--- a/configs/chromebox_panther_defconfig
+++ b/configs/chromebox_panther_defconfig
@@ -21,7 +21,7 @@ CONFIG_CROS_EC_LPC=y
 CONFIG_SPI_FLASH=y
 CONFIG_DM_PCI=y
 CONFIG_DM_RTC=y
-CONFIG_DM_TPM=y
+CONFIG_TIMER=y
 CONFIG_TPM_TIS_LPC=y
 CONFIG_VIDEO_VESA=y
 CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
index 438e43b..3f2cff8 100644
--- a/configs/coreboot-x86_defconfig
+++ b/configs/coreboot-x86_defconfig
@@ -17,7 +17,7 @@ CONFIG_DM_ETH=y
 CONFIG_E1000=y
 CONFIG_DM_PCI=y
 CONFIG_DM_RTC=y
-CONFIG_DM_TPM=y
+CONFIG_TIMER=y
 CONFIG_TPM_TIS_LPC=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
diff --git a/configs/crownbay_defconfig b/configs/crownbay_defconfig
index d036c05..7ae36b6 100644
--- a/configs/crownbay_defconfig
+++ b/configs/crownbay_defconfig
@@ -23,6 +23,7 @@ CONFIG_E1000=y
 CONFIG_PCH_GBE=y
 CONFIG_DM_PCI=y
 CONFIG_DM_RTC=y
+CONFIG_TIMER=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_VIDEO_VESA=y
diff --git a/configs/efi-x86_defconfig b/configs/efi-x86_defconfig
index 43fb0c4..fd90739 100644
--- a/configs/efi-x86_defconfig
+++ b/configs/efi-x86_defconfig
@@ -13,4 +13,5 @@ CONFIG_DEBUG_EFI_CONSOLE=y
 CONFIG_DEBUG_UART_BASE=0
 CONFIG_DEBUG_UART_CLOCK=0
 # CONFIG_X86_SERIAL is not set
+CONFIG_TIMER=y
 CONFIG_EFI=y
diff --git a/configs/galileo_defconfig b/configs/galileo_defconfig
index 1e1ce95..6421e5b 100644
--- a/configs/galileo_defconfig
+++ b/configs/galileo_defconfig
@@ -18,6 +18,7 @@ CONFIG_DM_ETH=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_DM_PCI=y
 CONFIG_DM_RTC=y
+CONFIG_TIMER=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USE_PRIVATE_LIBGCC=y
diff --git a/configs/minnowmax_defconfig b/configs/minnowmax_defconfig
index 0f500e6..a89f6910 100644
--- a/configs/minnowmax_defconfig
+++ b/configs/minnowmax_defconfig
@@ -26,6 +26,7 @@ CONFIG_DEBUG_UART=y
 CONFIG_DEBUG_UART_NS16550=y
 CONFIG_DEBUG_UART_BASE=0x3f8
 CONFIG_DEBUG_UART_CLOCK=1843200
+CONFIG_TIMER=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_VIDEO_VESA=y
diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig
index 366ccc4..e2ab562 100644
--- a/configs/qemu-x86_defconfig
+++ b/configs/qemu-x86_defconfig
@@ -19,6 +19,7 @@ CONFIG_DM_ETH=y
 CONFIG_E1000=y
 CONFIG_DM_PCI=y
 CONFIG_DM_RTC=y
+CONFIG_TIMER=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_VIDEO_VESA=y
-- 
1.8.2.1

[U-Boot] [PATCH v3 09/11] x86: Convert to use driver model timer

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 10/11] x86: tsc: Remove legacy timer codes

[Bin Meng]

Now that we have converted all x86 boards to use driver model timer,
remove these legacy timer codes in the tsc driver.

Note this also removes the TSC_CALIBRATION_BYPASS Kconfig option,
as it is not needed with driver model.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/x86/Kconfig                   | 20 --------------
 arch/x86/cpu/qemu/Kconfig          |  1 -
 arch/x86/cpu/quark/Kconfig         |  5 ----
 arch/x86/include/asm/global_data.h |  1 -
 arch/x86/lib/tsc_timer.c           | 53 --------------------------------------
 configs/coreboot-x86_defconfig     |  1 -
 configs/efi-x86_defconfig          |  1 -
 7 files changed, 82 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 8914be3..fef5601 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -279,26 +279,6 @@ config AP_STACK_SIZE
 	  the memory used by this initialisation process. Typically 4KB is
 	  enough space.
 
-config TSC_CALIBRATION_BYPASS
-	bool "Bypass Time-Stamp Counter (TSC) calibration"
-	default n
-	help
-	  By default U-Boot automatically calibrates Time-Stamp Counter (TSC)
-	  running frequency via Model-Specific Register (MSR) and Programmable
-	  Interval Timer (PIT). If the calibration does not work on your board,
-	  select this option and provide a hardcoded TSC running frequency with
-	  CONFIG_TSC_FREQ_IN_MHZ below.
-
-	  Normally this option should be turned on in a simulation environment
-	  like qemu.
-
-config TSC_FREQ_IN_MHZ
-	int "Time-Stamp Counter (TSC) running frequency in MHz"
-	depends on TSC_CALIBRATION_BYPASS
-	default 1000
-	help
-	  The running frequency in MHz of Time-Stamp Counter (TSC).
-
 config HAVE_VGA_BIOS
 	bool "Add a VGA BIOS image"
 	help
diff --git a/arch/x86/cpu/qemu/Kconfig b/arch/x86/cpu/qemu/Kconfig
index fb775d7..4f98621 100644
--- a/arch/x86/cpu/qemu/Kconfig
+++ b/arch/x86/cpu/qemu/Kconfig
@@ -6,7 +6,6 @@
 
 config QEMU
 	bool
-	select TSC_CALIBRATION_BYPASS
 
 if QEMU
 
diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
index bc961ef..163caac 100644
--- a/arch/x86/cpu/quark/Kconfig
+++ b/arch/x86/cpu/quark/Kconfig
@@ -7,7 +7,6 @@
 config INTEL_QUARK
 	bool
 	select HAVE_RMU
-	select TSC_CALIBRATION_BYPASS
 
 if INTEL_QUARK
 
@@ -119,8 +118,4 @@ config SYS_CAR_SIZE
 	  Space in bytes in eSRAM used as Cache-As-ARM (CAR).
 	  Note this size must not exceed eSRAM's total size.
 
-config TSC_FREQ_IN_MHZ
-	int
-	default 400
-
 endif
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
index 5966b7c..0ca518c 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -54,7 +54,6 @@ struct arch_global_data {
 	uint8_t x86_mask;
 	uint32_t x86_device;
 	uint64_t tsc_base;		/* Initial value returned by rdtsc() */
-	uint32_t tsc_mhz;		/* TSC frequency in MHz */
 	void *new_fdt;			/* Relocated FDT */
 	uint32_t bist;			/* Built-in self test value */
 	enum pei_boot_mode_t pei_boot_mode;
diff --git a/arch/x86/lib/tsc_timer.c b/arch/x86/lib/tsc_timer.c
index 4a95959..6aa2437 100644
--- a/arch/x86/lib/tsc_timer.c
+++ b/arch/x86/lib/tsc_timer.c
@@ -280,63 +280,12 @@ success:
 	return delta / 1000;
 }
 
-#ifndef CONFIG_TIMER
-void timer_set_base(u64 base)
-{
-	gd->arch.tsc_base = base;
-}
-
-/*
- * Get the number of CPU time counter ticks since it was read first time after
- * restart. This yields a free running counter guaranteed to take almost 6
- * years to wrap around even at 100GHz clock rate.
- */
-u64 notrace get_ticks(void)
-{
-	u64 now_tick = rdtsc();
-
-	/* We assume that 0 means the base hasn't been set yet */
-	if (!gd->arch.tsc_base)
-		panic("No tick base available");
-	return now_tick - gd->arch.tsc_base;
-}
-#endif /* CONFIG_TIMER */
-
 /* Get the speed of the TSC timer in MHz */
 unsigned notrace long get_tbclk_mhz(void)
 {
-#ifdef CONFIG_TIMER
 	return get_tbclk() / 1000000;
-#else
-	unsigned long fast_calibrate;
-
-	if (gd->arch.tsc_mhz)
-		return gd->arch.tsc_mhz;
-
-#ifdef CONFIG_TSC_CALIBRATION_BYPASS
-	fast_calibrate = CONFIG_TSC_FREQ_IN_MHZ;
-#else
-	fast_calibrate = try_msr_calibrate_tsc();
-	if (!fast_calibrate) {
-
-		fast_calibrate = quick_pit_calibrate();
-		if (!fast_calibrate)
-			panic("TSC frequency is ZERO");
-	}
-#endif
-
-	gd->arch.tsc_mhz = fast_calibrate;
-	return fast_calibrate;
-#endif
 }
 
-#ifndef CONFIG_TIMER
-unsigned long get_tbclk(void)
-{
-	return get_tbclk_mhz() * 1000 * 1000;
-}
-#endif
-
 static ulong get_ms_timer(void)
 {
 	return (get_ticks() * 1000) / get_tbclk();
@@ -386,7 +335,6 @@ int timer_init(void)
 	return 0;
 }
 
-#ifdef CONFIG_TIMER
 static int tsc_timer_get_count(struct udevice *dev, u64 *count)
 {
 	u64 now_tick = rdtsc();
@@ -439,4 +387,3 @@ U_BOOT_DRIVER(tsc_timer) = {
 	.ops	= &tsc_timer_ops,
 	.flags = DM_FLAG_PRE_RELOC,
 };
-#endif /* CONFIG_TIMER */
diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
index 3f2cff8..6fb38f0 100644
--- a/configs/coreboot-x86_defconfig
+++ b/configs/coreboot-x86_defconfig
@@ -1,7 +1,6 @@
 CONFIG_X86=y
 CONFIG_VENDOR_COREBOOT=y
 CONFIG_TARGET_COREBOOT=y
-CONFIG_TSC_CALIBRATION_BYPASS=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
diff --git a/configs/efi-x86_defconfig b/configs/efi-x86_defconfig
index fd90739..7354f04 100644
--- a/configs/efi-x86_defconfig
+++ b/configs/efi-x86_defconfig
@@ -2,7 +2,6 @@ CONFIG_X86=y
 CONFIG_VENDOR_EFI=y
 CONFIG_DEFAULT_DEVICE_TREE="efi"
 CONFIG_TARGET_EFI=y
-CONFIG_TSC_CALIBRATION_BYPASS=y
 # CONFIG_CMD_BOOTM is not set
 # CONFIG_CMD_NET is not set
 CONFIG_OF_CONTROL=y
-- 
1.8.2.1

[U-Boot] [PATCH v3 10/11] x86: tsc: Remove legacy timer codes

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 11/11] x86: tsc: Move tsc_timer.c to drivers/timer

[Bin Meng]

To group all dm timer drivers together, move tsc timer to
drivers/timer directory.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>

---

Changes in v3: None
Changes in v2: None

 arch/x86/lib/Makefile                       | 1 -
 drivers/timer/Kconfig                       | 7 +++++++
 drivers/timer/Makefile                      | 1 +
 {arch/x86/lib => drivers/timer}/tsc_timer.c | 0
 include/configs/x86-common.h                | 2 --
 5 files changed, 8 insertions(+), 3 deletions(-)
 rename {arch/x86/lib => drivers/timer}/tsc_timer.c (100%)

diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index d676e2c..cd5ecb6 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -34,7 +34,6 @@ obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
 obj-y	+= string.o
 obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi_table.o
 obj-y	+= tables.o
-obj-$(CONFIG_SYS_X86_TSC_TIMER)	+= tsc_timer.o
 obj-$(CONFIG_CMD_ZBOOT)	+= zimage.o
 obj-$(CONFIG_HAVE_FSP) += fsp/
 
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index 029af64..2b10d2b 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -23,4 +23,11 @@ config SANDBOX_TIMER
 	  Select this to enable an emulated timer for sandbox. It gets
 	  time from host os.
 
+config X86_TSC_TIMER
+	bool "x86 Time-Stamp Counter (TSC) timer support"
+	depends on TIMER && X86
+	default y if X86
+	help
+	  Select this to enable Time-Stamp Counter (TSC) timer for x86.
+
 endmenu
diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
index 300946e..fe954ec 100644
--- a/drivers/timer/Makefile
+++ b/drivers/timer/Makefile
@@ -7,3 +7,4 @@
 obj-$(CONFIG_TIMER)		+= timer-uclass.o
 obj-$(CONFIG_ALTERA_TIMER)	+= altera_timer.o
 obj-$(CONFIG_SANDBOX_TIMER)	+= sandbox_timer.o
+obj-$(CONFIG_X86_TSC_TIMER)	+= tsc_timer.o
diff --git a/arch/x86/lib/tsc_timer.c b/drivers/timer/tsc_timer.c
similarity index 100%
rename from arch/x86/lib/tsc_timer.c
rename to drivers/timer/tsc_timer.c
diff --git a/include/configs/x86-common.h b/include/configs/x86-common.h
index ab9fa0b..7c3b673 100644
--- a/include/configs/x86-common.h
+++ b/include/configs/x86-common.h
@@ -154,8 +154,6 @@
  * CPU Features
  */
 
-#define CONFIG_SYS_X86_TSC_TIMER
-
 #define CONFIG_SYS_STACK_SIZE			(32 * 1024)
 #define CONFIG_SYS_MONITOR_BASE		CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_MALLOC_LEN			0x200000
-- 
1.8.2.1

[U-Boot] [PATCH v3 11/11] x86: tsc: Move tsc_timer.c to drivers/timer

[Simon Glass]

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v3 00/11] dm: timer: x86: 64-bit counter support and tsc timer dm conversion

[Thomas Chou]

Hi Bin,

On 2015?11?13? 16:11, Bin Meng wrote:
> This series enhances timer uclass driver to support 64-bit counter
> value, and convert tsc timer to driver model to be used by all x86
> boards.
>
> As a result of dm conversion, the TSC_CALIBRATION_BYPASS Kconfig
> option is no longer needed, and the TSC frequency can be specified
> in the board device tree.
>
> This v2 is rebased on top of u-boot-dm/master, to resolve conflicts
> with Altera timer updates and the new Sandbox timer driver.
>
> Changes in v3:
> - Update commit message to reflect the v2 changes (ie: there is
>    no "counter-64bit" property)
> - Really remove "counter-64bit" property from tsc_timer.dtsi
>
> Changes in v2:
> - Rebase on u-boot-dm/master
> - Change 'Timer' to 'timer' in the sandbox timer Kconfig
> - New patch to use device tree to pass the clock frequency
> - Do not use "counter-64bit" property, instead create an inline
>    function for 32-bit timer driver to construct a 64-bit timer value.
> - Remove "counter-64bit" property
>
> Bin Meng (11):
>    dm: timer: Fix several nits
>    dm: timer: Implement pre_probe()
>    timer: altera: Remove the codes to get clock frequency
>    timer: sandbox: Use device tree to pass the clock frequency
>    dm: timer: Support 64-bit counter
>    x86: Reomve MIN_PORT80_KCLOCKS_DELAY
>    x86: tsc: Use notrace from <linux/compiler.h>
>    x86: tsc: Add driver model timer support
>    x86: Convert to use driver model timer
>    x86: tsc: Remove legacy timer codes
>    x86: tsc: Move tsc_timer.c to drivers/timer
>
>   arch/sandbox/dts/sandbox.dts                |   1 +
>   arch/x86/Kconfig                            |  20 ------
>   arch/x86/cpu/baytrail/valleyview.c          |   3 -
>   arch/x86/cpu/coreboot/timestamp.c           |  22 ------
>   arch/x86/cpu/cpu.c                          |  18 -----
>   arch/x86/cpu/efi/efi.c                      |   4 --
>   arch/x86/cpu/ivybridge/cpu.c                |   1 -
>   arch/x86/cpu/qemu/Kconfig                   |   1 -
>   arch/x86/cpu/qemu/qemu.c                    |   3 -
>   arch/x86/cpu/quark/Kconfig                  |   5 --
>   arch/x86/cpu/quark/quark.c                  |   3 -
>   arch/x86/cpu/queensbay/tnc.c                |   3 -
>   arch/x86/dts/bayleybay.dts                  |   1 +
>   arch/x86/dts/broadwell_som-6896.dts         |   1 +
>   arch/x86/dts/chromebook_link.dts            |   1 +
>   arch/x86/dts/chromebox_panther.dts          |   1 +
>   arch/x86/dts/crownbay.dts                   |   1 +
>   arch/x86/dts/efi.dts                        |   5 ++
>   arch/x86/dts/galileo.dts                    |   5 ++
>   arch/x86/dts/minnowmax.dts                  |   1 +
>   arch/x86/dts/qemu-x86_i440fx.dts            |   5 ++
>   arch/x86/dts/qemu-x86_q35.dts               |   5 ++
>   arch/x86/dts/tsc_timer.dtsi                 |   6 ++
>   arch/x86/include/asm/global_data.h          |   3 -
>   arch/x86/lib/Makefile                       |   1 -
>   configs/bayleybay_defconfig                 |   1 +
>   configs/chromebook_link_defconfig           |   2 +-
>   configs/chromebox_panther_defconfig         |   2 +-
>   configs/coreboot-x86_defconfig              |   3 +-
>   configs/crownbay_defconfig                  |   1 +
>   configs/efi-x86_defconfig                   |   2 +-
>   configs/galileo_defconfig                   |   1 +
>   configs/minnowmax_defconfig                 |   1 +
>   configs/qemu-x86_defconfig                  |   1 +
>   drivers/timer/Kconfig                       |  19 +++--
>   drivers/timer/Makefile                      |   1 +
>   drivers/timer/altera_timer.c                |  10 +--
>   drivers/timer/sandbox_timer.c               |   6 +-
>   drivers/timer/timer-uclass.c                |  19 +++--
>   {arch/x86/lib => drivers/timer}/tsc_timer.c | 104 ++++++++++++++++------------
>   include/configs/x86-common.h                |   2 -
>   include/timer.h                             |  34 ++++++---
>   lib/time.c                                  |   9 ++-
>   43 files changed, 164 insertions(+), 174 deletions(-)
>   create mode 100644 arch/x86/dts/tsc_timer.dtsi
>   rename {arch/x86/lib => drivers/timer}/tsc_timer.c (87%)
>

Thanks a lot. For the series,

Acked-by: Thomas Chou <thomas@wytron.com.tw>

Best regards,
Thomas

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Simon Glass]

From: Bin Meng <bmeng.cn@gmail.com>

There are timers with a 64-bit counter value but current timer
uclass driver assumes a 32-bit one. Modify timer_get_count()
to ask timer driver to always return a 64-bit counter value,
and provide an inline helper function timer_conv_64() to handle
the 32-bit/64-bit conversion automatically.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Change timer_conv_64() to a normal function instead of inline
- Avoid using DECLARE_GLOBAL_DATA_PTR in the timer.h header file

 drivers/timer/altera_timer.c  |  4 ++--
 drivers/timer/sandbox_timer.c |  2 +-
 drivers/timer/timer-uclass.c  | 15 ++++++++++++---
 include/timer.h               | 14 +++++++++++---
 lib/time.c                    |  9 ++++++---
 5 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
index 2364f1b..89fe05b 100644
--- a/drivers/timer/altera_timer.c
+++ b/drivers/timer/altera_timer.c
@@ -34,7 +34,7 @@ struct altera_timer_platdata {
 	struct altera_timer_regs *regs;
 };
 
-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;
@@ -46,7 +46,7 @@ static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
 	/* Read timer value */
 	val = readl(&regs->snapl) & 0xffff;
 	val |= (readl(&regs->snaph) & 0xffff) << 16;
-	*count = ~val;
+	*count = timer_conv_64(~val);
 
 	return 0;
 }
diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
index 4b20af2..00a9944 100644
--- a/drivers/timer/sandbox_timer.c
+++ b/drivers/timer/sandbox_timer.c
@@ -18,7 +18,7 @@ void sandbox_timer_add_offset(unsigned long offset)
 	sandbox_timer_offset += offset;
 }
 
-static int sandbox_timer_get_count(struct udevice *dev, unsigned long *count)
+static int sandbox_timer_get_count(struct udevice *dev, u64 *count)
 {
 	*count = os_get_nsec() / 1000 + sandbox_timer_offset * 1000;
 
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index 0218591..aca421b 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -13,14 +13,14 @@ 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 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);
 
@@ -47,6 +47,15 @@ static int timer_pre_probe(struct udevice *dev)
 	return 0;
 }
 
+u64 timer_conv_64(u32 count)
+{
+	/* increment tbh if tbl has rolled over */
+	if (count < gd->timebase_l)
+		gd->timebase_h++;
+	gd->timebase_l = count;
+	return ((u64)gd->timebase_h << 32) | gd->timebase_l;
+}
+
 UCLASS_DRIVER(timer) = {
 	.id		= UCLASS_TIMER,
 	.name		= "timer",
diff --git a/include/timer.h b/include/timer.h
index ed5c685..7fee17e 100644
--- a/include/timer.h
+++ b/include/timer.h
@@ -8,13 +8,21 @@
 #define _TIMER_H_
 
 /*
+ * timer_conv_64 - convert 32-bit counter value to 64-bit
+ *
+ * @count: 32-bit counter value
+ * @return: 64-bit counter value
+ */
+u64 timer_conv_64(u32 count);
+
+/*
  * Get the current timer count
  *
  * @dev: The timer device
  * @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
@@ -35,10 +43,10 @@ struct timer_ops {
 	 * Get the current timer count
 	 *
 	 * @dev: The timer device
-	 * @count: pointer that returns the current timer count
+	 * @count: pointer that returns the current 64-bit 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);
 };
 
 /*
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)
 {
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Bin Meng]

Hi Simon,

On Wed, Nov 25, 2015 at 4:31 AM, Simon Glass <sjg@chromium.org> wrote:
> From: Bin Meng <bmeng.cn@gmail.com>
>
> There are timers with a 64-bit counter value but current timer
> uclass driver assumes a 32-bit one. Modify timer_get_count()
> to ask timer driver to always return a 64-bit counter value,
> and provide an inline helper function timer_conv_64() to handle
> the 32-bit/64-bit conversion automatically.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v3:
> - Change timer_conv_64() to a normal function instead of inline
> - Avoid using DECLARE_GLOBAL_DATA_PTR in the timer.h header file
>
>  drivers/timer/altera_timer.c  |  4 ++--
>  drivers/timer/sandbox_timer.c |  2 +-
>  drivers/timer/timer-uclass.c  | 15 ++++++++++++---
>  include/timer.h               | 14 +++++++++++---
>  lib/time.c                    |  9 ++++++---
>  5 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
> index 2364f1b..89fe05b 100644
> --- a/drivers/timer/altera_timer.c
> +++ b/drivers/timer/altera_timer.c
> @@ -34,7 +34,7 @@ struct altera_timer_platdata {
>         struct altera_timer_regs *regs;
>  };
>
> -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;
> @@ -46,7 +46,7 @@ static int altera_timer_get_count(struct udevice *dev, unsigned long *count)
>         /* Read timer value */
>         val = readl(&regs->snapl) & 0xffff;
>         val |= (readl(&regs->snaph) & 0xffff) << 16;
> -       *count = ~val;
> +       *count = timer_conv_64(~val);
>
>         return 0;
>  }
> diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
> index 4b20af2..00a9944 100644
> --- a/drivers/timer/sandbox_timer.c
> +++ b/drivers/timer/sandbox_timer.c
> @@ -18,7 +18,7 @@ void sandbox_timer_add_offset(unsigned long offset)
>         sandbox_timer_offset += offset;
>  }
>
> -static int sandbox_timer_get_count(struct udevice *dev, unsigned long *count)
> +static int sandbox_timer_get_count(struct udevice *dev, u64 *count)
>  {
>         *count = os_get_nsec() / 1000 + sandbox_timer_offset * 1000;
>
> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
> index 0218591..aca421b 100644
> --- a/drivers/timer/timer-uclass.c
> +++ b/drivers/timer/timer-uclass.c
> @@ -13,14 +13,14 @@ 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 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);
>
> @@ -47,6 +47,15 @@ static int timer_pre_probe(struct udevice *dev)
>         return 0;
>  }
>
> +u64 timer_conv_64(u32 count)
> +{
> +       /* increment tbh if tbl has rolled over */
> +       if (count < gd->timebase_l)
> +               gd->timebase_h++;
> +       gd->timebase_l = count;
> +       return ((u64)gd->timebase_h << 32) | gd->timebase_l;
> +}
> +
>  UCLASS_DRIVER(timer) = {
>         .id             = UCLASS_TIMER,
>         .name           = "timer",
> diff --git a/include/timer.h b/include/timer.h
> index ed5c685..7fee17e 100644
> --- a/include/timer.h
> +++ b/include/timer.h
> @@ -8,13 +8,21 @@
>  #define _TIMER_H_
>
>  /*
> + * timer_conv_64 - convert 32-bit counter value to 64-bit
> + *
> + * @count: 32-bit counter value
> + * @return: 64-bit counter value
> + */
> +u64 timer_conv_64(u32 count);
> +
> +/*
>   * Get the current timer count
>   *
>   * @dev: The timer device
>   * @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
> @@ -35,10 +43,10 @@ struct timer_ops {
>          * Get the current timer count
>          *
>          * @dev: The timer device
> -        * @count: pointer that returns the current timer count
> +        * @count: pointer that returns the current 64-bit 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);
>  };
>
>  /*
> 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)
>  {
> --

Looks good to me. thanks!

Regards,
Bin