Hello Wolfgang,
Wolfgang Denk wrote:
> In message <498027a2.8060...@denx.de> you 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.
> ...
>> - act = getenv("ethact");
>> + if ((*act == 0) || (env_changed_id < get_env_id()))
>> + {
>> + act = getenv("ethact");
>> + env_changed_id = get_env_id();
>> + }
>
> Incorrect brace style.
Argh.
> Note that you are calling get_env_id() twice - in the test and then
> again. This can be avoided. Also, I recommend to use "!=" instead of
> "<".
OK, I change this.
>> --- a/net/net.c
>> +++ b/net/net.c
> ...
>> +int env_changed_id = 0;
>> +
>> void ArpRequest (void)
>> {
>> int i;
>> @@ -341,63 +343,67 @@ restart:
>> * packets and timer events.
>> */
>>
>> - switch (protocol) {
>> + /* update just, if the environment has changed */
>> + if (env_changed_id < get_env_id ()) {
>> + switch (protocol) {
>
> Please change the comment into something like "update only when the
> environment has changed" or so.
>
> Better use "!=" instead of "<".
OK.
>> #if defined(CONFIG_CMD_NFS)
>> - case NFS:
>> + case NFS:
>> #endif
>> #if defined(CONFIG_CMD_PING)
>> - case PING:
>> + case PING:
>> #endif
>> #if defined(CONFIG_CMD_SNTP)
>> - case SNTP:
>> + case SNTP:
>> #endif
>> - case NETCONS:
>> - case TFTP:
>> - NetCopyIP(&NetOurIP, &bd->bi_ip_addr);
>> - NetOurGatewayIP = getenv_IPaddr ("gatewayip");
>> - NetOurSubnetMask= getenv_IPaddr ("netmask");
>> - NetOurVLAN = getenv_VLAN("vlan");
>> - NetOurNativeVLAN = getenv_VLAN("nvlan");
>> + case NETCONS:
>> + case TFTP:
>> + NetCopyIP(&NetOurIP, &bd->bi_ip_addr);
>> + NetOurGatewayIP = getenv_IPaddr ("gatewayip");
>> + NetOurSubnetMask= getenv_IPaddr ("netmask");
>> + NetOurVLAN = getenv_VLAN("vlan");
>> + NetOurNativeVLAN = getenv_VLAN("nvlan");
>>
>> - switch (protocol) {
>> + switch (protocol) {
>> #if defined(CONFIG_CMD_NFS)
>> - case NFS:
>> + case NFS:
>> #endif
>> - case NETCONS:
>> - case TFTP:
>> - NetServerIP = getenv_IPaddr ("serverip");
>> - break;
>> + case NETCONS:
>> + case TFTP:
>> + NetServerIP = getenv_IPaddr ("serverip");
>> + break;
>> #if defined(CONFIG_CMD_PING)
>> - case PING:
>> - /* nothing */
>> - break;
>> + case PING:
>> + /* nothing */
>> + break;
>> #endif
>> #if defined(CONFIG_CMD_SNTP)
>> - case SNTP:
>> - /* nothing */
>> - break;
>> + case SNTP:
>> + /* nothing */
>> + break;
>> #endif
>> + default:
>> + break;
>> + }
>> +
>> + break;
>> + case BOOTP:
>> + case RARP:
>> + /*
>> + * initialize our IP addr to 0 in order to accept ANY
>> + * IP addr assigned to us by the BOOTP / RARP server
>> + */
>> + NetOurIP = 0;
>> + NetServerIP = getenv_IPaddr ("serverip");
>> + NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must
>> be read */
>> + NetOurNativeVLAN = getenv_VLAN("nvlan");
>> + case CDP:
>> + NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must
>> be read */
>> + NetOurNativeVLAN = getenv_VLAN("nvlan");
>> + break;
>> default:
>> break;
>> }
>> -
>> - break;
>> - case BOOTP:
>> - case RARP:
>> - /*
>> - * initialize our IP addr to 0 in order to accept ANY
>> - * IP addr assigned to us by the BOOTP / RARP server
>> - */
>> - NetOurIP = 0;
>> - NetServerIP = getenv_IPaddr ("serverip");
>> - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */
>> - NetOurNativeVLAN = getenv_VLAN("nvlan");
>> - case CDP:
>> - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */
>> - NetOurNativeVLAN = getenv_VLAN("nvlan");
>> - break;
>> - default:
>> - break;
>> + env_changed_id = get_env_id ();
>
> You are calling get_env_id() twice - avoid that.
OK, I try it.
> Instead of adding yet anothe rlevel of indentation to that switch I
> recommend to split it in a separate fuinction - it's big enough for
> that anyway.
OK, will change 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
