[U-Boot] [PATCH] net/net.c: add get_timer_ms()

[Wolfgang Denk]

Dear Daniel Mack,

In message <20081205144031.GB1821 at 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 at 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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I read part of it all the way through.