[U-Boot] [PATCH] netloop: updates for NetLoop

Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common/cmd_nvedit.c - moved some variables in fn scope instead of file scope - NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de --- common/cmd_nvedit.c | 6 ++++++ net/eth.c | 5 ++--- net/net.c | 9 ++++----- 3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 95eebb5..3caa841 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -75,6 +75,12 @@ DECLARE_GLOBAL_DATA_PTR; static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; #define N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+/* + * This variable is incremented on each do_setenv (), so it can + * be used as an indication, if the environment has changed or not. + * So it is possible to reread an environment variable only if the + * environment was changed ... done so for example in NetInitLoop() + */ static int env_id = 1;
int get_env_id (void) diff --git a/net/eth.c b/net/eth.c index 4bbf84b..a494912 100644 --- a/net/eth.c +++ b/net/eth.c @@ -57,9 +57,6 @@ int eth_setenv_enetaddr(char *name, const uchar *enetaddr)
#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
-static char *act = NULL; -static int env_changed_id = 0; - /* * CPU and board-specific Ethernet initializations. Aliased function * signals caller to move on @@ -471,6 +468,8 @@ void eth_try_another(int first_restart) #ifdef CONFIG_NET_MULTI void eth_set_current(void) { + static char *act = NULL; + static int env_changed_id = 0; struct eth_device* old_current; int env_id;
diff --git a/net/net.c b/net/net.c index a89f6a0..a80c814 100644 --- a/net/net.c +++ b/net/net.c @@ -209,8 +209,6 @@ uchar NetArpWaitPacketBuf[PKTSIZE_ALIGN + PKTALIGN]; ulong NetArpWaitTimerStart; int NetArpWaitTry;
-int env_changed_id = 0; - void ArpRequest (void) { int i; @@ -278,15 +276,16 @@ void ArpTimeoutCheck(void) } }
-int +static void NetInitLoop(proto_t protocol) { + static int env_changed_id = 0; bd_t *bd = gd->bd; int env_id = get_env_id ();
/* update only when the environment has changed */ if (env_changed_id == env_id) - return 0; + return;
switch (protocol) { #if defined(CONFIG_CMD_NFS) @@ -347,7 +346,7 @@ NetInitLoop(proto_t protocol) break; } env_changed_id = env_id; - return 0; + return; }
/**********************************************************************/

On Wednesday 25 February 2009 07:40:45 Heiko Schocher wrote:
+/*
- This variable is incremented on each do_setenv (), so it can
- be used as an indication, if the environment has changed or not.
- So it is possible to reread an environment variable only if the
- environment was changed ... done so for example in NetInitLoop()
- */
i would tweak it to say "... it can be used via get_env_id() as an ..."
- return 0;
- return;
}
a return at the end of a function is pointless imo. just delete it. -mike

[PATCH v2] netloop: updates for NetLoop
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c - moved some variables in fn scope instead of file scope - NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de --- changes since v1: - added comments from Mike Frysinger
common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 8 +++----- 3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 95eebb5..30e296c 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -75,6 +75,13 @@ DECLARE_GLOBAL_DATA_PTR; static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; #define N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+/* + * This variable is incremented on each do_setenv (), so it can + * be used via get_env_id() as an indication, if the environment + * has changed or not. So it is possible to reread an environment + * variable only if the environment was changed ... done so for + * example in NetInitLoop() + */ static int env_id = 1;
int get_env_id (void) diff --git a/net/eth.c b/net/eth.c index 4bbf84b..a494912 100644 --- a/net/eth.c +++ b/net/eth.c @@ -57,9 +57,6 @@ int eth_setenv_enetaddr(char *name, const uchar *enetaddr)
#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
-static char *act = NULL; -static int env_changed_id = 0; - /* * CPU and board-specific Ethernet initializations. Aliased function * signals caller to move on @@ -471,6 +468,8 @@ void eth_try_another(int first_restart) #ifdef CONFIG_NET_MULTI void eth_set_current(void) { + static char *act = NULL; + static int env_changed_id = 0; struct eth_device* old_current; int env_id;
diff --git a/net/net.c b/net/net.c index a89f6a0..0887f6e 100644 --- a/net/net.c +++ b/net/net.c @@ -209,8 +209,6 @@ uchar NetArpWaitPacketBuf[PKTSIZE_ALIGN + PKTALIGN]; ulong NetArpWaitTimerStart; int NetArpWaitTry;
-int env_changed_id = 0; - void ArpRequest (void) { int i; @@ -278,15 +276,16 @@ void ArpTimeoutCheck(void) } }
-int +static void NetInitLoop(proto_t protocol) { + static int env_changed_id = 0; bd_t *bd = gd->bd; int env_id = get_env_id ();
/* update only when the environment has changed */ if (env_changed_id == env_id) - return 0; + return;
switch (protocol) { #if defined(CONFIG_CMD_NFS) @@ -347,7 +346,6 @@ NetInitLoop(proto_t protocol) break; } env_changed_id = env_id; - return 0; }
/**********************************************************************/

On Thursday 26 February 2009 02:16:52 Heiko Schocher wrote:
[PATCH v2] netloop: updates for NetLoop
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
Acked-by: Mike Frysinger vapier@gentoo.org -mike

Dear Ben,
In message 49A641E4.8000001@denx.de you wrote:
[PATCH v2] netloop: updates for NetLoop
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
changes since v1:
- added comments from Mike Frysinger
common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 8 +++----- 3 files changed, 12 insertions(+), 8 deletions(-)
Except for an Ack from Mike I did not see any comments?
Ditto for this one:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/55341
Best regards,
Wolfgang Denk

Dear Wolfgang & Ben,
Ditto for this one:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/55341
The patch content is obsolete due to the "NetLoop initialization bug" fix.
Regards, Michael

Dear Ben,
In message 20090403215728.930BE83797DC@gemini.denx.de I wrote:
Dear Ben,
In message 49A641E4.8000001@denx.de you wrote:
[PATCH v2] netloop: updates for NetLoop
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
changes since v1:
- added comments from Mike Frysinger
common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 8 +++----- 3 files changed, 12 insertions(+), 8 deletions(-)
Except for an Ack from Mike I did not see any comments?
Ditto for this one:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/55341
Please comment -- if you want, I can apply directly.
Best regards,
Wolfgang Denk

Hi Wolfgang,
Wolfgang Denk wrote:
Dear Ben,
In message 20090403215728.930BE83797DC@gemini.denx.de I wrote:
Dear Ben,
In message 49A641E4.8000001@denx.de you wrote:
[PATCH v2] netloop: updates for NetLoop
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
changes since v1:
- added comments from Mike Frysinger
common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 8 +++----- 3 files changed, 12 insertions(+), 8 deletions(-)
Except for an Ack from Mike I did not see any comments?
Ditto for this one:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/55341
Please comment -- if you want, I can apply directly.
Best regards,
Wolfgang Denk
Sorry, thought I was caught up. This looks good - please apply directly.
thanks, Ben

Dear Heiko Schocher,
In message 49A641E4.8000001@denx.de you wrote:
[PATCH v2] netloop: updates for NetLoop
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
changes since v1:
- added comments from Mike Frysinger
I'm sorry, but this does not apply any more. Can you please rebase it?
Thanks.
Best regards,
Wolfgang Denk

Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c - moved some variables in fn scope instead of file scope - NetInitLoop now static void
Signed-off-by: Heiko Schocher <hs at denx.de> --- changes since v1: - added comments from Mike Frysinger
changes since v2: - rebased against current head, commit 28afe0160f87ff74574150d703055a965f91422a --- common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 7 +++---- 3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 163765a..3ee971a 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -77,6 +77,13 @@ SPI_FLASH|MG_DISK|NVRAM|NOWHERE} static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; #define N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+/* + * This variable is incremented on each do_setenv (), so it can + * be used via get_env_id() as an indication, if the environment + * has changed or not. So it is possible to reread an environment + * variable only if the environment was changed ... done so for + * example in NetInitLoop() + */ static int env_id = 1;
int get_env_id (void) diff --git a/net/eth.c b/net/eth.c index c6fa5b9..8940ebf 100644 --- a/net/eth.c +++ b/net/eth.c @@ -57,9 +57,6 @@ int eth_setenv_enetaddr(char *name, const uchar *enetaddr)
#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
-static char *act = NULL; -static int env_changed_id = 0; - /* * CPU and board-specific Ethernet initializations. Aliased function * signals caller to move on @@ -471,6 +468,8 @@ void eth_try_another(int first_restart) #ifdef CONFIG_NET_MULTI void eth_set_current(void) { + static char *act = NULL; + static int env_changed_id = 0; struct eth_device* old_current; int env_id;
diff --git a/net/net.c b/net/net.c index b8648bd..5637cf5 100644 --- a/net/net.c +++ b/net/net.c @@ -209,8 +209,6 @@ uchar NetArpWaitPacketBuf[PKTSIZE_ALIGN + PKTALIGN]; ulong NetArpWaitTimerStart; int NetArpWaitTry;
-int env_changed_id = 0; - void ArpRequest (void) { int i; @@ -278,9 +276,10 @@ void ArpTimeoutCheck(void) } }
-int +static void NetInitLoop(proto_t protocol) { + static int env_changed_id = 0; bd_t *bd = gd->bd; int env_id = get_env_id ();
@@ -295,7 +294,7 @@ NetInitLoop(proto_t protocol) env_changed_id = env_id; }
- return 0; + return; }
/**********************************************************************/

Hi Heiko,
Heiko Schocher wrote:
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher <hs at denx.de>
Please put your real e-mail address here.
Also, can you please verify that everything still works fine when this patch is applied to net/next? It has Michael Zaidman's "NetLoop initialization bug" fix applied.
thanks, Ben

Hello Ben,
Ben Warren wrote:
Heiko Schocher wrote:
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher <hs at denx.de>
Please put your real e-mail address here.
Ups, sorry, send a new one ...
Also, can you please verify that everything still works fine when this patch is applied to net/next? It has Michael Zaidman's "NetLoop initialization bug" fix applied.
This patch from Michael is also in actual u-boot.git, so I tested my patch with actual u-boot.git, on a mpc8xx, mpc82xx and on a mpc83xx based board, without showing the bug which Michael's patch fixed, so it seems to work fine.
bye Heiko

Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c - moved some variables in fn scope instead of file scope - NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de --- changes since v1: - added comments from Mike Frysinger
changes since v2: - rebased against current head, commit 28afe0160f87ff74574150d703055a965f91422a
changes since v3: - corrected my EMail address, as Ben Warren suggested.
common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 7 +++---- 3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 163765a..3ee971a 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -77,6 +77,13 @@ SPI_FLASH|MG_DISK|NVRAM|NOWHERE} static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; #define N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+/* + * This variable is incremented on each do_setenv (), so it can + * be used via get_env_id() as an indication, if the environment + * has changed or not. So it is possible to reread an environment + * variable only if the environment was changed ... done so for + * example in NetInitLoop() + */ static int env_id = 1;
int get_env_id (void) diff --git a/net/eth.c b/net/eth.c index c6fa5b9..8940ebf 100644 --- a/net/eth.c +++ b/net/eth.c @@ -57,9 +57,6 @@ int eth_setenv_enetaddr(char *name, const uchar *enetaddr)
#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
-static char *act = NULL; -static int env_changed_id = 0; - /* * CPU and board-specific Ethernet initializations. Aliased function * signals caller to move on @@ -471,6 +468,8 @@ void eth_try_another(int first_restart) #ifdef CONFIG_NET_MULTI void eth_set_current(void) { + static char *act = NULL; + static int env_changed_id = 0; struct eth_device* old_current; int env_id;
diff --git a/net/net.c b/net/net.c index b8648bd..5637cf5 100644 --- a/net/net.c +++ b/net/net.c @@ -209,8 +209,6 @@ uchar NetArpWaitPacketBuf[PKTSIZE_ALIGN + PKTALIGN]; ulong NetArpWaitTimerStart; int NetArpWaitTry;
-int env_changed_id = 0; - void ArpRequest (void) { int i; @@ -278,9 +276,10 @@ void ArpTimeoutCheck(void) } }
-int +static void NetInitLoop(proto_t protocol) { + static int env_changed_id = 0; bd_t *bd = gd->bd; int env_id = get_env_id ();
@@ -295,7 +294,7 @@ NetInitLoop(proto_t protocol) env_changed_id = env_id; }
- return 0; + return; }
/**********************************************************************/

