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