Re: [ECOS] delay_us()

[Nick Garnett <nickg at ecoscentric dot com>]

Motoya Kurotsu <kurotsu at allied-telesis dot co dot jp> writes:

> Hi, jifl;
> 
> > > 
> > > The delay_us() take cares the large number of delay without kernel, 
> > > calling hal_delay_us() -- at least in mips architecture -- but not do so 
> > > with kernel. I think that it should not vary how to take care the large number 
> > > of delay depending on the use of kernel. Considering that some ethernet 
> > > drivers actually use this function with the value of several seconds, 
> > 
> > That's unfortunate, but I see it's true, or at least I see 100ms max for 
> > most now anyway, which is still quite a lot. However the CF driver and the 
> > 82559 driver go up to 500ms and 2 *seconds* respectively. Do you have any 
> > other fixed examples of ethernet drivers and specific HAL platforms this 
> > has problems right now with?
> >
> 
> No, except the drivers which you mentioned above.
> 
> > > I think that delay_us() should take care of the large number of delay 
> > > as well with the kernel.
> > 
> > Hmm... really the drivers shouldn't be using such large values, and should 
> > instead loop with smaller values, i.e. for (i=0;i<10;i++) HAL_DELAY_US(10000);
> > 
> > > Please tell me how you think ahout it?
> >
> 
> Reconsidering my opinion, I agree with yours. Certainly the micro second 
> precision isn't always necessary when the value is large.
> 
> > Not sure about adding a dependency on a 64-bit type in hal common code.
> > 
> > I think I'd try and fix the problem by adding loops where required, and 
> > then impose an API limit on this by also adding an assertion in hal_if.c's 
> > delay_us() to barf if some maximum value is exceeded, perhaps 100000 us, 
> > i.e. 100ms.
> > 
> 
> I think that it is good idea to impose an API limit. It is my unfortunate 
> experience that it took several hours to find out the overflow in delay_us() 
> in my code.
> 

The HAL implementations of hal_delay_us() go to some trouble to ensure
that they can cope with any length of delay -- at least the ones I
wrote do. It would be trivially easy to fix the implementation in
hal_if.c to do the same. We certainly do not want to impose some
arbitrary limit on the API -- what value do we choose? It depends
entirely on the resolution of the harware clock where the overflow
might come. We also don't want to comb through all the sources looking
for usec delays that might exceed whatever arbitrary limit is imposed.

Frankly I am surprised that the hal_if.c implementation was not
written properly in the first place -- we were certainly well aware of
the timer overflow issues at the time. 

The algorithm needs to be rearranged to count in microseconds rather
than clock ticks, avoiding the potential overflows.

I'll look at preparing a patch to do this.


-- 
Nick Garnett                    eCos Kernel Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


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