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

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

[Jisheng Zhang]

Let's assume the counter value is 0xf000000, the pistachio clocksource
read cycles function would return 0xffffffff0fffffff, but it should
return 0xfffffff.

We fix this issue by calculating bitwise-not counter, then cast to
cycle_t.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
 drivers/clocksource/time-pistachio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/time-pistachio.c b/drivers/clocksource/time-pistachio.c
index bba6799..3269d9e 100644
--- a/drivers/clocksource/time-pistachio.c
+++ b/drivers/clocksource/time-pistachio.c
@@ -84,7 +84,7 @@ pistachio_clocksource_read_cycles(struct clocksource *cs)
 	counter = gpt_readl(pcs->base, TIMER_CURRENT_VALUE, 0);
 	raw_spin_unlock_irqrestore(&pcs->lock, flags);
 
-	return ~(cycle_t)counter;
+	return (cycle_t)~counter;
 }
 
 static u64 notrace pistachio_read_sched_clock(void)
-- 
2.6.2

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

[Daniel Lezcano]

On 11/25/2015 04:42 PM, Jisheng Zhang wrote:
> Let's assume the counter value is 0xf000000, the pistachio clocksource
> read cycles function would return 0xffffffff0fffffff, but it should
> return 0xfffffff.
>
> We fix this issue by calculating bitwise-not counter, then cast to
> cycle_t.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>

Hi Jisheng,

I tried to reproduce this behavior on x86_64 but without success.

On which architecture did you produce this result ? Do you have a simple 
test program to check with ?

Thanks

   -- Daniel


-- 
  <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

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

[Jisheng Zhang]

Dear Daniel,

On Tue, 15 Dec 2015 21:59:30 +0100 Daniel Lezcano wrote:

> On 11/25/2015 04:42 PM, Jisheng Zhang wrote:
> > Let's assume the counter value is 0xf000000, the pistachio clocksource
> > read cycles function would return 0xffffffff0fffffff, but it should
> > return 0xfffffff.
> >
> > We fix this issue by calculating bitwise-not counter, then cast to
> > cycle_t.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>  
> 
> Hi Jisheng,
> 
> I tried to reproduce this behavior on x86_64 but without success.
> 
> On which architecture did you produce this result ? Do you have a simple 
> test program to check with ?

I have no HW platforms with pistachio, just read the code and run the
following test code in x86_64 and x86_32:

#include <stdio.h>
unsigned long long pistachio_clocksource_read_cycles()
{
	unsigned int counter = 0xf000000;
	return ~(unsigned long long)counter;
}
int main()
{
	printf("%llx\n", pistachio_clocksource_read_cycles());
	return 0;
}

Thanks,
Jisheng

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

[Jisheng Zhang]

On Wed, 16 Dec 2015 15:11:25 +0800
Jisheng Zhang  wrote:

> Dear Daniel,
> 
> On Tue, 15 Dec 2015 21:59:30 +0100 Daniel Lezcano wrote:
> 
> > On 11/25/2015 04:42 PM, Jisheng Zhang wrote:  
> > > Let's assume the counter value is 0xf000000, the pistachio clocksource

oops, sorry, should be 0xf0000000. Do I need to send a v2 patch?

> > > read cycles function would return 0xffffffff0fffffff, but it should
> > > return 0xfffffff.
> > >
> > > We fix this issue by calculating bitwise-not counter, then cast to
> > > cycle_t.
> > >
> > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>    
> > 
> > Hi Jisheng,
> > 
> > I tried to reproduce this behavior on x86_64 but without success.
> > 
> > On which architecture did you produce this result ? Do you have a simple 
> > test program to check with ?  
> 
> I have no HW platforms with pistachio, just read the code and run the
> following test code in x86_64 and x86_32:
> 
> #include <stdio.h>
> unsigned long long pistachio_clocksource_read_cycles()
> {
> 	unsigned int counter = 0xf000000;

should be unsigned int counter = 0xf0000000;

> 	return ~(unsigned long long)counter;
> }
> int main()
> {
> 	printf("%llx\n", pistachio_clocksource_read_cycles());
> 	return 0;
> }
> 
> Thanks,
> Jisheng
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jisheng Zhang]

On Wed, 16 Dec 2015 15:28:07 +0800 wrote:

> On Wed, 16 Dec 2015 15:11:25 +0800
> Jisheng Zhang  wrote:
> 
> > Dear Daniel,
> > 
> > On Tue, 15 Dec 2015 21:59:30 +0100 Daniel Lezcano wrote:
> >   
> > > On 11/25/2015 04:42 PM, Jisheng Zhang wrote:    
> > > > Let's assume the counter value is 0xf000000, the pistachio clocksource  
> 
> oops, sorry, should be 0xf0000000. Do I need to send a v2 patch?

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?

Thank you very much,
Jisheng

> 
> > > > read cycles function would return 0xffffffff0fffffff, but it should
> > > > return 0xfffffff.
> > > >
> > > > We fix this issue by calculating bitwise-not counter, then cast to
> > > > cycle_t.
> > > >
> > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>      
> > > 
> > > Hi Jisheng,
> > > 
> > > I tried to reproduce this behavior on x86_64 but without success.
> > > 
> > > On which architecture did you produce this result ? Do you have a simple 
> > > test program to check with ?    
> > 
> > I have no HW platforms with pistachio, just read the code and run the
> > following test code in x86_64 and x86_32:
> > 
> > #include <stdio.h>
> > unsigned long long pistachio_clocksource_read_cycles()
> > {
> > 	unsigned int counter = 0xf000000;  
> 
> should be unsigned int counter = 0xf0000000;
> 
> > 	return ~(unsigned long long)counter;
> > }
> > int main()
> > {
> > 	printf("%llx\n", pistachio_clocksource_read_cycles());
> > 	return 0;
> > }
> > 
> > Thanks,
> > Jisheng
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel  
> 

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

[Jisheng Zhang]

On Wed, 16 Dec 2015 15:36:09 +0800 Jisheng Zhang wrote:

> On Wed, 16 Dec 2015 15:28:07 +0800 wrote:
> 
> > On Wed, 16 Dec 2015 15:11:25 +0800
> > Jisheng Zhang  wrote:
> >   
> > > Dear Daniel,
> > > 
> > > On Tue, 15 Dec 2015 21:59:30 +0100 Daniel Lezcano wrote:
> > >     
> > > > On 11/25/2015 04:42 PM, Jisheng Zhang wrote:      
> > > > > Let's assume the counter value is 0xf000000, the pistachio clocksource    
> > 
> > oops, sorry, should be 0xf0000000. Do I need to send a v2 patch?  
> 
> And in fact, clocksource_mmio_readw_down() also has similar issue, but it masks

oops, sorry, I really need a cup of coffee. I mean clocksource_mmio_readl_down()

> 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?
> 
> Thank you very much,
> Jisheng
> 
> >   
> > > > > read cycles function would return 0xffffffff0fffffff, but it should
> > > > > return 0xfffffff.
> > > > >
> > > > > We fix this issue by calculating bitwise-not counter, then cast to
> > > > > cycle_t.
> > > > >
> > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>        
> > > > 
> > > > Hi Jisheng,
> > > > 
> > > > I tried to reproduce this behavior on x86_64 but without success.
> > > > 
> > > > On which architecture did you produce this result ? Do you have a simple 
> > > > test program to check with ?      
> > > 
> > > I have no HW platforms with pistachio, just read the code and run the
> > > following test code in x86_64 and x86_32:
> > > 
> > > #include <stdio.h>
> > > unsigned long long pistachio_clocksource_read_cycles()
> > > {
> > > 	unsigned int counter = 0xf000000;    
> > 
> > should be unsigned int counter = 0xf0000000;
> >   
> > > 	return ~(unsigned long long)counter;
> > > }
> > > int main()
> > > {
> > > 	printf("%llx\n", pistachio_clocksource_read_cycles());
> > > 	return 0;
> > > }
> > > 
> > > Thanks,
> > > Jisheng
> > > 
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel    
> >   
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Daniel Lezcano]

