2.6.19 timer API changes

2.6.19 timer API changes

[Daniel Laird]

Hi All,

I am porting the Philips/PNX8550 kernel from 2.6.17.13 to 2.6.19.  I am
having some issues:
One issue I am having is with the new Timer API that replaced the board
specific API.

I have made all of the important changes 
board_timer_setup -> plat_timer_setup 

When I run the kernel it hangs in the calibrate_delay function.
Eventually the complete kernel does run but it runs very slow.
This is usually an issue with the Timer Interuppt setup etc.  But I have
looked at the other MIPS ports and seem to have made the same changes.

On the PNX8550 it does not use the CP0 timer but use a different timer (the
Custom MIPS core has 3 extra timers)

I replaced the arch/mips/kernel/time.c with a merge between 2.6.17.13 and
2.6.19 and I can get the kernel to boot at the correct speed straight
through the calibrate_delay function and the entire system seems to be
working correctly.

I was wondering if anyone might have any ideas on how to debug this problem
as I would like a clean port to 2.6.19. 
Maybe the new timer code will not work properly on the PNX8550 in which case
maybe some patches are required.

I am continuing to debug at my end and once I have a working system with the
smallest set of changes to the time.c file I will post them in the hope that
someone will point out a silly error I have made in the CPU/board setup.

In the mean time any helpful ideas on debugging, tracing, even solving this
issue would be really appreciated

Daniel 
 




-- 
View this message in context: http://www.nabble.com/2.6.19-timer-API-changes-tf2838715.html#a7925588
Sent from the linux-mips main mailing list archive at Nabble.com.

2.6.19 timer API changes

[Daniel Laird]

Hi All, 

I am porting the Philips/PNX8550 kernel from 2.6.17.13 to 2.6.19.  I am
having some issues: 
One issue I am having is with the new Timer API that replaced the board
specific API. 

I have made all of the important changes 
board_timer_setup -> plat_timer_setup 

When I run the kernel it hangs in the calibrate_delay function. 
Eventually the complete kernel does run but it runs very slow. 
This is usually an issue with the Timer Interuppt setup etc.  But I have
looked at the other MIPS ports and seem to have made the same changes. 

On the PNX8550 it does not use the CP0 timer but use a different timer (the
Custom MIPS core has 3 extra timers) 

I replaced the arch/mips/kernel/time.c with a merge between 2.6.17.13 and
2.6.19 and I can get the kernel to boot at the correct speed straight
through the calibrate_delay function and the entire system seems to be
working correctly. 

I was wondering if anyone might have any ideas on how to debug this problem
as I would like a clean port to 2.6.19. 
Maybe the new timer code will not work properly on the PNX8550 in which case
maybe some patches are required. 

I am continuing to debug at my end and once I have a working system with the
smallest set of changes to the time.c file I will post them in the hope that
someone will point out a silly error I have made in the CPU/board setup. 

In the mean time any helpful ideas on debugging, tracing, even solving this
issue would be really appreciated 

Daniel 
  




-- 
View this message in context: http://www.nabble.com/2.6.19-timer-API-changes-tf2838715.html#a7943218
Sent from the linux-mips main mailing list archive at Nabble.com.

Re: 2.6.19 timer API changes

[Atsushi Nemoto]

On Tue, 19 Dec 2006 00:17:24 -0800 (PST), Daniel Laird <danieljlaird@hotmail.com> wrote:
> When I run the kernel it hangs in the calibrate_delay function. 
> Eventually the complete kernel does run but it runs very slow. 
> This is usually an issue with the Timer Interuppt setup etc.  But I have
> looked at the other MIPS ports and seem to have made the same changes. 
> 
> On the PNX8550 it does not use the CP0 timer but use a different timer (the
> Custom MIPS core has 3 extra timers) 

Hmm, do the TIMER1 and CP0_COUNTER run at same speed?  If no, the
PNX8550 port should be broken (i.e. gettimeofday() did not work
properly) even without the timer API changes.  You should provide
custom clocksource.mips_read (previously named mips_hpt_read) function
which returns TIMER1 counter value.  If the TIMER1 was not 32-bit
free-run counter, some trick would be required.  Refer sb1250 or
jmr3927 for example.

---
Atsushi Nemoto

Re: 2.6.19 timer API changes

[Daniel Laird]

Atsushi Nemoto wrote:
> 
> Hmm, do the TIMER1 and CP0_COUNTER run at same speed?  If no, the
> PNX8550 port should be broken (i.e. gettimeofday() did not work
> properly) even without the timer API changes.  You should provide
> custom clocksource.mips_read (previously named mips_hpt_read) function
> which returns TIMER1 counter value.  If the TIMER1 was not 32-bit
> free-run counter, some trick would be required.  Refer sb1250 or
> jmr3927 for example.
> 
> ---
> Atsushi Nemoto
> 
> 
> 
I am just starting to look into this (thankyou for your first comments).
I have reduced the problem code, so if I change the following:
/* For use both as a high precision timer and an interrupt source.  */
static void __init c0_hpt_timer_init(void)
{
	expirelo = read_c0_count() + cycles_per_jiffy;
	write_c0_compare(expirelo);
} (the 2.6.19 version)
to the following:
/* For use both as a high precision timer and an interrupt source.  */
static void __init c0_hpt_timer_init(void)
{
    unsigned int count = read_c0_count() - mips_hpt_read();
	expirelo = (count / cycles_per_jiffy + 1) * cycles_per_jiffy;
	write_c0_count(expirelo - cycles_per_jiffy);
	write_c0_compare(expirelo);
	write_c0_count(count);
}

