From: Tristram Ha <[email protected]>
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 <[email protected]>
---
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},
/* 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,
} };
--
2.34.1
On Tue, 2024-05-28 at 14:37 -0700, [email protected] 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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>