| Message ID | 1716932220-3622-1-git-send-email-Tristram.Ha@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: phy: micrel: fix KSZ9477 PHY issues after suspend/resume | expand |
On Tue, 2024-05-28 at 14:37 -0700, Tristram.Ha@microchip.com wrote: > CAUTION: This email originated from outside of the organization. Do > not click links or open attachments unless you recognize the sender > and know the content is safe. > > From: Tristram Ha <tristram.ha@microchip.com> > > When the PHY is powered up after powered down most of the registers > are > reset, so the PHY setup code needs to be done again. In addition the > interrupt register will need to be setup again so that link status > indication works again. > > Fixes: 26dd2974c5b5 ("net: phy: micrel: Move KSZ9477 errata fixes to > PHY driver") > Signed-off-by: Tristram Ha <tristram.ha@microchip.com> > --- > drivers/net/phy/micrel.c | 60 +++++++++++++++++++++++++++++++++++--- > -- > 1 file changed, 53 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > index 2b8f8b7f1517..902d9a262c8a 100644 > --- a/drivers/net/phy/micrel.c > +++ b/drivers/net/phy/micrel.c > @@ -1918,7 +1918,7 @@ static const struct ksz9477_errata_write > ksz9477_errata_writes[] = { > {0x01, 0x75, 0x0060}, > {0x01, 0xd3, 0x7777}, > {0x1c, 0x06, 0x3008}, > - {0x1c, 0x08, 0x2000}, > + {0x1c, 0x08, 0x2001}, This change wasn't mentioned in the commit message, but I see that the latest errata sheet says "The value of this register may read back as either 0x2000 or 0x2001. Bit 0 is read-only, and is not a fixed value." > > /* Transmit waveform amplitude can be improved (1000BASE-T, > 100BASE-TX, 10BASE-Te) */ > {0x1c, 0x04, 0x00d0}, > @@ -1939,7 +1939,7 @@ static const struct ksz9477_errata_write > ksz9477_errata_writes[] = { > {0x1c, 0x20, 0xeeee}, > }; > > -static int ksz9477_config_init(struct phy_device *phydev) > +static int ksz9477_phy_errata(struct phy_device *phydev) > { > int err; > int i; > @@ -1967,16 +1967,26 @@ static int ksz9477_config_init(struct > phy_device *phydev) > return err; > } > > + return err; > +} > + > +static int ksz9477_config_init(struct phy_device *phydev) > +{ > + int err; > + > + /* Only KSZ9897 family of switches needs this fix. */ > + if ((phydev->phy_id & 0xf) == 1) { > + err = ksz9477_phy_errata(phydev); > + if (err) > + return err; > + } > + > /* According to KSZ9477 Errata DS80000754C (Module 4) all EEE > modes > * in this switch shall be regarded as broken. > */ > if (phydev->dev_flags & MICREL_NO_EEE) > phydev->eee_broken_modes = -1; > > - err = genphy_restart_aneg(phydev); > - if (err) > - return err; > - ksz9477_phy_errata is setting the PHY for 100 Mbps speed with auto- negotiation off, as the errata sheet requests before applying the errata writes, so it seems wrong to remove this auto-negotiation restart here as it would presumably be left in that state otherwise. Maybe it should be moved into that function instead? > return kszphy_config_init(phydev); > } > > @@ -2085,6 +2095,42 @@ static int kszphy_resume(struct phy_device > *phydev) > return 0; > } > > +static int ksz9477_resume(struct phy_device *phydev) > +{ > + int ret; > + > + /* No need to initialize registers if not powered down. */ > + ret = phy_read(phydev, MII_BMCR); > + if (ret < 0) > + return ret; > + if (!(ret & BMCR_PDOWN)) > + return 0; > + > + genphy_resume(phydev); > + > + /* After switching from power-down to normal mode, an > internal global > + * reset is automatically generated. Wait a minimum of 1 ms > before > + * read/write access to the PHY registers. > + */ > + usleep_range(1000, 2000); > + > + /* Only KSZ9897 family of switches needs this fix. */ > + if ((phydev->phy_id & 0xf) == 1) { > + ret = ksz9477_phy_errata(phydev); > + if (ret) > + return ret; > + } > + > + /* Enable PHY Interrupts */ > + if (phy_interrupt_is_valid(phydev)) { > + phydev->interrupts = PHY_INTERRUPT_ENABLED; > + if (phydev->drv->config_intr) > + phydev->drv->config_intr(phydev); > + } > + > + return 0; > +} > + > static int kszphy_probe(struct phy_device *phydev) > { > const struct kszphy_type *type = phydev->drv->driver_data; > @@ -5493,7 +5539,7 @@ static struct phy_driver ksphy_driver[] = { > .config_intr = kszphy_config_intr, > .handle_interrupt = kszphy_handle_interrupt, > .suspend = genphy_suspend, > - .resume = genphy_resume, > + .resume = ksz9477_resume, > .get_features = ksz9477_get_features, > } }; > > -- > 2.34.1 >
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index 2b8f8b7f1517..902d9a262c8a 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -1918,7 +1918,7 @@ static const struct ksz9477_errata_write ksz9477_errata_writes[] = { {0x01, 0x75, 0x0060}, {0x01, 0xd3, 0x7777}, {0x1c, 0x06, 0x3008}, - {0x1c, 0x08, 0x2000}, + {0x1c, 0x08, 0x2001}, /* Transmit waveform amplitude can be improved (1000BASE-T, 100BASE-TX, 10BASE-Te) */ {0x1c, 0x04, 0x00d0}, @@ -1939,7 +1939,7 @@ static const struct ksz9477_errata_write ksz9477_errata_writes[] = { {0x1c, 0x20, 0xeeee}, }; -static int ksz9477_config_init(struct phy_device *phydev) +static int ksz9477_phy_errata(struct phy_device *phydev) { int err; int i; @@ -1967,16 +1967,26 @@ static int ksz9477_config_init(struct phy_device *phydev) return err; } + return err; +} + +static int ksz9477_config_init(struct phy_device *phydev) +{ + int err; + + /* Only KSZ9897 family of switches needs this fix. */ + if ((phydev->phy_id & 0xf) == 1) { + err = ksz9477_phy_errata(phydev); + if (err) + return err; + } + /* According to KSZ9477 Errata DS80000754C (Module 4) all EEE modes * in this switch shall be regarded as broken. */ if (phydev->dev_flags & MICREL_NO_EEE) phydev->eee_broken_modes = -1; - err = genphy_restart_aneg(phydev); - if (err) - return err; - return kszphy_config_init(phydev); } @@ -2085,6 +2095,42 @@ static int kszphy_resume(struct phy_device *phydev) return 0; } +static int ksz9477_resume(struct phy_device *phydev) +{ + int ret; + + /* No need to initialize registers if not powered down. */ + ret = phy_read(phydev, MII_BMCR); + if (ret < 0) + return ret; + if (!(ret & BMCR_PDOWN)) + return 0; + + genphy_resume(phydev); + + /* After switching from power-down to normal mode, an internal global + * reset is automatically generated. Wait a minimum of 1 ms before + * read/write access to the PHY registers. + */ + usleep_range(1000, 2000); + + /* Only KSZ9897 family of switches needs this fix. */ + if ((phydev->phy_id & 0xf) == 1) { + ret = ksz9477_phy_errata(phydev); + if (ret) + return ret; + } + + /* Enable PHY Interrupts */ + if (phy_interrupt_is_valid(phydev)) { + phydev->interrupts = PHY_INTERRUPT_ENABLED; + if (phydev->drv->config_intr) + phydev->drv->config_intr(phydev); + } + + return 0; +} + static int kszphy_probe(struct phy_device *phydev) { const struct kszphy_type *type = phydev->drv->driver_data; @@ -5493,7 +5539,7 @@ static struct phy_driver ksphy_driver[] = { .config_intr = kszphy_config_intr, .handle_interrupt = kszphy_handle_interrupt, .suspend = genphy_suspend, - .resume = genphy_resume, + .resume = ksz9477_resume, .get_features = ksz9477_get_features, } };