From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Mack Date: Fri, 5 Dec 2008 21:43:57 +0100 Subject: [U-Boot] [PATCH] net/net.c: add get_timer_ms() In-Reply-To: <20081205202627.9031F834B020@gemini.denx.de> References: <20081128162529.GA14044@buzzloop.caiaq.de> <20081203005808.GD2012@buzzloop.caiaq.de> <4938CECF.6090901@gmail.com> <20081205144031.GB1821@buzzloop.caiaq.de> <20081205202627.9031F834B020@gemini.denx.de> Message-ID: <20081205204357.GD3940@buzzloop.caiaq.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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