Skip to contentSkip to conversation

net: stmmac: qcom-ethqos: cleanups and re-organise SerDes handling

RFC PATCH In review Russell King (Oracle) 5 participants 23 messages Feb 12 – Feb 17

Subsystems (8)
Networking Network drivers Arm Qualcomm mailing list Arm i.MX and Layerscape +4 more subsystems
lore
Share & tools
lore

Russell King (Oracle)

author
#0 lore raw

cover net: stmmac: qcom-ethqos: cleanups and re-organise SerDes handling

Hi,

As the last series had issues with stability, I've changed the approach in this series to concentrate on keeping much of the SerDes related code within the qcom-ethqos driver rather than trying to move it out at this stage. This means it should be possible to bisect these patches and pinpoint exactly the code movement that causes any instability.

This series starts with various cleanups to qcom-ethqos (the first four patches) before beginning to move code, passing phylink's phy interface (which will change) to the fix_mac_speed() method, and then using that to configure the serdes and inband setting before moving the SerDes code.

Please test this patch set, and let me know whether this works, or where it breaks.

Thanks.

 .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c    |  11 +-
 .../net/ethernet/stmicro/stmmac/dwmac-loongson.c   |   3 +-
 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 114 ++++++++++++---------
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c    |  11 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |   3 +-
 include/linux/stmmac.h                             |   3 +-
 8 files changed, 90 insertions(+), 61 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#1 lore raw

RFC PATCH 1/9 net: stmmac: qcom-ethqos: rename "por" memebers to "rgmii_por"

Rename the "por" and "num_por" members to indicate that they are for RGMII mode only as ethqos_configure_rgmii() is the only place that the values are programmed into the registers.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 34 +++++++++----------
 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +17 -17
index af8204c0e188..690bd5c7e1a6 100644
@@ -87,8 +87,8 @@ struct ethqos_emac_por {
 };
 
 struct ethqos_emac_driver_data {
-	const struct ethqos_emac_por *por;
-	unsigned int num_por;
+	const struct ethqos_emac_por *rgmii_por;
+	unsigned int num_rgmii_por;
 	bool rgmii_config_loopback_en;
 	bool has_emac_ge_3;
 	const char *link_clk_name;

@@ -108,8 +108,8 @@ struct qcom_ethqos {
 	int serdes_speed;
 	phy_interface_t phy_mode;
 
-	const struct ethqos_emac_por *por;
-	unsigned int num_por;
+	const struct ethqos_emac_por *rgmii_por;
+	unsigned int num_rgmii_por;
 	bool rgmii_config_loopback_en;
 	bool has_emac_ge_3;
 	bool needs_sgmii_loopback;

@@ -218,8 +218,8 @@ static const struct ethqos_emac_por emac_v2_3_0_por[] = {
 };
 
 static const struct ethqos_emac_driver_data emac_v2_3_0_data = {
-	.por = emac_v2_3_0_por,
-	.num_por = ARRAY_SIZE(emac_v2_3_0_por),
+	.rgmii_por = emac_v2_3_0_por,
+	.num_rgmii_por = ARRAY_SIZE(emac_v2_3_0_por),
 	.rgmii_config_loopback_en = true,
 	.has_emac_ge_3 = false,
 };

@@ -234,8 +234,8 @@ static const struct ethqos_emac_por emac_v2_1_0_por[] = {
 };
 
 static const struct ethqos_emac_driver_data emac_v2_1_0_data = {
-	.por = emac_v2_1_0_por,
-	.num_por = ARRAY_SIZE(emac_v2_1_0_por),
+	.rgmii_por = emac_v2_1_0_por,
+	.num_rgmii_por = ARRAY_SIZE(emac_v2_1_0_por),
 	.rgmii_config_loopback_en = false,
 	.has_emac_ge_3 = false,
 };

@@ -250,8 +250,8 @@ static const struct ethqos_emac_por emac_v3_0_0_por[] = {
 };
 
 static const struct ethqos_emac_driver_data emac_v3_0_0_data = {
-	.por = emac_v3_0_0_por,
-	.num_por = ARRAY_SIZE(emac_v3_0_0_por),
+	.rgmii_por = emac_v3_0_0_por,
+	.num_rgmii_por = ARRAY_SIZE(emac_v3_0_0_por),
 	.rgmii_config_loopback_en = false,
 	.has_emac_ge_3 = true,
 	.dwmac4_addrs = {

@@ -282,8 +282,8 @@ static const struct ethqos_emac_por emac_v4_0_0_por[] = {
 };
 
 static const struct ethqos_emac_driver_data emac_v4_0_0_data = {
-	.por = emac_v4_0_0_por,
-	.num_por = ARRAY_SIZE(emac_v4_0_0_por),
+	.rgmii_por = emac_v4_0_0_por,
+	.num_rgmii_por = ARRAY_SIZE(emac_v4_0_0_por),
 	.rgmii_config_loopback_en = false,
 	.has_emac_ge_3 = true,
 	.link_clk_name = "phyaux",

@@ -522,9 +522,9 @@ static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)
 	u32 val;
 
 	/* Reset to POR values and enable clk */
-	for (i = 0; i < ethqos->num_por; i++)
-		rgmii_writel(ethqos, ethqos->por[i].value,
-			     ethqos->por[i].offset);
+	for (i = 0; i < ethqos->num_rgmii_por; i++)
+		rgmii_writel(ethqos, ethqos->rgmii_por[i].value,
+			     ethqos->rgmii_por[i].offset);
 	ethqos_set_func_clk_en(ethqos);
 
 	/* Initialize the DLL first */

@@ -780,8 +780,8 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 				     "Failed to map rgmii resource\n");
 
 	data = of_device_get_match_data(dev);
-	ethqos->por = data->por;
-	ethqos->num_por = data->num_por;
+	ethqos->rgmii_por = data->rgmii_por;
+	ethqos->num_rgmii_por = data->num_rgmii_por;
 	ethqos->rgmii_config_loopback_en = data->rgmii_config_loopback_en;
 	ethqos->has_emac_ge_3 = data->has_emac_ge_3;
 	ethqos->needs_sgmii_loopback = data->needs_sgmii_loopback;
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 1/9
#2 lore raw

Re: [PATCH RFC net-next 1/9] net: stmmac: qcom-ethqos: rename "por" memebers to "rgmii_por"

Quoted 5 lines · Russell King (Oracle)

Rename the "por" and "num_por" members to indicate that they are for RGMII mode only as ethqos_configure_rgmii() is the only place that the values are programmed into the registers.

Signed-off-by Russell King (Oracle) <[email protected]>
Reviewed-by Mohd Ayaan Anwar <[email protected]>

Ayaan

#3 lore raw

RFC PATCH 2/9 net: stmmac: qcom-ethqos: remove register field value obfuscations

In reply to Russell King (Oracle)

Convert the register field values to something more human readable.

For example, using (BIT(29) | BIT(27)) to update a register field that consists of bits 29:27 is an obfuscated way of writing decimal 5 for this field. The comment above needs to explain that this value is 5.

Worse still is BIT(12) | GENMASK(9, 8), which is used to hide the decimal value 19 for the bitfield 16:8.

Fix these, and a few others by using FIELD_PREP(). While it means we have bare numeric constants, this is more preferable than having the obfuscation.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 23 ++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +15 -8
index 690bd5c7e1a6..50b95fd19f9d 100644
@@ -361,10 +361,12 @@ static int ethqos_dll_configure(struct qcom_ethqos *ethqos)
 			      SDCC_HC_REG_DLL_CONFIG2);
 
 		rgmii_updatel(ethqos, SDCC_DLL_CONFIG2_MCLK_FREQ_CALC,
-			      0x1A << 10, SDCC_HC_REG_DLL_CONFIG2);
+			      FIELD_PREP(SDCC_DLL_CONFIG2_MCLK_FREQ_CALC, 26),
+			      SDCC_HC_REG_DLL_CONFIG2);
 
 		rgmii_updatel(ethqos, SDCC_DLL_CONFIG2_DDR_TRAFFIC_INIT_SEL,
-			      BIT(2), SDCC_HC_REG_DLL_CONFIG2);
+			      FIELD_PREP(SDCC_DLL_CONFIG2_DDR_TRAFFIC_INIT_SEL,
+					 1), SDCC_HC_REG_DLL_CONFIG2);
 
 		rgmii_setmask(ethqos, SDCC_DLL_CONFIG2_DDR_TRAFFIC_INIT_SW,
 			      SDCC_HC_REG_DLL_CONFIG2);

