Hello Mike,
Mike Frysinger wrote:
> On Tuesday 10 February 2009 03:38:52 Heiko Schocher wrote:
>> NetLoop polls every cycle with getenv some environment variables.
>> This is horribly slow, especially when the environment is big.
>>
>> This patch reads only the environment variables in NetLoop,
>> when they were changed.
>>
>> Also moved the init part of the NetLoop function in a seperate
>> function.
>
> a bit late, but oh well ...
better late then never ;-)
>> --- a/common/cmd_nvedit.c
>> +++ b/common/cmd_nvedit.c
>> +static int env_id = 1;
>>
>> +int get_env_id (void)
>> +{
>> + return env_id;
>> +}
>
> there's no documentation anywhere in the source about these new functions.
> people have to read the changelog to figure out what the function is for and
> how it is used. at least to me, the usage is not conveyed in any way by the
> function name. a simple comment block right above these would go a long way.
Ok, I try to add a comment.
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -28,6 +28,9 @@
>>
>> #if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
>>
>> +static char *act = NULL;
>> +static int env_changed_id = 0;
>
> since these are only used by eth_set_current(), shouldnt they be in that
> function instead of file scope ? nothing else in eth.c uses them.
Yes, you are right.
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -209,6 +209,8 @@ uchar NetArpWaitPacketBuf[PKTSIZE_ALIGN +
>> PKTALIGN];
>> +int env_changed_id = 0;
>
> why isnt marked static ? and since only NetInitLoop() uses it, it should be
> in that function rather than file scope.
Yep.
>> +int
>> +NetInitLoop(proto_t protocol)
>> +{
>
> this function always returns 0, and the only caller is in this file, and it
> doesnt actually check the return value. so this function should be marked
> static and changed to void.
> -mike
OK, I sent a patch for this.
thanks
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
