Patchwork [U-Boot] lsxl: set ncip to broadcast address

login
register
mail settings
Submitter Michael Walle
Date Oct. 3, 2012, 3:44 p.m.
Message ID <1349279076-17807-1-git-send-email-michael@walle.cc>
Download mbox | patch
Permalink /patch/188814/
State Superseded
Headers show

Comments

Michael Walle - Oct. 3, 2012, 3:44 p.m.
Instead of using the serverip we get from the DHCP server, use the
broadcast address. That way it isn't necessary to use a special DHCP
configuration to set the netconsole peer.

Signed-off-by: Michael Walle <michael@walle.cc>
Cc: Prafulla Wadaskar <prafulla@marvell.com>
---

Hi Prafulla,

although it isn't a bugfix, it would be nice, if this patch made it into
the 2012.10 release.

 include/configs/lsxl.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Prafulla Wadaskar - Oct. 4, 2012, 10:47 a.m.
> -----Original Message-----
> From: Michael Walle [mailto:michael@walle.cc]
> Sent: 03 October 2012 21:15
> To: u-boot@lists.denx.de
> Cc: Michael Walle; Prafulla Wadaskar
> Subject: [PATCH] lsxl: set ncip to broadcast address
> 
> Instead of using the serverip we get from the DHCP server, use the
> broadcast address. That way it isn't necessary to use a special DHCP
> configuration to set the netconsole peer.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Cc: Prafulla Wadaskar <prafulla@marvell.com>
> ---
> 
> Hi Prafulla,
> 
> although it isn't a bugfix, it would be nice, if this patch made it
> into
> the 2012.10 release.
> 
>  include/configs/lsxl.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/configs/lsxl.h b/include/configs/lsxl.h
> index 0db559c..663c5e2 100644
> --- a/include/configs/lsxl.h
> +++ b/include/configs/lsxl.h
> @@ -146,7 +146,7 @@
>  	"config_nc_dhcp=setenv autoload_old ${autoload}; "		\
>  		"setenv autoload no "					\
>  		"&& bootp "						\
> -		"&& setenv ncip ${serverip} "				\
> +		"&& setenv ncip 255.255.255.255 "			\

Michael
Hard coding an ip-address is not recommended in u-boot environment.
I would need ack from Joe and Wolfgang to pull this patch.

Regards...
Prafulla . . .
Michael Walle - Oct. 4, 2012, 11:14 a.m.
On Thu, October 4, 2012 12:47, Prafulla Wadaskar wrote:
>> -----Original Message-----
>> From: Michael Walle [mailto:michael@walle.cc]
>> Sent: 03 October 2012 21:15
>> To: u-boot@lists.denx.de
>> Cc: Michael Walle; Prafulla Wadaskar
>> Subject: [PATCH] lsxl: set ncip to broadcast address
>>
>> Instead of using the serverip we get from the DHCP server, use the
>> broadcast address. That way it isn't necessary to use a special DHCP
>> configuration to set the netconsole peer.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> Cc: Prafulla Wadaskar <prafulla@marvell.com>
>> ---
>>
>> Hi Prafulla,
>>
>> although it isn't a bugfix, it would be nice, if this patch made it
>> into
>> the 2012.10 release.
>>
>>  include/configs/lsxl.h |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/configs/lsxl.h b/include/configs/lsxl.h
>> index 0db559c..663c5e2 100644
>> --- a/include/configs/lsxl.h
>> +++ b/include/configs/lsxl.h
>> @@ -146,7 +146,7 @@
>>  	"config_nc_dhcp=setenv autoload_old ${autoload}; "		\
>>  		"setenv autoload no "					\
>>  		"&& bootp "						\
>> -		"&& setenv ncip ${serverip} "				\
>> +		"&& setenv ncip 255.255.255.255 "			\
>
> Michael
> Hard coding an ip-address is not recommended in u-boot environment.
> I would need ack from Joe and Wolfgang to pull this patch.

Hi Prafulla,

i wouldn't call 255.255.255.255 an IP address ;) i could actually unset
ncip. In that case the netconsole driver itself implicitly assumes the
255.255.255.255 broadcast address. I found my solution more transparent,
because you see the used target IP address at first sight.

But if you see the broadcast address as an hardcoded IP address, i'll
prepare another version of the patch with "setenv ncip" (unset) this
evening.
Prafulla Wadaskar - Oct. 4, 2012, 11:19 a.m.
> -----Original Message-----
> From: Michael Walle [mailto:michael@walle.cc]
> Sent: 04 October 2012 16:45
> To: Prafulla Wadaskar
> Cc: Michael Walle; u-boot@lists.denx.de; Joe Hershberger; Wolfgang
> Denk
> Subject: RE: [PATCH] lsxl: set ncip to broadcast address
> 
> On Thu, October 4, 2012 12:47, Prafulla Wadaskar wrote:
> >> -----Original Message-----
> >> From: Michael Walle [mailto:michael@walle.cc]
> >> Sent: 03 October 2012 21:15
> >> To: u-boot@lists.denx.de
> >> Cc: Michael Walle; Prafulla Wadaskar
> >> Subject: [PATCH] lsxl: set ncip to broadcast address
> >>
> >> Instead of using the serverip we get from the DHCP server, use the
> >> broadcast address. That way it isn't necessary to use a special
> DHCP
> >> configuration to set the netconsole peer.
> >>
> >> Signed-off-by: Michael Walle <michael@walle.cc>
> >> Cc: Prafulla Wadaskar <prafulla@marvell.com>
> >> ---
> >>
> >> Hi Prafulla,
> >>
> >> although it isn't a bugfix, it would be nice, if this patch made it
> >> into
> >> the 2012.10 release.
> >>
> >>  include/configs/lsxl.h |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/include/configs/lsxl.h b/include/configs/lsxl.h
> >> index 0db559c..663c5e2 100644
> >> --- a/include/configs/lsxl.h
> >> +++ b/include/configs/lsxl.h
> >> @@ -146,7 +146,7 @@
> >>  	"config_nc_dhcp=setenv autoload_old ${autoload}; "		\
> >>  		"setenv autoload no "					\
> >>  		"&& bootp "						\
> >> -		"&& setenv ncip ${serverip} "				\
> >> +		"&& setenv ncip 255.255.255.255 "			\
> >
> > Michael
> > Hard coding an ip-address is not recommended in u-boot environment.
> > I would need ack from Joe and Wolfgang to pull this patch.
> 
> Hi Prafulla,
> 
> i wouldn't call 255.255.255.255 an IP address ;) i could actually
> unset
> ncip. In that case the netconsole driver itself implicitly assumes the
> 255.255.255.255 broadcast address. I found my solution more
> transparent,
> because you see the used target IP address at first sight.
> 
> But if you see the broadcast address as an hardcoded IP address, i'll
> prepare another version of the patch with "setenv ncip" (unset) this
> evening.

I think that would be good :-)

Regards...
Prafulla . . .
Wolfgang Denk - Oct. 4, 2012, 12:57 p.m.
Dear Prafulla Wadaskar,

In message <F766E4F80769BD478052FB6533FA745D235D228460@SC-VEXCH4.marvell.com> you wrote:
> 
> > --- a/include/configs/lsxl.h
> > +++ b/include/configs/lsxl.h
> > @@ -146,7 +146,7 @@
> >  	"config_nc_dhcp=3Dsetenv autoload_old ${autoload}; "		\
> >  		"setenv autoload no "					\
> >  		"&& bootp "						\
> > -		"&& setenv ncip ${serverip} "				\
> > +		"&& setenv ncip 255.255.255.255 "			\
> 
> Michael
> Hard coding an ip-address is not recommended in u-boot environment.
> I would need ack from Joe and Wolfgang to pull this patch.

"not recommended" is an understatement.  "not permitted" is more
correct.

NAK.

Best regards,

Wolfgang Denk

Patch

diff --git a/include/configs/lsxl.h b/include/configs/lsxl.h
index 0db559c..663c5e2 100644
--- a/include/configs/lsxl.h
+++ b/include/configs/lsxl.h
@@ -146,7 +146,7 @@ 
 	"config_nc_dhcp=setenv autoload_old ${autoload}; "		\
 		"setenv autoload no "					\
 		"&& bootp "						\
-		"&& setenv ncip ${serverip} "				\
+		"&& setenv ncip 255.255.255.255 "			\
 		"&& setenv autoload ${autoload_old}; "			\
 		"setenv autoload_old\0"					\
 	"standard_env=setenv ipaddr; setenv netmask; setenv serverip; "	\