@@ -425,11 +427,13 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 		if (ethqos->has_emac_ge_3) {
 			/* 0.9 ns */
 			rgmii_updatel(ethqos, SDCC_DDR_CONFIG_PRG_RCLK_DLY,
-				      115, SDCC_HC_REG_DDR_CONFIG);
+				      FIELD_PREP(SDCC_DDR_CONFIG_PRG_RCLK_DLY,
+						 115), SDCC_HC_REG_DDR_CONFIG);
 		} else {
 			/* 1.8 ns */
 			rgmii_updatel(ethqos, SDCC_DDR_CONFIG_PRG_RCLK_DLY,
-				      57, SDCC_HC_REG_DDR_CONFIG);
+				      FIELD_PREP(SDCC_DDR_CONFIG_PRG_RCLK_DLY,
+						 57), SDCC_HC_REG_DDR_CONFIG);
 		}
 		rgmii_setmask(ethqos, SDCC_DDR_CONFIG_PRG_DLY_EN,
 			      SDCC_HC_REG_DDR_CONFIG);

@@ -451,7 +455,8 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 		rgmii_updatel(ethqos, RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN,
 			      phase_shift, RGMII_IO_MACRO_CONFIG2);
 		rgmii_updatel(ethqos, RGMII_CONFIG_MAX_SPD_PRG_2,
-			      BIT(6), RGMII_IO_MACRO_CONFIG);
+			      FIELD_PREP(RGMII_CONFIG_MAX_SPD_PRG_2, 1),
+			      RGMII_IO_MACRO_CONFIG);
 		rgmii_clrmask(ethqos, RGMII_CONFIG2_RSVD_CONFIG15,
 			      RGMII_IO_MACRO_CONFIG2);
 

