Re: [PATCH] clocksource/drivers/pistachio: Fix wrong calculated clocksource read value

From: Daniel Lezcano
Date: Wed Dec 16 2015 - 11:23:49 EST


On 12/16/2015 11:38 AM, Russell King - ARM Linux wrote:
On Wed, Dec 16, 2015 at 11:32:17AM +0100, Daniel Lezcano wrote:
On 12/16/2015 10:33 AM, Russell King - ARM Linux wrote:
On Wed, Dec 16, 2015 at 10:21:55AM +0100, Daniel Lezcano wrote:
On 12/16/2015 08:36 AM, Jisheng Zhang wrote:
And in fact, clocksource_mmio_readw_down() also has similar issue, but it masks
with c->mask before return, the c->mask is less than 32 bit (because the
clocksource_mmio_init think number of valid bits > 32 or < 16 is invalid.)
the higher 32 bits are masked off, so we never saw such issue. But we'd better
to fix that, what's your opinion?

I think we should have a look to this portion closely.

There is no need to return more bits than are specified. If you have
a N-bit counter, then the high (64-N)-bits can be any value, because:

static inline cycle_t clocksource_delta(cycle_t now, cycle_t last, cycle_t mask)
{
return (now - last) & mask;
}

where 'now' is the current value returned from the clock source read
function, 'last' is a previously returned value, and 'mask' is the
bit mask. This has the effect of ignoring the high order bits.

I think this approach is perfectly sane. When I said we should look at this
portion closely, I meant we should double check the bitwise-nor order
regarding the explicit cast. The clocksource's mask makes sense and must
stay untouched.

That's not my point. Whether you do:

~(cycle_t)readl(...)

or

(cycle_t)~readl(...)

is irrelevant - the result is the same as far as the core code is
concerned as it doesn't care about the higher order bits.

The only thing about which should be done is really which is faster
in the general case, since this is a fast path in the time keeping
code.

Ah, ok. Yes, I agree.


--
<http://www.linaro.org/> Linaro.org â Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/