Then i get the system to boot up and all seems well.  I am new to this and
am looking into why this change makes the system boot up.  As always though
any help is appreciated.

Cheers
Dan
-- 
View this message in context: http://www.nabble.com/2.6.19-timer-API-changes-tf2838715.html#a7948316
Sent from the linux-mips main mailing list archive at Nabble.com.

Re: 2.6.19 timer API changes

[Atsushi Nemoto]

On Tue, 19 Dec 2006 00:17:24 -0800 (PST), Daniel Laird <danieljlaird@hotmail.com> wrote:
> On the PNX8550 it does not use the CP0 timer but use a different timer (the
> Custom MIPS core has 3 extra timers) 

Do you know what this ifndef line mean?

#ifndef CONFIG_SOC_PNX8550	/* pnx8550 resets to zero */
	/* Ack this timer interrupt and set the next one.  */
	expirelo += cycles_per_jiffy;
#endif

If it means "On PNX8550, writing to COMPARE register resets COUNTER to
zero", new time.c might be broken for PNX8550.  Could you try this
patch?

diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index 11aab6d..4eb0741 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -119,7 +119,11 @@ static cycle_t c0_hpt_read(void)
 /* For use both as a high precision timer and an interrupt source.  */
 static void __init c0_hpt_timer_init(void)
 {
+#ifdef CONFIG_SOC_PNX8550	/* pnx8550 resets to zero */
+	expirelo = cycles_per_jiffy;
+#else
 	expirelo = read_c0_count() + cycles_per_jiffy;
+#endif
 	write_c0_compare(expirelo);
 }
 

Re: 2.6.19 timer API changes

[Daniel Laird]

Atsushi Nemoto wrote:
> 
> On Tue, 19 Dec 2006 00:17:24 -0800 (PST), Daniel Laird
> <danieljlaird@hotmail.com> wrote:
>> On the PNX8550 it does not use the CP0 timer but use a different timer
>> (the
>> Custom MIPS core has 3 extra timers) 
> 
> Do you know what this ifndef line mean?
> 
> #ifndef CONFIG_SOC_PNX8550	/* pnx8550 resets to zero */
> 	/* Ack this timer interrupt and set the next one.  */
> 	expirelo += cycles_per_jiffy;
> #endif
> 
> If it means "On PNX8550, writing to COMPARE register resets COUNTER to
> zero", new time.c might be broken for PNX8550.  Could you try this
> patch?
> 
> diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
> index 11aab6d..4eb0741 100644
> --- a/arch/mips/kernel/time.c
> +++ b/arch/mips/kernel/time.c
> @@ -119,7 +119,11 @@ static cycle_t c0_hpt_read(void)
>  /* For use both as a high precision timer and an interrupt source.  */
>  static void __init c0_hpt_timer_init(void)
>  {
> +#ifdef CONFIG_SOC_PNX8550	/* pnx8550 resets to zero */
> +	expirelo = cycles_per_jiffy;
> +#else
>  	expirelo = read_c0_count() + cycles_per_jiffy;
> +#endif
>  	write_c0_compare(expirelo);
>  }
>  
> 
> 
> 
I am just digging out the mips core user manual...  
However I have tried this change you suggested, it still takes a long time
to get past the calibrate delay function (~10seconds).
However after this it seems to run at full speed where as before it used to
run very slow.
So an improvement, I think this does mean the new time.c has broken 8550
support hopefully I can find otu what the core does so it can be fixed.

-- 
View this message in context: http://www.nabble.com/2.6.19-timer-API-changes-tf2838715.html#a7949125
Sent from the linux-mips main mailing list archive at Nabble.com.

Re: 2.6.19 timer API changes

[Sergei Shtylyov]

Hello.

Atsushi Nemoto wrote:

>>When I run the kernel it hangs in the calibrate_delay function. 
>>Eventually the complete kernel does run but it runs very slow. 
>>This is usually an issue with the Timer Interuppt setup etc.  But I have
>>looked at the other MIPS ports and seem to have made the same changes. 

>>On the PNX8550 it does not use the CP0 timer but use a different timer (the
>>Custom MIPS core has 3 extra timers) 

> Hmm, do the TIMER1 and CP0_COUNTER run at same speed?  If no, the
> PNX8550 port should be broken (i.e. gettimeofday() did not work
> properly) even without the timer API changes.  You should provide
> custom clocksource.mips_read (previously named mips_hpt_read) function
> which returns TIMER1 counter value.  If the TIMER1 was not 32-bit
> free-run counter, some trick would be required.  Refer sb1250 or
> jmr3927 for example.

    I would like to discourage you from repeating those JMR3927 clocksource 
