[ECOS] MS_TICKS() revisited

[ECOS] MS_TICKS() revisited

[Grant Edwards]

I've modified my ms_ticks stuff so that there are two routines.
Both return the current system time, but one delays for a
millisecond and the other doesn't.

do_ms_tick()      // delays for 1ms, returns system time
get_ms_ticks()    // returns system time

#define MS_TICKS()  get_ms_ticks()
#define MS_TICKS_DELAY() -> do_ms_tick()

This saves a few milliseconds of overhead here and there
(nothing worth worrying about), but mostly it makes it clear
whether the caller wants to delay for a millisecond or just to
read the system time.

Attached is a patch against CVS sources for anybody who wants it.

I've only done limited testing, so it's possible I'm missing a
delay where I need one.  IOW, there may be an MS_TICKS() that
should be MS_TICKS_DELAY() -- but I don't think so.

NB: this patch also includes a fix to net_io_getc_nonblock() so
    that it always sets its return value properly.  AFAICT, the
    old version worked OK because whatever happened to be left
    in R0 when the function fell off the bottom always had a
    non-zero.

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] MS_TICKS() revisited

[Jonathan Larmour]

Grant Edwards wrote:
> 
> I've modified my ms_ticks stuff so that there are two routines.
> Both return the current system time, but one delays for a
> millisecond and the other doesn't.
> 
> do_ms_tick()      // delays for 1ms, returns system time
> get_ms_ticks()    // returns system time
> 
> #define MS_TICKS()  get_ms_ticks()
> #define MS_TICKS_DELAY() -> do_ms_tick()
> 
> This saves a few milliseconds of overhead here and there
> (nothing worth worrying about), but mostly it makes it clear
> whether the caller wants to delay for a millisecond or just to
> read the system time.
> 
> Attached is a patch against CVS sources for anybody who wants it.
> 
> I've only done limited testing, so it's possible I'm missing a
> delay where I need one.  IOW, there may be an MS_TICKS() that
> should be MS_TICKS_DELAY() -- but I don't think so.

Thanks for the patch. One query before I apply it:

+#if !defined(CYGPKG_REDBOOT_NETWORKING)
+#define MS_TICKS_DELAY() hal_delay_us(1000)
+#endif
+

Any reason this isn't do_ms_tick() ?

> NB: this patch also includes a fix to net_io_getc_nonblock() so
>     that it always sets its return value properly.  AFAICT, the
>     old version worked OK because whatever happened to be left
>     in R0 when the function fell off the bottom always had a
>     non-zero.

I've already fixed this in the repository, so Gary, if you get there before
I do, be warned :-).

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] MS_TICKS() revisited

[Grant Edwards]

On Fri, Feb 09, 2001 at 07:50:04PM +0000, Jonathan Larmour wrote:

> Thanks for the patch. One query before I apply it:
> 
> +#if !defined(CYGPKG_REDBOOT_NETWORKING)
> +#define MS_TICKS_DELAY() hal_delay_us(1000)
> +#endif
> +
> 
> Any reason this isn't do_ms_tick() ?

IIRC, I was thinking at the time that do_ms_tick() was part of
the networking package.  It isn't, so it probably should be
do_ms_tick().  Now that I think of it, it might be a better
idea to move the definitions for MS_TICKS() and
MS_TICKS_DELAY() from net.h into a more general spot like
redboot.h.  Then we don't need that chunk at all.

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] MS_TICKS() revisited

[Jonathan Larmour]

Grant Edwards wrote:
> 
> On Fri, Feb 09, 2001 at 07:50:04PM +0000, Jonathan Larmour wrote:
> 
> > Thanks for the patch. One query before I apply it:
> >
> > +#if !defined(CYGPKG_REDBOOT_NETWORKING)
> > +#define MS_TICKS_DELAY() hal_delay_us(1000)
> > +#endif
> > +
> >
> > Any reason this isn't do_ms_tick() ?
> 
> IIRC, I was thinking at the time that do_ms_tick() was part of
> the networking package.  It isn't, so it probably should be
> do_ms_tick().  Now that I think of it, it might be a better
> idea to move the definitions for MS_TICKS() and
> MS_TICKS_DELAY() from net.h into a more general spot like
> redboot.h.  Then we don't need that chunk at all.

I think that would be fine. If Gary T also agrees (CC'd) could you resubmit
your patch with that change?

Thanks!

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] MS_TICKS() revisited

[Grant Edwards]

