[U-Boot] [PATCH v5 0/2] timer: Add High Precision Event Timers (HPET) support

[U-Boot] [PATCH v5 0/2] timer: Add High Precision Event Timers (HPET) support

[Ivan Gorinov]

Add HPET driver as an alternative timer for x86 (default is TSC).
HPET counter has constant frequency and does not need calibration.
This change also makes TSC timer driver optional on x86.
New HPET driver can also be selected as the early timer on x86.

v5:
  Using new readq() and writeq() definitions.

v4:
  Using 64-bit pointer for main counter access.

v3:
  Added early timer choice in x86-specific configuration.

v2:
  Moved duplicated code to static functions.

Ivan Gorinov (2):
  x86: Add 64-bit memory-mapped I/O functions
  timer: Add High Precision Event Timers (HPET) support

 arch/Kconfig               |   2 +-
 arch/x86/Kconfig           |  21 ++++++
 arch/x86/dts/hpet.dtsi     |   7 ++
 arch/x86/include/asm/io.h  |  16 ++--
 drivers/timer/Kconfig      |   9 +++
 drivers/timer/Makefile     |   1 +
 drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/timer/tsc_timer.c  |   8 ++
 8 files changed, 236 insertions(+), 7 deletions(-)
 create mode 100644 arch/x86/dts/hpet.dtsi
 create mode 100644 drivers/timer/hpet_timer.c

-- 
2.7.4

[U-Boot] [PATCH v5 1/2] x86: Add 64-bit memory-mapped I/O functions

[Ivan Gorinov]

Add readq() and writeq() definitions for x86.

Please note: in 32-bit code readq/writeq will generate two 32-bit
memory access instructions instead of one atomic 64-bit operation.

Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
---
 arch/x86/include/asm/io.h | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 263dd8f..c7f6fcb 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -58,19 +58,23 @@
  * memory location directly.
  */
 
-#define readb(addr) (*(volatile unsigned char *) (addr))
-#define readw(addr) (*(volatile unsigned short *) (addr))
-#define readl(addr) (*(volatile unsigned int *) (addr))
+#define readb(addr) (*(volatile u8 *) (addr))
+#define readw(addr) (*(volatile u16 *) (addr))
+#define readl(addr) (*(volatile u32 *) (addr))
+#define readq(addr) (*(volatile u64 *) (addr))
 #define __raw_readb readb
 #define __raw_readw readw
 #define __raw_readl readl
+#define __raw_readq readq
 
-#define writeb(b,addr) (*(volatile unsigned char *) (addr) = (b))
-#define writew(b,addr) (*(volatile unsigned short *) (addr) = (b))
-#define writel(b,addr) (*(volatile unsigned int *) (addr) = (b))
+#define writeb(b, addr) (*(volatile u8 *) (addr) = (b))
+#define writew(b, addr) (*(volatile u16 *) (addr) = (b))
+#define writel(b, addr) (*(volatile u32 *) (addr) = (b))
+#define writeq(b, addr) (*(volatile u64 *) (addr) = (b))
 #define __raw_writeb writeb
 #define __raw_writew writew
 #define __raw_writel writel
+#define __raw_writeq writeq
 
 #define memset_io(a,b,c)	memset((a),(b),(c))
 #define memcpy_fromio(a,b,c)	memcpy((a),(b),(c))
-- 
2.7.4

[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Ivan Gorinov]

Add HPET driver as an alternative timer for x86 (default is TSC).
HPET counter has constant frequency and does not need calibration.
This change also makes TSC timer driver optional on x86.
New HPET driver can also be selected as the early timer on x86.

HPET can be selected as the tick timer in the Device Tree "chosen" node:

    /include/ "hpet.dtsi"

...

    chosen {
        tick-timer = "/hpet";
    };

Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
---
 arch/Kconfig               |   2 +-
 arch/x86/Kconfig           |  21 ++++++
 arch/x86/dts/hpet.dtsi     |   7 ++
 drivers/timer/Kconfig      |   9 +++
 drivers/timer/Makefile     |   1 +
 drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/timer/tsc_timer.c  |   8 ++
 7 files changed, 226 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/dts/hpet.dtsi
 create mode 100644 drivers/timer/hpet_timer.c

diff --git a/arch/Kconfig b/arch/Kconfig
index e599e7a..37dabae 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -104,7 +104,7 @@ config X86
 	select DM_PCI
 	select PCI
 	select TIMER
-	select X86_TSC_TIMER
+	imply X86_TSC_TIMER
 	imply BLK
 	imply DM_ETH
 	imply DM_GPIO
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5c23b2c..2fe5b6a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -119,6 +119,27 @@ source "arch/x86/cpu/tangier/Kconfig"
 
 # architecture-specific options below
 
+choice
+	prompt "Select which timer to use early on x86"
+	depends on X86
+	default X86_EARLY_TIMER_TSC
+
+config X86_EARLY_TIMER_TSC
+	bool "TSC"
+	depends on X86_TSC_TIMER
+	help
+	  This selects x86 Time Stamp Counter (TSC) as the early timer.
+	  See CONFIG_TIMER_EARLY for the early timer description.
+
+config X86_EARLY_TIMER_HPET
+	bool "HPET"
+	depends on HPET_TIMER
+	help
+	  This selects High Precision Event Timers as the early timer.
+	  Early HPET base address is specified by CONFIG_HPET_ADDRESS.
+
+endchoice
+
 config AHCI
 	default y
 
diff --git a/arch/x86/dts/hpet.dtsi b/arch/x86/dts/hpet.dtsi
new file mode 100644
index 0000000..a74f739
--- /dev/null
+++ b/arch/x86/dts/hpet.dtsi
@@ -0,0 +1,7 @@
+/ {
+	hpet: hpet at fed00000 {
+		compatible = "hpet-x86";
+		u-boot,dm-pre-reloc;
+		reg = <0xfed00000 0x1000>;
+	};
+};
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index 2c96896..26743b7 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -65,6 +65,15 @@ config X86_TSC_TIMER
 	help
 	  Select this to enable Time-Stamp Counter (TSC) timer for x86.
 
