On Fri, Mar 23, 2007 at 01:30:02PM +0100, Gabriel Paubert wrote:

In this driver, the default ethernet address is first set by by calling
eth_port_uc_addr_get() which reads the relevant registers of the 
corresponding port as initially set by firmware. However that function 
used the port_num field accessed through the private area of net_dev 
before it was set.  

Gabriel, you're right.  I introduced the bug and I'm sorry for your
trouble.

The result was that one board I have ended up with the unicast address 
set to 00:00:00:00:00:00 (only port 1 is connected on this board). The
problem appeared after commit 84dd619e4dc3b0b1c40dafd98c90fd950bce7bc5.

This patch fixes the bug by making eth_port_uc_get_addr() more similar 
to eth_port_uc_set_addr(), i.e., by using the port number as the first
parameter instead of a pointer to struct net_device.

Signed-off-by: Gabriel Paubert <redacted>

--

The minimal patch I first tried consisted in just moving mp->port_num
to before the call to eth_port_uc_get_addr().

Hmm.  That should have fixed it.  I reproduced the problem here and
this fixed it for me:

diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
index 1ee27c3..643ea31 100644
--- a/drivers/net/mv643xx_eth.c
+++ b/drivers/net/mv643xx_eth.c

@@ -1379,7 +1379,7 @@ #endif
 
 	spin_lock_init(&mp->lock);
 
-	port_num = pd->port_number;
+	port_num = mp->port_num = pd->port_number;
 
 	/* set default config values */
 	eth_port_uc_addr_get(dev, dev->dev_addr);

@@ -1411,8 +1411,6 @@ #endif
 	duplex = pd->duplex;
 	speed = pd->speed;
 
-	mp->port_num = port_num;
-
 	/* Hook up MII support for ethtool */
 	mp->mii.dev = dev;
 	mp->mii.mdio_read = mv643xx_mdio_read;

Would you please confirm that this fixes it for you?  If so, I'll submit
it upstream as coming from you, since you did all the work.  OK?

The other question is why
the driver never gets the info from the device tree on this PPC board,
but that's for another list despite the fact I lost some time looking 
for bugs in the OF interface before stumbling on this use of a field
before it was initialized.

Probably just because the mac address in the hardware was correct and
it didn't seem necessary to overwrite it.

Thank you,
-Dale