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 ...
> --- 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.
> --- 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.
> --- 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.
> +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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
