[net] net: phy: fix flag masking in __set_phy_supported

Message ID c0ca1da0-2b55-19c0-708a-ccab9a97ec5c@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series
  • [net] net: phy: fix flag masking in __set_phy_supported
Related show

Commit Message

Heiner Kallweit July 3, 2018, 8:34 p.m.
Currently also the pause flags are removed from phydev->supported because
they're not included in PHY_DEFAULT_FEATURES. I don't think this is
intended, especially when considering that this function can be called
via phy_set_max_speed() anywhere in a driver. Change the masking to mask
out only the values we're going to change. In addition remove the
misleading comment, job of this small function is just to adjust the
supported and advertised speeds.

Fixes: f3a6bd393c2c ("phylib: Add phy_set_max_speed helper")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Andrew Lunn July 4, 2018, 2:13 p.m. | #1
On Tue, Jul 03, 2018 at 10:34:54PM +0200, Heiner Kallweit wrote:
> Currently also the pause flags are removed from phydev->supported because
> they're not included in PHY_DEFAULT_FEATURES. I don't think this is
> intended, especially when considering that this function can be called
> via phy_set_max_speed() anywhere in a driver. Change the masking to mask
> out only the values we're going to change. In addition remove the
> misleading comment, job of this small function is just to adjust the
> supported and advertised speeds.
> 
> Fixes: f3a6bd393c2c ("phylib: Add phy_set_max_speed helper")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Florian Fainelli July 4, 2018, 7:58 p.m. | #2
On July 3, 2018 10:34:54 PM GMT+02:00, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>Currently also the pause flags are removed from phydev->supported
>because
>they're not included in PHY_DEFAULT_FEATURES. I don't think this is
>intended, especially when considering that this function can be called
>via phy_set_max_speed() anywhere in a driver. Change the masking to
>mask
>out only the values we're going to change. In addition remove the
>misleading comment, job of this small function is just to adjust the
>supported and advertised speeds.
>
>Fixes: f3a6bd393c2c ("phylib: Add phy_set_max_speed helper")
>Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
David Miller July 5, 2018, 12:48 a.m. | #3
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 3 Jul 2018 22:34:54 +0200

> Currently also the pause flags are removed from phydev->supported because
> they're not included in PHY_DEFAULT_FEATURES. I don't think this is
> intended, especially when considering that this function can be called
> via phy_set_max_speed() anywhere in a driver. Change the masking to mask
> out only the values we're going to change. In addition remove the
> misleading comment, job of this small function is just to adjust the
> supported and advertised speeds.
> 
> Fixes: f3a6bd393c2c ("phylib: Add phy_set_max_speed helper")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied and queued up for -stable.

Patch

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index bd0f339f..b9f5f40a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1724,11 +1724,8 @@  EXPORT_SYMBOL(genphy_loopback);
 
 static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
-	/* The default values for phydev->supported are provided by the PHY
-	 * driver "features" member, we want to reset to sane defaults first
-	 * before supporting higher speeds.
-	 */
-	phydev->supported &= PHY_DEFAULT_FEATURES;
+	phydev->supported &= ~(PHY_1000BT_FEATURES | PHY_100BT_FEATURES |
+			       PHY_10BT_FEATURES);
 
 	switch (max_speed) {
 	default: