[U-Boot] MPC512x FEC/MII

Hi all,
I have recently updated the U-Boot on one of our MPC512x-based boards from 2009.11 (+some patches) to a current u-boot build and noticed that the git commit 525856d59910c72687ab6201f39cdf1c04cfc15 apparenty broke the mii commands (see below) on this board. The patch moved PHY initialization from probe into init routine. The mii read commands return zero values regardless of previous FEC usage. Has anyone else come across similar behavior?
Thanks a lot, Kolja
/* with git commit 525856d59910c72687ab6201f39cdf1c04cfc15 */ EM10A=> mii i PHY 0x00: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x01: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x02: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x03: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX /* snip */
/* without git commit 525856d59910c72687ab6201f39cdf1c04cfc15 */ EM10A=> mii i PHY 0x00: OUI = 0x0885, Model = 0x11, Rev = 0x03, 100baseT, FDX PHY 0x01: OUI = 0x0885, Model = 0x11, Rev = 0x03, 100baseT, FDX
=== modified file 'drivers/net/mpc512x_fec.c' --- drivers/net/mpc512x_fec.c 2010-05-03 21:52:48 +0000 +++ drivers/net/mpc512x_fec.c 2010-05-03 21:52:48 +0000 @@ -160,7 +160,7 @@ }
/********************************************************************/ -static void mpc512x_fec_set_hwaddr (mpc512x_fec_priv *fec, char *mac) +static void mpc512x_fec_set_hwaddr (mpc512x_fec_priv *fec, unsigned char *mac) { u8 currByte; /* byte for which to compute the CRC */ int byte; /* loop - counter */ @@ -226,6 +226,12 @@ printf ("mpc512x_fec_init... Begin\n"); #endif
+ mpc512x_fec_set_hwaddr (fec, dev->enetaddr); + out_be32(&fec->eth->gaddr1, 0x00000000); + out_be32(&fec->eth->gaddr2, 0x00000000); + + mpc512x_fec_init_phy (dev, bis); + /* Set interrupt mask register */ out_be32(&fec->eth->imask, 0x00000000);
@@ -611,8 +617,6 @@ volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR; mpc512x_fec_priv *fec; struct eth_device *dev; - int i; - char *tmp, *end, env_enetaddr[6]; void * bd;
fec = (mpc512x_fec_priv *) malloc (sizeof(*fec)); @@ -663,25 +667,6 @@ */ out_be32(&fec->eth->ievent, 0xffffffff);
- /* - * Try to set the mac address now. The fec mac address is - * a garbage after reset. When not using fec for booting - * the Linux fec driver will try to work with this garbage. - */ - tmp = getenv ("ethaddr"); - if (tmp) { - for (i=0; i<6; i++) { - env_enetaddr[i] = tmp ? simple_strtoul (tmp, &end, 16) : 0; - if (tmp) - tmp = (*end) ? end+1 : end; - } - mpc512x_fec_set_hwaddr (fec, env_enetaddr); - out_be32(&fec->eth->gaddr1, 0x00000000); - out_be32(&fec->eth->gaddr2, 0x00000000); - } - - mpc512x_fec_init_phy (dev, bis); - return 1; }
Kolja Schneider, Software Design MEN Mikro Elektronik GmbH Neuwieder Straße 5-7 90411 Nürnberg, Germany Phone +49-911-99 33 5-251 Fax +49-911-99 33 5-910 Kolja.Schneider@men.de www.men.de
MEN Mikro Elektronik GmbH - Manfred Schmitz (CTO), Udo Fuchs (CFO) - Handelsregister/Trade Register AG Nürnberg HRB 5540 Please consider the environment before printing this e-mail

Hello Kolja,
we also get broken output from mii_cmd when the commit which you show below is applied, reverting it helps.
Hardware: Our own MPC5121ADS-based board and the MPC5121ADS development board.
The commit is titled:
mpc512x_fec: Move PHY initialization from probe into init routin This saves the autonegotation delay when not using ethernet in U-Boot",
This exactly what it does, the effect is no suprise at all:
Of course, the move of the PHY init from the fec_probe (executed before the u-boot prompt appears) to the fec_init (called only and every time a new network command is executed) has the effect which we see:
The PHY is only initialized while a network command (such as dhcp, tftp, ...) is being executed.
Also, when network commands terminate, the network device's halt routine is executed and in the case of the MPC512x FEC driver, it resets the FEC unit of the chip, bringing the MII communication down again. The result is that we never see a working mii command with this patch.
But not only the mii_cmd is broken by this change, also the Ethernet autonegotation process which is executed at the end of mpc512x_fec_init_phy() is moved from network device probe call to the netcommand init calls:
This causes the the Ethernet link is down after the network device probe and the autonegotiation process is executed EVERY time a network command runs. This means every time you run a dhcp, tftp, nfs, dns or other network command, you get a new autonegotiation process which may be fast but may also be slow. You may notice these delay at every network command when you run a series of commands to net-boot your device, e.g.:
dhcp; tftp of a boot-script (as part of the dhcp call);tftp kernel; tftp initramfs; tftp fdt
In our case, the routers seem to be set up so that after a system appears on the network (auto-negotiation complete), it takes several seconds before packets are being forwarded to the DHCP and TFTP servers, so it became quite painful.
We had to revert this change and I replaced it with a patch to not wait for the auto-negoation to complete in probe. This means that the PHY can auto-negotiatiate in the background and the board does not waste 2.5 seconds waiting for it if no cable is connected, which achieves the goal of the patch (to not wait for auto-neg when not needed).
We've had two iterations of this patch so far and I need to review the second iteration more closely before posting.
Best Regards, Bernhard
Am 09.09.2011 11:51, schrieb Schneider, Kolja:
Hi all,
I have recently updated the U-Boot on one of our MPC512x-based boards from 2009.11 (+some patches) to a current u-boot build and noticed that the git commit 525856d59910c72687ab6201f39cdf1c04cfc15 apparenty broke the mii commands (see below) on this board. The patch moved PHY initialization from probe into init routine. The mii read commands return zero values regardless of previous FEC usage. Has anyone else come across similar behavior?
Thanks a lot, Kolja
/* with git commit 525856d59910c72687ab6201f39cdf1c04cfc15 */ EM10A=> mii i PHY 0x00: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x01: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x02: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x03: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX /* snip */
/* without git commit 525856d59910c72687ab6201f39cdf1c04cfc15 */ EM10A=> mii i PHY 0x00: OUI = 0x0885, Model = 0x11, Rev = 0x03, 100baseT, FDX PHY 0x01: OUI = 0x0885, Model = 0x11, Rev = 0x03, 100baseT, FDX
=== modified file 'drivers/net/mpc512x_fec.c' --- drivers/net/mpc512x_fec.c 2010-05-03 21:52:48 +0000 +++ drivers/net/mpc512x_fec.c 2010-05-03 21:52:48 +0000 @@ -160,7 +160,7 @@ }
/********************************************************************/ -static void mpc512x_fec_set_hwaddr (mpc512x_fec_priv *fec, char *mac) +static void mpc512x_fec_set_hwaddr (mpc512x_fec_priv *fec, unsigned char *mac) { u8 currByte; /* byte for which to compute the CRC */ int byte; /* loop - counter */ @@ -226,6 +226,12 @@ printf ("mpc512x_fec_init... Begin\n"); #endif
- mpc512x_fec_set_hwaddr (fec, dev->enetaddr);
- out_be32(&fec->eth->gaddr1, 0x00000000);
- out_be32(&fec->eth->gaddr2, 0x00000000);
- mpc512x_fec_init_phy (dev, bis);
- /* Set interrupt mask register */ out_be32(&fec->eth->imask, 0x00000000);
@@ -611,8 +617,6 @@ volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR; mpc512x_fec_priv *fec; struct eth_device *dev;
int i;
char *tmp, *end, env_enetaddr[6]; void * bd;
fec = (mpc512x_fec_priv *) malloc (sizeof(*fec));
@@ -663,25 +667,6 @@ */ out_be32(&fec->eth->ievent, 0xffffffff);
- /*
* Try to set the mac address now. The fec mac address is
* a garbage after reset. When not using fec for booting
* the Linux fec driver will try to work with this garbage.
*/
- tmp = getenv ("ethaddr");
- if (tmp) {
for (i=0; i<6; i++) {
env_enetaddr[i] = tmp ? simple_strtoul (tmp,&end, 16) : 0;
if (tmp)
tmp = (*end) ? end+1 : end;
}
mpc512x_fec_set_hwaddr (fec, env_enetaddr);
out_be32(&fec->eth->gaddr1, 0x00000000);
out_be32(&fec->eth->gaddr2, 0x00000000);
- }
- mpc512x_fec_init_phy (dev, bis);
- return 1; }
Kolja Schneider, Software Design MEN Mikro Elektronik GmbH Neuwieder Straße 5-7 90411 Nürnberg, Germany Phone +49-911-99 33 5-251 Fax +49-911-99 33 5-910 Kolja.Schneider@men.de www.men.de
MEN Mikro Elektronik GmbH - Manfred Schmitz (CTO), Udo Fuchs (CFO) - Handelsregister/Trade Register AG Nürnberg HRB 5540 Please consider the environment before printing this e-mail
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi Bernhard,
we also get broken output from mii_cmd when the commit which you show below is applied, reverting it helps.
Hardware: Our own MPC5121ADS-based board and the MPC5121ADS development board.
The commit is titled:
mpc512x_fec: Move PHY initialization from probe into init routin This saves the autonegotation delay when not using ethernet in U-Boot",
This exactly what it does, the effect is no suprise at all:
Of course, the move of the PHY init from the fec_probe (executed before the u-boot prompt appears) to the fec_init (called only and every time a new network command is executed) has the effect which we see:
The PHY is only initialized while a network command (such as dhcp, tftp, ...) is being executed.
Well, this is actually in line with the U-Boot design principles of initializing a device only if its needed[1]. This principle was disregarded regularly in the past and so it seems kind of naturla that "fixing" it results in changes of behaviour. We should however fix the fallout rather than going back to violating the principles.
If the mii routines need initializations, then they should ensure that they have been done.
Cheers Detlev
[1] http://www.denx.de/wiki/view/U-Boot/DesignPrinciples#2_Keep_it_Fast

Hi Detlev,
-----Ursprüngliche Nachricht----- Von: Detlev Zundel [mailto:dzu@denx.de] Gesendet: Dienstag, 27. September 2011 16:28 An: Bernhard Kaindl Cc: Schneider, Kolja; u-boot@lists.denx.de Betreff: Re: [U-Boot] MPC512x FEC/MII
Hi Bernhard,
we also get broken output from mii_cmd when the commit which you
show
below is applied, reverting it helps.
Hardware: Our own MPC5121ADS-based board and the MPC5121ADS
development
board.
The commit is titled:
mpc512x_fec: Move PHY initialization from probe into init routin This saves the autonegotation delay when not using ethernet in U-Boot",
This exactly what it does, the effect is no suprise at all:
Of course, the move of the PHY init from the fec_probe (executed before the u-boot prompt appears) to the fec_init (called only and every time a new network command is executed) has the effect which we see:
The PHY is only initialized while a network command (such as dhcp, tftp, ...) is being executed.
Well, this is actually in line with the U-Boot design principles of initializing a device only if its needed[1]. This principle was disregarded regularly in the past and so it seems kind of naturla that "fixing" it results in changes of behaviour. We should however fix the fallout rather than going back to violating the principles.
If the mii routines need initializations, then they should ensure that they have been done.
This really is a shame as the mii tools are quite helpful for debugging/hardware verification. If the mii-command performs a re-initialisation each time they are invoked, they are probably close to useless (as one would need several commands to perform certain tasks...).
However I understand this is something that is not quite in the scope of a bootloader. As long as an easy workaround is available...
:) Kolja

Hi Kolja,
[...]
The PHY is only initialized while a network command (such as dhcp, tftp, ...) is being executed.
Well, this is actually in line with the U-Boot design principles of initializing a device only if its needed[1]. This principle was disregarded regularly in the past and so it seems kind of naturla that "fixing" it results in changes of behaviour. We should however fix the fallout rather than going back to violating the principles.
If the mii routines need initializations, then they should ensure that they have been done.
This really is a shame as the mii tools are quite helpful for debugging/hardware verification. If the mii-command performs a re-initialisation each time they are invoked, they are probably close to useless (as one would need several commands to perform certain tasks...).
I never said that the mii commands should initialize the phy _every_ time they are called. All I said was that if they need an init, they should ensure it has been run - this can also be done only once.
However I understand this is something that is not quite in the scope of a bootloader. As long as an easy workaround is available...
I think this is in the scope of a bootloader - all I want to say is that we should fix the problems that we encounter getting closer to our own goals rather than giving up our goals to fix an individual problem.
Actually I believe that the move towards a driver model discussed on the ML only recently, can solve many of these problems currently handled in all the sub-modules and can centralize it in a single place. By then we would have a "initialized" field for every device and the commands (mii or net commands) that deal with a net device can ensure that its "init" method runs if this is not set.
Also with this approach, before booting an operating system, we can deinitialize all devices again. Although this sounds somewhat strange, this was one of the more hard to find bugs that I have seen - an usb controller happily updating frame counters in RAM while Linux booted...
So maybe the solution to this problem can be reached by helping other code changes ;)
Cheers Detlev
participants (3)
-
Bernhard Kaindl
-
Detlev Zundel
-
Schneider, Kolja