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

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.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 drivers/net/phy/phy.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

On Mon, Nov 14, 2011 at 10:24 PM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@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@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

Hi,

2011/11/15 Andy Fleming <afleming@gmail.com>:
> On Mon, Nov 14, 2011 at 10:24 PM, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@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.

This may be unable to acquire the right value despite Extended
Capability bit of
Extended register was "1".
Moreover, all may be 1 even if it can read a register.
#  And this may be a bug of hardware. :-(
Since this check was not work currently, I added.

>
>
>>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@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...
>

OK, I investigate more about this problem.

>
>> +                       if (gblpa == 0xFFFF | gblpa == -1) {
>
> You've used bitwise OR, instead of ||

Oh, thanks.
I fixed.

Best regards,
  Nobuhiro