[Xenomai-core] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Xenomai-core] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

I have been trying to bring my IXP425 based system up to date, and I
have found an apparent regression. Everything worked fine with ipipe
2.6.30 and Xenomai 2.4. With 2.6.31 an ipipe kernel still boots, but
starting with 2.6.33 the trouble begins (see below).

I see that Gilles refactored the TSC emulation starting with
2.6.33. I wonder whether this could be the cause?

I did not immediately see any obvious bug, but I don't fully
understand the newer TSC code. I would appreciate any hints.

Thanks,
Richard

** ipipe-2.6.31-arm-1.16-01

   Seems to boot fine.

** ipipe-2.6.33-arm-1.18-01

   Boots, but runs very slowly. FCSE on/off makes no difference.
   After a few seconds, it seem to hand, but it might just be very
   slow. Once I saw this on the console:

      hrtimer: interrupt took 1414795601278721 ns

   coming from kernel/hrtimer.c:1363

      printk_once(KERN_WARNING "hrtimer: interrupt took %llu ns\n", ...)

** ipipe-2.6.35.9-arm-1.18-01

   Does not boot with ipipe. FCSE on/off makes no difference.
   It *does* boot without ipipe enabled.

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Gilles Chanteperdrix]

Richard Cochran wrote:
> I have been trying to bring my IXP425 based system up to date, and I
> have found an apparent regression. Everything worked fine with ipipe
> 2.6.30 and Xenomai 2.4. With 2.6.31 an ipipe kernel still boots, but
> starting with 2.6.33 the trouble begins (see below).
> 
> I see that Gilles refactored the TSC emulation starting with
> 2.6.33. I wonder whether this could be the cause?

What this modification does is that:
- there is only one clocksource, which is now xenomai tsc, with a very
high rating;
- the tsc read code is written in assembly, and copied in the vectors
page, the one at 0xffff0000.

I think the ixp is a 32 bits free-running counter with match register,
wich is a configuration I tested. So, I would tend to think that the
issue is rather in the clock_events implementation.


-- 
					    Gilles.

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

On Thu, Apr 07, 2011 at 08:32:52PM +0200, Gilles Chanteperdrix wrote:
> Richard Cochran wrote:
> > I have been trying to bring my IXP425 based system up to date, and I
> > have found an apparent regression. Everything worked fine with ipipe
> > 2.6.30 and Xenomai 2.4. With 2.6.31 an ipipe kernel still boots, but
> > starting with 2.6.33 the trouble begins (see below).
> > 
> > I see that Gilles refactored the TSC emulation starting with
> > 2.6.33. I wonder whether this could be the cause?
> 
> What this modification does is that:
> - there is only one clocksource, which is now xenomai tsc, with a very
> high rating;
> - the tsc read code is written in assembly, and copied in the vectors
> page, the one at 0xffff0000.
> 
> I think the ixp is a 32 bits free-running counter with match register,
> wich is a configuration I tested. So, I would tend to think that the
> issue is rather in the clock_events implementation.

The IXP425 has one shot, count down timer (see below). Does that make
any difference?

(It might be that one of the other IXPxxx chips does have a match
register, but not this one.)

Thanks,
Richard

---
>From the 2.6.31 patch, in arch/arm/mach-ixp4xx/common.c

/*
 * Reprogram the timer
 *
 * The timer is aperiodic (most of the time) when running Xenomai, so
 * __ipipe_mach_set_dec is called for each timer tick and programs the
 * timer hardware for the next tick.
 *
 */
#define MIN_DELAY 333 /* 5 usec with the 66.66 MHz system clock */

void __ipipe_mach_set_dec(unsigned long delay)
{
	unsigned long flags;
	if (delay > MIN_DELAY) {
		local_irq_save_hw(flags);
		*IXP4XX_OSRT1 = delay | ONE_SHOT_ENABLE;
		local_irq_restore_hw(flags);
	} else {
	      	ipipe_trigger_irq(IRQ_IXP4XX_TIMER1);
	}
}

EXPORT_SYMBOL(__ipipe_mach_set_dec);

/*
 * This returns the number of clock ticks remaining.
 */
unsigned long __ipipe_mach_get_dec(void)
{
	return(*IXP4XX_OST1); /* remaining */
}

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

On Thu, Apr 07, 2011 at 09:02:58PM +0200, Richard Cochran wrote:
> On Thu, Apr 07, 2011 at 08:32:52PM +0200, Gilles Chanteperdrix wrote:
> > I think the ixp is a 32 bits free-running counter with match register,
> > wich is a configuration I tested. So, I would tend to think that the
> > issue is rather in the clock_events implementation.
> 
> The IXP425 has one shot, count down timer (see below). Does that make
> any difference?
> 
> (It might be that one of the other IXPxxx chips does have a match
> register, but not this one.)

What I meant to say:

Yes, the time register is a 32 bits free-running counter.

But, the interrupt source is a count down register, with or without
automatic reload.

Richard

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Gilles Chanteperdrix]

Richard Cochran wrote:
> On Thu, Apr 07, 2011 at 09:02:58PM +0200, Richard Cochran wrote:
>> On Thu, Apr 07, 2011 at 08:32:52PM +0200, Gilles Chanteperdrix wrote:
>>> I think the ixp is a 32 bits free-running counter with match register,
>>> wich is a configuration I tested. So, I would tend to think that the
>>> issue is rather in the clock_events implementation.
>> The IXP425 has one shot, count down timer (see below). Does that make
>> any difference?
>>
>> (It might be that one of the other IXPxxx chips does have a match
>> register, but not this one.)
> 
> What I meant to say:
> 
> Yes, the time register is a 32 bits free-running counter.
> 
> But, the interrupt source is a count down register, with or without
> automatic reload.

On my side, I meant to say, the issue is in the implementation of the
I-pipe one-shot timer, the function ipipe_mach_set_dec...


-- 
					    Gilles.

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

On Thu, Apr 07, 2011 at 10:26:02PM +0200, Gilles Chanteperdrix wrote:
> 
> On my side, I meant to say, the issue is in the implementation of the
> I-pipe one-shot timer, the function ipipe_mach_set_dec...

I couldn't find anything wrong there, but I did find this...

In ipipe_tsc_asm.S you have

 #ifndef CONFIG_CPU_ENDIAN_BE8
 /* Little endian */
	ldr	r0, [r0]
	cmp	r2, r0
	adc	r1, r3, #0
 #else /* Big endian */
	ldr	r1, [r0]
	cmp	r3, r1
	adc	r0, r2, #0
 #endif /* Big endian */

but my .config has

 CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
 CONFIG_CPU_BIG_ENDIAN=y
 # CONFIG_CPU_ENDIAN_BE8 is not set
 CONFIG_CPU_ENDIAN_BE32=y

and the machine really is big endian, so that might make a difference.

Richard

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

In ipipe_tsc_asm.S you have at the end of the file...

/* User-space entry-point: r0 is the hardware counter virtual address */
#ifndef CONFIG_CPU_BIG_ENDIAN
/* Little endian */

...

#else /* Big endian */
/* Little endian */
1: ldr	  r1, .LCdec16_last_tsc + 4
   ldr	  ip, [r0]
   ldr	  r2, .Ldec16_Clast_cnt
---------------^

Did you mean ".LCdec16_last_tsc" here?

Thanks,
Richard



   subs   ip, r2, ip
   addcs  ip, ip, #0x10000
   myldrd r2, r3, r3, .LCdec16_last_tsc
   cmp	  r1, r3
   bne	  1b
   adds	  r1, ip, r3
   adc	  r0, r2, #0
#endif /* Big endian */
       usr_ret	     lr

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Gilles Chanteperdrix]

Richard Cochran wrote:
> In ipipe_tsc_asm.S you have at the end of the file...
> 
> /* User-space entry-point: r0 is the hardware counter virtual address */
> #ifndef CONFIG_CPU_BIG_ENDIAN
> /* Little endian */
> 
> ...
> 
> #else /* Big endian */
> /* Little endian */
> 1: ldr	  r1, .LCdec16_last_tsc + 4
>    ldr	  ip, [r0]
>    ldr	  r2, .Ldec16_Clast_cnt
> ---------------^
> 
> Did you mean ".LCdec16_last_tsc" here?

You do not really care about this code, since it is for 16 bit
decrementers (currently only the integrator or the S3C has this, and
they are not big endian machines). But yes.

-- 
                                                                Gilles.

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Gilles Chanteperdrix]

Richard Cochran wrote:
> On Thu, Apr 07, 2011 at 10:26:02PM +0200, Gilles Chanteperdrix wrote:
>> On my side, I meant to say, the issue is in the implementation of the
>> I-pipe one-shot timer, the function ipipe_mach_set_dec...
> 
> I couldn't find anything wrong there, but I did find this...
> 
> In ipipe_tsc_asm.S you have
> 
>  #ifndef CONFIG_CPU_ENDIAN_BE8
>  /* Little endian */
> 	ldr	r0, [r0]
> 	cmp	r2, r0
> 	adc	r1, r3, #0
>  #else /* Big endian */
> 	ldr	r1, [r0]
> 	cmp	r3, r1
> 	adc	r0, r2, #0
>  #endif /* Big endian */
> 
> but my .config has
> 
>  CONFIG_ARCH_SUPPORTS_BIG_ENDIAN=y
>  CONFIG_CPU_BIG_ENDIAN=y
>  # CONFIG_CPU_ENDIAN_BE8 is not set
>  CONFIG_CPU_ENDIAN_BE32=y
> 
> and the machine really is big endian, so that might make a difference.

Yes, you can try changing the #ifdef. But note also that the big endian
code was never actually tested. So, you probably would be better
compiling a little user-space application to test this tsc code.

the Linksys WRT54GS is an ixp 425, right?.

-- 
                                                                Gilles.

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

On Fri, Apr 08, 2011 at 08:16:17AM +0200, Gilles Chanteperdrix wrote:
> Yes, you can try changing the #ifdef. But note also that the big endian
> code was never actually tested. So, you probably would be better
> compiling a little user-space application to test this tsc code.

