| Message ID | b317787e1e55f9c59c55a3cf5f9f02d477dd5a59.1676490952.git.yuiko.oshino@microchip.com |
|---|---|
| State | New |
| Headers | show |
| Series | net:usb:lan78xx: move LAN7800 internal phy register accesses to the phy driver. | expand |
>-----Original Message----- >From: Andrew Lunn <andrew@lunn.ch> >Sent: Thursday, February 16, 2023 10:09 AM >To: Yuiko Oshino - C18177 <Yuiko.Oshino@microchip.com> >Cc: enguerrand.de-ribaucourt@savoirfairelinux.com; Woojung Huh - C21699 ><Woojung.Huh@microchip.com>; hkallweit1@gmail.com; netdev@vger.kernel.org; >pabeni@redhat.com; davem@davemloft.net; UNGLinuxDriver ><UNGLinuxDriver@microchip.com>; linux@armlinux.org.uk; >edumazet@google.com; linux-usb@vger.kernel.org; kuba@kernel.org >Subject: Re: [PATCH net 1/2] net:usb:lan78xx: fix accessing the LAN7800's >internal phy specific registers from the MAC driver > >EXTERNAL EMAIL: Do not click links or open attachments unless you know the >content is safe > >On Thu, Feb 16, 2023 at 07:20:53AM -0700, Yuiko Oshino wrote: >> Move the LAN7800 internal phy (phy ID 0x0007c132) specific register accesses >to the phy driver (microchip.c). >> >> Fixes: 14437e3fa284f465dbbc8611fd4331ca8d60e986 ("lan78xx: workaround >> of forced 100 Full/Half duplex mode error") > >I would not say this is a fix which needs putting into stable. The architecture is >wrong to do it in the MAC driver, but i don't think it causes real issues? > >So please submit this to net-next, not net. > > Andrew Hi Andrew, Enguerrand reported the below on 20 Dec 2022, therefore, submitting to net. " 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." Thank you. Yuiko
> Hi Andrew, > > Enguerrand reported the below on 20 Dec 2022, therefore, submitting to net. > > " 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." O.K. So please include in the commit message this information. Then it becomes clear it really is a fix. Also, you did not add a fixes: tag to the second patch. There is a danger the first patch gets back ported, but not the second. Since you are just moving code around, i suggest you have just have one patch. Andrew
>-----Original Message----- >From: Andrew Lunn <andrew@lunn.ch> >Sent: Thursday, February 16, 2023 11:56 AM >To: Yuiko Oshino - C18177 <Yuiko.Oshino@microchip.com> >Cc: enguerrand.de-ribaucourt@savoirfairelinux.com; Woojung Huh - C21699 ><Woojung.Huh@microchip.com>; hkallweit1@gmail.com; netdev@vger.kernel.org; >pabeni@redhat.com; davem@davemloft.net; UNGLinuxDriver ><UNGLinuxDriver@microchip.com>; linux@armlinux.org.uk; >edumazet@google.com; linux-usb@vger.kernel.org; kuba@kernel.org >Subject: Re: [PATCH net 1/2] net:usb:lan78xx: fix accessing the LAN7800's >internal phy specific registers from the MAC driver > >EXTERNAL EMAIL: Do not click links or open attachments unless you know the >content is safe > >> Hi Andrew, >> >> Enguerrand reported the below on 20 Dec 2022, therefore, submitting to net. >> >> " 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." > >O.K. > >So please include in the commit message this information. Then it becomes clear it >really is a fix. > >Also, you did not add a fixes: tag to the second patch. There is a danger the first >patch gets back ported, but not the second. Since you are just moving code around, >i suggest you have just have one patch. > > Andrew No problem. One patch would be easier! I will add the comment from Enguerrand. I will do v2 tomorrow. Thank you. Yuiko
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index f18ab8e220db..068488890d57 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2115,33 +2115,8 @@ static void lan78xx_remove_mdio(struct lan78xx_net *dev) static void lan78xx_link_status_change(struct net_device *net) { struct phy_device *phydev = net->phydev; - int temp; - - /* At forced 100 F/H mode, chip may fail to set mode correctly - * when cable is switched between long(~50+m) and short one. - * As workaround, set to 10 before setting to 100 - * 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); - temp = phy_read(phydev, MII_BMCR); - temp &= ~(BMCR_SPEED100 | BMCR_SPEED1000); - phy_write(phydev, MII_BMCR, temp); /* set to 10 first */ - 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); - - /* enable phy interrupt back */ - temp = phy_read(phydev, LAN88XX_INT_MASK); - temp |= LAN88XX_INT_MASK_MDINTPIN_EN_; - phy_write(phydev, LAN88XX_INT_MASK, temp); - } + phy_print_status(phydev); } static int irq_map(struct irq_domain *d, unsigned int irq,
Move the LAN7800 internal phy (phy ID 0x0007c132) specific register accesses to the phy driver (microchip.c). Fixes: 14437e3fa284f465dbbc8611fd4331ca8d60e986 ("lan78xx: workaround of forced 100 Full/Half duplex mode error") Signed-off-by: Yuiko Oshino <yuiko.oshino@microchip.com> --- drivers/net/usb/lan78xx.c | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-)