diff mbox

[U-Boot] ARM: Add support for displaying second ethaddr in 'bdinfo' command

Message ID 1301433395-25203-1-git-send-email-gryrmln@gmail.com
State Not Applicable
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Gray Remlin March 29, 2011, 9:16 p.m. UTC
Signed-off-by: Gray Remlin <gryrmln@gmail.com>
---
 common/cmd_bdinfo.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Wolfgang Denk March 29, 2011, 9:49 p.m. UTC | #1
Dear Gray Remlin,

In message <1301433395-25203-1-git-send-email-gryrmln@gmail.com> you wrote:
> Signed-off-by: Gray Remlin <gryrmln@gmail.com>
> ---
>  common/cmd_bdinfo.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Why limit this to eth1addr?  What's the chances that ARM systems may
have more than 2 network interfaces?

Best regards,

Wolfgang Denk
Gray Remlin March 29, 2011, 10:36 p.m. UTC | #2
On 03/29/2011 10:49 PM, Wolfgang Denk wrote:
> Dear Gray Remlin,
>
> In message<1301433395-25203-1-git-send-email-gryrmln@gmail.com>  you wrote:
>> Signed-off-by: Gray Remlin<gryrmln@gmail.com>
>> ---
>>   common/cmd_bdinfo.c |    3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
> Why limit this to eth1addr?  What's the chances that ARM systems may
> have more than 2 network interfaces?
>
> Best regards,
>
> Wolfgang Denk
>
Good question, I have already asked myself this, so I have stuck with 
what I do know.

1. It has had the one ethaddr limit for a (relatively) long time, which 
it seems no-one else 'required'\'submitted a patch' to change it.
2. I only know (with my very limited knowledge) of ARM boards with a 
maximum of two interfaces as standard.
3. Why limit it to six (as in other parts of the source) ?

My answer:
It is not my place to dictate policy, that is the role of the Project 
Manager.
And no, that is not a 'cop-out', it is the only way to avoid anarchy.
Albert ARIBAUD March 30, 2011, 11:34 a.m. UTC | #3
Le 30/03/2011 00:36, Gray Remlin a écrit :
> On 03/29/2011 10:49 PM, Wolfgang Denk wrote:
>> Dear Gray Remlin,
>>
>> In message<1301433395-25203-1-git-send-email-gryrmln@gmail.com>   you wrote:
>>> Signed-off-by: Gray Remlin<gryrmln@gmail.com>
>>> ---
>>>    common/cmd_bdinfo.c |    3 +++
>>>    1 files changed, 3 insertions(+), 0 deletions(-)
>> Why limit this to eth1addr?  What's the chances that ARM systems may
>> have more than 2 network interfaces?
>>
>> Best regards,
>>
>> Wolfgang Denk
>>
> Good question, I have already asked myself this, so I have stuck with
> what I do know.
>
> 1. It has had the one ethaddr limit for a (relatively) long time, which
> it seems no-one else 'required'\'submitted a patch' to change it.
> 2. I only know (with my very limited knowledge) of ARM boards with a
> maximum of two interfaces as standard.
> 3. Why limit it to six (as in other parts of the source) ?
>
> My answer:
> It is not my place to dictate policy, that is the role of the Project
> Manager.
> And no, that is not a 'cop-out', it is the only way to avoid anarchy.

There is no ARM board right now which defined CONFIG_HAS_ETH2 or higher, 
and I have seen no ARM code which assumes otherwise (readers feel free 
to prove me wrong, of course).

Besides, while contributors are always welcome to generalize their 
patches beyond their needs if they so accept, by no means are they 
forced to do so against their will when their contribution is consistent 
with the existing code state.

I thus consider Gray's patch is OK and, unless some review is requested 
in the near future, will pull it in u-boot-arm (*not* for inclusion in 
2011-03, of course).

If someone wants support for CONFIG_HAS_ETH2 or higher on ARM, they are 
welcome to extend Gray's work, even to merge some ARM and PPC parts of 
common/cmd_bd_info.c.

Amicalement,
Detlev Zundel March 30, 2011, 12:47 p.m. UTC | #4
Hi Gray,

> On 03/29/2011 10:49 PM, Wolfgang Denk wrote:
>> Dear Gray Remlin,
>>
>> In message<1301433395-25203-1-git-send-email-gryrmln@gmail.com>  you wrote:
>>> Signed-off-by: Gray Remlin<gryrmln@gmail.com>
>>> ---
>>>   common/cmd_bdinfo.c |    3 +++
>>>   1 files changed, 3 insertions(+), 0 deletions(-)
>> Why limit this to eth1addr?  What's the chances that ARM systems may
>> have more than 2 network interfaces?
>>
>> Best regards,
>>
>> Wolfgang Denk
>>
> Good question, I have already asked myself this, so I have stuck with 
> what I do know.
>
> 1. It has had the one ethaddr limit for a (relatively) long time, which 
> it seems no-one else 'required'\'submitted a patch' to change it.
> 2. I only know (with my very limited knowledge) of ARM boards with a 
> maximum of two interfaces as standard.
> 3. Why limit it to six (as in other parts of the source) ?

Because todays CPUs get drowned with six saturated links? ;)

> My answer:
> It is not my place to dictate policy, that is the role of the Project 
> Manager.
> And no, that is not a 'cop-out', it is the only way to avoid anarchy.

If we extend something, it is a good idea to sync to other places in the
sources.

Cheers
  Detlev
Gray Remlin March 30, 2011, 1:35 p.m. UTC | #5
On 03/30/2011 12:34 PM, Albert ARIBAUD wrote:
> Le 30/03/2011 00:36, Gray Remlin a écrit :
>> On 03/29/2011 10:49 PM, Wolfgang Denk wrote:
>>> Dear Gray Remlin,
>>>
>>> In message<1301433395-25203-1-git-send-email-gryrmln@gmail.com>    you wrote:
>>>> Signed-off-by: Gray Remlin<gryrmln@gmail.com>
>>>> ---
>>>>     common/cmd_bdinfo.c |    3 +++
>>>>     1 files changed, 3 insertions(+), 0 deletions(-)
>>> Why limit this to eth1addr?  What's the chances that ARM systems may
>>> have more than 2 network interfaces?
>>>
>>> Best regards,
>>>
>>> Wolfgang Denk
>>>
>> Good question, I have already asked myself this, so I have stuck with
>> what I do know.
>>
>> 1. It has had the one ethaddr limit for a (relatively) long time, which
>> it seems no-one else 'required'\'submitted a patch' to change it.
>> 2. I only know (with my very limited knowledge) of ARM boards with a
>> maximum of two interfaces as standard.
>> 3. Why limit it to six (as in other parts of the source) ?
>>
>> My answer:
>> It is not my place to dictate policy, that is the role of the Project
>> Manager.
>> And no, that is not a 'cop-out', it is the only way to avoid anarchy.
> There is no ARM board right now which defined CONFIG_HAS_ETH2 or higher,
> and I have seen no ARM code which assumes otherwise (readers feel free
> to prove me wrong, of course).
>
> Besides, while contributors are always welcome to generalize their
> patches beyond their needs if they so accept, by no means are they
> forced to do so against their will when their contribution is consistent
> with the existing code state.
>
> I thus consider Gray's patch is OK and, unless some review is requested
> in the near future, will pull it in u-boot-arm (*not* for inclusion in
> 2011-03, of course).
>
> If someone wants support for CONFIG_HAS_ETH2 or higher on ARM, they are
> welcome to extend Gray's work, even to merge some ARM and PPC parts of
> common/cmd_bd_info.c.
>
> Amicalement,
I hold regard for 'small is beautiful' and want to see u-boot both
'complete' and 'concise', sometimes a difficult juggling act.

There are two different variants of the same ARM board that I am working
with, the variant I have has two ethernet ports, the other only one.
I am considering releasing a patch for the configuration header file 
with the
differences commented to make it obvious, until u-boot supports this, 
there is
no point in submitting the patch other than to confuse people.

There is a new recently released ARM board that also has two ethernet ports,
but as this differs in memory configuration, will not likely share the same
configuration header, but will still require support.

These three ARM boards are all Kirkwood family.
If they release another with more ports, I will most likely submit the 
patch myself
(providing I can test it first of course, any freebies Globalscale ?).
diff mbox

Patch

diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
index bba7374..c0553d5 100644
--- a/common/cmd_bdinfo.c
+++ b/common/cmd_bdinfo.c
@@ -340,6 +340,9 @@  int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 #if defined(CONFIG_CMD_NET)
 	print_eth(0);
+#if defined(CONFIG_HAS_ETH1)
+	print_eth(1);
+#endif
 	printf ("ip_addr     = %pI4\n", &bd->bi_ip_addr);
 #endif
 	printf ("baudrate    = %d bps\n", bd->bi_baudrate);