Hi,
On Fri, Dec 05, 2008 at 09:26:27PM +0100, Wolfgang Denk wrote:
> > net/net.c | 15 ++++++++++-----
> > 1 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 77e83b5..1c48236 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -206,6 +206,11 @@ uchar NetArpWaitPacketBuf[PKTSIZE_ALIGN +
> > PKTALIGN];
> > ulong NetArpWaitTimerStart;
> > int NetArpWaitTry;
> >
> > +static long get_timer_ms(long base)
> > +{
> > + return get_timer(base) / (CONFIG_SYS_HZ / 1000);
> > +}
> > +
>
> This is by definition a NO-OP at best, and misleading and wrong
> otherwise. get_timer() is defined to return millisecond resolution,
> and CONFIG_SYS_HZ is supposed to be 1000.
The timer implementation (at least the one for PXA processors) assumes
that the OSCR register increments 1000 times a second. Which it doesn't
for PXA3xx variants. Hence, all functions from cpu/pxa/interrupts.c will
behave entirely differently on a PXA270 compared to a PXA3xx, and so all
code using this functions will break.
That's what I've experienced, and as I didn't find a proper place to fix
it at a sane level, I fixed the problem locally. I agree that this might
not be the best place, so I'll happily accept better proposals.
> So in a correct configuration get_timer_ms() is the same as
> get_timer(), and if CONFIG_SYS_HZ is (incorrectly) not set to 1000
Why is a CONFIG_SYS_HZ != 1000 considered incorrect? Or let me spin it
that way: if that's incorrect, what does this variable exist for at all?
> while get_timer() is implemented correctly, then get_timer_ms()
> willnot do what it claims to do.
What is get_timer() supposed to return, anyway? I didn't find any
documentation about it and assumed that it straightly returns the
primary system timer of the CPU (which it perfectly does for PXAs).
> Not to mention what happens if someone has CONFIG_SYS_HZ defined as
> 999, for example.
Not sure whether I got your point here. If the system timer increments
999 times per second and CONFIG_SYS_HZ is set accordingly, my function
does the right thing, doesn't it? I'm not up to any flamewar, I just
want to understand where you see the problem.
As fas as I understand the big picture, a function like mine should
exist somewhere in the code. Probably not in net/net.c, though.
Best regards,
Daniel
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