"tricks" when you have 3 spare count/compare regs. This will warrant troubles 
when clockevents support gets merged into mainline (in fact, it was not 
necessary even on JMR3927 which has 3 timers). Although, if the timer isn't 
auto-reloading (I assume it isn't), the trick shouldn't be needed.

> ---
> Atsushi Nemoto

WBR, Sergei

Re: 2.6.19 timer API changes

[Sergei Shtylyov]

Daniel Laird wrote:

> Atsushi Nemoto wrote:

>>Hmm, do the TIMER1 and CP0_COUNTER run at same speed?  If no, the
>>PNX8550 port should be broken (i.e. gettimeofday() did not work
>>properly) even without the timer API changes.  You should provide
>>custom clocksource.mips_read (previously named mips_hpt_read) function

    Meaning clocksource_mips.read... :-)

>>which returns TIMER1 counter value.  If the TIMER1 was not 32-bit
>>free-run counter, some trick would be required.  Refer sb1250 or
>>jmr3927 for example.

>>---
>>Atsushi Nemoto
>>
>>
>>
> 
> I am just starting to look into this (thankyou for your first comments).
> I have reduced the problem code, so if I change the following:
> /* For use both as a high precision timer and an interrupt source.  */
> static void __init c0_hpt_timer_init(void)
> {
> 	expirelo = read_c0_count() + cycles_per_jiffy;
> 	write_c0_compare(expirelo);
> } (the 2.6.19 version)
> to the following:
> /* For use both as a high precision timer and an interrupt source.  */
> static void __init c0_hpt_timer_init(void)
> {
>     unsigned int count = read_c0_count() - mips_hpt_read();

     Doesn't make sense to me... Should be 0 or near.

> 	expirelo = (count / cycles_per_jiffy + 1) * cycles_per_jiffy;
> 	write_c0_count(expirelo - cycles_per_jiffy);
> 	write_c0_compare(expirelo);
> 	write_c0_count(count);
> }

    This code just shouldn't be executing at all, since the interrupts are 
coming from the other source than standard CP0 count/compare registers (so, 
I'd assume mips_timer_state should need to be set -- but it doesn't)... and at 
the same time the handler writes to them... well, PNX8550 must have really 
weird timers...

> Then i get the system to boot up and all seems well.  I am new to this and
> am looking into why this change makes the system boot up.  As always though
> any help is appreciated.

> Cheers
> Dan

WBR, Sergei

Re: 2.6.19 timer API changes

[Atsushi Nemoto]

On Tue, 19 Dec 2006 18:52:36 +0300, Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:
>     I would like to discourage you from repeating those JMR3927
> clocksource "tricks" when you have 3 spare count/compare regs. This
> will warrant troubles when clockevents support gets merged into
> mainline (in fact, it was not necessary even on JMR3927 which has 3
> timers). Although, if the timer isn't auto-reloading (I assume it
> isn't), the trick shouldn't be needed.

Indeed.  JMR3927 is not good for reference on writing new code.

Though I do not know the PNX8550 timer details, if writing to the
COMPARE reset the COUNTER or COUNTER wrapped to zero at a value in
COMPARE, perhaps we can not use c0_hpt_read for clocksource.

---
Atsushi Nemoto

Re: 2.6.19 timer API changes

[Atsushi Nemoto]

On Tue, 19 Dec 2006 07:34:54 -0800 (PST), Daniel Laird <danieljlaird@hotmail.com> wrote:
> I am just digging out the mips core user manual...  
> However I have tried this change you suggested, it still takes a long time
> to get past the calibrate delay function (~10seconds).
> However after this it seems to run at full speed where as before it used to
> run very slow.
> So an improvement, I think this does mean the new time.c has broken 8550
> support hopefully I can find otu what the core does so it can be fixed.

Hm, then it seems writing to COMPARE does not clear COUNT.

How about this?  You should still fix pnx8550_hpt_read() anyway, but I
suppose gettimeofday() on PNX8550 was broken long time.


Subject: [MIPS] Use custom timer_ack and clocksource_mips.read for PNX8550

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index 11aab6d..8aa544f 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -94,10 +94,8 @@ static void c0_timer_ack(void)
 {
 	unsigned int count;
 
-#ifndef CONFIG_SOC_PNX8550	/* pnx8550 resets to zero */
 	/* Ack this timer interrupt and set the next one.  */
 	expirelo += cycles_per_jiffy;
-#endif
 	write_c0_compare(expirelo);
 
 	/* Check to see if we have missed any timer interrupts.  */
diff --git a/arch/mips/philips/pnx8550/common/time.c b/arch/mips/philips/pnx8550/common/time.c
index 65c440e..e86905a 100644
--- a/arch/mips/philips/pnx8550/common/time.c
+++ b/arch/mips/philips/pnx8550/common/time.c
@@ -33,7 +33,18 @@ #include <asm/debug.h>
 #include <int.h>
 #include <cm.h>
 
-extern unsigned int mips_hpt_frequency;
+static unsigned long cycles_per_jiffy __read_mostly;
+
+static void pnx8550_timer_ack(void)
+{
+	write_c0_compare(cycles_per_jiffy);
+}
+
+static cycle_t pnx8550_hpt_read(void)
+{
+	/* FIXME: we should use timer2 or timer3 as freerun counter */
+	return read_c0_count();
+}
 
 /*
  * pnx8550_time_init() - it does the following things:
@@ -68,6 +79,11 @@ void pnx8550_time_init(void)
 	 * HZ timer interrupts per second.
 	 */
 	mips_hpt_frequency = 27UL * ((1000000UL * n)/(m * pow2p));
+	cycles_per_jiffy = (mips_hpt_frequency + HZ / 2) / HZ;
+	clocksource_mips.read = pnx8550_hpt_read;
+	mips_timer_ack = pnx8550_timer_ack;
+	write_c0_count(0);
+	write_c0_compare(cycles_per_jiffy);
 }
 
 void __init plat_timer_setup(struct irqaction *irq)

