| Message ID | 20221216144910.1416322-1-enguerrand.de-ribaucourt@savoirfairelinux.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: lan78xx: isolate LAN88XX specific operations | expand |
On Fri, 2022-12-16 at 15:49 +0100, Enguerrand de Ribaucourt wrote: > Some operations during the cable switch workaround modify the register > LAN88XX_INT_MASK of the PHY. However, this register is specific to the > LAN8835 PHY. For instance, if a DP8322I PHY is connected to the LAN7801, > that register (0x19), corresponds to the LED and MAC address > configuration, resulting in unapropriate behavior. > > Fixes: 14437e3fa284 ("lan78xx: workaround of forced 100 Full/Half duplex mode error") AFACS LAN7801 support was introduced after the above commit, so I guess 89b36fb5e532 ("lan78xx: Lan7801 Support for Fixed PHY") should be a better fixes tag. > Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> > --- > drivers/net/usb/lan78xx.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c > index f18ab8e220db..ea0a56e6cd40 100644 > --- a/drivers/net/usb/lan78xx.c > +++ b/drivers/net/usb/lan78xx.c > @@ -2116,6 +2116,7 @@ static void lan78xx_link_status_change(struct net_device *net) > { > struct phy_device *phydev = net->phydev; > int temp; > + bool lan88_fixup; > > /* At forced 100 F/H mode, chip may fail to set mode correctly > * when cable is switched between long(~50+m) and short one. > @@ -2123,10 +2124,15 @@ static void lan78xx_link_status_change(struct net_device *net) > * at forced 100 F/H mode. > */ > if (!phydev->autoneg && (phydev->speed == 100)) { > - /* disable phy interrupt */ > - temp = phy_read(phydev, LAN88XX_INT_MASK); > - temp &= ~LAN88XX_INT_MASK_MDINTPIN_EN_; > - phy_write(phydev, LAN88XX_INT_MASK, temp); > + lan88_fixup = (PHY_LAN8835 & 0xfffffff0) == > + (phydev->phy_id & 0xfffffff0); > + > + if(lan88_fixup) { > + /* disable phy interrupt */ > + temp = phy_read(phydev, LAN88XX_INT_MASK); > + temp &= ~LAN88XX_INT_MASK_MDINTPIN_EN_; > + phy_write(phydev, LAN88XX_INT_MASK, temp); > + } Why don't you use instead something alike: phy_config_interrupt(phydev, 0); //force 10 and 100 speed phy_config_interrupt(phydev, 1); so that you properly keep interrupt disabled regardless of the chip version? Thanks! Paolo
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index f18ab8e220db..ea0a56e6cd40 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2116,6 +2116,7 @@ static void lan78xx_link_status_change(struct net_device *net) { struct phy_device *phydev = net->phydev; int temp; + bool lan88_fixup; /* At forced 100 F/H mode, chip may fail to set mode correctly * when cable is switched between long(~50+m) and short one. @@ -2123,10 +2124,15 @@ static void lan78xx_link_status_change(struct net_device *net) * at forced 100 F/H mode. */ if (!phydev->autoneg && (phydev->speed == 100)) { - /* disable phy interrupt */ - temp = phy_read(phydev, LAN88XX_INT_MASK); - temp &= ~LAN88XX_INT_MASK_MDINTPIN_EN_; - phy_write(phydev, LAN88XX_INT_MASK, temp); + lan88_fixup = (PHY_LAN8835 & 0xfffffff0) == + (phydev->phy_id & 0xfffffff0); + + if(lan88_fixup) { + /* disable phy interrupt */ + temp = phy_read(phydev, LAN88XX_INT_MASK); + temp &= ~LAN88XX_INT_MASK_MDINTPIN_EN_; + phy_write(phydev, LAN88XX_INT_MASK, temp); + } temp = phy_read(phydev, MII_BMCR); temp &= ~(BMCR_SPEED100 | BMCR_SPEED1000); @@ -2134,13 +2140,15 @@ static void lan78xx_link_status_change(struct net_device *net) temp |= BMCR_SPEED100; phy_write(phydev, MII_BMCR, temp); /* set to 100 later */ - /* clear pending interrupt generated while workaround */ - temp = phy_read(phydev, LAN88XX_INT_STS); + if(lan88_fixup) { + /* clear pending interrupt generated while workaround */ + temp = phy_read(phydev, LAN88XX_INT_STS); - /* enable phy interrupt back */ - temp = phy_read(phydev, LAN88XX_INT_MASK); - temp |= LAN88XX_INT_MASK_MDINTPIN_EN_; - phy_write(phydev, LAN88XX_INT_MASK, temp); + /* enable phy interrupt back */ + temp = phy_read(phydev, LAN88XX_INT_MASK); + temp |= LAN88XX_INT_MASK_MDINTPIN_EN_; + phy_write(phydev, LAN88XX_INT_MASK, temp); + } } }
Some operations during the cable switch workaround modify the register LAN88XX_INT_MASK of the PHY. However, this register is specific to the LAN8835 PHY. For instance, if a DP8322I PHY is connected to the LAN7801, that register (0x19), corresponds to the LED and MAC address configuration, resulting in unapropriate behavior. Fixes: 14437e3fa284 ("lan78xx: workaround of forced 100 Full/Half duplex mode error") Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> --- drivers/net/usb/lan78xx.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)