Hi Michal,
On Wed, Aug 31, 2011 at 3:36 AM, Michal Simek <mon...@monstr.eu> wrote:
> Patch: "Put common autoload code into auto_load() function"
> (sha1: 093498669e77597635a24f326f11efeab213d394) is not simple code
> cleanup but code change which introduce new bug.
>
> If autoload variable is not setup it worked as autoload=yes.
>
> Currently if autoload is not setup dhcp sends request in
> forever loop.
Thanks for bring this up - there was some discussion at the time I
remember. Also please excuse the verbatim code in this email but I
want it to be clear.
The old code in v2011.03 is:
/* Obey the 'autoload' setting */
if ((s = getenv("autoload")) != NULL) {
if (*s == 'n') {
/*
* Just use BOOTP to configure system;
* Do not use TFTP to load the bootfile.
*/
NetState = NETLOOP_SUCCESS;
return;
#if defined(CONFIG_CMD_NFS)
} else if (strcmp(s, "NFS") == 0) {
/*
* Use NFS to load the bootfile.
*/
NfsStart();
return;
#endif
}
}
TftpStart();
return;
and
if ((s = getenv("autoload")) != NULL) {
if (*s == 'n') {
/*
* Just use BOOTP to configure system;
* Do not use TFTP to load the bootfile.
*/
NetState = NETLOOP_SUCCESS;
return;
#if defined(CONFIG_CMD_NFS)
} else if (strcmp(s, "NFS") == 0) {
/*
* Use NFS to load the bootfile.
*/
NfsStart();
return;
#endif
}
}
TftpStart();
These two sites were changed to a call to auto_load:
static void auto_load(void)
{
const char *s = getenv("autoload");
if (s != NULL) {
if (*s == 'n') {
/*
* Just use BOOTP to configure system;
* Do not use TFTP to load the bootfile.
*/
NetState = NETLOOP_SUCCESS;
return;
}
#if defined(CONFIG_CMD_NFS)
if (strcmp(s, "NFS") == 0) {
/*
* Use NFS to load the bootfile.
*/
NfsStart();
return;
}
#endif
TftpStart();
}
}
I don't see a change in the functionality here - I may be missing
something so please can you explain?
>From my experience U-Boot has always performed an autoload unless you
'setenv autoload n', even if the variable is not set. From the README:
autoload - if set to "no" (any string beginning with 'n'),
"bootp" will just load perform a lookup of the
configuration from the BOOTP server, but not try to
load any image using TFTP
which suggests that this is the correct behavior (i.e. it defaults to
autoload=yes)
>
> There are two options how to fix it:
> 1. Move TftpStart() which is in this patch
> 2. Change functionality if autoload is not setup, set NetSate and ends.
>
> @@ -165,7 +165,8 @@ static void auto_load(void)
> }
> #endif
> TftpStart();
> - }
> + } else
> + NetState = NETLOOP_SUCCESS;
> }
>
> CC: Eric Bénard <e...@eukrea.com>
> CC: Simon Glass <s...@chromium.org>
> Signed-off-by: Michal Simek <mon...@monstr.eu>
> ---
> net/bootp.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bootp.c b/net/bootp.c
> index 3db08ea..a003c42 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -164,8 +164,8 @@ static void auto_load(void)
> return;
> }
> #endif
> - TftpStart();
> }
> + TftpStart();
> }
Isn't this the opposite of the patch you want? I think you want to
move TftpStart() inside the loop, like this:
static void auto_load(void)
{
const char *s = getenv("autoload");
if (s != NULL) {
if (*s == 'n') {
/*
* Just use BOOTP to configure system;
* Do not use TFTP to load the bootfile.
*/
NetState = NETLOOP_SUCCESS;
return;
}
#if defined(CONFIG_CMD_NFS)
if (strcmp(s, "NFS") == 0) {
/*
* Use NFS to load the bootfile.
*/
NfsStart();
return;
TftpStart();
}
#endif
}
}
but we should first agree that this is the expected behavior, and
change the README.
Regards,
Simon
>
> #if !defined(CONFIG_CMD_DHCP)
> --
> 1.5.5.6
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
