[U-Boot-Users] [PATCH] Fix mpc8360emds board hang on second network operation

[U-Boot-Users] [PATCH] Fix mpc8360emds board hang on second network operation

[Jerry Van Baren]

The changeset that causes my problems is ee62ed32

The original code calls init_phy(dev) followed by phy_change(dev) *once*
during PHY initialization.  The part of changeset that appears to cause
my problems is calling phy_change(dev) every time uec_init() is called.

On my mpc8360emds board, without this change the second command that does
a network operation hangs the board (requires a hard reset to recover).
Example failure sequence is two "ping" commands: the first works, the
second hangs the board.

This also speeds up the network initialization and doesn't cause a packet
error on the first TFTP packet.

Signed-off-by: Gerald Van Baren <vanbaren@cideas.com>
---

Kim:

Does this make any sense?  Any thoughts why it isn't causing problems
for you?  It makes all the difference in the world for me.

One other interesting sequence difference with patch ee62ed32 is that
the ethernet MAC address is set on every uec_init() call.  The following
code was inside the "if (uec->the_first_run == 0)" conditional, now it
is outside the conditional and thus run on every uec_init() call.
Functionally, it doesn't seem to matter for me either way, but seems
unnecessary.

+       /* Set up the MAC address */
+       if (dev->enetaddr[0] & 0x01) {
+               printf("%s: MacAddress is multcast address\n",
+                        __FUNCTION__);
+               return -1;
+       }
+       uec_set_mac_address(uec, dev->enetaddr);

Best regards,
gvb

P.S. What's the Guiness World Book record for number of words written
to explain moving two lines of code?

 drivers/qe/uec.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
index 55f37cb..d447d96 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -1163,6 +1163,8 @@ static int uec_init(struct eth_device* dev, bd_t *bd)
 			return err;
 		}
 
+		phy_change(dev);
+
 		curphy = uec->mii_info->phyinfo;
 
 		if (curphy->config_aneg) {
@@ -1201,8 +1203,6 @@ static int uec_init(struct eth_device* dev, bd_t *bd)
 		return -1;
 	}
 
-	phy_change(dev);
-
 	return (uec->mii_info->link ? 0 : -1);
 }
 
-- 
1.5.4.1

[U-Boot-Users] [PATCH] Fix mpc8360emds board hang on second network operation

[Kim Phillips]

On Tue, 26 Feb 2008 21:32:30 -0500
Jerry Van Baren <gvb.uboot@gmail.com> wrote:

> The changeset that causes my problems is ee62ed32
> 
> The original code calls init_phy(dev) followed by phy_change(dev) *once*
> during PHY initialization.  The part of changeset that appears to cause
> my problems is calling phy_change(dev) every time uec_init() is called.

phy_change is there so that u-boot will renegotiate the link in case
its state has changed since the last networking command (this was the
original complaint that warranted the ee62ed32 patch, iirc).

> On my mpc8360emds board, without this change the second command that does
> a network operation hangs the board (requires a hard reset to recover).
> Example failure sequence is two "ping" commands: the first works, the
> second hangs the board.
> 
> This also speeds up the network initialization and doesn't cause a packet
> error on the first TFTP packet.
> 
> Signed-off-by: Gerald Van Baren <vanbaren@cideas.com>
> ---
> 
> Kim:
> 
> Does this make any sense?  Any thoughts why it isn't causing problems
> for you?  It makes all the difference in the world for me.
> 
> One other interesting sequence difference with patch ee62ed32 is that
> the ethernet MAC address is set on every uec_init() call.  The following
> code was inside the "if (uec->the_first_run == 0)" conditional, now it
> is outside the conditional and thus run on every uec_init() call.
> Functionally, it doesn't seem to matter for me either way, but seems
> unnecessary.
> 
> +       /* Set up the MAC address */
> +       if (dev->enetaddr[0] & 0x01) {
> +               printf("%s: MacAddress is multcast address\n",
> +                        __FUNCTION__);
> +               return -1;
> +       }
> +       uec_set_mac_address(uec, dev->enetaddr);
> 

this was done in order to be able to re-set eth[1]addr between
networking commands, and have the new eth[1]addr value actually
programmed on the interface.

I've tested 100BT, and experience the same as you.  The reason u-boot
doesn't recover is that i is not incremented in genmii_update_link(),
but adding that doesn't resolve the problem.  I believe the PHY status
is being incorrectly determined somehow - these PHYs are really
finicky :/.

Kim