Wolfgang,
Heiko Schocher wrote:
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
Acked-by: Ben Warren biggerbadderben@gmail.com
changes since v1:
- added comments from Mike Frysinger
changes since v2:
- rebased against current head, commit 28afe0160f87ff74574150d703055a965f91422a
changes since v3:
- corrected my EMail address, as Ben Warren suggested.
common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 7 +++---- 3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 163765a..3ee971a 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -77,6 +77,13 @@ SPI_FLASH|MG_DISK|NVRAM|NOWHERE} static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; #define N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+/*
- This variable is incremented on each do_setenv (), so it can
- be used via get_env_id() as an indication, if the environment
- has changed or not. So it is possible to reread an environment
- variable only if the environment was changed ... done so for
- example in NetInitLoop()
- */
static int env_id = 1;
int get_env_id (void) diff --git a/net/eth.c b/net/eth.c index c6fa5b9..8940ebf 100644 --- a/net/eth.c +++ b/net/eth.c @@ -57,9 +57,6 @@ int eth_setenv_enetaddr(char *name, const uchar *enetaddr)
#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
-static char *act = NULL; -static int env_changed_id = 0;
/*
- CPU and board-specific Ethernet initializations. Aliased function
- signals caller to move on
@@ -471,6 +468,8 @@ void eth_try_another(int first_restart) #ifdef CONFIG_NET_MULTI void eth_set_current(void) {
- static char *act = NULL;
- static int env_changed_id = 0; struct eth_device* old_current; int env_id;
diff --git a/net/net.c b/net/net.c index b8648bd..5637cf5 100644 --- a/net/net.c +++ b/net/net.c @@ -209,8 +209,6 @@ uchar NetArpWaitPacketBuf[PKTSIZE_ALIGN + PKTALIGN]; ulong NetArpWaitTimerStart; int NetArpWaitTry;
-int env_changed_id = 0;
void ArpRequest (void) { int i; @@ -278,9 +276,10 @@ void ArpTimeoutCheck(void) } }
-int +static void NetInitLoop(proto_t protocol) {
- static int env_changed_id = 0; bd_t *bd = gd->bd; int env_id = get_env_id ();
@@ -295,7 +294,7 @@ NetInitLoop(proto_t protocol) env_changed_id = env_id; }
- return 0;
- return;
}
/**********************************************************************/
Please apply directly.
regards, Ben

Dear Ben Warren,
In message 49F73B55.50603@gmail.com you wrote:
Heiko Schocher wrote:
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
Acked-by: Ben Warren biggerbadderben@gmail.com
...
Please apply directly.
Done, thanks.
Best regards,
Wolfgang Denk

Dear Heiko Schocher,
In message 49F6A3DB.2020907@denx.de you wrote:
Fix some issues introduced from commit: 2f70c49e5b9813635ad73666aa30f304c7fdeda9 suggested by Mike Frysinger.
- added some comment for the env_id variable in common_cmd_nvedit.c
- moved some variables in fn scope instead of file scope
- NetInitLoop now static void
Signed-off-by: Heiko Schocher hs@denx.de
changes since v1:
- added comments from Mike Frysinger
changes since v2:
- rebased against current head, commit 28afe0160f87ff74574150d703055a965f91422a
changes since v3:
- corrected my EMail address, as Ben Warren suggested.
common/cmd_nvedit.c | 7 +++++++ net/eth.c | 5 ++--- net/net.c | 7 +++---- 3 files changed, 12 insertions(+), 7 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (5)
-
Ben Warren
-
Heiko Schocher
-
Michael Zaidman
-
Mike Frysinger
-
Wolfgang Denk