[U-Boot] [PATCH] phy: Check return value for read MII_STAT1000 register

[Andy Fleming]

On Mon, Nov 14, 2011 at 10:24 PM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj at renesas.com> wrote:
> When Extended register is effective, there is not necessarily certainly
> register for 1000BASE. 0xFFFF may be able to be read although register
> is read. This adds this check.


I don't understand what you mean by this, and I suspect that this is
not the right fix for this.


>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj at renesas.com>
> ---
>  drivers/net/phy/phy.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 8da7688..f0b3c56 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -294,8 +294,14 @@ static int genphy_parse_link(struct phy_device *phydev)
>                         * both PHYs in the link
>                         */
>                        gblpa = phy_read(phydev, MDIO_DEVAD_NONE, MII_STAT1000);
> -                       gblpa &= phy_read(phydev,
> +                       /* If gblpa was 0xFFFF or -1, this chip does not have MII_STAT1000
> +                          register or read error. */

I'm fairly certain that this code is correct as-is. The MII_STAT1000
register must exist if the ERCAP bit is set. If, for some reason your
PHY doesn't conform to spec, you need to code up a phy-specific
parsing function. But I suspect that there's something else going on,
here...


> +                       if (gblpa == 0xFFFF | gblpa == -1) {

You've used bitwise OR, instead of ||

> +                               gblpa = 0;
> +                       } else {
> +                               gblpa &= phy_read(phydev,
>                                        MDIO_DEVAD_NONE, MII_CTRL1000) << 2;
> +                       }
>                }

Andy