URL:
<http://savannah.nongnu.org/bugs/?30363>
Summary: _delay_xx() functions in <util/delay.h> are broken
Project: AVR C Runtime Library
Submitted by: bperrybap
Submitted on: Mon 05 Jul 2010 10:34:10 PM GMT
Category: Header
Severity: 3 - Normal
Priority: 5 - Normal
Item Group: libc code
Status: None
Percent Complete: 0%
Assigned to: None
Open/Closed: Open
Discussion Lock: Any
Release: 1.7.*
Fixed Release: None
_______________________________________________________
Details:
WIth the conversion to using __builtin_delay_cycles() the _delay_us() and
_delay_ms() functions in <util/delay.h> were broken.
The problem is quite obvious, but the solution is not.
The problem is that the code is using a temporary variable that was
calculating the number of "ticks" not cycles. The ticks value was 3 clocks in
_delay_us() and 4 clocks in _delay_ms(). The new code is passing this
temporary "ticks" value to the built in function __delay_builtin_cycles()
rather than a cycle count which results in delaying the incorrect number of
cycles.
The reason I say the solution is not obvious is due to both rounding concerns
and backward compatibility.
The old routines rounded to a "ticks" value which was more than a single AVR
cycle. The original code also always rounded down because of integer
truncation. However, when the tick count was zero, a tick count of 1 was
used.
This is potentially a backward compatibility issue when converting to the new
builtin delay function and use very short microsecond delays.
The reason it can break old code is that old code might only be working "by
accident". In other words the user might have been asking for an impossible
delay (shorter than a delay "tick") but the code would delay a full tick,
which for _delay_us() is 3 cycles. So while a user might have asked for 1 us
he would get 3 us. The new code would potentially give a 1 us delay, which
might actually break previously working code, which was working "by accident"
because the actual delay was longer than what was asked for.
The other issue that comes into play is rounding itself.
For example, there are 3 ways to round cycles.
- up (delay is always *at least* as long as requested)
- down (delay is closest as possible without going over but can be worst case
nearly 1 cycle shorter than requested)
- closest (picks closest cycle which could round up or down)
The are pluses and minuses to all 3 methods.
The real answer is have some sort of reconfigurability in the API itself but
that potentially breaks backward compatibility.
While it would be nice to have a rounding option in the API functiuon itself,
it could be handled in a limited way with ifdefs that could
be set by the makefile/compiler or by the user setting a #define before
including the <util/delay.h> file.
My concern and focus in the past has been on ensuring that the delay
functions can be used for low level hardware setup times, which simply was not
possible with the previous routines as the truncation would often generate
delays that were too short and when the delay were very small the delays might
be 1 or 2 cycles longer than what would have been possible.
This can be a very big deal when writing an open source library for something
like an i/o intensive GLCD that has many hardware setup times and will be
running on a variety of AVRs at different clock rates.
Delays that are too short, and it doesn't function, delays longer than
necessary and the performance goes way down.
I have also assumed that when people are asking for delays in the millisecond
and longer range that they are not too concerned about the individual cycle
rounding.
So until there is a new API for delays that supports a rounding option, I
would propose that the code always round up to ensure that a delay is always
at least as long as requested.
Yes that means that somebody that asks for a 0.200us (200ns) delay on a 1Mhz
AVR gets a 1 us (1 AVR clock) delay, but that is the shortest delay possible
in that environment.
I have attached a modified header file to show what I'm talking about (Note:
the file attached is untested and is there for example)
If you guys are ok with this method, I will flesh it out, which includes not
only full testing but also updating the doxygen documentation in the header
file as well to reflect this new delay method its rounding details.
If you think there should be a rounding define/ifdef to allow uses to
configure how the rounding works, I can even add that in as well.
A few things to consider when considering something other than "round up".
What happens if the number of cycles needed is less than 1?
Do you delay zero cycles or do you delay 1?
Arguments can be made either way, but for hardware delays, often no delay
won't work, so you have to delay at least 1 cycle which on slow F_CPU rates,
can be WAY longer than what is desired, but is often still necessary to make
things work.
This rounding dilemma is avoided when always rounding up.
--- bill
_______________________________________________________
File Attachments:
-------------------------------------------------------
Date: Mon 05 Jul 2010 10:34:10 PM GMT Name: delay.h.in Size: 6kB By:
bperrybap
Example (untested) <util/delay.h> header file.
<http://savannah.nongnu.org/bugs/download.php?file_id=20904>
_______________________________________________________
Reply to this item at:
<http://savannah.nongnu.org/bugs/?30363>
_______________________________________________
Message sent via/by Savannah
http://savannah.nongnu.org/
_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@nongnu.org
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev
