net: phy: Correctly handle MII ioctl which changes autonegotiation.
Subsystems (3)
PATCH 1/1 net: phy: Correctly handle MII ioctl which changes autonegotiation.
When advertised capabilities are changed with mii-tool, such as:
mii-tool -A 10baseT
the existing handler has two errors.
- An actual PHY register value is provided by mii-tool, and this must be mapped to internal state with mii_adv_to_ethtool_adv_t().
- The PHY state machine needs to be told that autonegotiation has again been performed. If not, the MAC will not be notified of the new link speed and duplex, resulting in a possible config mismatch.
--- drivers/net/phy/phy.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-)
drivers/net/phy/phy.c +24 -12
@@ -352,6 +352,7 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd) {struct mii_ioctl_data *mii_data =if_mii (ifr);u16 val = mii_data->val_in; +int change_autoneg =0 ;switch (cmd) {case SIOCGMIIPHY :
@@ -367,22 +368,29 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)if (mii_data->phy_id == phydev->addr) {switch (mii_data->reg_num) {case MII_BMCR : -if ((val & (BMCR_RESET |BMCR_ANENABLE )) ==0 ) +if ((val & (BMCR_RESET |BMCR_ANENABLE )) ==0 ) { +if (phydev->autoneg ==AUTONEG_ENABLE ) + change_autoneg =1 ; phydev->autoneg =AUTONEG_DISABLE ; -else +if (val &BMCR_FULLDPLX ) + phydev->duplex =DUPLEX_FULL ; +else + phydev->duplex =DUPLEX_HALF ; +if (val &BMCR_SPEED1000 ) + phydev->speed =SPEED_1000 ; +else if (val &BMCR_SPEED100 ) + phydev->speed =SPEED_100 ; +else phydev->speed =SPEED_10 ; + } +else { +if (phydev->autoneg ==AUTONEG_DISABLE ) + change_autoneg =1 ; phydev->autoneg =AUTONEG_ENABLE ; -if (!phydev->autoneg && (val &BMCR_FULLDPLX )) - phydev->duplex =DUPLEX_FULL ; -else - phydev->duplex =DUPLEX_HALF ; -if (!phydev->autoneg && (val &BMCR_SPEED1000 )) - phydev->speed =SPEED_1000 ; -else if (!phydev->autoneg && - (val &BMCR_SPEED100 )) - phydev->speed =SPEED_100 ; + }break ;case MII_ADVERTISE : - phydev->advertising = val; + phydev->advertising =mii_adv_to_ethtool_adv_t (val); + change_autoneg =1 ;break ;default :/* do nothing */
@@ -396,6 +404,10 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)if (mii_data->reg_num ==MII_BMCR && val &BMCR_RESET )return phy_init_hw (phydev); + +if (change_autoneg) +return phy_start_aneg (phydev); +return 0 ;case SIOCSHWTSTAMP :
Signature · 1 line
1.7.9.5
Hi Brian,
2014-11-10 11:18 GMT-08:00 Brian Hill <[email protected]>:
Quoted 6 lines
When advertised capabilities are changed with mii-tool, such as:
mii-tool -A 10baseT
the existing handler has two errors.
Quoted 6 lines
- An actual PHY register value is provided by mii-tool, and this must be mapped to internal state with
mii_adv_to_ethtool_adv_t().
- The PHY state machine needs to be told that autonegotiation has again been performed. If not, the MAC will not be notified of the new link speed and duplex, resulting in a possible config mismatch.
Looks very good to me, just one minor thing I did not notice initially. Once you fix that, feel free to resubmit with:
Quoted 11 lines
Signed-off-by Brian Hill <[email protected]>--- drivers/net/phy/phy.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-)drivers/net/phy/phy.c +0
-0
drivers/net/phy/phy.c int phy_mii_ioctl(struct phy_device *phydev, struct · line 356
Quoted 5 lines
drivers/net/phy/phy.c +1
-0@@ -352,6 +352,7 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd) {struct mii_ioctl_data *mii_data =if_mii (ifr);u16 val = mii_data->val_in; +int change_autoneg =0 ;
This should be a "bool autoneg = false;" here
Quoted 65 lines
drivers/net/phy/phy.c +19
-12switch (cmd) {case SIOCGMIIPHY :@@ -367,22 +368,29 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd) if (mii_data->phy_id == phydev->addr) {switch (mii_data->reg_num) {case MII_BMCR : -if ((val & (BMCR_RESET |BMCR_ANENABLE )) ==0 ) +if ((val & (BMCR_RESET |BMCR_ANENABLE )) ==0 ) { +if (phydev->autoneg ==AUTONEG_ENABLE ) + change_autoneg =1 ; phydev->autoneg =AUTONEG_DISABLE ; -else +if (val &BMCR_FULLDPLX ) + phydev->duplex =DUPLEX_FULL ; +else + phydev->duplex =DUPLEX_HALF ; +if (val &BMCR_SPEED1000 ) + phydev->speed =SPEED_1000 ; +else if (val &BMCR_SPEED100 ) + phydev->speed =SPEED_100 ; +else phydev->speed =SPEED_10 ; + } +else { +if (phydev->autoneg ==AUTONEG_DISABLE ) + change_autoneg =1 ; phydev->autoneg =AUTONEG_ENABLE ; -if (!phydev->autoneg && (val &BMCR_FULLDPLX )) - phydev->duplex =DUPLEX_FULL ; -else - phydev->duplex =DUPLEX_HALF ; -if (!phydev->autoneg && (val &BMCR_SPEED1000 )) - phydev->speed =SPEED_1000 ; -else if (!phydev->autoneg && - (val &BMCR_SPEED100 )) - phydev->speed =SPEED_100 ; + }break ;case MII_ADVERTISE : - phydev->advertising = val; + phydev->advertising =mii_adv_to_ethtool_adv_t (val); + change_autoneg =1 ;break ;default :/* do nothing */drivers/net/phy/phy.c +4
-0@@ -396,6 +404,10 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd) if (mii_data->reg_num == MII_BMCR && val & BMCR_RESET) return phy_init_hw(phydev); + + if (change_autoneg) + return phy_start_aneg(phydev); +return 0 ;case SIOCSHWTSTAMP:
--
1.7.9.5--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 1 line
Florian