Re: 2.6.19 timer API changes

[Daniel Laird]

Atsushi Nemoto wrote:
> 
> <snip>
> Hm, then it seems writing to COMPARE does not clear COUNT.
> 
> How about this?  You should still fix pnx8550_hpt_read() anyway, but I
> suppose gettimeofday() on PNX8550 was broken long time.
> 
> 
> Subject: [MIPS] Use custom timer_ack and clocksource_mips.read for PNX8550
> 

I have not tried the suggested change yet, but I have fiund the mips manual
and here is what it says:

The 32-bit register is both readable and writable through the MTC0 and MFC0
instructions as CP0 register 9. Upon reset, the Count register is set to 0.
Count and Compare together make up Timer_1 in the PR4450 (see Section 3.12).
The timer is active by default. When active, Count register contains a free
running
counter; on each processor clock-tick, the value in the register increments
by one.
However, when bit ST1 bit[3] in the CP0 Configuration register is enabled,
the timer is
stopped. When the ST1 bit is disabled, the timer returns to its default
state.
Timer_1 is active when the PR4450 is in Sleep mode, but is switched off when
the
PR4450 is in Coma mode.
When active, the register can be reset with the assertion of the Terminal
Count
(TC_1) signal, which is asserted when the values in the Count and Compare
registers
match. After the Count register is reset, it restarts its count on the next
processor
clock-tick.
In the PR4450, the TC_1 signal is fed to the external hardware interrupt
signal to
invoke an interrupt handler e.g., the timer interrupt. The TC_1 signal can
only be
reset to 0 when the interrupt handler performs a write to the Compare
register.
Consecutive writes to Count will result in undefined contents. At least one
instruction
must be inserted between consecutive writes to Count.

This seems to suggest that writing to the compare register does in fact
clear count.
If I do the following:
static void __init c0_hpt_timer_init(void)
{
#ifdef CONFIG_SOC_PNX8550 /* pnx8550 resets to zero */
    expirelo = cycles_per_jiffy;
#else
    expirelo = read_c0_count() + cycles_per_jiffy;
#endif
    write_c0_compare(expirelo);
    write_c0_count(cycles_per_jiffy); //Added DJL
}
Then I get  a normal startup.  i.e it boots fast (no 10second hang).  If I
remove the write_c0_count then I get the 10 second hang.
I have no idea if gettimeofday is broken.  ANy ideas on testing this? Is
there a test package / application that will do this?  Before I write my own

Thanks for the suggestions keep them coming!
Cheers
Dan


-- 
View this message in context: http://www.nabble.com/2.6.19-timer-API-changes-tf2838715.html#a7987092
Sent from the linux-mips main mailing list archive at Nabble.com.

Re: 2.6.19 timer API changes

[Sergei Shtylyov]

Hello.

Daniel Laird wrote:

>>Hm, then it seems writing to COMPARE does not clear COUNT.

>>How about this?  You should still fix pnx8550_hpt_read() anyway, but I
>>suppose gettimeofday() on PNX8550 was broken long time.

>>Subject: [MIPS] Use custom timer_ack and clocksource_mips.read for PNX8550

> I have not tried the suggested change yet, but I have fiund the mips manual
> and here is what it says:

> The 32-bit register is both readable and writable through the MTC0 and MFC0
> instructions as CP0 register 9. Upon reset, the Count register is set to 0.
> Count and Compare together make up Timer_1 in the PR4450 (see Section 3.12).
> The timer is active by default. When active, Count register contains a free
> running
> counter; on each processor clock-tick, the value in the register increments
> by one.
> However, when bit ST1 bit[3] in the CP0 Configuration register is enabled,
> the timer is
> stopped. When the ST1 bit is disabled, the timer returns to its default
> state.
> Timer_1 is active when the PR4450 is in Sleep mode, but is switched off when
> the
> PR4450 is in Coma mode.
> When active, the register can be reset with the assertion of the Terminal
> Count
> (TC_1) signal, which is asserted when the values in the Count and Compare
> registers
> match. After the Count register is reset, it restarts its count on the next
> processor
> clock-tick.
> In the PR4450, the TC_1 signal is fed to the external hardware interrupt
> signal to
> invoke an interrupt handler e.g., the timer interrupt. The TC_1 signal can
> only be
> reset to 0 when the interrupt handler performs a write to the Compare
> register.
> Consecutive writes to Count will result in undefined contents. At least one
> instruction
> must be inserted between consecutive writes to Count.

