Hello Michael,
Michael Zaidman wrote:
> Hi Heiko,
>
> The patch "netloop: speed up NetLoop" you delivered
> into the u-boot-2009.03 introduced bug I have described
> below a few days ago. Could you please take a look at
> the proposed fix?
>
Sorry, seems I missed it.
> Thanks,
> Michael
>
> ---------- Forwarded message ----------
> From: Michael Zaidman <michael.zaid...@gmail.com>
> Date: Sun, Mar 22, 2009 at 8:35 PM
> Subject: [U-Boot] [PATCH] NetLoop initialization bug
> To: u-boot@lists.denx.de
>
>
> [U-Boot] [PATCH] NetLoop initialization bug
>
Thanks for catching this.
Your patch doesn;t apply, I get "malformed patch". Before
sending a new one, please have a look at my comments:
>
>
> Upon u-boot's start up the first ping command causes a failure of the
> consequent TFTP command. It happens in the recently added mechanism of
> the NetLoop initialization where initialization of global network parameters
> is
> separated in the NetInitLoop routine which is called per env_id change.
> Thus, ping request will initialize the network parameters necessary for ping
> operation only, afterwards the env_changed_id will be set to the env_id
> that will prevent all following initialization requests from other protocols.
> The problem is that the initialized by ping subset of network parameters is
> not
> sufficient for other protocols and particularly for TFTP
> which requires the NetServerIp also.
>
> Signed-off-by: michael.zaid...@gmail.com
>
> diff --git a/net/net.c b/net/net.c
> index a89f6a0..dc98d0f 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -288,64 +288,13 @@ NetInitLoop(proto_t protocol)
> if (env_changed_id == env_id)
> return 0;
>
> - switch (protocol) {
> -#if defined(CONFIG_CMD_NFS)
> - case NFS:
> -#endif
> -#if defined(CONFIG_CMD_PING)
> - case PING:
> -#endif
> -#if defined(CONFIG_CMD_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");
> + NetCopyIP(&NetOurIP, &bd->bi_ip_addr);
> + NetOurGatewayIP = getenv_IPaddr ("gatewayip");
> + NetOurSubnetMask= getenv_IPaddr ("netmask");
> + NetServerIP = getenv_IPaddr ("serverip");
>
The following 2 vars are just used, if CONFIG_CMD_CDP
is used, can we do a "#if defined" around it?
> + NetOurNativeVLAN = getenv_VLAN("nvlan");
> + NetOurVLAN = getenv_VLAN("vlan");
>
[...]
> @@ -443,18 +392,19 @@ restart:
> /* Start with a clean slate... */
> BootpTry = 0;
> NetOurIP = 0;
> - NetServerIP = getenv_IPaddr ("serverip");
> DhcpRequest(); /* Basically same as BOOTP */
> break;
> #endif
>
> case BOOTP:
> BootpTry = 0;
> + NetOurIP = 0;
>
why we need this here?
> BootpRequest ();
> break;
>
> case RARP:
> RarpTry = 0;
> + NetOurIP = 0;
>
also here?
Has this to do something with the bugfix?
bye
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
