Re: [PATCH] mips/alchemy: define eth platform devices in the correct order

[Florian Fainelli <florian@openwrt.org>]

Hi 

Le Saturday 17 July 2010 19:17:52, Wolfgang Grandegger a écrit :
> On 07/17/2010 07:01 PM, Manuel Lauss wrote:
> > Servus Wolfgang,
> > 
> > On Sat, Jul 17, 2010 at 4:38 PM, Wolfgang Grandegger <wg@grandegger.com> 
wrote:
> >> From: Wolfgang Grandegger <wg@denx.de>
> >> 
> >> Currently, the eth devices are probed in the inverse order, first
> >> au1xxx_eth1_device and then au1xxx_eth0_device. On the GPR board,
> >> 
> >> this makes trouble:
> >>  # ifconfig|grep HWaddr
> >>  eth0      Link encap:Ethernet  HWaddr 00:50:C2:0C:30:01
> >>  eth1      Link encap:Ethernet  HWaddr 66:22:01:80:38:10
> >> 
> >> A bogous ethernet hwaddr is assigned to the first device and
> >> au1xxx_eth0_device is mapped to eth1, which even does not work

Most likely prom_get_ethernet_addr is failing for the first device because 
pdev->id == 1 that is why you get such an address, take a look at au1000-
eth.c.

> >> 
> >> properly. With this patch, the problems are gone:
> >>  # ifconfig|grep HWaddr
> >>  eth0      Link encap:Ethernet  HWaddr 66:22:11:32:38:10
> >>  eth1      Link encap:Ethernet  HWaddr 66:22:11:32:38:11
> > 
> > Interesting.  I don't disagree with the patch; what do you think about
> > passing MAC address via platform_data?   I don't particularly like
> > how the driver is trying to get a MAC address using the prom interface.

The patch is actually good, because there are no reasons to register the 
second MAC before the first one, sorry about that. However, the real fix also 
involves au1000-eth,c in au1000_probe(). We currently only handle the case 
where the pdev->id is 0, not 1. When pdev->id == 1 you end up defaulting to 
the default au1000-eth defined MAC address. So I would do it that way:

- make the board code get the ethernet MAC address for the given adapter
- pass it via platform_data
- au1000-eth checks for its validity or generates a random one

> 
> Well, I don't think it's a good idea. Each board should have a different
> mac address and it's nomally stored somewhere in the boards non-volatile
> storage during board bringup.
> 
> > I'll try to cook something up.
> 
> But not via platform data, please. Or have I missed something. With the
> flat device tree (as used for PowerPC) the situation is different
> because the boot-loader can fixup the MAC address before booting Linux.

MIPS has no mainlined support for DT yet, but still, you would have to cope 
with existing boards running YAMON as a bootloader, so using prom_getenv() is 
imho a good solution.
--
Florian