From: Heiko Schocher <hs@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] netloop: speed up NetLoop
Date: Wed, 25 Feb 2009 08:46:36 +0100 [thread overview]
Message-ID: <49A4F75C.4010101@denx.de> (raw)
In-Reply-To: <200902241545.06473.vapier@gentoo.org>
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
next prev parent reply other threads:[~2009-02-25 7:46 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-10 8:38 [U-Boot] netloop: speed up NetLoop Heiko Schocher
2009-02-21 21:42 ` Wolfgang Denk
2009-02-23 7:53 ` Ben Warren
2009-02-24 20:45 ` Mike Frysinger
2009-02-25 7:46 ` Heiko Schocher [this message]
2009-02-24 20:54 ` Mike Frysinger
2009-02-25 7:46 ` Heiko Schocher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49A4F75C.4010101@denx.de \
--to=hs@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox