From 83f71ef55866d77c8e84c99d4c06c55836b7820a Mon Sep 17 00:00:00 2001 From: James Byrne Date: Mon, 4 Mar 2019 17:40:33 +0000 Subject: [PATCH] net: phy: micrel: Use correct skew values on KSZ9021 Commit ff7bd212cb8a ("net: phy: micrel: fix divisor value for KSZ9031 phy skew") fixed the skew value divisor for the KSZ9031, but left the code using the same divisor for the KSZ9021, which is incorrect. The preceding commit c16e69f702b1 ("net: phy: micrel: add documentation for Micrel KSZ90x1 binding") added the DTS documentation for the KSZ90x1, changing it from the equivalent file in the Linux kernel to correctly state that for this part the skew value is set in 120ps steps, whereas the Linux documentation and driver continue to this day to use the incorrect value of 200 that came from the original KSZ9021 datasheet before it was corrected in revision 1.2 (Feb 2014). This commit sorts out the resulting confusion in a consistent way by making the following changes: - Update the documentation to be clear about what the skew values mean, in the same was as for the KSZ9031. - Update the Micrel PHY driver to select the appropriate divisor for both parts. - Adjust all the device trees that state skew values for KSZ9021 PHYs to use values based on 120ps steps instead of 200ps steps. This will result in the same values being programmed into the skew registers as the equivalent device trees in the Linux kernel do, where it incorrectly uses 200ps steps (since that's where all these device trees were copied from). Signed-off-by: James Byrne Acked-by: Joe Hershberger --- arch/arm/dts/sama5d3xcm.dtsi | 32 ++++++++++++------------- arch/arm/dts/sama5d3xcm_cmp.dtsi | 32 ++++++++++++------------- arch/arm/dts/socfpga_arria5_socdk.dts | 4 ++-- arch/arm/dts/socfpga_cyclone5_is1.dts | 4 ++-- arch/arm/dts/socfpga_cyclone5_socdk.dts | 4 ++-- arch/arm/dts/socfpga_cyclone5_sockit.dts | 4 ++-- arch/arm/dts/socfpga_cyclone5_vining_fpga.dts | 4 ++-- doc/device-tree-bindings/net/micrel-ksz90x1.txt | 27 +++++++++++++++++++++ drivers/net/phy/micrel_ksz90x1.c | 14 +++++++---- 9 files changed, 79 insertions(+), 46 deletions(-) diff --git a/arch/arm/dts/sama5d3xcm.dtsi b/arch/arm/dts/sama5d3xcm.dtsi index 2cf9c36..d123057 100644 --- a/arch/arm/dts/sama5d3xcm.dtsi +++ b/arch/arm/dts/sama5d3xcm.dtsi @@ -44,28 +44,28 @@ reg = <0x1>; interrupt-parent = <&pioB>; interrupts = <25 IRQ_TYPE_EDGE_FALLING>; - txen-skew-ps = <800>; - txc-skew-ps = <3000>; - rxdv-skew-ps = <400>; - rxc-skew-ps = <3000>; - rxd0-skew-ps = <400>; - rxd1-skew-ps = <400>; - rxd2-skew-ps = <400>; - rxd3-skew-ps = <400>; + txen-skew-ps = <480>; + txc-skew-ps = <1800>; + rxdv-skew-ps = <240>; + rxc-skew-ps = <1800>; + rxd0-skew-ps = <240>; + rxd1-skew-ps = <240>; + rxd2-skew-ps = <240>; + rxd3-skew-ps = <240>; }; ethernet-phy@7 { reg = <0x7>; interrupt-parent = <&pioB>; interrupts = <25 IRQ_TYPE_EDGE_FALLING>; - txen-skew-ps = <800>; - txc-skew-ps = <3000>; - rxdv-skew-ps = <400>; - rxc-skew-ps = <3000>; - rxd0-skew-ps = <400>; - rxd1-skew-ps = <400>; - rxd2-skew-ps = <400>; - rxd3-skew-ps = <400>; + txen-skew-ps = <480>; + txc-skew-ps = <1800>; + rxdv-skew-ps = <240>; + rxc-skew-ps = <1800>; + rxd0-skew-ps = <240>; + rxd1-skew-ps = <240>; + rxd2-skew-ps = <240>; + rxd3-skew-ps = <240>; }; }; }; diff --git a/arch/arm/dts/sama5d3xcm_cmp.dtsi b/arch/arm/dts/sama5d3xcm_cmp.dtsi index 77638c3..332b057 100644 --- a/arch/arm/dts/sama5d3xcm_cmp.dtsi +++ b/arch/arm/dts/sama5d3xcm_cmp.dtsi @@ -43,28 +43,28 @@ reg = <0x1>; interrupt-parent = <&pioB>; interrupts = <25 IRQ_TYPE_EDGE_FALLING>; - txen-skew-ps = <800>; - txc-skew-ps = <3000>; - rxdv-skew-ps = <400>; - rxc-skew-ps = <3000>; - rxd0-skew-ps = <400>; - rxd1-skew-ps = <400>; - rxd2-skew-ps = <400>; - rxd3-skew-ps = <400>; + txen-skew-ps = <480>; + txc-skew-ps = <1800>; + rxdv-skew-ps = <240>; + rxc-skew-ps = <1800>; + rxd0-skew-ps = <240>; + rxd1-skew-ps = <240>; + rxd2-skew-ps = <240>; + rxd3-skew-ps = <240>; }; ethernet-phy@7 { reg = <0x7>; interrupt-parent = <&pioB>; interrupts = <25 IRQ_TYPE_EDGE_FALLING>; - txen-skew-ps = <800>; - txc-skew-ps = <3000>; - rxdv-skew-ps = <400>; - rxc-skew-ps = <3000>; - rxd0-skew-ps = <400>; - rxd1-skew-ps = <400>; - rxd2-skew-ps = <400>; - rxd3-skew-ps = <400>; + txen-skew-ps = <480>; + txc-skew-ps = <1800>; + rxdv-skew-ps = <240>; + rxc-skew-ps = <1800>; + rxd0-skew-ps = <240>; + rxd1-skew-ps = <240>; + rxd2-skew-ps = <240>; + rxd3-skew-ps = <240>; }; }; diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts b/arch/arm/dts/socfpga_arria5_socdk.dts index 90e676e..fa972e2 100644 --- a/arch/arm/dts/socfpga_arria5_socdk.dts +++ b/arch/arm/dts/socfpga_arria5_socdk.dts @@ -67,9 +67,9 @@ rxd2-skew-ps = <0>; rxd3-skew-ps = <0>; txen-skew-ps = <0>; - txc-skew-ps = <2600>; + txc-skew-ps = <1560>; rxdv-skew-ps = <0>; - rxc-skew-ps = <2000>; + rxc-skew-ps = <1200>; }; &gpio0 { diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts b/arch/arm/dts/socfpga_cyclone5_is1.dts index 2d31412..a769498 100644 --- a/arch/arm/dts/socfpga_cyclone5_is1.dts +++ b/arch/arm/dts/socfpga_cyclone5_is1.dts @@ -43,9 +43,9 @@ rxd2-skew-ps = <0>; rxd3-skew-ps = <0>; txen-skew-ps = <0>; - txc-skew-ps = <2600>; + txc-skew-ps = <1560>; rxdv-skew-ps = <0>; - rxc-skew-ps = <2000>; + rxc-skew-ps = <1200>; }; &gpio1 { diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts b/arch/arm/dts/socfpga_cyclone5_socdk.dts index 6f138b2..95c7619 100644 --- a/arch/arm/dts/socfpga_cyclone5_socdk.dts +++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts @@ -71,9 +71,9 @@ rxd2-skew-ps = <0>; rxd3-skew-ps = <0>; txen-skew-ps = <0>; - txc-skew-ps = <2600>; + txc-skew-ps = <1560>; rxdv-skew-ps = <0>; - rxc-skew-ps = <2000>; + rxc-skew-ps = <1200>; }; &gpio0 { diff --git a/arch/arm/dts/socfpga_cyclone5_sockit.dts b/arch/arm/dts/socfpga_cyclone5_sockit.dts index c155ff0..90669cd 100644 --- a/arch/arm/dts/socfpga_cyclone5_sockit.dts +++ b/arch/arm/dts/socfpga_cyclone5_sockit.dts @@ -128,9 +128,9 @@ rxd2-skew-ps = <0>; rxd3-skew-ps = <0>; txen-skew-ps = <0>; - txc-skew-ps = <2600>; + txc-skew-ps = <1560>; rxdv-skew-ps = <0>; - rxc-skew-ps = <2000>; + rxc-skew-ps = <1200>; }; &gpio0 { /* GPIO 0..29 */ diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts index 355b3db..ac57f41 100644 --- a/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts +++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga.dts @@ -85,9 +85,9 @@ rxd2-skew-ps = <0>; rxd3-skew-ps = <0>; txen-skew-ps = <0>; - txc-skew-ps = <2600>; + txc-skew-ps = <1560>; rxdv-skew-ps = <0>; - rxc-skew-ps = <2000>; + rxc-skew-ps = <1200>; }; }; }; diff --git a/doc/device-tree-bindings/net/micrel-ksz90x1.txt b/doc/device-tree-bindings/net/micrel-ksz90x1.txt index 307f53f..a214d35 100644 --- a/doc/device-tree-bindings/net/micrel-ksz90x1.txt +++ b/doc/device-tree-bindings/net/micrel-ksz90x1.txt @@ -14,6 +14,33 @@ KSZ9021: value is 0, the maximum value is 1800, and it is incremented by 120ps steps. + The KSZ9021 hardware supports a range of skew values from negative to + positive, where the specific range is property dependent. All values + specified in the devicetree are offset by the minimum value so they + can be represented as positive integers in the devicetree since it's + difficult to represent a negative number in the devictree. + + The following 4-bit values table applies to all the skew properties: + + Pad Skew Value Delay (ps) Devicetree Value + ------------------------------------------------------ + 0000 -840ps 0 + 0001 -720ps 120 + 0010 -600ps 240 + 0011 -480ps 360 + 0100 -360ps 480 + 0101 -240ps 600 + 0110 -120ps 720 + 0111 0ps 840 + 1000 120ps 960 + 1001 240ps 1080 + 1010 360ps 1200 + 1011 480ps 1320 + 1100 600ps 1440 + 1101 720ps 1560 + 1110 840ps 1680 + 1111 960ps 1800 + Optional properties: - rxc-skew-ps : Skew control of RXC pad diff --git a/drivers/net/phy/micrel_ksz90x1.c b/drivers/net/phy/micrel_ksz90x1.c index 63e7b02..1f8d86a 100644 --- a/drivers/net/phy/micrel_ksz90x1.c +++ b/drivers/net/phy/micrel_ksz90x1.c @@ -33,10 +33,14 @@ #define CTRL1000_CONFIG_MASTER (1 << 11) #define CTRL1000_MANUAL_CONFIG (1 << 12) +#define KSZ9021_PS_TO_REG 120 + /* KSZ9031 PHY Registers */ #define MII_KSZ9031_MMD_ACCES_CTRL 0x0d #define MII_KSZ9031_MMD_REG_DATA 0x0e +#define KSZ9031_PS_TO_REG 60 + static int ksz90xx_startup(struct phy_device *phydev) { unsigned phy_ctl; @@ -102,11 +106,11 @@ static const struct ksz90x1_reg_field ksz9031_clk_grp[] = { }; static int ksz90x1_of_config_group(struct phy_device *phydev, - struct ksz90x1_ofcfg *ofcfg) + struct ksz90x1_ofcfg *ofcfg, + int ps_to_regval) { struct udevice *dev = phydev->dev; struct phy_driver *drv = phydev->drv; - const int ps_to_regval = 60; int val[4]; int i, changed = 0, offset, max; u16 regval = 0; @@ -148,7 +152,8 @@ static int ksz9021_of_config(struct phy_device *phydev) int i, ret = 0; for (i = 0; i < ARRAY_SIZE(ofcfg); i++) { - ret = ksz90x1_of_config_group(phydev, &(ofcfg[i])); + ret = ksz90x1_of_config_group(phydev, &ofcfg[i], + KSZ9021_PS_TO_REG); if (ret) return ret; } @@ -167,7 +172,8 @@ static int ksz9031_of_config(struct phy_device *phydev) int i, ret = 0; for (i = 0; i < ARRAY_SIZE(ofcfg); i++) { - ret = ksz90x1_of_config_group(phydev, &(ofcfg[i])); + ret = ksz90x1_of_config_group(phydev, &ofcfg[i], + KSZ9031_PS_TO_REG); if (ret) return ret; } -- 2.7.4