Yes, it worked. I sent a patch for 2.6.33.

> the Linksys WRT54GS is an ixp 425, right?.

I don't know about that one.

The NSLU is ixp42x, but many people run it in little endian.

Thanks,
Richard

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Gilles Chanteperdrix]

Richard Cochran wrote:
> On Fri, Apr 08, 2011 at 08:16:17AM +0200, Gilles Chanteperdrix wrote:
>> Yes, you can try changing the #ifdef. But note also that the big endian
>> code was never actually tested. So, you probably would be better
>> compiling a little user-space application to test this tsc code.
> 
> Yes, it worked. I sent a patch for 2.6.33.
> 
>> the Linksys WRT54GS is an ixp 425, right?.
> 
> I don't know about that one.
> 
> The NSLU is ixp42x, but many people run it in little endian.

Ok. Thanks queued. I will also add the hostrt stuff.

-- 
                                                                Gilles.

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

On Fri, Apr 08, 2011 at 08:35:37AM +0200, Gilles Chanteperdrix wrote:
> Ok. Thanks queued. I will also add the hostrt stuff.

BTW, patch works for both 2.6.33 and 2.6.35.
Both are booting fine, now.

Thanks,
Richard

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Gilles Chanteperdrix]

Gilles Chanteperdrix wrote:
> Richard Cochran wrote:
>> I have been trying to bring my IXP425 based system up to date, and I
>> have found an apparent regression. Everything worked fine with ipipe
>> 2.6.30 and Xenomai 2.4. With 2.6.31 an ipipe kernel still boots, but
>> starting with 2.6.33 the trouble begins (see below).
>>
>> I see that Gilles refactored the TSC emulation starting with
>> 2.6.33. I wonder whether this could be the cause?
> 
> What this modification does is that:
> - there is only one clocksource, which is now xenomai tsc, with a very
> high rating;
> - the tsc read code is written in assembly, and copied in the vectors
> page, the one at 0xffff0000.
> 
> I think the ixp is a 32 bits free-running counter with match register,
> wich is a configuration I tested. So, I would tend to think that the
> issue is rather in the clock_events implementation.

Just to explain a little further about this modification. The reasons
for this refactoring are:
- the TSC emulation code is factored, so that when porting to a new
port, we avoid copy-pasting the TSC emulation;
- it is written in assembly, because even for such a simple thing, I was
unable to get gcc to generate optimal code without useless register moves;
- it is ready for multi-SOCs kernels (all the TSC emulations are
compiled-in, the one chosen is copied to the vectors page when the
ipipe_tsc_register function is called). This is the direction ARM linux
is going, if you followed the ARM wrestling on the linux-arm-kernel
mailing list.
- the Linux clocksource code also uses this TSC, this keeps it in cache,
and gets us ready for hostrt (actually, hostrt is already there on ARM,
what is missing is just the equivalent of the code that was sent to the
ARM mailing list for x86_32).
- and most of all, this implementation is in the vector page, so is
available to user-space applications. This means that Xenomai user-space
support can be compiled without any knowledge of the actual TSC
involved. IOW, it means that Xenomai user-space support compiled with
--enable-arm-mach=generic (which is going to be the default) can use TSC
emulation..

-- 
                                                                Gilles.

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Richard Cochran]

On Fri, Apr 08, 2011 at 08:35:03AM +0200, Gilles Chanteperdrix wrote:

> - the TSC emulation code is factored, so that when porting to a new
> port, we avoid copy-pasting the TSC emulation;

The ARM porting page on the wiki is out of date with regards to this.

> - it is ready for multi-SOCs kernels (all the TSC emulations are
> compiled-in, the one chosen is copied to the vectors page when the
> ipipe_tsc_register function is called). This is the direction ARM linux
> is going, if you followed the ARM wrestling on the linux-arm-kernel
> mailing list.

Yes, that thread was really entertaining ;)


Thanks for all your good work,

Richard

Re: [Xenomai-core] [Adeos-main] ARM IXP: ipipe regression from 2.6.31 to .33 and .35

[Gilles Chanteperdrix]

Richard Cochran wrote:
> On Fri, Apr 08, 2011 at 08:35:03AM +0200, Gilles Chanteperdrix wrote:
> 
>> - the TSC emulation code is factored, so that when porting to a new
>> port, we avoid copy-pasting the TSC emulation;
> 
> The ARM porting page on the wiki is out of date with regards to this.

Yes, and about the GPIO demuxing too. I will update it when I am done
with the Panda port, which will allow me to document the SMP bits...

> 
>> - it is ready for multi-SOCs kernels (all the TSC emulations are
>> compiled-in, the one chosen is copied to the vectors page when the
>> ipipe_tsc_register function is called). This is the direction ARM linux
>> is going, if you followed the ARM wrestling on the linux-arm-kernel
>> mailing list.
> 
> Yes, that thread was really entertaining ;)
> 
> 
> Thanks for all your good work,

You are welcome, it is a pleasure to work with people who do not expect
us to do all the fixing on our own.

-- 
					    Gilles.