+config HPET_TIMER
+	bool "High Precision Event Timers (HPET) support"
+	depends on TIMER
+	default y if X86
+	help
+	  Select this to enable High Precision Event Timers (HPET) on x86.
+	  HPET main counter increments at constant rate and does not need
+	  calibration.
+
 config OMAP_TIMER
 	bool "Omap timer support"
 	depends on TIMER
diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
index a6e7832..557fecc 100644
--- a/drivers/timer/Makefile
+++ b/drivers/timer/Makefile
@@ -8,6 +8,7 @@ obj-y += 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
+obj-$(CONFIG_HPET_TIMER)	+= hpet_timer.o
 obj-$(CONFIG_OMAP_TIMER)	+= omap-timer.o
 obj-$(CONFIG_AST_TIMER)	+= ast_timer.o
 obj-$(CONFIG_STI_TIMER)		+= sti-timer.o
diff --git a/drivers/timer/hpet_timer.c b/drivers/timer/hpet_timer.c
new file mode 100644
index 0000000..b1ce226
--- /dev/null
+++ b/drivers/timer/hpet_timer.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <malloc.h>
+#include <timer.h>
+#include <asm/cpu.h>
+#include <asm/io.h>
+#include <asm/u-boot-x86.h>
+
+#define HPET_PERIOD_REG 0x004
+#define HPET_CONFIG_REG 0x010
+#define HPET_MAIN_COUNT 0x0f0
+
+#define ENABLE_CNF 1
+
+#define HPET_MAX_PERIOD 100000000
+
+struct hpet_timer_priv {
+	void *regs;
+};
+
+/*
+ * Returns HPET clock frequency in HZ
+ * (rounding to the nearest integer),
+ * or 0 if HPET is not available.
+ */
+static inline u32 get_clock_frequency(void *regs)
+{
+	u64 d = 1000000000000000ull;
+	u32 period;
+
+	period = readl(regs + HPET_PERIOD_REG);
+	if (period == 0)
+		return 0;
+	if (period > HPET_MAX_PERIOD)
+		return 0;
+
+	d += period / 2;
+
+	return d / period;
+}
+
+/* Reset and start the main counter. */
+static void start_main_counter(void *regs)
+{
+	u32 config;
+
+	config = readl(regs + HPET_CONFIG_REG);
+	config &= ~ENABLE_CNF;
+	writel(config, regs + HPET_CONFIG_REG);
+	writeq(0, regs + HPET_MAIN_COUNT);
+	config |= ENABLE_CNF;
+	writel(config, regs + HPET_CONFIG_REG);
+}
+
+/* Read the main counter, repeat if 32-bit rollover happens. */
+static u64 read_main_counter(void *regs)
+{
+	u64 t, t0;
+
+	t = readq(regs + HPET_MAIN_COUNT);
+	do {
+		t0 = t;
+		t = readq(regs + HPET_MAIN_COUNT);
+	} while ((t >> 32) != (t0 >> 32));
+
+	return t;
+}
+
+static int hpet_timer_get_count(struct udevice *dev, u64 *count)
+{
+	struct hpet_timer_priv *priv = dev_get_priv(dev);
+
+	*count = read_main_counter(priv->regs);
+
+	return 0;
+}
+
+static int hpet_timer_probe(struct udevice *dev)
+{
+	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct hpet_timer_priv *priv = dev_get_priv(dev);
+	u32 rate;
+
+	rate = get_clock_frequency(priv->regs);
+	if (rate == 0)
+		return -ENODEV;
+
+	start_main_counter(priv->regs);
+
+	uc_priv->clock_rate = rate;
+
+	return 0;
+}
+
+#ifdef CONFIG_X86_EARLY_TIMER_HPET
+
+static void *early_regs = (void *)CONFIG_HPET_ADDRESS;
+
+unsigned long notrace timer_early_get_rate(void)
+{
+	return get_clock_frequency(early_regs);
+}
+
+u64 notrace timer_early_get_count(void)
+{
+	return read_main_counter(early_regs);
+}
+
+int timer_init(void)
+{
+	if (get_clock_frequency(early_regs) == 0)
+		return -ENODEV;
+
+	start_main_counter(early_regs);
+
+	return 0;
+}
+
+ulong timer_get_boot_us(void)
+{
+	u32 period;
+	u64 d;
+
+	period = readl(early_regs + HPET_PERIOD_REG);
+	if (period == 0)
+		return 0;
+	if (period > HPET_MAX_PERIOD)
+		return 0;
+
+	d = read_main_counter(early_regs);
+
+	/*
+	 * Multiplication overflow
+	 * at 2^64 femtoseconds
+	 * (more than 5 hours)
+	 */
+
+	d *= period;
+
+	return d / 1000000000;
+}
+
+#endif /* CONFIG_X86_EARLY_TIMER_HPET */
+
+static int hpet_timer_ofdata_to_platdata(struct udevice *dev)
+{
+	struct hpet_timer_priv *priv = dev_get_priv(dev);
+
+	priv->regs = map_physmem(devfdt_get_addr(dev), 0x1000, MAP_NOCACHE);
+
+	return 0;
+}
+
+static const struct timer_ops hpet_timer_ops = {
+	.get_count = hpet_timer_get_count,
+};
+
+static const struct udevice_id hpet_timer_ids[] = {
+	{ .compatible = "hpet-x86", },
+	{ .compatible = "intel,ce4100-hpet", },
+	{ }
+};
+
+U_BOOT_DRIVER(hpet_timer) = {
+	.name	= "hpet_timer",
+	.id	= UCLASS_TIMER,
+	.of_match = hpet_timer_ids,
+	.ofdata_to_platdata = hpet_timer_ofdata_to_platdata,
+	.priv_auto_alloc_size = sizeof(struct hpet_timer_priv),
+	.probe = hpet_timer_probe,
+	.ops	= &hpet_timer_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
index 9296de6..bd0e75c 100644
--- a/drivers/timer/tsc_timer.c
+++ b/drivers/timer/tsc_timer.c
@@ -277,6 +277,8 @@ success:
 	return delta / 1000;
 }
 
