[U-Boot] Incorrect MDIO clock setup in 4xx network driver

[U-Boot] Incorrect MDIO clock setup in 4xx network driver

[Felix Radensky]

Hi,

The following piece of code in ppc_4xx_eth_init() results in incorrect MDIO
clock setup on Canyonlands, in the case when OPB clock is 100Mhz

    /* Whack the M1 register */
    mode_reg = 0x0;
    mode_reg &= ~0x00000038;
    if (sysinfo.freqOPB <= 50000000);
    else if (sysinfo.freqOPB <= 66666667)
        mode_reg |= EMAC_M1_OBCI_66;
    else if (sysinfo.freqOPB <= 83333333)
        mode_reg |= EMAC_M1_OBCI_83;
    else if (sysinfo.freqOPB <= 100000000)
        mode_reg |= EMAC_M1_OBCI_100;
    else
        mode_reg |= EMAC_M1_OBCI_GT100;

   out_be32((void *)EMAC_M1 + hw_p->hw_addr, mode_reg);

When OPB clock runs at 100Mhz, the value of sysinfo.freqOPB is 100000001,
so this code configures MDC clock to 3.3Mhz instead of less than 2.5Hz. This
is not a problem on Canyonlands, where Marvell PHY MDIO clock can run at
8.3Mhz. However other PHYs may actually require MDIO clock to be less than
2.5Mhz.

How about rewriting this code as

    freq = sysinfo.freqOPB / 1000000;
    if (freq <= 50)
    else if (freq <= 66)
       mode_reg |= EMAC_M1_OBCI_66;
    else if (freq <= 83)
       mode_reg |= EMAC_M1_OBCI_83;
    else if (freq <= 100)
       mode_reg |= EMAC_M1_OBCI_100;
    else
        mode_reg |= EMAC_M1_OBCI_GT100;

Felix.

[U-Boot] Incorrect MDIO clock setup in 4xx network driver

[Stefan Roese]

Hi Felix,

(btw, please cc me on 4xx related patches, otherwise I might miss them)

On Thursday 28 May 2009 22:20:26 Felix Radensky wrote:
> The following piece of code in ppc_4xx_eth_init() results in incorrect MDIO
> clock setup on Canyonlands, in the case when OPB clock is 100Mhz
>
>     /* Whack the M1 register */
>     mode_reg = 0x0;
>     mode_reg &= ~0x00000038;
>     if (sysinfo.freqOPB <= 50000000);
>     else if (sysinfo.freqOPB <= 66666667)
>         mode_reg |= EMAC_M1_OBCI_66;
>     else if (sysinfo.freqOPB <= 83333333)
>         mode_reg |= EMAC_M1_OBCI_83;
>     else if (sysinfo.freqOPB <= 100000000)
>         mode_reg |= EMAC_M1_OBCI_100;
>     else
>         mode_reg |= EMAC_M1_OBCI_GT100;
>
>    out_be32((void *)EMAC_M1 + hw_p->hw_addr, mode_reg);
>
> When OPB clock runs at 100Mhz, the value of sysinfo.freqOPB is 100000001,
> so this code configures MDC clock to 3.3Mhz instead of less than 2.5Hz.
> This is not a problem on Canyonlands, where Marvell PHY MDIO clock can run
> at 8.3Mhz. However other PHYs may actually require MDIO clock to be less
> than 2.5Mhz.
>
> How about rewriting this code as
>
>     freq = sysinfo.freqOPB / 1000000;
>     if (freq <= 50)
>     else if (freq <= 66)
>        mode_reg |= EMAC_M1_OBCI_66;
>     else if (freq <= 83)
>        mode_reg |= EMAC_M1_OBCI_83;
>     else if (freq <= 100)
>        mode_reg |= EMAC_M1_OBCI_100;
>     else
>         mode_reg |= EMAC_M1_OBCI_GT100;

Looks good to me. Do you plan to send a patch for this?

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================