[PATCH] net: phy: TI DP83869 fix invalid clock delay configuration

Frank de Brabander debrabander at gmail.com
Fri Oct 6 14:24:39 CEST 2023


Setting the clock delay from the device tree settings
rx-internal-delay-ps and tx-internal-delay-ps was broken:

 - The expected value in the device tree is suppose to be a
   delay in picoseconds, but the driver only allowed an array index.
 - Driver converted this array index to the actual delay in
   picoseconds and tried to apply this in the device register. This
   however is not a valid register value. The actual logic here was
   reversed, it converted an register representation of the delay to
   the device tree delay in picoseconds.

Only when the internal delays were NOT configured in the device tree
and they default value of 7 (=2000ps) was used, a valid value was
loaded in the register.

Signed-off-by: Frank de Brabander <debrabander at gmail.com>
---
 drivers/net/phy/dp83869.c | 53 +++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 8d32d73b07..f9d4782580 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -81,7 +81,10 @@
 
 /* RGMIIDCTL bits */
 #define DP83869_RGMII_TX_CLK_DELAY_SHIFT	4
-#define DP83869_CLK_DELAY_DEF				7
+#define DP83869_CLK_DELAY_STEP			250
+#define DP83869_CLK_DELAY_MIN			250
+#define DP83869_CLK_DELAY_MAX			4000
+#define DP83869_CLK_DELAY_DEFAULT		2000
 
 /* CFG2 bits */
 #define MII_DP83869_CFG2_SPEEDOPT_10EN		0x0040
@@ -157,10 +160,6 @@ static int dp83869_config_port_mirroring(struct phy_device *phydev)
 	return 0;
 }
 
-static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
-					     1750, 2000, 2250, 2500, 2750, 3000,
-					     3250, 3500, 3750, 4000};
-
 static int dp83869_set_strapped_mode(struct phy_device *phydev)
 {
 	struct dp83869_private *dp83869 = phydev->priv;
@@ -183,7 +182,6 @@ static int dp83869_set_strapped_mode(struct phy_device *phydev)
 static int dp83869_of_init(struct phy_device *phydev)
 {
 	struct dp83869_private * const dp83869 = phydev->priv;
-	const int delay_entries = ARRAY_SIZE(dp83869_internal_delay);
 	int ret;
 	ofnode node;
 
@@ -238,32 +236,45 @@ static int dp83869_of_init(struct phy_device *phydev)
 	dp83869->tx_fifo_depth = ofnode_read_s32_default(node, "tx-fifo-depth",
 							 DP83869_PHYCR_FIFO_DEPTH_4_B_NIB);
 
+	/* Internal clock delay values can be configured in steps of
+	 * 250ps (0.25ns). The register field for clock delay is 4-bits wide,
+	 * the values range from 0b0000 for 0.25ns to 0b1111 for 4ns.
+	 */
+
 	/* RX delay *must* be specified if internal delay of RX is used. */
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
-		dp83869->rx_int_delay = ofnode_read_u32_default(node, "rx-internal-delay-ps",
-								DP83869_CLK_DELAY_DEF);
-		if (dp83869->rx_int_delay > delay_entries) {
-			dp83869->rx_int_delay = DP83869_CLK_DELAY_DEF;
-			pr_debug("rx-internal-delay-ps not set/invalid, default to %ups\n",
-				 dp83869_internal_delay[dp83869->rx_int_delay]);
+		dp83869->rx_int_delay = ofnode_read_u32_default(node,
+			"rx-internal-delay-ps", DP83869_CLK_DELAY_DEFAULT);
+		if (dp83869->rx_int_delay > DP83869_CLK_DELAY_MAX ||
+		    dp83869->rx_int_delay < DP83869_CLK_DELAY_MIN ||
+		    dp83869->rx_int_delay % DP83869_CLK_DELAY_STEP) {
+			dp83869->rx_int_delay = DP83869_CLK_DELAY_DEFAULT;
+			pr_warn("rx-internal-delay-ps not set/invalid, default"
+				" to %ups\n", DP83869_CLK_DELAY_DEFAULT);
 		}
 
-		dp83869->rx_int_delay = dp83869_internal_delay[dp83869->rx_int_delay];
+		dp83869->rx_int_delay =
+			(dp83869->rx_int_delay - DP83869_CLK_DELAY_STEP)
+			/ DP83869_CLK_DELAY_STEP;
 	}
 
-	/* TX delay *must* be specified if internal delay of RX is used. */
+	/* TX delay *must* be specified if internal delay of TX is used. */
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
-		dp83869->tx_int_delay = ofnode_read_u32_default(node, "tx-internal-delay-ps",
-								DP83869_CLK_DELAY_DEF);
-		if (dp83869->tx_int_delay > delay_entries) {
-			dp83869->tx_int_delay = DP83869_CLK_DELAY_DEF;
-			pr_debug("tx-internal-delay-ps not set/invalid, default to %ups\n",
-				 dp83869_internal_delay[dp83869->tx_int_delay]);
+		dp83869->tx_int_delay = ofnode_read_u32_default(node,
+			"tx-internal-delay-ps", DP83869_CLK_DELAY_DEFAULT);
+		if (dp83869->tx_int_delay > DP83869_CLK_DELAY_MAX ||
+		    dp83869->tx_int_delay < DP83869_CLK_DELAY_MIN ||
+		    dp83869->tx_int_delay % DP83869_CLK_DELAY_STEP) {
+			dp83869->tx_int_delay = DP83869_CLK_DELAY_DEFAULT;
+			pr_warn("tx-internal-delay-ps not set/invalid, default"
+				" to %ups\n", DP83869_CLK_DELAY_DEFAULT);
 		}
 
-		dp83869->tx_int_delay = dp83869_internal_delay[dp83869->tx_int_delay];
+		dp83869->tx_int_delay =
+			(dp83869->tx_int_delay - DP83869_CLK_DELAY_STEP)
+			/ DP83869_CLK_DELAY_STEP;
 	}
 
 	return 0;
-- 
2.30.2



More information about the U-Boot mailing list