[U-Boot] [PATCH 1/2] net: phy: micrel: Use correct skew values on KSZ9021

[U-Boot] [PATCH 1/2] net: phy: micrel: Use correct skew values on KSZ9021

[James Byrne]

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 <james.byrne@origamienergy.com>

---

 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 +--
 .../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 2cf9c3611d..d123057f30 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 at 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 77638c3cbe..332b057e0a 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 at 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 90e676e701..fa972e287f 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 b7054bfd5a..31fa8dcfa6 100644
--- a/arch/arm/dts/socfpga_cyclone5_is1.dts
+++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
@@ -46,9 +46,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 6f138b2b26..95c7619b8d 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 c155ff02eb..90669cde45 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 355b3dbf43..ac57f41cb5 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 307f53f726..a214d35fc9 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 63e7b0242b..1f8d86ab2e 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.17.1

[U-Boot] [PATCH 2/2] net: phy: micrel: Find Micrel PHY node correctly

[James Byrne]

In some of the device trees that specify skew values for KSZ90x1 PHYs
the values are stored (incorrectly) in the MAC node, whereas in others
it is in an 'ethernet-phy' subnode. Previously the code would fail to
find and program these skew values, so this commit changes it to look
for an "ethernet-phy" subnode first, and revert to looking in the MAC
node if there isn't one.

The device trees affected (where the skew values are in a subnode) are
imx6qdl-icore-rqs.dtsi, r8a77970-eagle.dts, r8a77990-ebisu.dts,
r8a77995-draak.dts, salvator-common.dtsi, sama5d3xcm.dtsi,
sama5d3xcm_cmp.dtsi, socfpga_cyclone5_vining_fpga.dts,
socfpga_stratix10_socdk.dts and ulcb.dtsi. Before this change the skew
values in these device trees would be ignored.

The device trees where the skew values are in the MAC node are
socfpga_arria10_socdk.dtsi, socfpga_arria5_socdk.dts,
socfpga_cyclone5_de0_nano_soc.dts, socfpga_cyclone5_de10_nano.dts,
socfpga_cyclone5_de1_soc.dts, socfpga_cyclone5_is1.dts,
socfpga_cyclone5_socdk.dts, socfpga_cyclone5_sockit.dts. These should be
unaffected by this change.

The changes were tested on a sama5d3xcm.

Signed-off-by: James Byrne <james.byrne@origamienergy.com>
---

 drivers/net/phy/micrel_ksz90x1.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel_ksz90x1.c b/drivers/net/phy/micrel_ksz90x1.c
index 1f8d86ab2e..8dec9f2367 100644
--- a/drivers/net/phy/micrel_ksz90x1.c
+++ b/drivers/net/phy/micrel_ksz90x1.c
@@ -114,12 +114,20 @@ static int ksz90x1_of_config_group(struct phy_device *phydev,
 	int val[4];
 	int i, changed = 0, offset, max;
 	u16 regval = 0;
+	ofnode node;
 
 	if (!drv || !drv->writeext)
 		return -EOPNOTSUPP;
 
+	/* Look for a PHY node under the Ethernet node */
+	node = dev_read_subnode(dev, "ethernet-phy");
+	if (!ofnode_valid(node)) {
+		/* No node found, look in the Ethernet node */
+		node = dev_ofnode(dev);
+	}
+
 	for (i = 0; i < ofcfg->grpsz; i++) {
-		val[i] = dev_read_u32_default(dev, ofcfg->grp[i].name, ~0);
+		val[i] = ofnode_read_u32_default(node, ofcfg->grp[i].name, ~0);
 		offset = ofcfg->grp[i].off;
 		if (val[i] == -1) {
 			/* Default register value for KSZ9021 */
-- 
2.17.1

[U-Boot] [PATCH 1/2] net: phy: micrel: Use correct skew values on KSZ9021

[Joe Hershberger]

On Mon, Mar 4, 2019 at 11:46 AM James Byrne
<james.byrne@origamienergy.com> wrote:
>
> 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 <james.byrne@origamienergy.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 2/2] net: phy: micrel: Find Micrel PHY node correctly

[Joe Hershberger]

On Mon, Mar 4, 2019 at 11:47 AM James Byrne
<james.byrne@origamienergy.com> wrote:
>
> In some of the device trees that specify skew values for KSZ90x1 PHYs
> the values are stored (incorrectly) in the MAC node, whereas in others
> it is in an 'ethernet-phy' subnode. Previously the code would fail to
> find and program these skew values, so this commit changes it to look
> for an "ethernet-phy" subnode first, and revert to looking in the MAC
> node if there isn't one.
>
> The device trees affected (where the skew values are in a subnode) are
> imx6qdl-icore-rqs.dtsi, r8a77970-eagle.dts, r8a77990-ebisu.dts,
> r8a77995-draak.dts, salvator-common.dtsi, sama5d3xcm.dtsi,
> sama5d3xcm_cmp.dtsi, socfpga_cyclone5_vining_fpga.dts,
> socfpga_stratix10_socdk.dts and ulcb.dtsi. Before this change the skew
> values in these device trees would be ignored.
>
> The device trees where the skew values are in the MAC node are
> socfpga_arria10_socdk.dtsi, socfpga_arria5_socdk.dts,
> socfpga_cyclone5_de0_nano_soc.dts, socfpga_cyclone5_de10_nano.dts,
> socfpga_cyclone5_de1_soc.dts, socfpga_cyclone5_is1.dts,
> socfpga_cyclone5_socdk.dts, socfpga_cyclone5_sockit.dts. These should be
> unaffected by this change.
>
> The changes were tested on a sama5d3xcm.
>
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] net: phy: micrel: Use correct skew values on KSZ9021

[Joe Hershberger]

Hi James,

https://patchwork.ozlabs.org/patch/1051234/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

[U-Boot] net: phy: micrel: Find Micrel PHY node correctly

[Joe Hershberger]

Hi James,

https://patchwork.ozlabs.org/patch/1051237/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe