diff mbox series

[1/3] net: ethernet: stmmac: dwmac-rk: Disable delayline if it is invalid

Message ID 20220623162850.245608-2-sebastian.reichel@collabora.com
State New
Headers show
Series [1/3] net: ethernet: stmmac: dwmac-rk: Disable delayline if it is invalid | expand

Commit Message

Sebastian Reichel June 23, 2022, 4:28 p.m. UTC 
From: David Wu <david.wu@rock-chips.com>

Explicitly disable delayline if it is no value is configured in DT.

Signed-off-by: David Wu <david.wu@rock-chips.com>
[rebase]
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 52 +++++++++----------
 1 file changed, 25 insertions(+), 27 deletions(-)

Comments

Sebastian Reichel June 24, 2022, 8:15 p.m. UTC | #1 
Hi,

On Fri, Jun 24, 2022 at 07:37:08PM +0200, Andrew Lunn wrote:
> > The Rockchip hardware supports enabling/disabling delays and
> > configuring a delay value of 0x00-0x7f. For the RK3588 evaluation
> > board the RX delay should be disabled.
> 
> So you can just put 0 in DT then.

My understanding is, that there is a difference between
enabled delay with a delay value of 0 and delay fully
disabled. But I could not find any details aboout this
in the datasheet unfortunately.

-- Sebastian
Peter Geis June 24, 2022, 11:43 p.m. UTC | #2 
On Fri, Jun 24, 2022 at 4:16 PM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hi,
>
> On Fri, Jun 24, 2022 at 07:37:08PM +0200, Andrew Lunn wrote:
> > > The Rockchip hardware supports enabling/disabling delays and
> > > configuring a delay value of 0x00-0x7f. For the RK3588 evaluation
> > > board the RX delay should be disabled.
> >
> > So you can just put 0 in DT then.
>
> My understanding is, that there is a difference between
> enabled delay with a delay value of 0 and delay fully
> disabled. But I could not find any details aboout this
> in the datasheet unfortunately.

The driver already sets the delays to 0 in case of the rgmii-id modes.
0 is disabled, even in this patch. The only thing this patch does is
change the behavior when the delays are not set. If the rx delays
should be 0, they should be defined as 0 in the device tree. There is
rgmii-rxid for a reason as well, but if they are setting the rx delay
to 0 with rgmii that implies this hardware is fundamentally broken.

Very Respectfully,
Peter Geis

>
> -- Sebastian
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index c469abc91fa1..56ccd4fbd6c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -75,8 +75,16 @@  struct rk_priv_data {
 #define GRF_CLR_BIT(nr)	(BIT(nr+16))
 
 #define DELAY_ENABLE(soc, tx, rx) \
-	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) | \
-	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
+	((((tx) >= 0) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) | \
+	 (((rx) >= 0) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
+
+#define DELAY_ENABLE_BY_ID(soc, tx, rx, id) \
+	((((tx) >= 0) ? soc##_GMAC_TXCLK_DLY_ENABLE(id) : soc##_GMAC_TXCLK_DLY_DISABLE(id)) | \
+	 (((rx) >= 0) ? soc##_GMAC_RXCLK_DLY_ENABLE(id) : soc##_GMAC_RXCLK_DLY_DISABLE(id)))
+
+#define DELAY_VALUE(soc, tx, rx) \
+	((((tx) >= 0) ? soc##_GMAC_CLK_TX_DL_CFG(tx) : 0) | \
+	 (((rx) >= 0) ? soc##_GMAC_CLK_RX_DL_CFG(rx) : 0))
 
 #define PX30_GRF_GMAC_CON1		0x0904
 
@@ -179,8 +187,7 @@  static void rk3128_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3128_GMAC_RMII_MODE_CLR);
 	regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON0,
 		     DELAY_ENABLE(RK3128, tx_delay, rx_delay) |
-		     RK3128_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3128_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3128, tx_delay, rx_delay));
 }
 
 static void rk3128_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -296,8 +303,7 @@  static void rk3228_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     DELAY_ENABLE(RK3228, tx_delay, rx_delay));
 
 	regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON0,
-		     RK3228_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3228_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3128, tx_delay, rx_delay));
 }
 
 static void rk3228_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -417,8 +423,7 @@  static void rk3288_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3288_GMAC_RMII_MODE_CLR);
 	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON3,
 		     DELAY_ENABLE(RK3288, tx_delay, rx_delay) |
-		     RK3288_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3288_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3288, tx_delay, rx_delay));
 }
 
 static void rk3288_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -579,12 +584,10 @@  static void rk3328_set_to_rgmii(struct rk_priv_data *bsp_priv,
 	regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
 		     RK3328_GMAC_PHY_INTF_SEL_RGMII |
 		     RK3328_GMAC_RMII_MODE_CLR |
-		     RK3328_GMAC_RXCLK_DLY_ENABLE |
-		     RK3328_GMAC_TXCLK_DLY_ENABLE);
+		     DELAY_ENABLE(RK3328, tx_delay, rx_delay));
 
 	regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON0,
-		     RK3328_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3328_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3328, tx_delay, rx_delay));
 }
 
 static void rk3328_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -709,8 +712,7 @@  static void rk3366_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3366_GMAC_RMII_MODE_CLR);
 	regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON7,
 		     DELAY_ENABLE(RK3366, tx_delay, rx_delay) |
-		     RK3366_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3366_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3366, tx_delay, rx_delay));
 }
 
 static void rk3366_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -820,8 +822,7 @@  static void rk3368_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3368_GMAC_RMII_MODE_CLR);
 	regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON16,
 		     DELAY_ENABLE(RK3368, tx_delay, rx_delay) |
-		     RK3368_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3368_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3368, tx_delay, rx_delay));
 }
 
 static void rk3368_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -931,8 +932,7 @@  static void rk3399_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3399_GMAC_RMII_MODE_CLR);
 	regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON6,
 		     DELAY_ENABLE(RK3399, tx_delay, rx_delay) |
-		     RK3399_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3399_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3399, tx_delay, rx_delay));
 }
 
 static void rk3399_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -1037,13 +1037,11 @@  static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
 				     RK3568_GRF_GMAC0_CON1;
 
 	regmap_write(bsp_priv->grf, con0,
-		     RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
-		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+		     DELAY_VALUE(RK3568, tx_delay, rx_delay));
 
 	regmap_write(bsp_priv->grf, con1,
 		     RK3568_GMAC_PHY_INTF_SEL_RGMII |
-		     RK3568_GMAC_RXCLK_DLY_ENABLE |
-		     RK3568_GMAC_TXCLK_DLY_ENABLE);
+		     DELAY_ENABLE(RK3568, tx_delay, rx_delay));
 }
 
 static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
@@ -1422,7 +1420,7 @@  static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 
 	ret = of_property_read_u32(dev->of_node, "tx_delay", &value);
 	if (ret) {
-		bsp_priv->tx_delay = 0x30;
+		bsp_priv->tx_delay = -1;
 		dev_err(dev, "Can not read property: tx_delay.");
 		dev_err(dev, "set tx_delay to 0x%x\n",
 			bsp_priv->tx_delay);
@@ -1433,7 +1431,7 @@  static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 
 	ret = of_property_read_u32(dev->of_node, "rx_delay", &value);
 	if (ret) {
-		bsp_priv->rx_delay = 0x10;
+		bsp_priv->rx_delay = -1;
 		dev_err(dev, "Can not read property: rx_delay.");
 		dev_err(dev, "set rx_delay to 0x%x\n",
 			bsp_priv->rx_delay);
@@ -1507,15 +1505,15 @@  static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 		break;
 	case PHY_INTERFACE_MODE_RGMII_ID:
 		dev_info(dev, "init for RGMII_ID\n");
-		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, 0);
+		bsp_priv->ops->set_to_rgmii(bsp_priv, -1, -1);
 		break;
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 		dev_info(dev, "init for RGMII_RXID\n");
-		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay, 0);
+		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay, -1);
 		break;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		dev_info(dev, "init for RGMII_TXID\n");
-		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, bsp_priv->rx_delay);
+		bsp_priv->ops->set_to_rgmii(bsp_priv, -1, bsp_priv->rx_delay);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
 		dev_info(dev, "init for RMII\n");