RE: Xenomai on Freescale i.MX21 (ARM926J)

Hi Philippe,

Here is a new version of the patch considering your remarks.

> -----Message d'origine-----
> De : Philippe Gerum [mailto:rpm@xxxxxxxxxxx]
> Envoyé : vendredi, 21. avril 2006 17:44
> À : ROSSIER Daniel
> Cc : xenomai-core@xxxxxxx; adeos-main@xxxxxxx
> Objet : Re: [Xenomai-core] Xenomai on Freescale i.MX21 (ARM926J)
> 
> 
> Hi Daniel,
> 
> Some comments about the Adeos patch for the i.MX21/CSB535FS board. The
> inlined patch
> has been rebuilt by diff'ing the vanilla Adeos 1.2-00 support with yours.
> 
>  > --- linux-2.6.14/arch/arm/kernel/ipipe-core.c      2006-04-21
> 16:17:02.000000000 +0200
>  > +++ linux-2.6.14-heig/arch/arm/kernel/ipipe-core.c 2006-04-21
> 15:33:47.000000000 +0200
>  > @@ -22,6 +22,12 @@
>  >   * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-
> 1307, USA.
>  >   *
>  >   * Architecture-dependent I-PIPE core support for ARM.
>  > + *
>  > + * April 2006 :
>  > + * Adapted to ARM9 i.MX21/CSB535FS board with Xenomai by S.Gerber,
> G.Boutillier and D.Rossier
>  > + * University of Applied Sciences Western Switzerland
>  > + * Reconfigurable & Embedded Digital Systems, REDS Institute
>  > + *
>  >   */
>  >
>  >  #include <linux/config.h>
>  > @@ -320,11 +326,19 @@ static void __ipipe_set_decr(void)
>  >  int ipipe_tune_timer(unsigned long ns, int flags)
>  >  {
>  >    unsigned long x, ticks;
>  > +  unsigned long us;
>  >
>  >    if (flags & IPIPE_RESET_TIMER)
>  >            ticks = __ipipe_mach_ticks_per_jiffy;
>  >    else {
>  > -          ticks = ns * __ipipe_mach_ticks_per_jiffy / (1000000000 / HZ);
>  > +
>  > +          /*
>  > +           * FIXME : Temporary convert ns to us. With nanosecondes we
> have a problem
>  > +                 *         with overflow.
>  > +           *         ticks = ns * __ipipe_mach_ticks_per_jiffy /
> (1000000000 / HZ);
>  > +           */
>  > +          us = ns / 1000;
>  > +          ticks = ((us * __ipipe_mach_ticks_per_jiffy) / 10000);
>  >
> 
> This change is going to be wrong when CONFIG_HZ != 1000,
> this is why HZ should remain in the picture. This said, the original
> expression looks
> overly complicated, something like the following should do without risking
> the overflow:
> 
> -             ticks = ns * __ipipe_mach_ticks_per_jiffy / (1000000000 / HZ);
> +             ticks = ns / (TICKS_PER_uSEC * 1000);
> 
>  >            if (ticks > __ipipe_mach_ticks_per_jiffy)
>  >                    return -EINVAL;
> 

Agreed. We propose the following changes in order to get a better precision:


+               ticks = __ipipe_mach_ticks_per_jiffy;
+       else {
+
+               //FIXME : Problem with value of ns < 1000
+               ticks = (ns / 1000) * (__ipipe_mach_ticks_per_jiffy) / (1000000 
/ HZ);
+
+               if (ticks > __ipipe_mach_ticks_per_jiffy)
+                       return -EINVAL;
+       }



>  > diff -uNrp linux-2.6.14/arch/arm/kernel/process.c linux-2.6.14-
> heig/arch/arm/kernel/process.c
>  > --- linux-2.6.14/arch/arm/kernel/process.c 2006-04-21
> 16:17:02.000000000 +0200
>  > +++ linux-2.6.14-heig/arch/arm/kernel/process.c    2006-04-21
> 15:33:47.000000000 +0200
>  > @@ -7,6 +7,12 @@
>  >   * This program is free software; you can redistribute it and/or
> modify
>  >   * it under the terms of the GNU General Public License version 2 as
>  >   * published by the Free Software Foundation.
>  > + *
>  > + * April 2006 :
>  > + * Adapted to ARM9 i.MX21/CSB535FS board with Xenomai by S.Gerber,
> G.Boutillier and D.Rossier
>  > + * University of Applied Sciences Western Switzerland
>  > + * Reconfigurable & Embedded Digital Systems, REDS Institute
>  > + *
>  >   */
>  >  #include <stdarg.h>
>  >
>  > @@ -111,7 +117,14 @@ void cpu_idle(void)
>  >            preempt_disable();
>  >            leds_event(led_idle_start);
>  >            while (!need_resched())
>  > +/*
>  > + * Width idle -> latenca of interrupts
>  > + */
>  > +#ifndef CONFIG_IPIPE
>  >                    idle();
>  > +#else
>  > +                  ;
>  > +#endif
> 
> Ouch. It would be saner to implement the idle mode switch one could pass
> as a boot argument to the kernel (e.g. arch/i386/kernel/process.c),
> accepting "poll" or "default", instead of hard-wiring the poll mode.

Fixed. We have introduced a new config variable called XENO_OPT_POLL_IDLE

> 
>  >            leds_event(led_idle_end);
>  >            preempt_enable();
>  >            schedule();
>  > diff -uNrp linux-2.6.14/init/Kconfig linux-2.6.14-heig/init/Kconfig
>  > --- linux-2.6.14/init/Kconfig      2006-04-21 16:17:02.000000000 +0200
>  > +++ linux-2.6.14-heig/init/Kconfig 2006-04-21 15:34:01.000000000
> +0200
>  > @@ -502,3 +502,20 @@ config STOP_MACHINE
>  >    help
>  >      Need stop_machine() primitive.
>  >  endmenu
>  > +
>  > +menu "Real-time sub-system"
>  > +
> 
> The Xenomai-related stuff does not belong to the Adeos patch.
> 
> --
> 
> Philippe.




We still have to send further info about latencies. This should be done in the 
next days.




Daniel

adeos-ipipe-2.6.14-arm-imx21-0.1-01.patch
Description: adeos-ipipe-2.6.14-arm-imx21-0.1-01.patch

_______________________________________________
Xenomai-core mailing list
Xenomai-core@xxxxxxx
https://mail.gna.org/listinfo/xenomai-core