On Fri, Feb 09, 2001 at 08:03:29PM +0000, Jonathan Larmour wrote:
> Grant Edwards wrote:
> > > Thanks for the patch. One query before I apply it:
> > >
> > > +#if !defined(CYGPKG_REDBOOT_NETWORKING)
> > > +#define MS_TICKS_DELAY() hal_delay_us(1000)
> > > +#endif
> > > +
> > >
> > > Any reason this isn't do_ms_tick() ?
> > 
> > IIRC, I was thinking at the time that do_ms_tick() was part of
> > the networking package.  It isn't, so it probably should be
> > do_ms_tick().  Now that I think of it, it might be a better
> > idea to move the definitions for MS_TICKS() and
> > MS_TICKS_DELAY() from net.h into a more general spot like
> > redboot.h.  Then we don't need that chunk at all.
> 
> I think that would be fine. If Gary T also agrees (CC'd) could you resubmit
> your patch with that change?

Uh, wait, that piece of pre-processor code is in a file
(net_io.c) that is part of the networking package, so we can
ditch it without moving the definitions out of net.h.

I've no idea why I thought that needed to be there.  :/

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] MS_TICKS() revisited

[Jonathan Larmour]

Grant Edwards wrote:
> 
> Uh, wait, that piece of pre-processor code is in a file
> (net_io.c) that is part of the networking package, so we can
> ditch it without moving the definitions out of net.h.

Ack.

I've checked your patch in with appropriate changes. It'll show up in the
next anoncvs snapshot. Thanks!

One teeny favour though: in future can you write a ChangeLog entry? It
saves time and means I don't misrepresent your change :).

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] MS_TICKS() revisited

[Grant Edwards]

On Fri, Feb 09, 2001 at 10:03:07PM +0000, Jonathan Larmour wrote:
> Grant Edwards wrote:
> > 
> > Uh, wait, that piece of pre-processor code is in a file
> > (net_io.c) that is part of the networking package, so we can
> > ditch it without moving the definitions out of net.h.
> 
> Ack.
> 
> I've checked your patch in with appropriate changes. It'll show up in the
> next anoncvs snapshot. Thanks!
> 
> One teeny favour though: in future can you write a ChangeLog entry? It
> saves time and means I don't misrepresent your change :).

Sure.  I should have thought of that.

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] MS_TICKS() revisited

[Gary Thomas]

On 09-Feb-2001 Jonathan Larmour wrote:
>> NB: this patch also includes a fix to net_io_getc_nonblock() so
>>     that it always sets its return value properly.  AFAICT, the
>>     old version worked OK because whatever happened to be left
>>     in R0 when the function fell off the bottom always had a
>>     non-zero.
> 
> I've already fixed this in the repository, so Gary, if you get there before
> I do, be warned :-).
> 

I think there are some uses of 'MS_TICKS()' [the old one] that you've
missed.  In particular, in the xyzModem code - this gets used to allow
time to pass.  Also, ping, and some others I'm sure.

When I get a chance, I'll verify these changes.

[ECOS] Re: MS_TICKS() revisited

[Grant Edwards]

Gary Thomas writes:

> I think there are some uses of 'MS_TICKS()' [the old one] that you've
> missed.  In particular, in the xyzModem code - this gets used to allow
> time to pass.  Also, ping, and some others I'm sure.

In my snapshot of RedBoot (probably a month or two old now), the only
files that contain uses of MS_TICK\x13() are in the sorc/net directory:
arp.c, bootp.c, timers.c, udp.c and net_io.c.  The xyzModem stuff
doesn't, and I'm not sure what "ping" is (in regards to RedBoot, that
is). 

-- 
Grant Edwards
grante@visi.com

[ECOS] Re: MS_TICKS() revisited

[Gary Thomas]

On 12-Feb-2001 Grant Edwards wrote:
> 
> Gary Thomas writes:
> 
>> I think there are some uses of 'MS_TICKS()' [the old one] that you've
>> missed.  In particular, in the xyzModem code - this gets used to allow
>> time to pass.  Also, ping, and some others I'm sure.
> 
> In my snapshot of RedBoot (probably a month or two old now), the only
> files that contain uses of MS_TICK\x13() are in the sorc/net directory:
> arp.c, bootp.c, timers.c, udp.c and net_io.c.  The xyzModem stuff
> doesn't, and I'm not sure what "ping" is (in regards to RedBoot, that
> is). 

'ping' is a new RedBoot command I added - more or less the functionality
of the desktop program used to check connectivity between network devices.

If your snapshot is more than two weeks old, you won't have it.

Re: [ECOS] Re: MS_TICKS() revisited

[Jonathan Larmour]

Gary Thomas wrote:
> 
> 'ping' is a new RedBoot command I added - more or less the functionality
> of the desktop program used to check connectivity between network devices.
> 
> If your snapshot is more than two weeks old, you won't have it.

It's probably not in anoncvs yet. Because of meetings, I've not had a
chance to update it. I'll see if I can today.

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine