[ECOS] hal_delay_us() patch to at91_misc + multithreading

[ECOS] hal_delay_us() patch to at91_misc + multithreading

[Øyvind Harboe]

Original message:

http://sourceware.org/ml/ecos-discuss/2005-05/msg00357.html

Your fix seems approperiate. Perhaps you should try to post it to
ecos-patches?

Q: As near as I can tell, HAL_DELAY_US() is not thread safe in
at91_misc.c. Is this correctly observed?

I can't tell by reading the eCos documentation whether or not
hal_delay_us or HAL_DELAY_US is supposed to be thread safe.

http://sources.redhat.com/ecos/docs-2.0/ref/hal-interrupt-handling.html

The implementation of I2C uses hal_delay_us, but since I'm not currently
using that code(our home rolled I2C code has not been phased out yet),
I'm not sure whether or not interrupts are disabled while hal_delay_us()
is invoked inside the I2C code.

-- 
Øyvind Harboe
http://www.zylin.com


--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] hal_delay_us() patch to at91_misc + multithreading

[Bart Veer]

>>>>> "Oyvind" == =?ISO-8859-1?Q?=D8yvind?= Harboe <ISO-8859-1> writes:

    Oyvind> Q: As near as I can tell, HAL_DELAY_US() is not thread
    Oyvind> safe in at91_misc.c. Is this correctly observed?

    Oyvind> I can't tell by reading the eCos documentation whether or
    Oyvind> not hal_delay_us or HAL_DELAY_US is supposed to be thread
    Oyvind> safe.

    Oyvind> http://sources.redhat.com/ecos/docs-2.0/ref/hal-interrupt-handling.html

    Oyvind> The implementation of I2C uses hal_delay_us, but since I'm
    Oyvind> not currently using that code(our home rolled I2C code has
    Oyvind> not been phased out yet), I'm not sure whether or not
    Oyvind> interrupts are disabled while hal_delay_us() is invoked
    Oyvind> inside the I2C code.

The actual definition of HAL_DELAY_US() is in
http://ecos.sourceware.org/docs-latest/ref/hal-clocks-and-timers.html
It lists the macro as optional and does not address thread-safety.
There are still some architectures where it has not been implemented
including the synthetic target, sparc, sparclite and v85x.

There are two main ways of implementing HAL_DELAY_US(). The first is a
counting loop, typically written in inline assembler, using an outer
loop for the microseconds and an inner loop that consumes
approximately 1us. With a bit of experimenting it is usually
straightforward to get this to within a couple of percent of the
desired delay. The implementation is automatically thread-safe and
does not require manipulating any hardware, disabling interrupts, or
anything like that. If the thread happens to be timesliced or
preempted by a higher-priority thread in the middle of the busy loop
then the delay will be longer than intended, but that is always a
possibility anyway. This approach assumes the cpu clock speed is known
at compile-time and won't change. On the synthetic target it would
probably require reading /proc/cpuinfo during initialization,
extracting the bogomips rating, and using that to calibrate a busy
loop.

The second way is to use one of the hardware clocks, possibly the
system clock. Personally I do not like this approach, it adds a
dependency on the hardware and is vulnerable to other code
manipulating the clock. However currently this is the more common
implementation.

HAL_DELAY_US() is already used in various places, not just the I2C
bitbang code, for example the lan91cxx ethernet driver. It is
generally useful functionality. I would like to see the macro
definition upgraded to make it compulsory rather than optional, and
require the macro to be thread-safe. I am happy to do a synthetic
target implementation, but that still leaves other architectures where
the macro would need to be added or fixed.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] hal_delay_us() patch to at91_misc + multithreading

[Øyvind Harboe]

> I would like to see the macro
> definition upgraded to make it compulsory rather than optional, and
> require the macro to be thread-safe. 

Makes sense. I2C seems to be based upon these assumptions. 

> I am happy to do a synthetic
> target implementation, but that still leaves other architectures where
> the macro would need to be added or fixed.

How about calibrating a CPU counter loop implementaiton upon startup
using the default hardware timer in the HAL?

- a couple of ms to the startup time is a small price to pay for
  a default implementation of a HAL_DELAY_US()
- The HAL_DELAY_US() definition should state that the the accuracy
  is not very good, but that it is guaranteed to wait at least
  n us.


Is there a fundamental reason why HAL_DELAY_US() needs to be very
accurate(i.e. say <20-30%)

> 
> Bart
> 


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] hal_delay_us() patch to at91_misc + multithreading

[Bart Veer]

>>>>> "Oyvind" == =?ISO-8859-1?Q?=D8yvind?= Harboe <ISO-8859-1> writes:

    >> I would like to see the macro definition upgraded to make it
    >> compulsory rather than optional, and require the macro to be
    >> thread-safe.

    Oyvind> Makes sense. I2C seems to be based upon these assumptions. 

    >> I am happy to do a synthetic target implementation, but that
    >> still leaves other architectures where the macro would need to
    >> be added or fixed.

    Oyvind> How about calibrating a CPU counter loop implementaiton
    Oyvind> upon startup using the default hardware timer in the HAL?

    Oyvind> - a couple of ms to the startup time is a small price to pay for
    Oyvind>   a default implementation of a HAL_DELAY_US()
    Oyvind> - The HAL_DELAY_US() definition should state that the the accuracy
    Oyvind>   is not very good, but that it is guaranteed to wait at least
    Oyvind>   n us.

Doesn't work for the synthetic target. The only easy way for the
synthetic target to read a clock is a gettimeofday() system call, i.e.
real time rather than process elapsed time. This would be fine if
there was nothing else running, but the synthetic target may get
preempted at any time including during this calibration loop. Hence
there is no guarantee the gettimeofday() values bear any resemblance
to the loop count. You could try doing the measurements several times
and picking the most sensible result, but at that level of complexity
you might as well read /proc/cpuinfo.

For embedded targets, often a couple of ms extra boot time is not
significant. But not always. There are systems which have a hard
requirement that some application code gets to run within a couple of
ms or less of power-up. Instead wherever possible the HAL_DELAY_US()
loop count should be determined during the porting process, not at
run-time.

    Oyvind> Is there a fundamental reason why HAL_DELAY_US() needs to
    Oyvind> be very accurate(i.e. say <20-30%)

It should not be less than the specified delay, give or take a couple
of percent. Device drivers can have fairly strict minimum intervals
between operations. Once interrupts are enabled HAL_DELAY_US() can
take arbitrary amounts of time longer than the specified delay, but it
is still a good idea to make it reasonably accurate. The usual
implementation is a polling loop during which no other work can get
done, making that loop use more cpu cycles than are absolutely
necessary is wasteful.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] hal_delay_us() patch to at91_misc + multithreading

[Øyvind Harboe]

[lots of valid objections to my suggested HAL_DELAY_US() implementation
 deleted]

There is one issue with making HAL_DELAY_US() compulsory that has not
been discussed: how likely is it to get a consensus to do so?

My thinking was that a default implementation of HAL_DELAY_US() would
improve that chances of getting a consensus. 

The crummy HAL_DELAY_US() I suggested scored on this point. It would
work correctly on almost any platform.



-- 
Øyvind Harboe
http://www.zylin.com


--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss