diff mbox series

[v2,1/7] ARM: imx6q: remove PHY fixup for KSZ9031

Message ID 20210309112615.625-2-o.rempel@pengutronix.de (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series remove different PHY fixups | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Oleksij Rempel March 9, 2021, 11:26 a.m. UTC
Starting with:

    bcf3440c6dd7 ("net: phy: micrel: add phy-mode support for the KSZ9031 PHY")

the micrel phy driver started respecting phy-mode for the KSZ9031 PHY.
At least with kernel v5.8 configuration provided by this fixup was
overwritten by the micrel driver.

This fixup was providing following configuration:

RX path: 2.58ns delay
    rx -0.42 (left shift) + rx_clk  +0.96ns (right shift) =
        1,38 + 1,2 internal RX delay = 2.58ns
TX path: 0.96ns delay
    tx (no delay) + tx_clk 0.96ns (right shift) = 0.96ns

This configuration is outside of the recommended RGMII clock skew delays
and about in the middle of: rgmii-idrx and rgmii-id

Since most embedded systems do not have enough place to introduce
significant clock skew, rgmii-id is the way to go.

In case this patch breaks network functionality on your system, build
kernel with enabled MICREL_PHY. If it is still not working then try
following device tree options:
1. Set (or change) phy-mode in DT to:
   phy-mode = "rgmii-id";
   This actives internal delay for both RX and TX.
1. Set (or change) phy-mode in DT to:
   phy-mode = "rgmii-idrx";
   This actives internal delay for RX only.
3. Use following DT properties:
   phy-mode = "rgmii";
   txen-skew-psec = <0>;
   rxdv-skew-psec = <0>;
   rxd0-skew-psec = <0>;
   rxd1-skew-psec = <0>;
   rxd2-skew-psec = <0>;
   rxd3-skew-psec = <0>;
   rxc-skew-psec = <1860>;
   txc-skew-psec = <1860>;
   This activates the internal delays for RX and TX, with the value as
   the fixup that is removed in this patch.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Philippe Schenker <philippe.schenker@toradex.com>
---
 arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts |  2 +-
 arch/arm/mach-imx/mach-imx6q.c          | 23 -----------------------
 2 files changed, 1 insertion(+), 24 deletions(-)

Comments

Shawn Guo May 11, 2021, 1:49 a.m. UTC | #1
On Tue, Mar 09, 2021 at 12:26:09PM +0100, Oleksij Rempel wrote:
> Starting with:
> 
>     bcf3440c6dd7 ("net: phy: micrel: add phy-mode support for the KSZ9031 PHY")
> 
> the micrel phy driver started respecting phy-mode for the KSZ9031 PHY.
> At least with kernel v5.8 configuration provided by this fixup was
> overwritten by the micrel driver.
> 
> This fixup was providing following configuration:
> 
> RX path: 2.58ns delay
>     rx -0.42 (left shift) + rx_clk  +0.96ns (right shift) =
>         1,38 + 1,2 internal RX delay = 2.58ns
> TX path: 0.96ns delay
>     tx (no delay) + tx_clk 0.96ns (right shift) = 0.96ns
> 
> This configuration is outside of the recommended RGMII clock skew delays
> and about in the middle of: rgmii-idrx and rgmii-id
> 
> Since most embedded systems do not have enough place to introduce
> significant clock skew, rgmii-id is the way to go.
> 
> In case this patch breaks network functionality on your system, build
> kernel with enabled MICREL_PHY. If it is still not working then try
> following device tree options:
> 1. Set (or change) phy-mode in DT to:
>    phy-mode = "rgmii-id";
>    This actives internal delay for both RX and TX.
> 1. Set (or change) phy-mode in DT to:
>    phy-mode = "rgmii-idrx";
>    This actives internal delay for RX only.
> 3. Use following DT properties:
>    phy-mode = "rgmii";
>    txen-skew-psec = <0>;
>    rxdv-skew-psec = <0>;
>    rxd0-skew-psec = <0>;
>    rxd1-skew-psec = <0>;
>    rxd2-skew-psec = <0>;
>    rxd3-skew-psec = <0>;
>    rxc-skew-psec = <1860>;
>    txc-skew-psec = <1860>;
>    This activates the internal delays for RX and TX, with the value as
>    the fixup that is removed in this patch.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Acked-by: Philippe Schenker <philippe.schenker@toradex.com>
> ---
>  arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts |  2 +-
>  arch/arm/mach-imx/mach-imx6q.c          | 23 -----------------------

I have different branch for DTS and mach-imx change.  Please split the
changes.

Shawn

>  2 files changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
> index fa2307d8ce86..c713ac03b3b9 100644
> --- a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
> +++ b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
> @@ -112,7 +112,7 @@ flash: m25p80@0 {
>  &fec {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_enet>;
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";
>  	phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
>  	phy-supply = <&vgen2_1v2_eth>;
>  	status = "okay";
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index 703998ebb52e..78205f90da27 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -40,27 +40,6 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
>  	return 0;
>  }
>  
> -static void mmd_write_reg(struct phy_device *dev, int device, int reg, int val)
> -{
> -	phy_write(dev, 0x0d, device);
> -	phy_write(dev, 0x0e, reg);
> -	phy_write(dev, 0x0d, (1 << 14) | device);
> -	phy_write(dev, 0x0e, val);
> -}
> -
> -static int ksz9031rn_phy_fixup(struct phy_device *dev)
> -{
> -	/*
> -	 * min rx data delay, max rx/tx clock delay,
> -	 * min rx/tx control delay
> -	 */
> -	mmd_write_reg(dev, 2, 4, 0);
> -	mmd_write_reg(dev, 2, 5, 0);
> -	mmd_write_reg(dev, 2, 8, 0x003ff);
> -
> -	return 0;
> -}
> -
>  /*
>   * fixup for PLX PEX8909 bridge to configure GPIO1-7 as output High
>   * as they are used for slots1-7 PERST#
> @@ -152,8 +131,6 @@ static void __init imx6q_enet_phy_init(void)
>  	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>  		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>  				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_AR8031, 0xffffffef,
>  				ar8031_phy_fixup);
>  		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
> -- 
> 2.29.2
>
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
index fa2307d8ce86..c713ac03b3b9 100644
--- a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
+++ b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
@@ -112,7 +112,7 @@  flash: m25p80@0 {
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
 	phy-supply = <&vgen2_1v2_eth>;
 	status = "okay";
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 703998ebb52e..78205f90da27 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -40,27 +40,6 @@  static int ksz9021rn_phy_fixup(struct phy_device *phydev)
 	return 0;
 }
 
-static void mmd_write_reg(struct phy_device *dev, int device, int reg, int val)
-{
-	phy_write(dev, 0x0d, device);
-	phy_write(dev, 0x0e, reg);
-	phy_write(dev, 0x0d, (1 << 14) | device);
-	phy_write(dev, 0x0e, val);
-}
-
-static int ksz9031rn_phy_fixup(struct phy_device *dev)
-{
-	/*
-	 * min rx data delay, max rx/tx clock delay,
-	 * min rx/tx control delay
-	 */
-	mmd_write_reg(dev, 2, 4, 0);
-	mmd_write_reg(dev, 2, 5, 0);
-	mmd_write_reg(dev, 2, 8, 0x003ff);
-
-	return 0;
-}
-
 /*
  * fixup for PLX PEX8909 bridge to configure GPIO1-7 as output High
  * as they are used for slots1-7 PERST#
@@ -152,8 +131,6 @@  static void __init imx6q_enet_phy_init(void)
 	if (IS_BUILTIN(CONFIG_PHYLIB)) {
 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
 				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_AR8031, 0xffffffef,
 				ar8031_phy_fixup);
 		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,