+#ifdef CONFIG_X86_EARLY_TIMER_TSC
+
 /* Get the speed of the TSC timer in MHz */
 unsigned notrace long get_tbclk_mhz(void)
 {
@@ -322,6 +324,8 @@ void __udelay(unsigned long usec)
 #endif
 }
 
+#endif /* CONFIG_X86_EARLY_TIMER_TSC */
+
 static int tsc_timer_get_count(struct udevice *dev, u64 *count)
 {
 	u64 now_tick = rdtsc();
@@ -365,6 +369,8 @@ static int tsc_timer_probe(struct udevice *dev)
 	return 0;
 }
 
+#ifdef CONFIG_X86_EARLY_TIMER_TSC
+
 unsigned long notrace timer_early_get_rate(void)
 {
 	tsc_timer_ensure_setup();
@@ -377,6 +383,8 @@ u64 notrace timer_early_get_count(void)
 	return rdtsc() - gd->arch.tsc_base;
 }
 
+#endif
+
 static const struct timer_ops tsc_timer_ops = {
 	.get_count = tsc_timer_get_count,
 };
-- 
2.7.4

[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Andy Shevchenko]

On Fri, 2018-04-06 at 12:18 -0700, Ivan Gorinov wrote:
> Add HPET driver as an alternative timer for x86 (default is TSC).
> HPET counter has constant frequency and does not need calibration.
> This change also makes TSC timer driver optional on x86.
> New HPET driver can also be selected as the early timer on x86.
> 
> HPET can be selected as the tick timer in the Device Tree "chosen"
> node:
> 
>     /include/ "hpet.dtsi"
> 
> ...
> 
>     chosen {
>         tick-timer = "/hpet";
>     };

