Dear Daniel Mack,
In message 20081205144031.GB1821@buzzloop.caiaq.de you wrote:
Make timeout implementation in net/net.c take into account the CONFIG_SYS_HZ variable. This is needed for all CPUs where the default timer is running on anything else than 1000.
Signed-off-by: Daniel Mack daniel@caiaq.de
I reject this patch.
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.
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 while get_timer() is implemented correctly, then get_timer_ms() willnot do what it claims to do.
Not to mention what happens if someone has CONFIG_SYS_HZ defined as 999, for example.
The whole aproach is broken - instead of coating bugs we should fix the cause of the bugs.
NAK.
Best regards,
Wolfgang Denk