Re: [PATCH net-next] net: dsa: microchip: ksz9477: follow errata sheet when applying fixups

[Robert Hancock]

On Mon, 2023-06-19 at 10:16 +0200, Rasmus Villemoes 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.
> 
> The errata sheets for both ksz9477 and ksz9567 begin with
> 
>   IMPORTANT NOTE
> 
>   Multiple errata workarounds in this document call for changing PHY
>   registers for each PHY port. PHY registers 0x0 to 0x1F are in the
>   address range 0xN100 to 0xN13F, while indirect (MMD) PHY registers
>   are accessed via the PHY MMD Setup Register and the PHY MMD Data
>   Register.
> 
>   Before configuring the PHY MMD registers, it is necessary to set
> the
>   PHY to 100 Mbps speed with auto-negotiation disabled by writing to
>   register 0xN100-0xN101. After writing the MMD registers, and after
>   all errata workarounds that involve PHY register settings, write
>   register 0xN100-0xN101 again to enable and restart auto-
> negotiation.
> 
> Without that explicit auto-neg restart, we do sometimes have problems
> establishing link.
> 
> Rather than writing back the hardcoded 0x1340 value the errata sheet
> suggests (which likely just corresponds to the most common strap
> configuration), restore the original value, setting the
> PORT_AUTO_NEG_RESTART bit if PORT_AUTO_NEG_ENABLE is set.
> 
> Fixes: 1fc33199185d ("net: dsa: microchip: Add PHY errata
> workarounds")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> ---
> While I do believe this is a fix, I don't think it's post-rc7
> material, hence targeting net-next with cc stable.

I don't think this will apply to net-next as the relevant code has been
moved to the Micrel PHY driver and removed from this one in the
following commits, and effectively the same change to disable autoneg
before the register writes and re-enable afterwards was incorporated:

commit 26dd2974c5b5caef358784530c9e72715adc8f5b
Author: Robert Hancock <robert.hancock@xxxxxxxxxx>
Date:   Mon Jun 5 09:39:42 2023 -0600

    net: phy: micrel: Move KSZ9477 errata fixes to PHY driver

commit 6068e6d7ba5001dfb96bb8b7b92e2ed2a5877786
Author: Robert Hancock <robert.hancock@xxxxxxxxxx>
Date:   Mon Jun 5 09:39:43 2023 -0600

    net: dsa: microchip: remove KSZ9477 PHY errata handling

However, your patch may be reasonable to apply to -rc7 or stable as a
more targeted change for those releases.

> 
>  drivers/net/dsa/microchip/ksz9477.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c
> b/drivers/net/dsa/microchip/ksz9477.c
> index bf13d47c26cf..9a712ea71ee7 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -902,6 +902,16 @@ static void ksz9477_port_mmd_write(struct
> ksz_device *dev, int port,
> 
>  static void ksz9477_phy_errata_setup(struct ksz_device *dev, int
> port)
>  {
> +       u16 cr;
> +
> +       /* Errata document says the PHY must be configured to 100Mbps
> +        * with auto-neg disabled before configuring the PHY MMD
> +        * registers.
> +        */
> +       ksz_pread16(dev, port, REG_PORT_PHY_CTRL, &cr);
> +       ksz_pwrite16(dev, port, REG_PORT_PHY_CTRL,
> +                    PORT_SPEED_100MBIT | PORT_FULL_DUPLEX);
> +
>         /* Apply PHY settings to address errata listed in
>          * KSZ9477, KSZ9897, KSZ9896, KSZ9567, KSZ8565
>          * Silicon Errata and Data Sheet Clarification documents:
> @@ -943,6 +953,13 @@ static void ksz9477_phy_errata_setup(struct
> ksz_device *dev, int port)
>         ksz9477_port_mmd_write(dev, port, 0x1c, 0x1d, 0xe7ff);
>         ksz9477_port_mmd_write(dev, port, 0x1c, 0x1e, 0xefff);
>         ksz9477_port_mmd_write(dev, port, 0x1c, 0x20, 0xeeee);
> +
> +       /* Restore PHY CTRL register, restart auto-negotiation if
> +        * enabled in the original value.
> +        */
> +       if (cr & PORT_AUTO_NEG_ENABLE)
> +               cr |= PORT_AUTO_NEG_RESTART;
> +       ksz_pwrite16(dev, port, REG_PORT_PHY_CTRL, cr);
>  }
> 
>  void ksz9477_get_caps(struct ksz_device *dev, int port,
> --
> 2.37.2
> 

-- 
Robert Hancock <robert.hancock@xxxxxxxxxx>