FWIW,

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
> ---
>  arch/Kconfig               |   2 +-
>  arch/x86/Kconfig           |  21 ++++++
>  arch/x86/dts/hpet.dtsi     |   7 ++
>  drivers/timer/Kconfig      |   9 +++
>  drivers/timer/Makefile     |   1 +
>  drivers/timer/hpet_timer.c | 179
> +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/timer/tsc_timer.c  |   8 ++
>  7 files changed, 226 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/dts/hpet.dtsi
>  create mode 100644 drivers/timer/hpet_timer.c
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e599e7a..37dabae 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -104,7 +104,7 @@ config X86
>  	select DM_PCI
>  	select PCI
>  	select TIMER
> -	select X86_TSC_TIMER
> +	imply X86_TSC_TIMER
>  	imply BLK
>  	imply DM_ETH
>  	imply DM_GPIO
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5c23b2c..2fe5b6a 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -119,6 +119,27 @@ source "arch/x86/cpu/tangier/Kconfig"
>  
>  # architecture-specific options below
>  
> +choice
> +	prompt "Select which timer to use early on x86"
> +	depends on X86
> +	default X86_EARLY_TIMER_TSC
> +
> +config X86_EARLY_TIMER_TSC
> +	bool "TSC"
> +	depends on X86_TSC_TIMER
> +	help
> +	  This selects x86 Time Stamp Counter (TSC) as the early
> timer.
> +	  See CONFIG_TIMER_EARLY for the early timer description.
> +
> +config X86_EARLY_TIMER_HPET
> +	bool "HPET"
> +	depends on HPET_TIMER
> +	help
> +	  This selects High Precision Event Timers as the early
> timer.
> +	  Early HPET base address is specified by
> CONFIG_HPET_ADDRESS.
> +
> +endchoice
> +
>  config AHCI
>  	default y
>  
> diff --git a/arch/x86/dts/hpet.dtsi b/arch/x86/dts/hpet.dtsi
> new file mode 100644
> index 0000000..a74f739
> --- /dev/null
> +++ b/arch/x86/dts/hpet.dtsi
> @@ -0,0 +1,7 @@
> +/ {
> +	hpet: hpet at fed00000 {
> +		compatible = "hpet-x86";
> +		u-boot,dm-pre-reloc;
> +		reg = <0xfed00000 0x1000>;
> +	};
> +};
> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> index 2c96896..26743b7 100644
> --- a/drivers/timer/Kconfig
> +++ b/drivers/timer/Kconfig
> @@ -65,6 +65,15 @@ config X86_TSC_TIMER
>  	help
>  	  Select this to enable Time-Stamp Counter (TSC) timer for
> x86.
>  
> +config HPET_TIMER
> +	bool "High Precision Event Timers (HPET) support"
> +	depends on TIMER
> +	default y if X86
> +	help
> +	  Select this to enable High Precision Event Timers (HPET) on
> x86.
> +	  HPET main counter increments at constant rate and does not
> need
> +	  calibration.
> +
>  config OMAP_TIMER
>  	bool "Omap timer support"
>  	depends on TIMER
> diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
> index a6e7832..557fecc 100644
> --- a/drivers/timer/Makefile
> +++ b/drivers/timer/Makefile
> @@ -8,6 +8,7 @@ obj-y += 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
> +obj-$(CONFIG_HPET_TIMER)	+= hpet_timer.o
>  obj-$(CONFIG_OMAP_TIMER)	+= omap-timer.o
>  obj-$(CONFIG_AST_TIMER)	+= ast_timer.o
>  obj-$(CONFIG_STI_TIMER)		+= sti-timer.o
> diff --git a/drivers/timer/hpet_timer.c b/drivers/timer/hpet_timer.c
> new file mode 100644
> index 0000000..b1ce226
> --- /dev/null
> +++ b/drivers/timer/hpet_timer.c
> @@ -0,0 +1,179 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <malloc.h>
> +#include <timer.h>
> +#include <asm/cpu.h>
> +#include <asm/io.h>
> +#include <asm/u-boot-x86.h>
> +
> +#define HPET_PERIOD_REG 0x004
> +#define HPET_CONFIG_REG 0x010
> +#define HPET_MAIN_COUNT 0x0f0
> +
> +#define ENABLE_CNF 1
> +
> +#define HPET_MAX_PERIOD 100000000
> +
> +struct hpet_timer_priv {
> +	void *regs;
> +};
> +
> +/*
> + * Returns HPET clock frequency in HZ
> + * (rounding to the nearest integer),
> + * or 0 if HPET is not available.
> + */
> +static inline u32 get_clock_frequency(void *regs)
> +{
> +	u64 d = 1000000000000000ull;
> +	u32 period;
> +
> +	period = readl(regs + HPET_PERIOD_REG);
> +	if (period == 0)
> +		return 0;
> +	if (period > HPET_MAX_PERIOD)
> +		return 0;
> +
> +	d += period / 2;
> +
> +	return d / period;
> +}
> +
> +/* Reset and start the main counter. */
> +static void start_main_counter(void *regs)
> +{
> +	u32 config;
> +
> +	config = readl(regs + HPET_CONFIG_REG);
> +	config &= ~ENABLE_CNF;
> +	writel(config, regs + HPET_CONFIG_REG);
> +	writeq(0, regs + HPET_MAIN_COUNT);
> +	config |= ENABLE_CNF;
> +	writel(config, regs + HPET_CONFIG_REG);
> +}
> +
> +/* Read the main counter, repeat if 32-bit rollover happens. */
> +static u64 read_main_counter(void *regs)
> +{
> +	u64 t, t0;
> +
> +	t = readq(regs + HPET_MAIN_COUNT);
> +	do {
> +		t0 = t;
> +		t = readq(regs + HPET_MAIN_COUNT);
> +	} while ((t >> 32) != (t0 >> 32));
> +
> +	return t;
> +}
> +
> +static int hpet_timer_get_count(struct udevice *dev, u64 *count)
> +{
> +	struct hpet_timer_priv *priv = dev_get_priv(dev);
> +
> +	*count = read_main_counter(priv->regs);
> +
> +	return 0;
> +}
> +
> +static int hpet_timer_probe(struct udevice *dev)
> +{
> +	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +	struct hpet_timer_priv *priv = dev_get_priv(dev);
> +	u32 rate;
> +
> +	rate = get_clock_frequency(priv->regs);
> +	if (rate == 0)
> +		return -ENODEV;
> +
> +	start_main_counter(priv->regs);
> +
> +	uc_priv->clock_rate = rate;
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_X86_EARLY_TIMER_HPET
> +
> +static void *early_regs = (void *)CONFIG_HPET_ADDRESS;
> +
> +unsigned long notrace timer_early_get_rate(void)
> +{
> +	return get_clock_frequency(early_regs);
> +}
> +
> +u64 notrace timer_early_get_count(void)
> +{
> +	return read_main_counter(early_regs);
> +}
> +
> +int timer_init(void)
> +{
> +	if (get_clock_frequency(early_regs) == 0)
> +		return -ENODEV;
> +
> +	start_main_counter(early_regs);
> +
> +	return 0;
> +}
> +
> +ulong timer_get_boot_us(void)
> +{
> +	u32 period;
> +	u64 d;
> +
> +	period = readl(early_regs + HPET_PERIOD_REG);
> +	if (period == 0)
> +		return 0;
> +	if (period > HPET_MAX_PERIOD)
> +		return 0;
> +
> +	d = read_main_counter(early_regs);
> +
> +	/*
> +	 * Multiplication overflow
> +	 * at 2^64 femtoseconds
> +	 * (more than 5 hours)
> +	 */
> +
> +	d *= period;
> +
> +	return d / 1000000000;
> +}
> +
> +#endif /* CONFIG_X86_EARLY_TIMER_HPET */
> +
> +static int hpet_timer_ofdata_to_platdata(struct udevice *dev)
> +{
> +	struct hpet_timer_priv *priv = dev_get_priv(dev);
> +
> +	priv->regs = map_physmem(devfdt_get_addr(dev), 0x1000,
> MAP_NOCACHE);
> +
> +	return 0;
> +}
> +
> +static const struct timer_ops hpet_timer_ops = {
> +	.get_count = hpet_timer_get_count,
> +};
> +
> +static const struct udevice_id hpet_timer_ids[] = {
> +	{ .compatible = "hpet-x86", },
> +	{ .compatible = "intel,ce4100-hpet", },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(hpet_timer) = {
> +	.name	= "hpet_timer",
> +	.id	= UCLASS_TIMER,
> +	.of_match = hpet_timer_ids,
> +	.ofdata_to_platdata = hpet_timer_ofdata_to_platdata,
> +	.priv_auto_alloc_size = sizeof(struct hpet_timer_priv),
> +	.probe = hpet_timer_probe,
> +	.ops	= &hpet_timer_ops,
> +	.flags = DM_FLAG_PRE_RELOC,
> +};
> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
> index 9296de6..bd0e75c 100644
> --- a/drivers/timer/tsc_timer.c
> +++ b/drivers/timer/tsc_timer.c
> @@ -277,6 +277,8 @@ success:
>  	return delta / 1000;
>  }
>  
> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
> +
>  /* Get the speed of the TSC timer in MHz */
>  unsigned notrace long get_tbclk_mhz(void)
>  {
> @@ -322,6 +324,8 @@ void __udelay(unsigned long usec)
>  #endif
>  }
>  
> +#endif /* CONFIG_X86_EARLY_TIMER_TSC */
> +
>  static int tsc_timer_get_count(struct udevice *dev, u64 *count)
>  {
>  	u64 now_tick = rdtsc();
> @@ -365,6 +369,8 @@ static int tsc_timer_probe(struct udevice *dev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
> +
>  unsigned long notrace timer_early_get_rate(void)
>  {
>  	tsc_timer_ensure_setup();
> @@ -377,6 +383,8 @@ u64 notrace timer_early_get_count(void)
>  	return rdtsc() - gd->arch.tsc_base;
>  }
>  
> +#endif
> +
>  static const struct timer_ops tsc_timer_ops = {
>  	.get_count = tsc_timer_get_count,
>  };

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[U-Boot] [PATCH v5 1/2] x86: Add 64-bit memory-mapped I/O functions

[Andy Shevchenko]

On Fri, 2018-04-06 at 12:17 -0700, Ivan Gorinov wrote:
> Add readq() and writeq() definitions for x86.
> 
> Please note: in 32-bit code readq/writeq will generate two 32-bit
> memory access instructions instead of one atomic 64-bit operation.
> 

 
> -#define readb(addr) (*(volatile unsigned char *) (addr))
> -#define readw(addr) (*(volatile unsigned short *) (addr))
> -#define readl(addr) (*(volatile unsigned int *) (addr))
> +#define readb(addr) (*(volatile u8 *) (addr))
> +#define readw(addr) (*(volatile u16 *) (addr))
> +#define readl(addr) (*(volatile u32 *) (addr))


> -#define writeb(b,addr) (*(volatile unsigned char *) (addr) = (b))
> -#define writew(b,addr) (*(volatile unsigned short *) (addr) = (b))
> -#define writel(b,addr) (*(volatile unsigned int *) (addr) = (b))
> +#define writeb(b, addr) (*(volatile u8 *) (addr) = (b))
> +#define writew(b, addr) (*(volatile u16 *) (addr) = (b))
> +#define writel(b, addr) (*(volatile u32 *) (addr) = (b))

What's wrong with the existing types?

Even in ARM case they are using unsigned long long for 64-bit variant.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Bin Meng]

Hi Ivan,

On Sat, Apr 7, 2018 at 3:18 AM, Ivan Gorinov <ivan.gorinov@intel.com> wrote:
> Add HPET driver as an alternative timer for x86 (default is TSC).
> HPET counter has constant frequency and does not need calibration.
> This change also makes TSC timer driver optional on x86.
> New HPET driver can also be selected as the early timer on x86.
>
> HPET can be selected as the tick timer in the Device Tree "chosen" node:
>
>     /include/ "hpet.dtsi"
>
> ...
>
>     chosen {
>         tick-timer = "/hpet";
>     };
>
> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
> ---
>  arch/Kconfig               |   2 +-
>  arch/x86/Kconfig           |  21 ++++++
>  arch/x86/dts/hpet.dtsi     |   7 ++
>  drivers/timer/Kconfig      |   9 +++
>  drivers/timer/Makefile     |   1 +
>  drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/timer/tsc_timer.c  |   8 ++
>  7 files changed, 226 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/dts/hpet.dtsi
>  create mode 100644 drivers/timer/hpet_timer.c
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e599e7a..37dabae 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -104,7 +104,7 @@ config X86
>         select DM_PCI
>         select PCI
>         select TIMER
> -       select X86_TSC_TIMER
> +       imply X86_TSC_TIMER
>         imply BLK
>         imply DM_ETH
>         imply DM_GPIO
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5c23b2c..2fe5b6a 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -119,6 +119,27 @@ source "arch/x86/cpu/tangier/Kconfig"
>
>  # architecture-specific options below
>
> +choice
> +       prompt "Select which timer to use early on x86"

nits: "on x86" is not needed since this option is only visible in the
x86 sub-menu

> +       depends on X86

This depends is not necessary as this option is already included in
arch/x86/Kconfig.
But it should really depend on CONFIG_TIMER_EARLY

> +       default X86_EARLY_TIMER_TSC
> +
> +config X86_EARLY_TIMER_TSC
> +       bool "TSC"
> +       depends on X86_TSC_TIMER
> +       help
> +         This selects x86 Time Stamp Counter (TSC) as the early timer.
> +         See CONFIG_TIMER_EARLY for the early timer description.
> +
> +config X86_EARLY_TIMER_HPET
> +       bool "HPET"
> +       depends on HPET_TIMER
> +       help
> +         This selects High Precision Event Timers as the early timer.
> +         Early HPET base address is specified by CONFIG_HPET_ADDRESS.
> +
> +endchoice
> +
>  config AHCI
>         default y
>
> diff --git a/arch/x86/dts/hpet.dtsi b/arch/x86/dts/hpet.dtsi
> new file mode 100644
> index 0000000..a74f739
> --- /dev/null
> +++ b/arch/x86/dts/hpet.dtsi
> @@ -0,0 +1,7 @@
> +/ {
> +       hpet: hpet at fed00000 {
> +               compatible = "hpet-x86";
> +               u-boot,dm-pre-reloc;
> +               reg = <0xfed00000 0x1000>;
> +       };
> +};
> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> index 2c96896..26743b7 100644
> --- a/drivers/timer/Kconfig
> +++ b/drivers/timer/Kconfig
> @@ -65,6 +65,15 @@ config X86_TSC_TIMER
>         help
>           Select this to enable Time-Stamp Counter (TSC) timer for x86.
>
> +config HPET_TIMER
> +       bool "High Precision Event Timers (HPET) support"
> +       depends on TIMER
> +       default y if X86
> +       help
> +         Select this to enable High Precision Event Timers (HPET) on x86.
> +         HPET main counter increments at constant rate and does not need
> +         calibration.
> +
>  config OMAP_TIMER
>         bool "Omap timer support"
>         depends on TIMER
> diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
> index a6e7832..557fecc 100644
> --- a/drivers/timer/Makefile
> +++ b/drivers/timer/Makefile
> @@ -8,6 +8,7 @@ obj-y += 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
> +obj-$(CONFIG_HPET_TIMER)       += hpet_timer.o
>  obj-$(CONFIG_OMAP_TIMER)       += omap-timer.o
>  obj-$(CONFIG_AST_TIMER)        += ast_timer.o
>  obj-$(CONFIG_STI_TIMER)                += sti-timer.o
> diff --git a/drivers/timer/hpet_timer.c b/drivers/timer/hpet_timer.c
> new file mode 100644
> index 0000000..b1ce226
> --- /dev/null
> +++ b/drivers/timer/hpet_timer.c
> @@ -0,0 +1,179 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <malloc.h>
> +#include <timer.h>
> +#include <asm/cpu.h>
> +#include <asm/io.h>
> +#include <asm/u-boot-x86.h>
> +
> +#define HPET_PERIOD_REG 0x004
> +#define HPET_CONFIG_REG 0x010
> +#define HPET_MAIN_COUNT 0x0f0
> +
> +#define ENABLE_CNF 1
> +
> +#define HPET_MAX_PERIOD 100000000
> +
> +struct hpet_timer_priv {
> +       void *regs;
> +};
> +
> +/*
> + * Returns HPET clock frequency in HZ
> + * (rounding to the nearest integer),
> + * or 0 if HPET is not available.
> + */
> +static inline u32 get_clock_frequency(void *regs)
> +{
> +       u64 d = 1000000000000000ull;
> +       u32 period;
> +
> +       period = readl(regs + HPET_PERIOD_REG);
> +       if (period == 0)
> +               return 0;
> +       if (period > HPET_MAX_PERIOD)
> +               return 0;
> +
> +       d += period / 2;
> +
> +       return d / period;
> +}
> +
> +/* Reset and start the main counter. */

nits: remove the ending .

> +static void start_main_counter(void *regs)
> +{
> +       u32 config;
> +
> +       config = readl(regs + HPET_CONFIG_REG);
> +       config &= ~ENABLE_CNF;
> +       writel(config, regs + HPET_CONFIG_REG);
> +       writeq(0, regs + HPET_MAIN_COUNT);
> +       config |= ENABLE_CNF;
> +       writel(config, regs + HPET_CONFIG_REG);
> +}
> +
> +/* Read the main counter, repeat if 32-bit rollover happens. */

nits: remove the ending .

> +static u64 read_main_counter(void *regs)
> +{
> +       u64 t, t0;
> +
> +       t = readq(regs + HPET_MAIN_COUNT);
> +       do {
> +               t0 = t;
> +               t = readq(regs + HPET_MAIN_COUNT);
> +       } while ((t >> 32) != (t0 >> 32));
> +
> +       return t;
> +}
> +
> +static int hpet_timer_get_count(struct udevice *dev, u64 *count)
> +{
> +       struct hpet_timer_priv *priv = dev_get_priv(dev);
> +
> +       *count = read_main_counter(priv->regs);
> +
> +       return 0;
> +}
> +
> +static int hpet_timer_probe(struct udevice *dev)
> +{
> +       struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +       struct hpet_timer_priv *priv = dev_get_priv(dev);
> +       u32 rate;
> +
> +       rate = get_clock_frequency(priv->regs);
> +       if (rate == 0)
> +               return -ENODEV;
> +
> +       start_main_counter(priv->regs);
> +
> +       uc_priv->clock_rate = rate;
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_X86_EARLY_TIMER_HPET
> +
> +static void *early_regs = (void *)CONFIG_HPET_ADDRESS;
> +
> +unsigned long notrace timer_early_get_rate(void)
> +{
> +       return get_clock_frequency(early_regs);
> +}
> +
> +u64 notrace timer_early_get_count(void)
> +{
> +       return read_main_counter(early_regs);
> +}
> +
> +int timer_init(void)
> +{
> +       if (get_clock_frequency(early_regs) == 0)
> +               return -ENODEV;
> +
> +       start_main_counter(early_regs);
> +
> +       return 0;
> +}
> +
> +ulong timer_get_boot_us(void)
> +{
> +       u32 period;
> +       u64 d;
> +
> +       period = readl(early_regs + HPET_PERIOD_REG);
> +       if (period == 0)
> +               return 0;
> +       if (period > HPET_MAX_PERIOD)
> +               return 0;
> +
> +       d = read_main_counter(early_regs);
> +
> +       /*
> +        * Multiplication overflow
> +        * at 2^64 femtoseconds
> +        * (more than 5 hours)
> +        */
> +
> +       d *= period;
> +
> +       return d / 1000000000;
> +}
> +
> +#endif /* CONFIG_X86_EARLY_TIMER_HPET */
> +
> +static int hpet_timer_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct hpet_timer_priv *priv = dev_get_priv(dev);
> +
> +       priv->regs = map_physmem(devfdt_get_addr(dev), 0x1000, MAP_NOCACHE);
> +
> +       return 0;
> +}
> +
> +static const struct timer_ops hpet_timer_ops = {
> +       .get_count = hpet_timer_get_count,
> +};
> +
> +static const struct udevice_id hpet_timer_ids[] = {
> +       { .compatible = "hpet-x86", },
> +       { .compatible = "intel,ce4100-hpet", },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(hpet_timer) = {
> +       .name   = "hpet_timer",
> +       .id     = UCLASS_TIMER,
> +       .of_match = hpet_timer_ids,
> +       .ofdata_to_platdata = hpet_timer_ofdata_to_platdata,
> +       .priv_auto_alloc_size = sizeof(struct hpet_timer_priv),
> +       .probe = hpet_timer_probe,
> +       .ops    = &hpet_timer_ops,
> +       .flags = DM_FLAG_PRE_RELOC,
> +};
> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
> index 9296de6..bd0e75c 100644
> --- a/drivers/timer/tsc_timer.c
> +++ b/drivers/timer/tsc_timer.c
> @@ -277,6 +277,8 @@ success:
>         return delta / 1000;
>  }
>
> +#ifdef CONFIG_X86_EARLY_TIMER_TSC

Why do we surround the following APIs with CONFIG_X86_EARLY_TIMER_TSC?
These APIs are generic U-Boot timer APIs. If we select
CONFIG_X86_EARLY_TIMER_HPET, these APIs are not available and will
cause build error.

Simon, do you think we should fix such in the timer uclass driver?

> +
>  /* Get the speed of the TSC timer in MHz */
>  unsigned notrace long get_tbclk_mhz(void)
>  {
> @@ -322,6 +324,8 @@ void __udelay(unsigned long usec)
>  #endif
>  }
>
> +#endif /* CONFIG_X86_EARLY_TIMER_TSC */
> +
>  static int tsc_timer_get_count(struct udevice *dev, u64 *count)
>  {
>         u64 now_tick = rdtsc();
> @@ -365,6 +369,8 @@ static int tsc_timer_probe(struct udevice *dev)
>         return 0;
>  }
>
> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
> +
>  unsigned long notrace timer_early_get_rate(void)
>  {
>         tsc_timer_ensure_setup();
> @@ -377,6 +383,8 @@ u64 notrace timer_early_get_count(void)
>         return rdtsc() - gd->arch.tsc_base;
>  }
>
> +#endif
> +
>  static const struct timer_ops tsc_timer_ops = {
>         .get_count = tsc_timer_get_count,
>  };
> --

Regards,
Bin

[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Simon Glass]

Hi,

On 8 April 2018 at 19:22, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Ivan,
>
> On Sat, Apr 7, 2018 at 3:18 AM, Ivan Gorinov <ivan.gorinov@intel.com> wrote:
>> Add HPET driver as an alternative timer for x86 (default is TSC).
>> HPET counter has constant frequency and does not need calibration.
>> This change also makes TSC timer driver optional on x86.
>> New HPET driver can also be selected as the early timer on x86.
>>
>> HPET can be selected as the tick timer in the Device Tree "chosen" node:
>>
>>     /include/ "hpet.dtsi"
>>
>> ...
>>
>>     chosen {
>>         tick-timer = "/hpet";
>>     };
>>
>> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
>> ---
>>  arch/Kconfig               |   2 +-
>>  arch/x86/Kconfig           |  21 ++++++
>>  arch/x86/dts/hpet.dtsi     |   7 ++
>>  drivers/timer/Kconfig      |   9 +++
>>  drivers/timer/Makefile     |   1 +
>>  drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/timer/tsc_timer.c  |   8 ++
>>  7 files changed, 226 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/x86/dts/hpet.dtsi
>>  create mode 100644 drivers/timer/hpet_timer.c
>>

[..]
>> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
>> index 9296de6..bd0e75c 100644
>> --- a/drivers/timer/tsc_timer.c
>> +++ b/drivers/timer/tsc_timer.c
>> @@ -277,6 +277,8 @@ success:
>>         return delta / 1000;
>>  }
>>
>> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
>
> Why do we surround the following APIs with CONFIG_X86_EARLY_TIMER_TSC?
> These APIs are generic U-Boot timer APIs. If we select
> CONFIG_X86_EARLY_TIMER_HPET, these APIs are not available and will
> cause build error.
>
> Simon, do you think we should fix such in the timer uclass driver?

We should not have arch-specific code in the uclass, or in any generic driver.

Regards,
Simon

[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Bin Meng]

Hi Simon,

On Fri, Apr 13, 2018 at 12:42 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 8 April 2018 at 19:22, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Ivan,
>>
>> On Sat, Apr 7, 2018 at 3:18 AM, Ivan Gorinov <ivan.gorinov@intel.com> wrote:
>>> Add HPET driver as an alternative timer for x86 (default is TSC).
>>> HPET counter has constant frequency and does not need calibration.
>>> This change also makes TSC timer driver optional on x86.
>>> New HPET driver can also be selected as the early timer on x86.
>>>
>>> HPET can be selected as the tick timer in the Device Tree "chosen" node:
>>>
>>>     /include/ "hpet.dtsi"
>>>
>>> ...
>>>
>>>     chosen {
>>>         tick-timer = "/hpet";
>>>     };
>>>
>>> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
>>> ---
>>>  arch/Kconfig               |   2 +-
>>>  arch/x86/Kconfig           |  21 ++++++
>>>  arch/x86/dts/hpet.dtsi     |   7 ++
>>>  drivers/timer/Kconfig      |   9 +++
>>>  drivers/timer/Makefile     |   1 +
>>>  drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/timer/tsc_timer.c  |   8 ++
>>>  7 files changed, 226 insertions(+), 1 deletion(-)
>>>  create mode 100644 arch/x86/dts/hpet.dtsi
>>>  create mode 100644 drivers/timer/hpet_timer.c
>>>
>
> [..]
>>> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
>>> index 9296de6..bd0e75c 100644
>>> --- a/drivers/timer/tsc_timer.c
>>> +++ b/drivers/timer/tsc_timer.c
>>> @@ -277,6 +277,8 @@ success:
>>>         return delta / 1000;
>>>  }
>>>
>>> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
>>
>> Why do we surround the following APIs with CONFIG_X86_EARLY_TIMER_TSC?
>> These APIs are generic U-Boot timer APIs. If we select
>> CONFIG_X86_EARLY_TIMER_HPET, these APIs are not available and will
>> cause build error.
>>
>> Simon, do you think we should fix such in the timer uclass driver?
>
> We should not have arch-specific code in the uclass, or in any generic driver.

What I meant is these APIs like get_timer(). Can such APIs be moved to
timer uclass driver? To avoid conflicts with other timer drivers, in
this patch it was solved by using the config option
CONFIG_X86_EARLY_TIMER_TSC to hide this in the TSC driver, which does
not make sense.

Regards,
Bin

[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Simon Glass]

Hi Bin,

On 15 April 2018 at 23:06, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Fri, Apr 13, 2018 at 12:42 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi,
>>
>> On 8 April 2018 at 19:22, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Ivan,
>>>
>>> On Sat, Apr 7, 2018 at 3:18 AM, Ivan Gorinov <ivan.gorinov@intel.com> wrote:
>>>> Add HPET driver as an alternative timer for x86 (default is TSC).
>>>> HPET counter has constant frequency and does not need calibration.
>>>> This change also makes TSC timer driver optional on x86.
>>>> New HPET driver can also be selected as the early timer on x86.
>>>>
>>>> HPET can be selected as the tick timer in the Device Tree "chosen" node:
>>>>
>>>>     /include/ "hpet.dtsi"
>>>>
>>>> ...
>>>>
>>>>     chosen {
>>>>         tick-timer = "/hpet";
>>>>     };
>>>>
>>>> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
>>>> ---
>>>>  arch/Kconfig               |   2 +-
>>>>  arch/x86/Kconfig           |  21 ++++++
>>>>  arch/x86/dts/hpet.dtsi     |   7 ++
>>>>  drivers/timer/Kconfig      |   9 +++
>>>>  drivers/timer/Makefile     |   1 +
>>>>  drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  drivers/timer/tsc_timer.c  |   8 ++
>>>>  7 files changed, 226 insertions(+), 1 deletion(-)
>>>>  create mode 100644 arch/x86/dts/hpet.dtsi
>>>>  create mode 100644 drivers/timer/hpet_timer.c
>>>>
>>
>> [..]
>>>> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
>>>> index 9296de6..bd0e75c 100644
>>>> --- a/drivers/timer/tsc_timer.c
>>>> +++ b/drivers/timer/tsc_timer.c
>>>> @@ -277,6 +277,8 @@ success:
>>>>         return delta / 1000;
>>>>  }
>>>>
>>>> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
>>>
>>> Why do we surround the following APIs with CONFIG_X86_EARLY_TIMER_TSC?
>>> These APIs are generic U-Boot timer APIs. If we select
>>> CONFIG_X86_EARLY_TIMER_HPET, these APIs are not available and will
>>> cause build error.
>>>
>>> Simon, do you think we should fix such in the timer uclass driver?
>>
>> We should not have arch-specific code in the uclass, or in any generic driver.
>
> What I meant is these APIs like get_timer(). Can such APIs be moved to
> timer uclass driver? To avoid conflicts with other timer drivers, in
> this patch it was solved by using the config option
> CONFIG_X86_EARLY_TIMER_TSC to hide this in the TSC driver, which does

Well get_timer() just calls get_ticks() which uses DM, so what exactly
are you proposing here?

> not make sense.
>
> Regards,
> Bin

Regards,
Simon

[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Bin Meng]

Hi Simon,

On Tue, Apr 17, 2018 at 11:10 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 15 April 2018 at 23:06, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon,
>>
>> On Fri, Apr 13, 2018 at 12:42 AM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi,
>>>
>>> On 8 April 2018 at 19:22, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Hi Ivan,
>>>>
>>>> On Sat, Apr 7, 2018 at 3:18 AM, Ivan Gorinov <ivan.gorinov@intel.com> wrote:
>>>>> Add HPET driver as an alternative timer for x86 (default is TSC).
>>>>> HPET counter has constant frequency and does not need calibration.
>>>>> This change also makes TSC timer driver optional on x86.
>>>>> New HPET driver can also be selected as the early timer on x86.
>>>>>
>>>>> HPET can be selected as the tick timer in the Device Tree "chosen" node:
>>>>>
>>>>>     /include/ "hpet.dtsi"
>>>>>
>>>>> ...
>>>>>
>>>>>     chosen {
>>>>>         tick-timer = "/hpet";
>>>>>     };
>>>>>
>>>>> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
>>>>> ---
>>>>>  arch/Kconfig               |   2 +-
>>>>>  arch/x86/Kconfig           |  21 ++++++
>>>>>  arch/x86/dts/hpet.dtsi     |   7 ++
>>>>>  drivers/timer/Kconfig      |   9 +++
>>>>>  drivers/timer/Makefile     |   1 +
>>>>>  drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>  drivers/timer/tsc_timer.c  |   8 ++
>>>>>  7 files changed, 226 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 arch/x86/dts/hpet.dtsi
>>>>>  create mode 100644 drivers/timer/hpet_timer.c
>>>>>
>>>
>>> [..]
>>>>> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
>>>>> index 9296de6..bd0e75c 100644
>>>>> --- a/drivers/timer/tsc_timer.c
>>>>> +++ b/drivers/timer/tsc_timer.c
>>>>> @@ -277,6 +277,8 @@ success:
>>>>>         return delta / 1000;
>>>>>  }
>>>>>
>>>>> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
>>>>
>>>> Why do we surround the following APIs with CONFIG_X86_EARLY_TIMER_TSC?
>>>> These APIs are generic U-Boot timer APIs. If we select
>>>> CONFIG_X86_EARLY_TIMER_HPET, these APIs are not available and will
>>>> cause build error.
>>>>
>>>> Simon, do you think we should fix such in the timer uclass driver?
>>>
>>> We should not have arch-specific code in the uclass, or in any generic driver.
>>
>> What I meant is these APIs like get_timer(). Can such APIs be moved to
>> timer uclass driver? To avoid conflicts with other timer drivers, in
>> this patch it was solved by using the config option
>> CONFIG_X86_EARLY_TIMER_TSC to hide this in the TSC driver, which does
>
> Well get_timer() just calls get_ticks() which uses DM, so what exactly
> are you proposing here?
>

The tsc_timer.c has the get_timer() implementation while lib/time.c
provides a default one that calls into DM. When I saw this new HPET
driver I got confused and thought we should move such from timer
driver to the uclass driver. My original comments regarding to
CONFIG_X86_EARLY_TIMER_TSC is to make sure we support mixed use of
timers, eg: HPET as the early timer and TSC as the normal timer. I
will have a look at the latest patch.

Regards,
Bin