On 12/16/2015 08:36 AM, Jisheng Zhang wrote:
> On Wed, 16 Dec 2015 15:28:07 +0800 wrote:
>
>> On Wed, 16 Dec 2015 15:11:25 +0800
>> Jisheng Zhang  wrote:
>>
>>> Dear Daniel,
>>>
>>> On Tue, 15 Dec 2015 21:59:30 +0100 Daniel Lezcano wrote:
>>>
>>>> On 11/25/2015 04:42 PM, Jisheng Zhang wrote:
>>>>> Let's assume the counter value is 0xf000000, the pistachio clocksource
>>
>> oops, sorry, should be 0xf0000000. Do I need to send a v2 patch?
>
> 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.


-- 
  <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

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

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

[Daniel Lezcano]

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.



-- 
  <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

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

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

[Daniel Lezcano]

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

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

[Jisheng Zhang]

On Wed, 16 Dec 2015 10:38:03 +0000 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;
> > >}

So the "& c->mask" in "~(cycle_t)readl_relaxed(to_mmio_clksrc(c)->reg) & c->mask;"

isn't needed, I'm not sure I understand this correctly.


> > >
> > >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.
> 

Got it.

If there's no "& c->mask", just as the pistachio does, 

return (cycle_t)~readl_relaxed(to_mmio_clksrc(c)->reg)
  1c:   e1a0c00d        mov     ip, sp
  20:   e92dd800        push    {fp, ip, lr, pc}
  24:   e24cb004        sub     fp, ip, #4
  28:   e5103040        ldr     r3, [r0, #-64]  ; 0x40
  2c:   e5930000        ldr     r0, [r3]
  30:   e3a01000        mov     r1, #0
  34:   e1e00000        mvn     r0, r0
  38:   e89da800        ldm     sp, {fp, sp, pc}


is better than

return ~(cycle_t)readl_relaxed(to_mmio_clksrc(c)->reg);

  1c:   e1a0c00d        mov     ip, sp
  20:   e92dd800        push    {fp, ip, lr, pc}
  24:   e24cb004        sub     fp, ip, #4
  28:   e5103040        ldr     r3, [r0, #-64]  ; 0x40
  2c:   e5932000        ldr     r2, [r3]
  30:   e3a01000        mov     r1, #0
  34:   e1e00002        mvn     r0, r2
  38:   e1e01001        mvn     r1, r1
  3c:   e89da800        ldm     sp, {fp, sp, pc}

Thanks,
Jisheng

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

[Daniel Lezcano]

On 12/16/2015 08:28 AM, Jisheng Zhang wrote:
> On Wed, 16 Dec 2015 15:11:25 +0800
> Jisheng Zhang  wrote:
>
>> Dear Daniel,
>>
>> On Tue, 15 Dec 2015 21:59:30 +0100 Daniel Lezcano wrote:
>>
>>> On 11/25/2015 04:42 PM, Jisheng Zhang wrote:
>>>> Let's assume the counter value is 0xf000000, the pistachio clocksource
>
> oops, sorry, should be 0xf0000000. Do I need to send a v2 patch?

Nope, I fixed it.



-- 
  <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

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

[Daniel Lezcano]

On 12/16/2015 08:11 AM, Jisheng Zhang wrote:
> Dear Daniel,
>
> On Tue, 15 Dec 2015 21:59:30 +0100 Daniel Lezcano wrote:
>
>> On 11/25/2015 04:42 PM, Jisheng Zhang wrote:
>>> Let's assume the counter value is 0xf000000, the pistachio clocksource
>>> read cycles function would return 0xffffffff0fffffff, but it should
>>> return 0xfffffff.
>>>
>>> We fix this issue by calculating bitwise-not counter, then cast to
>>> cycle_t.
>>>
>>> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
>>
>> Hi Jisheng,
>>
>> I tried to reproduce this behavior on x86_64 but without success.
>>
>> On which architecture did you produce this result ? Do you have a simple
>> test program to check with ?
>
> I have no HW platforms with pistachio, just read the code and run the
> following test code in x86_64 and x86_32:
>
> #include <stdio.h>
> unsigned long long pistachio_clocksource_read_cycles()
> {
> 	unsigned int counter = 0xf000000;
> 	return ~(unsigned long long)counter;
> }
> int main()
> {
> 	printf("%llx\n", pistachio_clocksource_read_cycles());
> 	return 0;
> }

Ok, I reproduced it. I had an issue with the signed/unsigned type.

That's a good catch !


-- 
  <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