[PATCH] 44x ethernet MDIO access

There is a small bug in the emac_phy_write() routine defined in
drivers/net/ibm_emac/ibm_emac_core.c (present in latest 2.6.8-rc3).  At
the end of the function, check is done for phy error; however as written
this will always be false, because "stacr" contains the value _written_
to the hardware, rather than the value read back from hardware.

Simple on-line fix attached.

In a related issue, both the PHY read and write functions use a fixed
time delay (MDIO_DELAY = 50) presently... it seems I need a longer value
on my hardware, and having such hardcoded values is trouble anyways... I
suggest we change the code to poll for completion, with a short delay
and an upper limit on the number of loops.

=Ralph

Ralph Siemsen wrote:

In a related issue, both the PHY read and write functions use a fixed
time delay (MDIO_DELAY = 50) presently... it seems I need a longer value
on my hardware, and having such hardcoded values is trouble anyways... I
suggest we change the code to poll for completion, with a short delay
and an upper limit on the number of loops.

I've just checked the hardware, it seems that the MDIO clock is driven
at EPB clock divided by 10.  So on the IBM eval board its 7.6MHz.  Each
MDIO transfer involves moving 64 bits of data.  So that means just the
data transfer (never mind any processing in the PHY) takes 8.4us.

The important part to note is that EPB could vary over a considerable
range, so the time needed for MDIO is potentially longer than the 50us
currently used in the code.

Attached is a patch that "works for me" - turns the long udelay() into a
polling loop with short little delays instead.  This allows MDIO_DELAY
to be increased to a larger value (say 150) without slowing down most
systems.

Also I can be used to measure just how long the transfers actually take.
 If MDIO_DEBUG is enabled then you'll see how many "loops" were
needed... on the Ocotea board i get values around 25.

-Ralph

On Thu, 5 Aug 2004, Ralph Siemsen wrote:

Attached is a patch that "works for me" - turns the long udelay() into a
polling loop with short little delays instead.  This allows MDIO_DELAY
to be increased to a larger value (say 150) without slowing down most
systems.

                                                      ^^^^
						      MDIO_DELAY?

And perhaps you want to create a (possibly inlined) function just for this
loop?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Geert Uytterhoeven wrote:

quoted

-	udelay(MDIO_DELAY);
+	count = 0;
+	while (((stacr = in_be32(&emacp->em0stacr) & EMAC_STACR_OC) == 0)
+					&& (count++ < 5000))

                                                      ^^^^
						      MDIO_DELAY?

And perhaps you want to create a (possibly inlined) function just for this
loop?

Good catch - yes, should be MDIO_DELAY not 5000.  I had found that 50
was not enough so I had upped it in my testing - forgot to change it back :(

I'd also suggest the default MDIO_DELAY be increased from its present
value of 50 to perhaps 100 or 150...  Does anyone know what the slowest
possible EPB clock speed is?

-R

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/