| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <13cc6020cab02aba5848de43c8653a159d47a367.camel@calian.com>
Date: Tue, 28 May 2024 22:37:04 +0000
From: Robert Hancock <robert.hancock@...ian.com>
To: "linux@...linux.org.uk" <linux@...linux.org.uk>,
"Tristram.Ha@...rochip.com" <Tristram.Ha@...rochip.com>,
"hkallweit1@...il.com" <hkallweit1@...il.com>,
"andrew@...n.ch"
<andrew@...n.ch>
CC: "olteanv@...il.com" <olteanv@...il.com>,
"davem@...emloft.net"
<davem@...emloft.net>,
"vivien.didelot@...il.com" <vivien.didelot@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"pabeni@...hat.com"
<pabeni@...hat.com>,
"f.fainelli@...il.com" <f.fainelli@...il.com>,
"UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org"
<kuba@...nel.org>
Subject: Re: [PATCH net] net: phy: micrel: fix KSZ9477 PHY issues after
suspend/resume
On Tue, 2024-05-28 at 14:37 -0700, Tristram.Ha@...rochip.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@...rochip.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@...rochip.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
>
Powered by blists - more mailing lists