On Mon, Jul 25, 2016 at 11:50:19PM +0800, Fu Wei wrote:
Hi Will,
On 25 July 2016 at 17:02, Will Deacon will.deacon@arm.com wrote:
On Wed, Jul 20, 2016 at 02:17:59AM +0800, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
This patch simplify arch_counter_get_cntvct_mem function by using readq to get 64-bit CNTVCT value instead of readl_relaxed.
Signed-off-by: Fu Wei fu.wei@linaro.org
drivers/clocksource/arm_arch_timer.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index e6fd42d..483d2f9 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -418,15 +418,7 @@ u32 arch_timer_get_rate(void)
static u64 arch_counter_get_cntvct_mem(void) {
u32 vct_lo, vct_hi, tmp_hi;
do {
vct_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
vct_lo = readl_relaxed(arch_counter_base + CNTVCT_LO);
tmp_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
} while (vct_hi != tmp_hi);
return ((u64) vct_hi << 32) | vct_lo;
return readq(arch_counter_base + CNTVCT_LO);
Sorry, right after posting v9, I got your comment,
What's the benefit of doing this? If you use readq here, how can we
benefit:
- simplify the code
- from arch/arm64/include/asm/io.h, I guess readq is more efficient
And the harm is that it breaks the build on ARM, because ARM doesn't provide readq (as it can't, as not all CPUs on ARM support 64-bit reads) and it's not appropriate for architecture code to emulate it. Consider carefully why the original code has that loop present - only a device driver hows how to read a 64-bit register safely using two 32-bit reads. Such a loop may not be appropriate for some other device.
So... as the 0-day builder detected a failure on ARM with this, NAK.
If you want to make this conditional on readq() being present, that'd be acceptable, but you must support the case where readq() is not provided.