> This seems to suggest that writing to the compare register does in fact
> clear count.

    It actually doesn't suggest anything alike.  All it says is that "the 
register *can* be reset with the assertion of the Terminal Count (TC_1) signal 
which is asserted when the values in the Count and Compare registers match".
Whtat it doesn't say is what determines if it's really reset by TC_1 assertion 
or not.

> If I do the following:
> static void __init c0_hpt_timer_init(void)
> {
> #ifdef CONFIG_SOC_PNX8550 /* pnx8550 resets to zero */
>     expirelo = cycles_per_jiffy;
> #else
>     expirelo = read_c0_count() + cycles_per_jiffy;
> #endif
>     write_c0_compare(expirelo);
>     write_c0_count(cycles_per_jiffy); //Added DJL
> }

    Hmm, all it actually should do is warrant missing of the first jiffy 
interrupt... I'm even beginning to suspect the counter counts down, not up...
But well, after looking at some internal code, it seems that the Count 
register on PNX8550 always auto-reloads after mathing the Compare register.

> Then I get  a normal startup.  i.e it boots fast (no 10second hang).  If I
> remove the write_c0_count then I get the 10 second hang.
> I have no idea if gettimeofday is broken.  ANy ideas on testing this? Is

    It surely is, if I'm correct in my assumption... I'd suggest to use 
another PNX8550 timer as a clocksource (setting the its Compare register to 
all ones).

> there a test package / application that will do this?  Before I write my own

> Thanks for the suggestions keep them coming!
> Cheers
> Dan

WBR, Sergei

Re: 2.6.19 timer API changes

[Sergei Shtylyov]

Hello.

Atsushi Nemoto wrote:

>>I am just digging out the mips core user manual...  
>>However I have tried this change you suggested, it still takes a long time
>>to get past the calibrate delay function (~10seconds).
>>However after this it seems to run at full speed where as before it used to
>>run very slow.
>>So an improvement, I think this does mean the new time.c has broken 8550
>>support hopefully I can find otu what the core does so it can be fixed.

> Hm, then it seems writing to COMPARE does not clear COUNT.

    Looks like the count/compare match does this...

> How about this?  You should still fix pnx8550_hpt_read() anyway, but I
> suppose gettimeofday() on PNX8550 was broken long time.

    And nobody noticed. :-)

> Subject: [MIPS] Use custom timer_ack and clocksource_mips.read for PNX8550
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> ---
> diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
> index 11aab6d..8aa544f 100644
> --- a/arch/mips/kernel/time.c
> +++ b/arch/mips/kernel/time.c
> @@ -94,10 +94,8 @@ static void c0_timer_ack(void)
>  {
>  	unsigned int count;
>  
> -#ifndef CONFIG_SOC_PNX8550	/* pnx8550 resets to zero */
>  	/* Ack this timer interrupt and set the next one.  */
>  	expirelo += cycles_per_jiffy;
> -#endif
>  	write_c0_compare(expirelo);
>  
>  	/* Check to see if we have missed any timer interrupts.  */
> diff --git a/arch/mips/philips/pnx8550/common/time.c b/arch/mips/philips/pnx8550/common/time.c
> index 65c440e..e86905a 100644
> --- a/arch/mips/philips/pnx8550/common/time.c
> +++ b/arch/mips/philips/pnx8550/common/time.c
> @@ -33,7 +33,18 @@ #include <asm/debug.h>
>  #include <int.h>
>  #include <cm.h>

> -extern unsigned int mips_hpt_frequency;
> +static unsigned long cycles_per_jiffy __read_mostly;

   I wonder shouldn't it be added to <asm-mips/time.h> just for such occasions..

> +
> +static void pnx8550_timer_ack(void)
> +{
> +	write_c0_compare(cycles_per_jiffy);
> +}
> +
> +static cycle_t pnx8550_hpt_read(void)
> +{
> +	/* FIXME: we should use timer2 or timer3 as freerun counter */
> +	return read_c0_count();
 > +}

    I'd suggest read_c0_count2() here, possibly adding an interrupt handler 
for it since it will interrupt upon hitting compare2 reg. value (but we could 
probably just mask the IRQ off), and enabling the timer 2, of course (the 
current code disables it)...

WBR, Sergei

Re: 2.6.19 timer API changes

[Kevin D. Kissell]

> > The 32-bit register is both readable and writable through the MTC0 and MFC0
> > instructions as CP0 register 9. Upon reset, the Count register is set to 0.
> > Count and Compare together make up Timer_1 in the PR4450 (see Section 3.12).
> > The timer is active by default. When active, Count register contains a free
> > running counter; on each processor clock-tick, the value in the register increments
> > by one. However, when bit ST1 bit[3] in the CP0 Configuration register is enabled,
> > the timer is stopped. When the ST1 bit is disabled, the timer returns to its default
> > state. Timer_1 is active when the PR4450 is in Sleep mode, but is switched off when
> > the PR4450 is in Coma mode.
> > When active, the register can be reset with the assertion of the TerminalCount
> > (TC_1) signal, which is asserted when the values in the Count and Compare
> > registers match. After the Count register is reset, it restarts its count on the next
> > processor clock-tick.
> > In the PR4450, the TC_1 signal is fed to the external hardware interrupt
> > signal to invoke an interrupt handler e.g., the timer interrupt. The TC_1 signal can
> > only be reset to 0 when the interrupt handler performs a write to the Compare
> > register.
> > Consecutive writes to Count will result in undefined contents. At least one
> > instruction must be inserted between consecutive writes to Count.
> 
> > This seems to suggest that writing to the compare register does in fact
> > clear count.
> 
>     It actually doesn't suggest anything alike.  All it says is that "the 
> register *can* be reset with the assertion of the Terminal Count (TC_1) signal 
> which is asserted when the values in the Count and Compare registers match".
> Whtat it doesn't say is what determines if it's really reset by TC_1 assertion 
> or not.

In all fairness, the spec can be read to state that the assertion of TC_1 clears
the Count register.  Or rather that it "can" - there seems to be some assumed
mode of behavior that's not otherwise described.  I wonder if they're not 
talking about behavior specifc to "Coma mode", which was perhaps what
they are referring to when they begin the next sentence with "When active".
I can see how someone might think it advantageous to have a mode where
the Count register auto-resets on a timer tick, so that there's no need to
recalcuate Compare values.  But I've never seen that implemented on a
MIPS processor. Free-running Count registers have other uses that can
be shared with the timer interrupts, so long as it's Compare and not Count
that gets reprogrammed on an interrupt. I have a hard time believing that
the 8550 has the auto-reset as default behavior.

> > If I do the following:
> > static void __init c0_hpt_timer_init(void)
> > {
> > #ifdef CONFIG_SOC_PNX8550 /* pnx8550 resets to zero */
> >     expirelo = cycles_per_jiffy;
> > #else
> >     expirelo = read_c0_count() + cycles_per_jiffy;
> > #endif
> >     write_c0_compare(expirelo);
> >     write_c0_count(cycles_per_jiffy); //Added DJL
> > }

First of all, I think the conditional code is broken, even if you
believe that Count is reset to zero on every interrupt.  This is
the *init* code, that's getting called once at boot time to set
up the clock.  It's not a clock interrupt handler.

It is highly likely that the Count register has already gone
beyond the value of cycles_per_jiffy by the time this code
gets hit.  If that's true, programming Compare to zero+delta
means waiting for the counter to wrap around before the first
interrupt is delivered - a 10-second-ish hang.  Writing the same
value to Count that you just wrote to Compare will, on many
cores, cause a Count=Compare state and a prompt interrupt.

But the real fix is almost certainly to get rid of the conditional.

            Regards,

            Kevin K.

Re: 2.6.19 timer API changes

[Kevin D. Kissell]

> > The 32-bit register is both readable and writable through the MTC0 and MFC0
> > instructions as CP0 register 9. Upon reset, the Count register is set to 0.
> > Count and Compare together make up Timer_1 in the PR4450 (see Section 3.12).
> > The timer is active by default. When active, Count register contains a free
> > running counter; on each processor clock-tick, the value in the register increments
> > by one. However, when bit ST1 bit[3] in the CP0 Configuration register is enabled,
> > the timer is stopped. When the ST1 bit is disabled, the timer returns to its default
> > state. Timer_1 is active when the PR4450 is in Sleep mode, but is switched off when
> > the PR4450 is in Coma mode.
> > When active, the register can be reset with the assertion of the TerminalCount
> > (TC_1) signal, which is asserted when the values in the Count and Compare
> > registers match. After the Count register is reset, it restarts its count on the next
> > processor clock-tick.
> > In the PR4450, the TC_1 signal is fed to the external hardware interrupt
> > signal to invoke an interrupt handler e.g., the timer interrupt. The TC_1 signal can
> > only be reset to 0 when the interrupt handler performs a write to the Compare
> > register.
> > Consecutive writes to Count will result in undefined contents. At least one
> > instruction must be inserted between consecutive writes to Count.
> 
> > This seems to suggest that writing to the compare register does in fact
> > clear count.
> 
>     It actually doesn't suggest anything alike.  All it says is that "the 
> register *can* be reset with the assertion of the Terminal Count (TC_1) signal 
> which is asserted when the values in the Count and Compare registers match".
> Whtat it doesn't say is what determines if it's really reset by TC_1 assertion 
> or not.

In all fairness, the spec can be read to state that the assertion of TC_1 clears
the Count register.  Or rather that it "can" - there seems to be some assumed
mode of behavior that's not otherwise described.  I wonder if they're not 
talking about behavior specifc to "Coma mode", which was perhaps what
they are referring to when they begin the next sentence with "When active".
I can see how someone might think it advantageous to have a mode where
the Count register auto-resets on a timer tick, so that there's no need to
recalcuate Compare values.  But I've never seen that implemented on a
MIPS processor. Free-running Count registers have other uses that can
be shared with the timer interrupts, so long as it's Compare and not Count
that gets reprogrammed on an interrupt. I have a hard time believing that
the 8550 has the auto-reset as default behavior.

> > If I do the following:
> > static void __init c0_hpt_timer_init(void)
> > {
> > #ifdef CONFIG_SOC_PNX8550 /* pnx8550 resets to zero */
> >     expirelo = cycles_per_jiffy;
> > #else
> >     expirelo = read_c0_count() + cycles_per_jiffy;
> > #endif
> >     write_c0_compare(expirelo);
> >     write_c0_count(cycles_per_jiffy); //Added DJL
> > }

First of all, I think the conditional code is broken, even if you
believe that Count is reset to zero on every interrupt.  This is
the *init* code, that's getting called once at boot time to set
up the clock.  It's not a clock interrupt handler.

It is highly likely that the Count register has already gone
beyond the value of cycles_per_jiffy by the time this code
gets hit.  If that's true, programming Compare to zero+delta
means waiting for the counter to wrap around before the first
interrupt is delivered - a 10-second-ish hang.  Writing the same
value to Count that you just wrote to Compare will, on many
cores, cause a Count=Compare state and a prompt interrupt.

But the real fix is almost certainly to get rid of the conditional.

            Regards,

            Kevin K.

Re: 2.6.19 timer API changes

[Atsushi Nemoto]

On Wed, 20 Dec 2006 01:37:17 -0800 (PST), Daniel Laird <danieljlaird@hotmail.com> wrote:
> Then I get  a normal startup.  i.e it boots fast (no 10second hang).  If I
> remove the write_c0_count then I get the 10 second hang.

I think Kevin's analysis about this 10 second hang is correct.  Then I
think my last patch will work as well.

> I have no idea if gettimeofday is broken.  ANy ideas on testing this? Is
> there a test package / application that will do this?  Before I write my own

Calling gettimeofday() continuously many times (at least some tick
periods) and calculates times between each call.  Those differences
should be almost same.  Of course you must run this program on very
idle system (or you must raise its priority).

---
Atsushi Nemoto

Re: 2.6.19 timer API changes

[Atsushi Nemoto]

On Wed, 20 Dec 2006 17:29:25 +0300, Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:
> > How about this?  You should still fix pnx8550_hpt_read() anyway, but I
> > suppose gettimeofday() on PNX8550 was broken long time.
> 
>     And nobody noticed. :-)

I changed my mind a bit.  The pre-clocksource gettimeofday() might
work well on PNX8550.  There was timerlo variable which hold COUNT
value on last timer interrupt and fixed_gettimeoffset() subtracted
timerlo from COUNT value at the time.

On Wed, 20 Dec 2006 17:29:25 +0300, Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:

> > +static cycle_t pnx8550_hpt_read(void)
> > +{
> > +	/* FIXME: we should use timer2 or timer3 as freerun counter */
> > +	return read_c0_count();
>  > +}
> 
>     I'd suggest read_c0_count2() here, possibly adding an interrupt
> handler for it since it will interrupt upon hitting compare2
> reg. value (but we could probably just mask the IRQ off), and
> enabling the timer 2, of course (the current code disables it)...

It would be right direction.  And we should set set count2 frequency
to mips_hpt_frequency.  But I cannot test it by myself so I'd like to
leave it for others.  Good exercise ;)

---
Atsushi Nemoto

Re: 2.6.19 timer API changes

[Daniel Laird]

hello, 


Atsushi Nemoto wrote:
> 
> On Wed, 20 Dec 2006 01:37:17 -0800 (PST), Daniel Laird
> <danieljlaird@hotmail.com> wrote:
>> Then I get  a normal startup.  i.e it boots fast (no 10second hang).  If
>> I
>> remove the write_c0_count then I get the 10 second hang.
> 
> I think Kevin's analysis about this 10 second hang is correct.  Then I
> think my last patch will work as well.
> 
>> I have no idea if gettimeofday is broken.  ANy ideas on testing this? Is
>> there a test package / application that will do this?  Before I write my
>> own
> 
> Calling gettimeofday() continuously many times (at least some tick
> periods) and calculates times between each call.  Those differences
> should be almost same.  Of course you must run this program on very
> idle system (or you must raise its priority).
> 
> ---
> Atsushi Nemoto
> 
Thanks guys I can see how Kevin has come to his conclusion (On 10 secs And I
think I agree)!  
I will try your 2nd proposed solution, I gave it a very quick go but it did
not work.  I will give this some proper time, I think some mips_clocksource
needed to be externed etc.  (and add linux/clocksource.h ) to
pnx8550/common/time.c.
Hopefullly this will make it compile, and then I can trace the problem a bit
more.  It might be after Christmas though before I come back to this
Again cheers for the help.
Dan
-- 
View this message in context: http://www.nabble.com/2.6.19-timer-API-changes-tf2838715.html#a7992266
Sent from the linux-mips main mailing list archive at Nabble.com.

Re: 2.6.19 timer API changes