@@ -464,7 +469,8 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 
 		/* Write 0x5 to PRG_RCLK_DLY_CODE */
 		rgmii_updatel(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
-			      (BIT(29) | BIT(27)), SDCC_HC_REG_DDR_CONFIG);
+			      FIELD_PREP(SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
+					 5), SDCC_HC_REG_DDR_CONFIG);
 		rgmii_setmask(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY,
 			      SDCC_HC_REG_DDR_CONFIG);
 		rgmii_setmask(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_EN,

@@ -487,7 +493,7 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 		rgmii_updatel(ethqos, RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN,
 			      phase_shift, RGMII_IO_MACRO_CONFIG2);
 		rgmii_updatel(ethqos, RGMII_CONFIG_MAX_SPD_PRG_9,
-			      BIT(12) | GENMASK(9, 8),
+			      FIELD_PREP(RGMII_CONFIG_MAX_SPD_PRG_9, 19),
 			      RGMII_IO_MACRO_CONFIG);
 		rgmii_clrmask(ethqos, RGMII_CONFIG2_RSVD_CONFIG15,
 			      RGMII_IO_MACRO_CONFIG2);

@@ -499,7 +505,8 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 				      RGMII_IO_MACRO_CONFIG2);
 		/* Write 0x5 to PRG_RCLK_DLY_CODE */
 		rgmii_updatel(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
-			      (BIT(29) | BIT(27)), SDCC_HC_REG_DDR_CONFIG);
+			      FIELD_PREP(SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
+					 5), SDCC_HC_REG_DDR_CONFIG);
 		rgmii_setmask(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY,
 			      SDCC_HC_REG_DDR_CONFIG);
 		rgmii_setmask(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_EN,
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 2/9
#4 lore raw

Re: [PATCH RFC net-next 2/9] net: stmmac: qcom-ethqos: remove register field value obfuscations

Quoted 6 lines · Russell King (Oracle)

Convert the register field values to something more human readable.

For example, using (BIT(29) | BIT(27)) to update a register field that consists of bits 29:27 is an obfuscated way of writing decimal 5 for this field. The comment above needs to explain that this value is 5.

Quoted 8 lines

Worse still is BIT(12) | GENMASK(9, 8), which is used to hide the decimal value 19 for the bitfield 16:8.

Fix these, and a few others by using FIELD_PREP(). While it means we have bare numeric constants, this is more preferable than having the obfuscation.

Signed-off-by Russell King (Oracle) <[email protected]>
Reviewed-by Mohd Ayaan Anwar <[email protected]>

Ayaan

#5 lore raw

RFC PATCH 3/9 net: stmmac: qcom-ethqos: change ethqos_configure*() to return void

In reply to Russell King (Oracle)

The ethqos_configure*() family of functions always return zero, and the return value is never checked. Change the int return type to void.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c  | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +4 -8
index 50b95fd19f9d..168f0fed68c0 100644
@@ -100,7 +100,7 @@ struct ethqos_emac_driver_data {
 struct qcom_ethqos {
 	struct platform_device *pdev;
 	void __iomem *rgmii_base;
-	int (*configure_func)(struct qcom_ethqos *ethqos, int speed);
+	void (*configure_func)(struct qcom_ethqos *ethqos, int speed);
 
 	unsigned int link_clk_rate;
 	struct clk *link_clk;

@@ -522,7 +522,7 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 	return 0;
 }
 
-static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)
+static void ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)
 {
 	struct device *dev = &ethqos->pdev->dev;
 	unsigned int i;

@@ -587,8 +587,6 @@ static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)
 		ethqos_dll_configure(ethqos);
 
 	ethqos_rgmii_macro_init(ethqos, speed);
-
-	return 0;
 }
 
 static void ethqos_set_serdes_speed(struct qcom_ethqos *ethqos, int speed)

@@ -607,7 +605,7 @@ static void ethqos_pcs_set_inband(struct stmmac_priv *priv, bool enable)
 /* On interface toggle MAC registers gets reset.
  * Configure MAC block for SGMII on ethernet phy link up
  */
-static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)
+static void ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)
 {
 	struct net_device *dev = platform_get_drvdata(ethqos->pdev);
 	struct stmmac_priv *priv = netdev_priv(dev);

@@ -638,11 +636,9 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)
 		ethqos_pcs_set_inband(priv, true);
 		break;
 	}
-
-	return 0;
 }
 
-static int ethqos_configure(struct qcom_ethqos *ethqos, int speed)
+static void ethqos_configure(struct qcom_ethqos *ethqos, int speed)
 {
 	return ethqos->configure_func(ethqos, speed);
 }
Signature · 1 line
2.47.3
#7 lore raw

RFC PATCH 4/9 net: stmmac: qcom-ethqos: move qcom_ethqos_set_sgmii_loopback() up

In reply to Russell King (Oracle)

ethqos_set_func_clk_en() configures both SGMII loopback and the RGMII functional clock setting. qcom_ethqos_set_sgmii_loopback() is only called from within ethqos_set_func_clk_en(), and checks for PHY_INTERFACE_MODE_2500BASEX.

Move qcom_ethqos_set_sgmii_loopback() to the callers of ethqos_set_func_clk_en() except for ethqos_configure_rgmii() where we know that ethqos->phy_mode will not be PHY_INTERFACE_MODE_2500BASEX.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +4 -1
index 168f0fed68c0..964eca46a653 100644
@@ -204,7 +204,6 @@ qcom_ethqos_set_sgmii_loopback(struct qcom_ethqos *ethqos, bool enable)
 
 static void ethqos_set_func_clk_en(struct qcom_ethqos *ethqos)
 {
-	qcom_ethqos_set_sgmii_loopback(ethqos, true);
 	rgmii_setmask(ethqos, RGMII_CONFIG_FUNC_CLK_EN, RGMII_IO_MACRO_CONFIG);
 }
 

@@ -532,6 +531,7 @@ static void ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)
 	for (i = 0; i < ethqos->num_rgmii_por; i++)
 		rgmii_writel(ethqos, ethqos->rgmii_por[i].value,
 			     ethqos->rgmii_por[i].offset);
+
 	ethqos_set_func_clk_en(ethqos);
 
 	/* Initialize the DLL first */

@@ -701,6 +701,7 @@ static int ethqos_clks_config(void *priv, bool enabled)
 		 * cycled. The actual configuration will be adjusted once
 		 * ethqos_fix_mac_speed() is invoked.
 		 */
