diff mbox

net: phy: unmask link partner capabilities

Message ID 1392999496-29753-1-git-send-email-cristian.bercaru@freescale.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

cristian.bercaru@freescale.com Feb. 21, 2014, 4:18 p.m. UTC 
From: Cristian Bercaru <cristian.bercaru@freescale.com>

Masking the link partner's capabilities with local capabilities can be
misleading in autonegotiation scenarios such as PAUSE frame
autonegotiation.
Therefore, this patch unmasks the link partner's capabilities.

Signed-off-by: Cristian Bercaru <cristian.bercaru@freescale.com>
---
 drivers/net/phy/phy_device.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Florian Fainelli Feb. 21, 2014, 7:24 p.m. UTC | #1 
Hi,

2014-02-21 8:18 GMT-08:00  <cristian.bercaru@freescale.com>:
> From: Cristian Bercaru <cristian.bercaru@freescale.com>
>
> Masking the link partner's capabilities with local capabilities can be
> misleading in autonegotiation scenarios such as PAUSE frame
> autonegotiation.
> Therefore, this patch unmasks the link partner's capabilities.

You have only covered the lpa register here, is lpagb also affected?
It might make sense to cache (lpa & adv) in an intermediate variable
to help clarifiying the checks.

>
> Signed-off-by: Cristian Bercaru <cristian.bercaru@freescale.com>
> ---
>  drivers/net/phy/phy_device.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index a70b604..e57825e6 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -967,8 +967,6 @@ int genphy_read_status(struct phy_device *phydev)
>                 if (adv < 0)
>                         return adv;
>
> -               lpa &= adv;
> -
>                 phydev->speed = SPEED_10;
>                 phydev->duplex = DUPLEX_HALF;
>                 phydev->pause = 0;
> @@ -979,13 +977,13 @@ int genphy_read_status(struct phy_device *phydev)
>
>                         if (lpagb & LPA_1000FULL)
>                                 phydev->duplex = DUPLEX_FULL;
> -               } else if (lpa & (LPA_100FULL | LPA_100HALF)) {
> +               } else if (lpa & adv & (LPA_100FULL | LPA_100HALF)) {
>                         phydev->speed = SPEED_100;
>
> -                       if (lpa & LPA_100FULL)
> +                       if (lpa & adv & LPA_100FULL)
>                                 phydev->duplex = DUPLEX_FULL;
>                 } else
> -                       if (lpa & LPA_10FULL)
> +                       if (lpa & adv & LPA_10FULL)
>                                 phydev->duplex = DUPLEX_FULL;
>
>                 if (phydev->duplex == DUPLEX_FULL) {
> --
> 1.7.11.7
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a70b604..e57825e6 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -967,8 +967,6 @@  int genphy_read_status(struct phy_device *phydev)
 		if (adv < 0)
 			return adv;
 
-		lpa &= adv;
-
 		phydev->speed = SPEED_10;
 		phydev->duplex = DUPLEX_HALF;
 		phydev->pause = 0;
@@ -979,13 +977,13 @@  int genphy_read_status(struct phy_device *phydev)
 
 			if (lpagb & LPA_1000FULL)
 				phydev->duplex = DUPLEX_FULL;
-		} else if (lpa & (LPA_100FULL | LPA_100HALF)) {
+		} else if (lpa & adv & (LPA_100FULL | LPA_100HALF)) {
 			phydev->speed = SPEED_100;
 
-			if (lpa & LPA_100FULL)
+			if (lpa & adv & LPA_100FULL)
 				phydev->duplex = DUPLEX_FULL;
 		} else
-			if (lpa & LPA_10FULL)
+			if (lpa & adv & LPA_10FULL)
 				phydev->duplex = DUPLEX_FULL;
 
 		if (phydev->duplex == DUPLEX_FULL) {