All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set
@ 2011-09-19  7:54 Peter Korsgaard
  2011-09-19 13:47 ` Fabio Estevam
  2011-09-19 21:25 ` Wolfgang Denk
  0 siblings, 2 replies; 6+ messages in thread
From: Peter Korsgaard @ 2011-09-19  7:54 UTC (permalink / raw)
  To: u-boot

Commit 093498669 (Put common autoload code into auto_load() function)
broke handling of autoload environment variable not being set.
The bootp/dhcp code will just keep on requesting IP address forever
and never start TFTP download.

Fix it by moving TftpStart() outside the conditional like it was before.

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 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();
 }
 
 #if !defined(CONFIG_CMD_DHCP)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set
  2011-09-19  7:54 [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set Peter Korsgaard
@ 2011-09-19 13:47 ` Fabio Estevam
  2011-09-19 15:02   ` Simon Glass
  2011-09-19 21:25 ` Wolfgang Denk
  1 sibling, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2011-09-19 13:47 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 19, 2011 at 4:54 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> Commit 093498669 (Put common autoload code into auto_load() function)
> broke handling of autoload environment variable not being set.
> The bootp/dhcp code will just keep on requesting IP address forever
> and never start TFTP download.
>
> Fix it by moving TftpStart() outside the conditional like it was before.
>
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>

This makes mx31pdk networking to work again at my company's network.

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

Thanks,

Fabio Estevam

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set
  2011-09-19 13:47 ` Fabio Estevam
@ 2011-09-19 15:02   ` Simon Glass
  2011-09-19 15:29     ` Peter Korsgaard
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2011-09-19 15:02 UTC (permalink / raw)
  To: u-boot

Hi Peter,

On Mon, Sep 19, 2011 at 6:47 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Sep 19, 2011 at 4:54 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>> Commit 093498669 (Put common autoload code into auto_load() function)
>> broke handling of autoload environment variable not being set.
>> The bootp/dhcp code will just keep on requesting IP address forever
>> and never start TFTP download.
>>
>> Fix it by moving TftpStart() outside the conditional like it was before.
>>
>> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>
> This makes mx31pdk networking to work again at my company's network.
>
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

I think we just had this discussion at the end of August. It would be
good to get to the bottom of why this commit is considered a change of
behaviour - it is something subtle that I cannot see.

But first: if autoload is not defined, it is supposed to default to
'y'. If you want it to be 'n' then you need to set it. It that what
you expect?

Regards,
Simon

>
> Thanks,
>
> Fabio Estevam
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set
  2011-09-19 15:02   ` Simon Glass
@ 2011-09-19 15:29     ` Peter Korsgaard
  2011-09-19 15:48       ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2011-09-19 15:29 UTC (permalink / raw)
  To: u-boot

>>>>> "Simon" == Simon Glass <sjg@chromium.org> writes:

Hi,

 Simon> I think we just had this discussion at the end of August. It
 Simon> would be good to get to the bottom of why this commit is
 Simon> considered a change of behaviour - it is something subtle that I
 Simon> cannot see.

Because with that commit it no longer calls TftpStart() if the autoload
variable isn't set (it moved into the if (getenv..) conditional).

 Simon> But first: if autoload is not defined, it is supposed to default to
 Simon> 'y'. If you want it to be 'n' then you need to set it. It that what
 Simon> you expect?

Yes, no autoload should behave like autoload=y, E.G. call TftpStart().

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set
  2011-09-19 15:29     ` Peter Korsgaard
@ 2011-09-19 15:48       ` Simon Glass
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2011-09-19 15:48 UTC (permalink / raw)
  To: u-boot

Hi Peter,

On Mon, Sep 19, 2011 at 8:29 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>>>>>> "Simon" == Simon Glass <sjg@chromium.org> writes:
>
> Hi,
>
> ?Simon> I think we just had this discussion at the end of August. It
> ?Simon> would be good to get to the bottom of why this commit is
> ?Simon> considered a change of behaviour - it is something subtle that I
> ?Simon> cannot see.
>
> Because with that commit it no longer calls TftpStart() if the autoload
> variable isn't set (it moved into the if (getenv..) conditional).

OK I see thank you.

>
> ?Simon> But first: if autoload is not defined, it is supposed to default to
> ?Simon> 'y'. If you want it to be 'n' then you need to set it. It that what
> ?Simon> you expect?
>
> Yes, no autoload should behave like autoload=y, E.G. call TftpStart().

Acked-by: Simon Glass <sjg@chromium.org>

Regards,
Simon
>
> --
> Bye, Peter Korsgaard
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set
  2011-09-19  7:54 [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set Peter Korsgaard
  2011-09-19 13:47 ` Fabio Estevam
@ 2011-09-19 21:25 ` Wolfgang Denk
  1 sibling, 0 replies; 6+ messages in thread
From: Wolfgang Denk @ 2011-09-19 21:25 UTC (permalink / raw)
  To: u-boot

Dear Peter Korsgaard,

In message <1316418886-13495-1-git-send-email-jacmet@sunsite.dk> you wrote:
> Commit 093498669 (Put common autoload code into auto_load() function)
> broke handling of autoload environment variable not being set.
> The bootp/dhcp code will just keep on requesting IP address forever
> and never start TFTP download.
> 
> Fix it by moving TftpStart() outside the conditional like it was before.
> 
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> ---
>  net/bootp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Real computer scientists despise the idea of actual  hardware.  Hard-
ware has limitations, software doesn't. It's a real shame that Turing
machines are so poor at I/O.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-09-19 21:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-19  7:54 [U-Boot] [PATCH] net/bootp.c: fix tftp load if autoload environment var isn't set Peter Korsgaard
2011-09-19 13:47 ` Fabio Estevam
2011-09-19 15:02   ` Simon Glass
2011-09-19 15:29     ` Peter Korsgaard
2011-09-19 15:48       ` Simon Glass
2011-09-19 21:25 ` Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.