diff mbox

[U-Boot] net: bugfix: NetSetTimeout assumes CONFIG_SYS_HZ=1000

Message ID 4FE85B87.6090302@kmckk.co.jp
State Accepted
Delegated to: Joe Hershberger
Headers show

Commit Message

Tetsuyuki Kobayashi June 25, 2012, 12:37 p.m. UTC 
NetSetTimeout sets incorrect value to timeDelta when CONFIG_SYS_HZ != 1000.

Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
---
 net/net.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Wolfgang Denk July 8, 2012, 7:58 p.m. UTC | #1 
Dear Tetsuyuki Kobayashi,

In message <4FE85B87.6090302@kmckk.co.jp> you wrote:
> NetSetTimeout sets incorrect value to timeDelta when CONFIG_SYS_HZ != 1000.

Your patch is OK per se, but please note that it is a mandatory
requirement that CONFIG_SYS_HZ == 1000.  So if you have any board
where this is not the case, this board needs to be fixed, too.

Best regards,

Wolfgang Denk
Tetsuyuki Kobayashi July 9, 2012, 2:42 a.m. UTC | #2 
Dear Wolfgang Denk,

(07/09/2012 04:58 AM), Wolfgang Denk wrote:
> Dear Tetsuyuki Kobayashi,
> 
> In message <4FE85B87.6090302@kmckk.co.jp> you wrote:
>> NetSetTimeout sets incorrect value to timeDelta when CONFIG_SYS_HZ != 1000.
> 
> Your patch is OK per se, but please note that it is a mandatory
> requirement that CONFIG_SYS_HZ == 1000.  So if you have any board
> where this is not the case, this board needs to be fixed, too.

Thank you for reviewing my patch.

I didn't know that it is a mandatory requirement that CONFIG_SYS_HZ == 1000.
I grep'ed for CONFIG_SYS_HZ in README and under doc/, but I could not find any description about it.

  $ grep -r CONFIG_SYS_HZ README doc/*
  doc/README.LED:(CONFIG_SYS_HZ / <N>) where <N> is the frequency of the blink. Typical values

And I couldn't find that what value should be return in ulong get_timer(ulong base);
I hope "how to implement system timer" documented.

Best regards,

Tetsuyuki Kobayashi
Joe Hershberger July 10, 2012, 6:24 p.m. UTC | #3 
Hi Wolfgang,

On Sun, Jul 8, 2012 at 2:58 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Tetsuyuki Kobayashi,
>
> In message <4FE85B87.6090302@kmckk.co.jp> you wrote:
>> NetSetTimeout sets incorrect value to timeDelta when CONFIG_SYS_HZ != 1000.
>
> Your patch is OK per se, but please note that it is a mandatory
> requirement that CONFIG_SYS_HZ == 1000.  So if you have any board
> where this is not the case, this board needs to be fixed, too.

If that is a hard requirement, then I don't see a point to applying
this patch.  I also don't know why the CONFIG_SYS_HZ variable exists.

Thanks,
-Joe
Joe Hershberger Sept. 28, 2012, 3:50 p.m. UTC | #4 
Hi Tetsuyuki,

On Mon, Jun 25, 2012 at 7:37 AM, Tetsuyuki Kobayashi <koba@kmckk.co.jp> wrote:
> NetSetTimeout sets incorrect value to timeDelta when CONFIG_SYS_HZ != 1000.
>
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---

I decided this doesn't hurt and it sort-of documents what "must be" (tm).

Applied, thanks.

-Joe
diff mbox

Patch

diff --git a/net/net.c b/net/net.c
index 9de7d92..ac9c2c8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -653,7 +653,7 @@  NetSetTimeout(ulong iv, thand_f *f)
 			"--- NetLoop timeout handler set (%p)\n", f);
 		timeHandler = f;
 		timeStart = get_timer(0);
-		timeDelta = iv;
+		timeDelta = iv * CONFIG_SYS_HZ / 1000;
 	}
 }