[U-Boot] [PATCH] drivers/net/tsec.c - mii_parse_sr does not wait for auto-negotiation completion bug fix

[Michael Zaidman]

On Tue, Mar 24, 2009 at 9:01 PM, Peter Tyser <ptyser at xes-inc.com> wrote:
> Hi Michael,
>
> On Tue, 2009-03-24 at 19:57 +0200, Michael Zaidman wrote:
>> drivers/net/tsec.c - mii_parse_sr does not wait for auto-negotiation
>> completion bug fix
>>
>> In the case when the MIIM_STATUS_LINK is 0 i.e. link is down
>> and this is the situation immediately after power up,
>> the code of awaiting for auto-negotiation completion now will be executed.
>>
>> Signed-off-by: Michael Zaidman <michael.zaidman at gmail.com>
>> ---
>>  drivers/net/tsec.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
>> index 399116f..54279ca 100644
>> --- a/drivers/net/tsec.c
>> +++ b/drivers/net/tsec.c
>> @@ -363,7 +363,7 @@ uint mii_parse_sr(uint mii_reg, struct tsec_private * priv)
>>          * (ie - we're capable and it's not done)
>>          */
>>         mii_reg = read_phy_reg(priv, MIIM_STATUS);
>> -       if ((mii_reg & MIIM_STATUS_LINK) && (mii_reg & PHY_BMSR_AUTN_ABLE)
>> +       if (!(mii_reg & MIIM_STATUS_LINK) && (mii_reg & PHY_BMSR_AUTN_ABLE)
>>             && !(mii_reg & PHY_BMSR_AUTN_COMP)) {
>>                 int i = 0;
>
> This issue has been brought up once before:
> http://lists.denx.de/pipermail/u-boot/2009-February/046829.html
>
> There was some concern about adding delay to the boot process in the
> case that the primary ethernet port(s) did not achieve link.
>
> There had also been some discussion about performing PHY
> auto-negotiation in parallel, but this seems like a much larger can of
> worms.
>
> I had proposed this:
> http://lists.denx.de/pipermail/u-boot/2009-February/048489.html
>
> Andy/Ben, any feedback on getting a patch such as Michael's or mine
> merged in?
>
> Thanks,
> Peter
>
>

Hi Peter,

Thanks for the reference. I found this thread very interesting.

    Before I submitted the patch I also wondered if it is possible for
    the link status to be OK while auto-negotiation has not been completed yet.
    So I dived into the IEEE-802.3 spec (Clause 28) and figured out that
    according to the Arbitration state diagram (Figure 28-16) the
auto-negotiation
    completed indication is asserted after the link status became OK.
    It contradicts with Marvell’s 88E1111 phy behavior on my mpc8349 based board
    where link status OK (register 1.2) always comes at least with 1 tbclk delay
    after auto-negotiation completion indication (register 1.5).
    It probably explains why the delay of 500 ms “results in faster booting”
    as stated in the code - without this delay the CPU leaves the routine with
    MIIM_STATUS_LINK still being in the down state.
    By replacing the PHY_BMSR_AUTN_CMP with MIIM_STATUS_LINK in loop exiting
    condition we eliminate the 500 ms delay.

    The spec also specifies auto-negotiation timeout in table 28-9.
    Its worst case value can be calculated as:

    break_link_timer + autoneg_wait_timer + link_fai_inhibit_timer =
        1500 ms + 1000 ms + 1000 ms = 3500 ms.

    So I reduced the PHY_AUTONEGOTIATE_TIMEOUT to be 4000 ms.

    Here is the updated patch

Signed-off-by: Michael Zaidman <michael.zaidman at gmail.com>
---
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 399116f..28e9cc7 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -363,12 +363,12 @@ uint mii_parse_sr(uint mii_reg, struct
tsec_private * priv)
         * (ie - we're capable and it's not done)
         */
        mii_reg = read_phy_reg(priv, MIIM_STATUS);
-       if ((mii_reg & MIIM_STATUS_LINK) && (mii_reg & PHY_BMSR_AUTN_ABLE)
+       if (!(mii_reg & MIIM_STATUS_LINK) && (mii_reg & PHY_BMSR_AUTN_ABLE)
            && !(mii_reg & PHY_BMSR_AUTN_COMP)) {
                int i = 0;

                puts("Waiting for PHY auto negotiation to complete");
-               while (!(mii_reg & PHY_BMSR_AUTN_COMP)) {
+               while (!(mii_reg & MIIM_STATUS_LINK)) {
                        /*
                         * Timeout reached ?
                         */
@@ -386,7 +386,6 @@ uint mii_parse_sr(uint mii_reg, struct tsec_private * priv)
                }
                puts(" done\n");
                priv->link = 1;
-               udelay(500000); /* another 500 ms (results in faster booting) */
        } else {
                if (mii_reg & MIIM_STATUS_LINK)
                        priv->link = 1;
diff --git a/include/tsec.h b/include/tsec.h
index 7b52e06..ed4c855 100644
--- a/include/tsec.h
+++ b/include/tsec.h
@@ -56,7 +56,7 @@
 #define TSEC_TIMEOUT 1000
 #define TOUT_LOOP      1000000

-#define PHY_AUTONEGOTIATE_TIMEOUT      5000 /* in ms */
+#define PHY_AUTONEGOTIATE_TIMEOUT      4000 /* in ms */

 /* TBI register addresses */
 #define TBI_CR                 0x00


Thanks,
Michael