[Sergei Shtylyov]

Hello, I wrote:

>> +
>> +static void pnx8550_timer_ack(void)
>> +{
>> +    write_c0_compare(cycles_per_jiffy);
>> +}
>> +
>> +static cycle_t pnx8550_hpt_read(void)
>> +{
>> +    /* FIXME: we should use timer2 or timer3 as freerun counter */
>> +    return read_c0_count();
>> +}

>    I'd suggest read_c0_count2() here, possibly adding an interrupt 
> handler for it since it will interrupt upon hitting compare2 reg. value 
> (but we could probably just mask the IRQ off), and enabling the timer 2, 
> of course (the current code disables it)...

    No, we'll have to handle IRQ it once the timer is enabled -- there seems 
to be no provision to mask it off other than disabling the timer.

WBR, Sergei

Re: 2.6.19 timer API changes

[Daniel Laird]

Atsushi Nemoto wrote:
> 
> On Wed, 20 Dec 2006 17:29:25 +0300, Sergei Shtylyov
> <sshtylyov@ru.mvista.com> wrote:
>> > How about this?  You should still fix pnx8550_hpt_read() anyway, but I
>> > suppose gettimeofday() on PNX8550 was broken long time.
>> 
>>     And nobody noticed. :-)
> 
> I changed my mind a bit.  The pre-clocksource gettimeofday() might
> work well on PNX8550.  There was timerlo variable which hold COUNT
> value on last timer interrupt and fixed_gettimeoffset() subtracted
> timerlo from COUNT value at the time.
> 
> On Wed, 20 Dec 2006 17:29:25 +0300, Sergei Shtylyov
> <sshtylyov@ru.mvista.com> wrote:
> 
>> > +static cycle_t pnx8550_hpt_read(void)
>> > +{
>> > +	/* FIXME: we should use timer2 or timer3 as freerun counter */
>> > +	return read_c0_count();
>>  > +}
>> 
>>     I'd suggest read_c0_count2() here, possibly adding an interrupt
>> handler for it since it will interrupt upon hitting compare2
>> reg. value (but we could probably just mask the IRQ off), and
>> enabling the timer 2, of course (the current code disables it)...
> 
> It would be right direction.  And we should set set count2 frequency
> to mips_hpt_frequency.  But I cannot test it by myself so I'd like to
> leave it for others.  Good exercise ;)
> 
> ---
> Atsushi Nemoto
> 
> 
> 
It seems likely that this did work in previous releases, however it will
obviously need to work on this release and so once I have solved the startup
issues by using your second proposed patch I will do some testing to see if
I can get this to work (if it is indeed broken)
Cheers
Dan
-- 
View this message in context: http://www.nabble.com/2.6.19-timer-API-changes-tf2838715.html#a7992312
Sent from the linux-mips main mailing list archive at Nabble.com.

Re: 2.6.19 timer API changes

[Sergei Shtylyov]

Hello.

Kevin D. Kissell wrote:

> they are referring to when they begin the next sentence with "When active".
> I can see how someone might think it advantageous to have a mode where
> the Count register auto-resets on a timer tick, so that there's no need to
> recalcuate Compare values.  But I've never seen that implemented on a
> MIPS processor.

    Toshiba TX3927 mentioned in that thread before is an example...

> Free-running Count registers have other uses that can
> be shared with the timer interrupts, so long as it's Compare and not Count
> that gets reprogrammed on an interrupt. I have a hard time believing that
> the 8550 has the auto-reset as default behavior.

    Yet the code suggests that it does. PNX8550 seems to be a strange beast... :-)

>>>If I do the following:
>>>static void __init c0_hpt_timer_init(void)
>>>{
>>>#ifdef CONFIG_SOC_PNX8550 /* pnx8550 resets to zero */
>>>    expirelo = cycles_per_jiffy;
>>>#else
>>>    expirelo = read_c0_count() + cycles_per_jiffy;
>>>#endif
>>>    write_c0_compare(expirelo);
>>>    write_c0_count(cycles_per_jiffy); //Added DJL
>>>}

> First of all, I think the conditional code is broken, even if you
> believe that Count is reset to zero on every interrupt.  This is
> the *init* code, that's getting called once at boot time to set
> up the clock.  It's not a clock interrupt handler.

> It is highly likely that the Count register has already gone

    This is called why the count is disabled -- this is done in 
arch_init_irq(). Only plat_timer_setup() re-enables it.

> beyond the value of cycles_per_jiffy by the time this code
> gets hit.  If that's true, programming Compare to zero+delta
> means waiting for the counter to wrap around before the first
> interrupt is delivered - a 10-second-ish hang.  Writing the same
> value to Count that you just wrote to Compare will, on many
> cores, cause a Count=Compare state and a prompt interrupt.

> But the real fix is almost certainly to get rid of the conditional.

    That would be too simple... :-)
    Moreover, with presumed auto-reload behavior it's likely to warrant the 
wrong expirelo value (which in turn will cause jiffies to be longer than 
intended) -- all because of Count register possible being non-zero at this 
point...

>             Kevin K.

WBR, Sergei