+		qcom_ethqos_set_sgmii_loopback(ethqos, true);
 		ethqos_set_func_clk_en(ethqos);
 	} else {
 		clk_disable_unprepare(ethqos->link_clk);

@@ -809,6 +810,8 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 
 	ethqos->serdes_speed = SPEED_1000;
 	ethqos_update_link_clk(ethqos, SPEED_1000);
+
+	qcom_ethqos_set_sgmii_loopback(ethqos, true);
 	ethqos_set_func_clk_en(ethqos);
 
 	plat_dat->bsp_priv = ethqos;
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 4/9
#8 lore raw

Re: [PATCH RFC net-next 4/9] net: stmmac: qcom-ethqos: move qcom_ethqos_set_sgmii_loopback() up

Quoted 10 lines · Russell King (Oracle)

ethqos_set_func_clk_en() configures both SGMII loopback and the RGMII functional clock setting. qcom_ethqos_set_sgmii_loopback() is only called from within ethqos_set_func_clk_en(), and checks for PHY_INTERFACE_MODE_2500BASEX.

Move qcom_ethqos_set_sgmii_loopback() to the callers of ethqos_set_func_clk_en() except for ethqos_configure_rgmii() where we know that ethqos->phy_mode will not be PHY_INTERFACE_MODE_2500BASEX.

Signed-off-by Russell King (Oracle) <[email protected]>
Reviewed-by Mohd Ayaan Anwar <[email protected]>

Ayaan

#9 lore raw

RFC PATCH 5/9 net: stmmac: qcom-ethqos: move loopback disable to .mac_finish()

In reply to Russell King (Oracle)

Loopback is enabled to allow the dwmac soft reset to succeed. This is enabled when clocks are enabled in ethqos_clks_config(), which happens at driver probe and runtime PM resume - e.g. when the network device is administratively brought up.

Currently, the loopback is disabled when the link comes up (via .mac_link_up() calling this driver's .fix_mac_speed().)

Move the qcom_ethqos_set_sgmii_loopback() call which disables loopback from ethqos_fix_mac_speed() into ethqos' SerDes specific .mac_finish() method so that loopback is disabled a little earlier after reset has completed, and dwmac setup has completed.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +12 -1
index 964eca46a653..bd5d3bf90400 100644
@@ -647,7 +647,6 @@ static void ethqos_fix_mac_speed(void *priv, int speed, unsigned int mode)
 {
 	struct qcom_ethqos *ethqos = priv;
 
-	qcom_ethqos_set_sgmii_loopback(ethqos, false);
 	ethqos_update_link_clk(ethqos, speed);
 	ethqos_configure(ethqos, speed);
 }

@@ -684,6 +683,17 @@ static void qcom_ethqos_serdes_powerdown(struct net_device *ndev, void *priv)
 	phy_exit(ethqos->serdes_phy);
 }
 
+static int ethqos_mac_finish_serdes(struct net_device *ndev, void *priv,
+				    unsigned int mode,
+				    phy_interface_t interface)
+{
+	struct qcom_ethqos *ethqos = priv;
+
+	qcom_ethqos_set_sgmii_loopback(ethqos, false);
+
+	return 0;
+}
+
 static int ethqos_clks_config(void *priv, bool enabled)
 {
 	struct qcom_ethqos *ethqos = priv;

@@ -770,6 +780,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	case PHY_INTERFACE_MODE_2500BASEX:
 	case PHY_INTERFACE_MODE_SGMII:
 		ethqos->configure_func = ethqos_configure_sgmii;
+		plat_dat->mac_finish = ethqos_mac_finish_serdes;
 		break;
 	default:
 		dev_err(dev, "Unsupported phy mode %s\n",
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 5/9
#10 lore raw

Re: [PATCH RFC net-next 5/9] net: stmmac: qcom-ethqos: move loopback disable to .mac_finish()

Quoted 8 lines · Russell King (Oracle)

Loopback is enabled to allow the dwmac soft reset to succeed. This is enabled when clocks are enabled in ethqos_clks_config(), which happens at driver probe and runtime PM resume - e.g. when the network device is administratively brought up.

Currently, the loopback is disabled when the link comes up (via .mac_link_up() calling this driver's .fix_mac_speed().)

Quoted 6 lines

Move the qcom_ethqos_set_sgmii_loopback() call which disables loopback from ethqos_fix_mac_speed() into ethqos' SerDes specific .mac_finish() method so that loopback is disabled a little earlier after reset has completed, and dwmac setup has completed.

Signed-off-by Russell King (Oracle) <[email protected]>
Reviewed-by Mohd Ayaan Anwar <[email protected]>

Ayaan

#11 lore raw

RFC PATCH 6/9 net: stmmac: pass interface mode into fix_mac_speed() method

In reply to Russell King (Oracle)

Pass the current interface mode reported by phylink into the fix_mac_speed() method. This will be used by qcom-ethqos for its "SGMII" configuration.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c   |  3 ++-
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c       | 11 +++++++----
 drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c  |  3 ++-
 .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c   |  3 ++-
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c   |  3 ++-
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c       | 11 +++++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     |  3 ++-
 include/linux/stmmac.h                                |  3 ++-
 8 files changed, 26 insertions(+), 14 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c +2 -1
index d043bad4a862..0495437d3a6e 100644
@@ -112,7 +112,8 @@ static int dwc_qos_probe(struct platform_device *pdev,
 #define AUTO_CAL_STATUS 0x880c
 #define  AUTO_CAL_STATUS_ACTIVE BIT(31)
 
-static void tegra_eqos_fix_speed(void *bsp_priv, int speed, unsigned int mode)
+static void tegra_eqos_fix_speed(void *bsp_priv, phy_interface_t interface,
+				 int speed, unsigned int mode)
 {
 	struct tegra_eqos *eqos = bsp_priv;
 	bool needs_calibration = false;
drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c +7 -4
index c4e85197629d..9f5a15b81f8a 100644
@@ -48,7 +48,8 @@ struct imx_dwmac_ops {
 
 	int (*fix_soc_reset)(struct stmmac_priv *priv);
 	int (*set_intf_mode)(struct imx_priv_data *dwmac, u8 phy_intf_sel);
-	void (*fix_mac_speed)(void *priv, int speed, unsigned int mode);
+	void (*fix_mac_speed)(void *priv, phy_interface_t interface,
+			      int speed, unsigned int mode);
 };
 
 struct imx_priv_data {

@@ -160,7 +161,8 @@ static int imx_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i,
 	return stmmac_set_clk_tx_rate(bsp_priv, clk_tx_i, interface, speed);
 }
 
-static void imx_dwmac_fix_speed(void *priv, int speed, unsigned int mode)
+static void imx_dwmac_fix_speed(void *priv, phy_interface_t interface,
+				int speed, unsigned int mode)
 {
 	struct plat_stmmacenet_data *plat_dat;
 	struct imx_priv_data *dwmac = priv;

@@ -185,13 +187,14 @@ static void imx_dwmac_fix_speed(void *priv, int speed, unsigned int mode)
 		dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
 }
 
-static void imx93_dwmac_fix_speed(void *priv, int speed, unsigned int mode)
+static void imx93_dwmac_fix_speed(void *priv, phy_interface_t interface,
+				  int speed, unsigned int mode)
 {
 	struct imx_priv_data *dwmac = priv;
 	unsigned int iface;
 	int ctrl, old_ctrl;
 
-	imx_dwmac_fix_speed(priv, speed, mode);
+	imx_dwmac_fix_speed(priv, interface, speed, mode);
 
 	if (!dwmac || mode != MLO_AN_FIXED)
 		return;
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +2 -1
index d66ae6ea4df5..08ccd5b154a0 100644
@@ -143,7 +143,8 @@ static struct stmmac_pci_info loongson_gmac_pci_info = {
 	.setup = loongson_gmac_data,
 };
 
-static void loongson_gnet_fix_speed(void *priv, int speed, unsigned int mode)
+static void loongson_gnet_fix_speed(void *priv, phy_interface_t interface,
+				    int speed, unsigned int mode)
 {
 	struct loongson_data *ld = (struct loongson_data *)priv;
 	struct net_device *ndev = dev_get_drvdata(ld->dev);
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +2 -1
index bd5d3bf90400..9b29516a5a7c 100644
@@ -643,7 +643,8 @@ static void ethqos_configure(struct qcom_ethqos *ethqos, int speed)
 	return ethqos->configure_func(ethqos, speed);
 }
 
-static void ethqos_fix_mac_speed(void *priv, int speed, unsigned int mode)
+static void ethqos_fix_mac_speed(void *priv, phy_interface_t interface,
+				 int speed, unsigned int mode)
 {
 	struct qcom_ethqos *ethqos = priv;
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c +2 -1
index 4c8991f3b38d..c6b99814d391 100644
@@ -72,7 +72,8 @@ struct socfpga_dwmac {
 	const struct socfpga_dwmac_ops *ops;
 };
 
-static void socfpga_dwmac_fix_mac_speed(void *bsp_priv, int speed,
+static void socfpga_dwmac_fix_mac_speed(void *bsp_priv,
+					phy_interface_t interface, int speed,
 					unsigned int mode)
 {
 	struct socfpga_dwmac *dwmac = (struct socfpga_dwmac *)bsp_priv;
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c +7 -4
index f50547b67fbc..6ebbf95d158f 100644
@@ -91,11 +91,13 @@ struct sti_dwmac {
 	struct regmap *regmap;
 	bool gmac_en;
 	int speed;
-	void (*fix_retime_src)(void *priv, int speed, unsigned int mode);
+	void (*fix_retime_src)(void *priv, phy_interface_t interface,
+			       int speed, unsigned int mode);
 };
 
 struct sti_dwmac_of_data {
-	void (*fix_retime_src)(void *priv, int speed, unsigned int mode);
+	void (*fix_retime_src)(void *priv, phy_interface_t interface,
+			       int speed, unsigned int mode);
 };
 
 enum {

@@ -114,7 +116,8 @@ static u32 stih4xx_tx_retime_val[] = {
 				 | STIH4XX_ETH_SEL_INTERNAL_NOTEXT_PHYCLK,
 };
 
-static void stih4xx_fix_retime_src(void *priv, int spd, unsigned int mode)
+static void stih4xx_fix_retime_src(void *priv, phy_interface_t interface,
+				   int spd, unsigned int mode)
 {
 	struct sti_dwmac *dwmac = priv;
 	u32 src = dwmac->tx_retime_src;

@@ -170,7 +173,7 @@ static int sti_set_phy_intf_sel(void *bsp_priv, u8 phy_intf_sel)
 	val = (dwmac->interface == PHY_INTERFACE_MODE_REVMII) ? 0 : ENMII;
 	regmap_update_bits(regmap, reg, ENMII_MASK, val);
 
-	dwmac->fix_retime_src(dwmac, dwmac->speed, 0);
+	dwmac->fix_retime_src(dwmac, dwmac->interface, dwmac->speed, 0);
 
 	return 0;
 }
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2 -1
index c63099a77cc0..a3f209fe54d7 100644
@@ -1071,7 +1071,8 @@ static void stmmac_mac_link_up(struct phylink_config *config,
 	}
 
 	if (priv->plat->fix_mac_speed)
-		priv->plat->fix_mac_speed(priv->plat->bsp_priv, speed, mode);
+		priv->plat->fix_mac_speed(priv->plat->bsp_priv, interface,
+					  speed, mode);
 
 	if (!duplex)
 		ctrl &= ~priv->hw->link.duplex;
include/linux/stmmac.h +2 -1
index 32352a216567..b96ae9dadfab 100644
@@ -256,7 +256,8 @@ struct plat_stmmacenet_data {
 	int (*set_phy_intf_sel)(void *priv, u8 phy_intf_sel);
 	int (*set_clk_tx_rate)(void *priv, struct clk *clk_tx_i,
 			       phy_interface_t interface, int speed);
-	void (*fix_mac_speed)(void *priv, int speed, unsigned int mode);
+	void (*fix_mac_speed)(void *priv, phy_interface_t interface,
+			      int speed, unsigned int mode);
 	int (*fix_soc_reset)(struct stmmac_priv *priv);
 	int (*serdes_powerup)(struct net_device *ndev, void *priv);
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
Signature · 1 line
2.47.3
#13 lore raw

RFC PATCH 7/9 net: stmmac: qcom-ethqos: pass phy interface mode to configs

In reply to Russell King (Oracle)

Pass the current phylink phy interface mode to the RGMII and "SGMII" configuration functions.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c  | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +10 -6
index 9b29516a5a7c..e2dd334ffd31 100644
@@ -100,7 +100,8 @@ struct ethqos_emac_driver_data {
 struct qcom_ethqos {
 	struct platform_device *pdev;
 	void __iomem *rgmii_base;
-	void (*configure_func)(struct qcom_ethqos *ethqos, int speed);
+	void (*configure_func)(struct qcom_ethqos *ethqos,
+			       phy_interface_t interface, int speed);
 
 	unsigned int link_clk_rate;
 	struct clk *link_clk;

@@ -521,7 +522,8 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 	return 0;
 }
 
-static void ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)
+static void ethqos_configure_rgmii(struct qcom_ethqos *ethqos,
+				   phy_interface_t interface, int speed)
 {
 	struct device *dev = &ethqos->pdev->dev;
 	unsigned int i;

@@ -605,7 +607,8 @@ static void ethqos_pcs_set_inband(struct stmmac_priv *priv, bool enable)
 /* On interface toggle MAC registers gets reset.
  * Configure MAC block for SGMII on ethernet phy link up
  */
-static void ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)
+static void ethqos_configure_sgmii(struct qcom_ethqos *ethqos,
+				   phy_interface_t interface, int speed)
 {
 	struct net_device *dev = platform_get_drvdata(ethqos->pdev);
 	struct stmmac_priv *priv = netdev_priv(dev);

@@ -638,9 +641,10 @@ static void ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)
 	}
 }
 
-static void ethqos_configure(struct qcom_ethqos *ethqos, int speed)
+static void ethqos_configure(struct qcom_ethqos *ethqos,
+			     phy_interface_t interface, int speed)
 {
-	return ethqos->configure_func(ethqos, speed);
+	return ethqos->configure_func(ethqos, interface, speed);
 }
 
 static void ethqos_fix_mac_speed(void *priv, phy_interface_t interface,

@@ -649,7 +653,7 @@ static void ethqos_fix_mac_speed(void *priv, phy_interface_t interface,
 	struct qcom_ethqos *ethqos = priv;
 
 	ethqos_update_link_clk(ethqos, speed);
-	ethqos_configure(ethqos, speed);
+	ethqos_configure(ethqos, interface, speed);
 }
 
 static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
Signature · 1 line
2.47.3
#15 lore raw

RFC PATCH 8/9 net: stmmac: qcom-ethqos: use phy interface mode for inband

In reply to Russell King (Oracle)

qcom-ethqos currently forces inband to be enabled for the Cisco SGMII speeds (1G, 100M and 10M) but not for 2500BASE-X (2.5G).

Rather than using the speed to determine the forced inband state, use phylink's PHY interface mode which will switch between SGMII for the 10M, 100M and 1G speeds, and 2500BASE-X for 2.5G.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../stmicro/stmmac/dwmac-qcom-ethqos.c         | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +7 -11
index e2dd334ffd31..cd6d1660bdb2 100644
@@ -615,30 +615,26 @@ static void ethqos_configure_sgmii(struct qcom_ethqos *ethqos,
 
 	switch (speed) {
 	case SPEED_2500:
-		rgmii_setmask(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
-			      RGMII_IO_MACRO_CONFIG2);
-		ethqos_set_serdes_speed(ethqos, SPEED_2500);
-		ethqos_pcs_set_inband(priv, false);
-		break;
 	case SPEED_1000:
 		rgmii_setmask(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
 			      RGMII_IO_MACRO_CONFIG2);
-		ethqos_set_serdes_speed(ethqos, SPEED_1000);
-		ethqos_pcs_set_inband(priv, true);
 		break;
 	case SPEED_100:
-		ethqos_set_serdes_speed(ethqos, SPEED_1000);
-		ethqos_pcs_set_inband(priv, true);
 		break;
 	case SPEED_10:
 		rgmii_updatel(ethqos, RGMII_CONFIG_SGMII_CLK_DVDR,
 			      FIELD_PREP(RGMII_CONFIG_SGMII_CLK_DVDR,
 					 SGMII_10M_RX_CLK_DVDR),
 			      RGMII_IO_MACRO_CONFIG);
-		ethqos_set_serdes_speed(ethqos, SPEED_1000);
-		ethqos_pcs_set_inband(priv, true);
 		break;
 	}
+
+	if (interface == PHY_INTERFACE_MODE_SGMII)
+		ethqos_set_serdes_speed(ethqos, SPEED_1000);
+	else
+		ethqos_set_serdes_speed(ethqos, SPEED_2500);
+
+	ethqos_pcs_set_inband(priv, interface == PHY_INTERFACE_MODE_SGMII);
 }
 
 static void ethqos_configure(struct qcom_ethqos *ethqos,
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 8/9
#16 lore raw

Re: [PATCH RFC net-next 8/9] net: stmmac: qcom-ethqos: use phy interface mode for inband

Quoted 8 lines · Russell King (Oracle)

qcom-ethqos currently forces inband to be enabled for the Cisco SGMII speeds (1G, 100M and 10M) but not for 2500BASE-X (2.5G).

Rather than using the speed to determine the forced inband state, use phylink's PHY interface mode which will switch between SGMII for the 10M, 100M and 1G speeds, and 2500BASE-X for 2.5G.

Signed-off-by Russell King (Oracle) <[email protected]>
Reviewed-by Mohd Ayaan Anwar <[email protected]>

Ayaan

#17 lore raw

RFC PATCH 9/9 net: stmmac: qcom-ethqos: move SerDes speed configuration

In reply to Russell King (Oracle)

Move the SerDes speed configuration to phylink's .mac_finish() stage so that the SerDes is appropriately configured for the interface mode prior to the link coming up.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +5 -5
index cd6d1660bdb2..153f6abc77cd 100644
@@ -629,11 +629,6 @@ static void ethqos_configure_sgmii(struct qcom_ethqos *ethqos,
 		break;
 	}
 
-	if (interface == PHY_INTERFACE_MODE_SGMII)
-		ethqos_set_serdes_speed(ethqos, SPEED_1000);
-	else
-		ethqos_set_serdes_speed(ethqos, SPEED_2500);
-
 	ethqos_pcs_set_inband(priv, interface == PHY_INTERFACE_MODE_SGMII);
 }
 

@@ -692,6 +687,11 @@ static int ethqos_mac_finish_serdes(struct net_device *ndev, void *priv,
 
 	qcom_ethqos_set_sgmii_loopback(ethqos, false);
 
+	if (interface == PHY_INTERFACE_MODE_SGMII)
+		ethqos_set_serdes_speed(ethqos, SPEED_1000);
+	else if (interface == PHY_INTERFACE_MODE_2500BASEX)
+		ethqos_set_serdes_speed(ethqos, SPEED_2500);
+
 	return 0;
 }
Signature · 1 line
2.47.3
maintainer
#19 lore raw
In reply to Russell King (Oracle)

Hello Russell,

Quoted 19 lines · Russell King (Oracle)

Hi,

As the last series had issues with stability, I've changed the approach in this series to concentrate on keeping much of the SerDes related code within the qcom-ethqos driver rather than trying to move it out at this stage. This means it should be possible to bisect these patches and pinpoint exactly the code movement that causes any instability.

This series starts with various cleanups to qcom-ethqos (the first four patches) before beginning to move code, passing phylink's phy interface (which will change) to the fix_mac_speed() method, and then using that to configure the serdes and inband setting before moving the SerDes code.

Please test this patch set, and let me know whether this works, or where it breaks.

Thanks.

Quoted 10 lines
 .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c    |  11 +-
 .../net/ethernet/stmicro/stmmac/dwmac-loongson.c   |   3 +-
 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 114 ++++++++++++---------
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c    |  11 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |   3 +-
 include/linux/stmmac.h                             |   3 +-
 8 files changed, 90 insertions(+), 61 deletions(-)

Things look pretty stable on the QCS9100 Ride R3 board (with the AQR115C PHY). I would like to test a bit more and should be happy to provide a Tested-by once that's complete.

I may also get access to a board with an RGMII PHY on Monday, so I’m hoping to verify the changes to ethqos_rgmii_macro_init as well as your patch from the previous series (“net: stmmac: qcom-ethqos: convert to set_clk_tx_rate() method”) soon.

Ayaan

author
#20 lore raw

Hi,

Quoted 10 lines · Mohd Ayaan Anwar

Hello Russell,

Things look pretty stable on the QCS9100 Ride R3 board (with the AQR115C PHY). I would like to test a bit more and should be happy to provide a Tested-by once that's complete.

I may also get access to a board with an RGMII PHY on Monday, so I’m hoping to verify the changes to ethqos_rgmii_macro_init as well as your patch from the previous series (“net: stmmac: qcom-ethqos: convert to set_clk_tx_rate() method”) soon.

Looking forward to hearing the results, thanks!

As you seem to be fairly active with the qcom-ethqos stuff, please consider adding yourself to the "QUALCOMM ETHQOS ETHERNET DRIVER" maintainers entry. Thanks.

Vinod, are you still actively involved with this, and if not, should Mohd remove you from the entry for the dwmac-qcom-ethqos driver?

Thanks.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#21 lore raw
Quoted 16 lines · Russell King (Oracle)

Hi,

Quoted 10 lines · Mohd Ayaan Anwar

Hello Russell,

Things look pretty stable on the QCS9100 Ride R3 board (with the AQR115C PHY). I would like to test a bit more and should be happy to provide a Tested-by once that's complete.

I may also get access to a board with an RGMII PHY on Monday, so I’m hoping to verify the changes to ethqos_rgmii_macro_init as well as your patch from the previous series (“net: stmmac: qcom-ethqos: convert to set_clk_tx_rate() method”) soon.

Looking forward to hearing the results, thanks!

Quoted 7 lines

As you seem to be fairly active with the qcom-ethqos stuff, please consider adding yourself to the "QUALCOMM ETHQOS ETHERNET DRIVER" maintainers entry. Thanks.

Vinod, are you still actively involved with this, and if not, should Mohd remove you from the entry for the dwmac-qcom-ethqos driver?

Yeah I am not doing anything in this area, pls do drop me from this

Thanks

Signature · 1 line
~Vinod
maintainer Tested-by on the v1 series
#22 lore raw
In reply to Russell King (Oracle)
Quoted 19 lines · Russell King (Oracle)

Hi,

As the last series had issues with stability, I've changed the approach in this series to concentrate on keeping much of the SerDes related code within the qcom-ethqos driver rather than trying to move it out at this stage. This means it should be possible to bisect these patches and pinpoint exactly the code movement that causes any instability.

This series starts with various cleanups to qcom-ethqos (the first four patches) before beginning to move code, passing phylink's phy interface (which will change) to the fix_mac_speed() method, and then using that to configure the serdes and inband setting before moving the SerDes code.

Please test this patch set, and let me know whether this works, or where it breaks.

Thanks.

Quoted 10 lines
 .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c    |  11 +-
 .../net/ethernet/stmicro/stmmac/dwmac-loongson.c   |   3 +-
 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 114 ++++++++++++---------
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c    |  11 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |   3 +-
 include/linux/stmmac.h                             |   3 +-
 8 files changed, 90 insertions(+), 61 deletions(-)

Tested without issues on:
- QCS9100 Ride R3 (AQR115C PHY, 2500BASE-X) - 2.5G/1G/100M
- IQ9 EVK (QCA8081 PHY, 2500BASE-X) - 2.5G
- QCS615 Ride (KSZ9031 PHY, RGMII) [0][1] - 1G/100M

Tested-by Mohd Ayaan Anwar <[email protected]>

Ayaan
---
[0] https://lore.kernel.org/netdev/[email protected]/t/#ma85cac924488d580b971e6477e7df30dc7e48045
[1] Ethernet is not yet enabled for this board in the upstream kernel.
The changes from [0] were applied locally to test this series. I am
trying to figure out how the board deals with RGMII delays so that I
can revive the series.