[U-Boot] [PATCH v2] timer: add High Precision Event Timers (HPET) support

Adding HPET 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. If X86_TSC is disabled, early timer functions are provided by HPET.
Signed-off-by: Ivan Gorinov ivan.gorinov@intel.com --- arch/Kconfig | 2 +- arch/x86/dts/hpet.dtsi | 7 ++ drivers/timer/Kconfig | 6 ++ drivers/timer/Makefile | 1 + drivers/timer/hpet_timer.c | 191 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 206 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/dts/hpet.dtsi b/arch/x86/dts/hpet.dtsi new file mode 100644 index 0000000..037dc7e --- /dev/null +++ b/arch/x86/dts/hpet.dtsi @@ -0,0 +1,7 @@ +/ { + hpet0: hpet@fed00000 { + compatible = "hpet-x86"; + u-boot,dm-pre-reloc; + reg = <0xfed00000 0x1000>; + }; +}; diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 2c96896..72ae6c5 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -65,6 +65,12 @@ 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 && X86 + help + Select this to enable High Precision Event Timers (HPET) for x86. + 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..0bef859 --- /dev/null +++ b/drivers/timer/hpet_timer.c @@ -0,0 +1,191 @@ +/* + * 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_L 0x0f0 +#define HPET_MAIN_COUNT_H 0x0f4 + +#define HPET_MAX_PERIOD 100000000 + +DECLARE_GLOBAL_DATA_PTR; + +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 &= ~1; + writel(config, regs + HPET_CONFIG_REG); + writel(0, regs + HPET_MAIN_COUNT_L); + writel(0, regs + HPET_MAIN_COUNT_H); + config |= 1; + writel(config, regs + HPET_CONFIG_REG); +} + +/* + * Read the main counter as two 32-bit registers, + * repeat if rollover happens. + */ +static u64 read_main_counter(void *regs) +{ + u64 now_tick; + u32 tl, th, th0; + + th = readl(regs + HPET_MAIN_COUNT_H); + do { + th0 = th; + tl = readl(regs + HPET_MAIN_COUNT_L); + th = readl(regs + HPET_MAIN_COUNT_H); + } while (th != th0); + + now_tick = th; + now_tick <<= 32; + now_tick |= tl; + + return now_tick; +} + +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; +} + +#ifndef CONFIG_X86_TSC_TIMER + +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_TSC_TIMER */ + +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, +};

Hi Ivan,
On Fri, Mar 30, 2018 at 6:29 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:
Adding HPET 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. If X86_TSC is disabled, early timer functions are provided by HPET.
Signed-off-by: Ivan Gorinov ivan.gorinov@intel.com
arch/Kconfig | 2 +- arch/x86/dts/hpet.dtsi | 7 ++ drivers/timer/Kconfig | 6 ++ drivers/timer/Makefile | 1 + drivers/timer/hpet_timer.c | 191 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 206 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/dts/hpet.dtsi b/arch/x86/dts/hpet.dtsi new file mode 100644 index 0000000..037dc7e --- /dev/null +++ b/arch/x86/dts/hpet.dtsi @@ -0,0 +1,7 @@ +/ {
hpet0: hpet@fed00000 {
nits: use 'hpet' instead of 'hpet0'? since there is only one hpet timer in the system.
compatible = "hpet-x86";
u-boot,dm-pre-reloc;
reg = <0xfed00000 0x1000>;
};
+}; diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 2c96896..72ae6c5 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -65,6 +65,12 @@ 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 && X86
help
Select this to enable High Precision Event Timers (HPET) for x86.
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..0bef859 --- /dev/null +++ b/drivers/timer/hpet_timer.c @@ -0,0 +1,191 @@ +/*
- 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_L 0x0f0 +#define HPET_MAIN_COUNT_H 0x0f4
+#define HPET_MAX_PERIOD 100000000
+DECLARE_GLOBAL_DATA_PTR;
This is not needed.
+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: use one line comment format
+static void start_main_counter(void *regs) +{
u32 config;
config = readl(regs + HPET_CONFIG_REG);
config &= ~1;
writel(config, regs + HPET_CONFIG_REG);
writel(0, regs + HPET_MAIN_COUNT_L);
writel(0, regs + HPET_MAIN_COUNT_H);
config |= 1;
writel(config, regs + HPET_CONFIG_REG);
+}
+/*
- Read the main counter as two 32-bit registers,
- repeat if rollover happens.
- */
+static u64 read_main_counter(void *regs) +{
u64 now_tick;
u32 tl, th, th0;
th = readl(regs + HPET_MAIN_COUNT_H);
do {
th0 = th;
tl = readl(regs + HPET_MAIN_COUNT_L);
th = readl(regs + HPET_MAIN_COUNT_H);
} while (th != th0);
now_tick = th;
now_tick <<= 32;
now_tick |= tl;
return now_tick;
+}
+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;
+}
+#ifndef CONFIG_X86_TSC_TIMER
This looks not good. If we support more timers on x86, the #ifdef logic will become un-maintainable. Can we support such from the timer library instead?
+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_TSC_TIMER */
+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,
+};
I will try to test this patch on several x86 boards.
Regards, Bin

On Fri, Mar 30, 2018 at 1:29 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:
Adding HPET 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. If X86_TSC is disabled, early timer functions are provided by HPET.
Signed-off-by: Ivan Gorinov ivan.gorinov@intel.com
Changelog?
arch/Kconfig | 2 +-
+config HPET_TIMER
bool "High Precision Event Timers (HPET) support"
depends on TIMER && X86
Does X86 makes any difference from building point of view?
config = readl(regs + HPET_CONFIG_REG);
config &= ~1;
writel(config, regs + HPET_CONFIG_REG);
1 or BIT(0) is magic. Can be fixed in all places?
writel(0, regs + HPET_MAIN_COUNT_L);
writel(0, regs + HPET_MAIN_COUNT_H);
Can we use writeq() here?
tl = readl(regs + HPET_MAIN_COUNT_L);
th = readl(regs + HPET_MAIN_COUNT_H);
Ditto.

On Fri, Mar 30, 2018 at 10:46:40PM +0300, Andy Shevchenko wrote:
writel(0, regs + HPET_MAIN_COUNT_L);
writel(0, regs + HPET_MAIN_COUNT_H);
Can we use writeq() here?
I don't see readq/writeq defined for x86, even x86_64.
tl = readl(regs + HPET_MAIN_COUNT_L);
th = readl(regs + HPET_MAIN_COUNT_H);
Ditto.
If readq() is defined as two read operations in 32-bit code, main counter rollover (low part overflow, high part increment) can happen between them.

On Sat, Mar 31, 2018 at 4:03 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:
On Fri, Mar 30, 2018 at 10:46:40PM +0300, Andy Shevchenko wrote:
writel(0, regs + HPET_MAIN_COUNT_L);
writel(0, regs + HPET_MAIN_COUNT_H);
Can we use writeq() here?
I don't see readq/writeq defined for x86, even x86_64.
read_le64() and write_le64() in U-Boot source code.
Besides that there are memcpy_toio() / memcpy_fromio() defined (didn't check if they are really implemented).
tl = readl(regs + HPET_MAIN_COUNT_L);
th = readl(regs + HPET_MAIN_COUNT_H);
Ditto.
If readq() is defined as two read operations in 32-bit code, main counter rollover (low part overflow, high part increment) can happen between them.
And how this contradicts ther current code?

On Sat, Mar 31, 2018 at 06:31:03AM -0600, Andy Shevchenko wrote:
tl = readl(regs + HPET_MAIN_COUNT_L);
th = readl(regs + HPET_MAIN_COUNT_H);
Ditto.
If readq() is defined as two read operations in 32-bit code, main counter rollover (low part overflow, high part increment) can happen between them.
And how this contradicts ther current code?
It just does not make the code simpler, rollover check is still required if U-Boot is compiled as 32-bit code.
Can we do something like the following?
#ifdef CONFIG_X86_64
static u64 read_main_counter(void *regs) { return readq(regs + HPET_MAIN_COUNT); }
#else
/* * Read the main counter as two 32-bit registers, * repeat if rollover happens. */ static u64 read_main_counter(void *regs) { u64 now_tick; u32 tl, th, th0;
th = readl(regs + HPET_MAIN_COUNT_H); do { th0 = th; tl = readl(regs + HPET_MAIN_COUNT_L); th = readl(regs + HPET_MAIN_COUNT_H); now_tick = th; now_tick <<= 32; now_tick |= tl; } while (th != th0);
return now_tick; }
#endif

On Tue, Apr 3, 2018 at 2:00 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:
On Sat, Mar 31, 2018 at 06:31:03AM -0600, Andy Shevchenko wrote:
tl = readl(regs + HPET_MAIN_COUNT_L);
th = readl(regs + HPET_MAIN_COUNT_H);
Ditto.
If readq() is defined as two read operations in 32-bit code, main counter rollover (low part overflow, high part increment) can happen between them.
And how this contradicts ther current code?
It just does not make the code simpler,
...b/c you misread what I suggested.
rollover check is still required if U-Boot is compiled as 32-bit code.
Can we do something like the following?
Yes, but... why?
What's wrong with replacing two sequential 32-bit reads with one 64-bit?
See below.
#ifdef CONFIG_X86_64
static u64 read_main_counter(void *regs) { return readq(regs + HPET_MAIN_COUNT); }
#else
/*
- Read the main counter as two 32-bit registers,
- repeat if rollover happens.
*/ static u64 read_main_counter(void *regs) { u64 now_tick; u32 tl, th, th0;
th = readl(regs + HPET_MAIN_COUNT_H); do {
now_tick = readq();
th0 = th; tl = readl(regs + HPET_MAIN_COUNT_L); th = readl(regs + HPET_MAIN_COUNT_H); now_tick = th; now_tick <<= 32; now_tick |= tl;
} while (th != th0);
} while (th != (now_tick >> 32));
return now_tick;
}
#endif

On Tue, Apr 03, 2018 at 06:17:42AM -0600, Andy Shevchenko wrote:
If readq() is defined as two read operations in 32-bit code, main counter rollover (low part overflow, high part increment) can happen between them.
And how this contradicts ther current code?
It just does not make the code simpler,
...b/c you misread what I suggested.
rollover check is still required if U-Boot is compiled as 32-bit code. Can we do something like the following?
Yes, but... why? What's wrong with replacing two sequential 32-bit reads with one 64-bit?
Doesn't readX/writeX imply a single I/O operation? It may be misleading to define it as two.
Assuming MMX or SSE2 to be supported by all x86 processors, 64-bit I/O registers can be accessed as a single operation even in 32-bit code:
static inline u64 readq(void *addr) { u64 value;
asm volatile ("movq (%0), %%xmm0" : : "r" (addr)); asm volatile ("movq %%xmm0, %0" : "=m" (value));
return value; }
I can add these definitions to "asm/io.h".

On Wed, Apr 4, 2018 at 7:26 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:
On Tue, Apr 03, 2018 at 06:17:42AM -0600, Andy Shevchenko wrote:
If readq() is defined as two read operations in 32-bit code, main counter rollover (low part overflow, high part increment) can happen between them.
And how this contradicts ther current code?
It just does not make the code simpler,
...b/c you misread what I suggested.
rollover check is still required if U-Boot is compiled as 32-bit code. Can we do something like the following?
Yes, but... why? What's wrong with replacing two sequential 32-bit reads with one 64-bit?
Doesn't readX/writeX imply a single I/O operation? It may be misleading to define it as two.
Assuming MMX or SSE2 to be supported by all x86 processors, 64-bit I/O registers can be accessed as a single operation even in 32-bit code:
Adding such requirement (MMX or SSE2) to U-Boot is not good. Why do we require MMX or SSE2 for readq? Can we use general purpose registers?
static inline u64 readq(void *addr) { u64 value;
asm volatile ("movq (%0), %%xmm0" : : "r" (addr)); asm volatile ("movq %%xmm0, %0" : "=m" (value)); return value;
}
I can add these definitions to "asm/io.h".
Regards, Bin

On Wed, Apr 04, 2018 at 12:15:24PM +0800, Bin Meng wrote:
Doesn't readX/writeX imply a single I/O operation? It may be misleading to define it as two.
Assuming MMX or SSE2 to be supported by all x86 processors, 64-bit I/O registers can be accessed as a single operation even in 32-bit code:
Adding such requirement (MMX or SSE2) to U-Boot is not good. Why do we require MMX or SSE2 for readq? Can we use general purpose registers?
In 32-bit code, we can't make a 64-bit memory read operation using only general purpose registers.
static inline u64 readq(void *addr) { u64 value;
asm volatile ("movq (%0), %%xmm0" : : "r" (addr)); asm volatile ("movq %%xmm0, %0" : "=m" (value)); return value;
}
I can add these definitions to "asm/io.h".

On Wed, Apr 4, 2018 at 7:40 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:
On Wed, Apr 04, 2018 at 12:15:24PM +0800, Bin Meng wrote:
Doesn't readX/writeX imply a single I/O operation? It may be misleading to define it as two.
Assuming MMX or SSE2 to be supported by all x86 processors, 64-bit I/O registers can be accessed as a single operation even in 32-bit code:
Adding such requirement (MMX or SSE2) to U-Boot is not good. Why do we require MMX or SSE2 for readq? Can we use general purpose registers?
In 32-bit code, we can't make a 64-bit memory read operation using only general purpose registers.
Exactly, that's why you have to use non-atomic variants.

On Wed, Apr 4, 2018 at 2:26 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:
On Tue, Apr 03, 2018 at 06:17:42AM -0600, Andy Shevchenko wrote:
If readq() is defined as two read operations in 32-bit code, main counter rollover (low part overflow, high part increment) can happen between them.
And how this contradicts ther current code?
It just does not make the code simpler,
...b/c you misread what I suggested.
rollover check is still required if U-Boot is compiled as 32-bit code. Can we do something like the following?
Yes, but... why? What's wrong with replacing two sequential 32-bit reads with one 64-bit?
Doesn't readX/writeX imply a single I/O operation?
Not always. It depends on atomicity you would like to get.
On 32-bit platform it's impossible to do readq() at once.
It may be misleading to define it as two.
Assuming MMX or SSE2 to be supported by all x86 processors, 64-bit I/O registers can be accessed as a single operation even in 32-bit code:
static inline u64 readq(void *addr) { u64 value;
asm volatile ("movq (%0), %%xmm0" : : "r" (addr)); asm volatile ("movq %%xmm0, %0" : "=m" (value)); return value;
}
I can add these definitions to "asm/io.h".
Please, no. This is wrong approach.
participants (3)
-
Andy Shevchenko
-
Bin Meng
-
Ivan Gorinov