[avr-libc-dev] [bug #30363] _delay_xx() functions in <util/delay.h> are broken

[Bill Perry]

Follow-up Comment #8, bug #30363 (project avr-libc):

While the fix is "ok". it doesn't solve some of the
other issues that plague these function calls.

The code I proposed a few months back was a quick and dirty solution to
correct the grossly miscalculated delays when the gcc compiler toolset had
built in delay support by the compiler.
It was not meant to be the long term fix.
I was assuming that this would be quickly tossed in and
the C library immediately re-released, while a better longer
term solution that delt with some of the other delay code issues was
created.
Since that didn't happen, I think we have the opportunity to create a better
longer term solution.

I also think the rounding issue needs to be better addressed
rather than simply always truncate down but round up when 0.
(My proposal was an attempt to always round up not just at 0)
Rounding down, does not work for some uses.

While the old delay code truncated but rounded 0 up,
the rounding in the old code was so "broken" and the granularity is now so
much better, that I don't think the new code needs to necessarily use this
truncation methodology.

None of this delay code  is difficult to write or fix.
I'll even write the code if we can agree on what it should do.

As I see it, there are 5 things that need to be addressed:
(some might be longer term than others)

1) fix the delay calculation to pass proper values to
  __builtin_avr_delay_cycles() based on users request.

2) How to round cycle delay values (up, down, closest)
    Maybe have a compile time ifdef to set this? 

3) How to deal with very long delays that go beyond
   the longest delay possible with the underlying delay
   mechanism.

4) backward compatibility

I think that non constant delay functions should also be
addressed for the long term since it is a issue/ problem that crops up over
and over again in the forums.

5) supporting delays using variables (non constants).


In my view this latest patch/solution, definitely solves #1, which was the
biggest issue. 
But it does not address #2, #3, or #5.

With respect to #2,
I would still argue that in the absence of being able to specify the rounding
style (up,down, closest) in the API, that any cycle rounding should always
round up rather than down.
This ensures that you always get a delay at least as long as you request,
which is what is needed when using these delays for hardware setup timing
delays.
Without a gurarantee of getting at least as long of delay as
requested, there is no way to use these delay functions
for hardware type delays and be guaranteed that you won't violate the
hardware timing.
So for me personally, better delay resolution but always rounding down would
not be enough for me to leave my existing _delay_us()/_delay_ns() solutions
and return back to using <util/delay.h> functions.
By always rounding down, they simply cannot solve my needs for 
creating minimum hardware setup timing independent of F_CPU
clock and I bet my needs are not unique.

Maybe there could be a #define that the user could set
to set the rounding style?

With respect to #3,
The code could easily be altered to do additional loops
to support longer delays.

With respect #5
I would highly recommend that we either add
a wrapper function to the library
delay_ms(ms) that calls _delay_ms(1) in a loop
or update the existing macros to detect non constant
arguments (which can be done) and then create a loop
so that delays using variables can be supported.

I think the easiest and cleanest 
would be simply create the new wrapper function
delay_ms() and require that the argument be an integer.

--------------------------------

I'll be happy to participate in anyway including writing
any code and corresponding documentation, but before any solution goes out,
I'd like to get some clarity and position on #2, #3, and #5.


--- bill



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?30363>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/