Re: [PATCH] ARM: mach-imx6q: add ksz9131rn_phy_fixup

Russell King - ARM Linux admin
2020-03-05T13:54:11Z

On Thu, Mar 05, 2020 at 02:49:28PM +0100, Philippe Schenker wrote:
> The MAC of the i.MX6 SoC is compliant with RGMII v1.3. The KSZ9131 PHY
> is like KSZ9031 adhering to RGMII v2.0 specification. This means the
> MAC should provide a delay to the TXC line. Because the i.MX6 MAC does
> not provide this delay this has to be done in the PHY.
>
> This patch adds by default ~1.6ns delay to the TXC line. This should
> be good for all boards that have the RGMII signals routed with the
> same length.
>
> The KSZ9131 has relatively high tolerances on skew registers from
> MMD 2.4 to MMD 2.8. Therefore the new DLL-based delay of 2ns is used
> and then as little as possibly subtracted from that so we get more
> accurate delay. This is actually needed because the i.MX6 SoC has
> an asynchron skew on TXC from -100ps to 900ps, to get all RGMII
> values within spec.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>
> ---
>
> arch/arm/mach-imx/mach-imx6q.c | 37 ++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index edd26e0ffeec..8ae5f2fa33e2 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -61,6 +61,14 @@ static void mmd_write_reg(struct phy_device *dev, int device, int reg, int val)
> phy_write(dev, 0x0e, val);
> }
>
> +static int mmd_read_reg(struct phy_device *dev, int device, int reg)
> +{
> + phy_write(dev, 0x0d, device);
> + phy_write(dev, 0x0e, reg);
> + phy_write(dev, 0x0d, (1 << 14) | device);
> + return phy_read(dev, 0x0e);
> +}

These look like the standard MII MMD registers, and it also looks like
you're reinventing phy_read_mmd() - but badly due to lack of locking.

I guess you need this because phy_read_mmd() may be modular - maybe
we should arrange for the accessors to be separately buildable into
the kernel, so that such fixups can stop badly reinventing the wheel?

> +
> static int ksz9031rn_phy_fixup(struct phy_device *dev)
> {
> /*
> @@ -74,6 +82,33 @@ static int ksz9031rn_phy_fixup(struct phy_device *dev)
> return 0;
> }
>
> +#define KSZ9131_RXTXDLL_BYPASS 12
> +
> +static int ksz9131rn_phy_fixup(struct phy_device *dev)
> +{
> + int tmp;
> +
> + tmp = mmd_read_reg(dev, 2, 0x4c);
> + /* disable rxdll bypass (enable 2ns skew delay on RXC) */
> + tmp &= ~(1 << KSZ9131_RXTXDLL_BYPASS);
> + mmd_write_reg(dev, 2, 0x4c, tmp);
> +
> + tmp = mmd_read_reg(dev, 2, 0x4d);
> + /* disable txdll bypass (enable 2ns skew delay on TXC) */
> + tmp &= ~(1 << KSZ9131_RXTXDLL_BYPASS);
> + mmd_write_reg(dev, 2, 0x4d, tmp);
> +
> + /*
> + * Subtract ~0.6ns from txdll = ~1.4ns delay.
> + * leave RXC path untouched
> + */
> + mmd_write_reg(dev, 2, 4, 0x007d);
> + mmd_write_reg(dev, 2, 6, 0xdddd);
> + mmd_write_reg(dev, 2, 8, 0x0007);
> +
> + return 0;
> +}
> +
> /*
> * fixup for PLX PEX8909 bridge to configure GPIO1-7 as output High
> * as they are used for slots1-7 PERST#
> @@ -167,6 +202,8 @@ static void __init imx6q_enet_phy_init(void)
> ksz9021rn_phy_fixup);
> phy_register_fixup_for_uid(PHY_ID_KSZ9031, MICREL_PHY_ID_MASK,
> ksz9031rn_phy_fixup);
> + phy_register_fixup_for_uid(PHY_ID_KSZ9131, MICREL_PHY_ID_MASK,
> + ksz9131rn_phy_fixup);
> phy_register_fixup_for_uid(PHY_ID_AR8031, 0xffffffef,
> ar8031_phy_fixup);
> phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
> --
> 2.25.1
>
>

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up