[PATCH net-next 0/4] net: stmmac: qcom-ethqos: simplifications

[PATCH net-next 0/4] net: stmmac: qcom-ethqos: simplifications

[Russell King (Oracle)]

Remove unnecessary code from the qcom-ethqos glue driver.

Start by consistently using -> serdes_speed to set the speed of the
serdes PHY rather than sometimes using ->serdes_speed and sometimes
using ->speed.

This then allows the removal of ->speed in the second patch.

There is no need to set the maximum speed just because we're using
2500BASE-X - phylink already knows that 2500BASE-X can't support
faster speeds.

This then makes qcom_ethqos_speed_mode_2500() redundant as it's
setting the interface mode to the value that was determined in the
switch statement that already determined that the interface mode
had this value.

Not tested on hardware.

 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 43 ++++++++--------------
 1 file changed, 15 insertions(+), 28 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH net-next 1/4] net: stmmac: qcom-ethqos: set serdes speed using serdes_speed

[Russell King (Oracle)]

ethqos->serdes_speed represents the current speed the serdes was
configured for, which should be the same as ethqos->speed. Since we
wish to remove ethqos->speed to simplify the code, switch to using the
serdes_speed instead.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 0e4da216f942..5d8cd4336a8c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -709,7 +709,7 @@ static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
 	if (ret)
 		return ret;
 
-	return phy_set_speed(ethqos->serdes_phy, ethqos->speed);
+	return phy_set_speed(ethqos->serdes_phy, ethqos->serdes_speed);
 }
 
 static void qcom_ethqos_serdes_powerdown(struct net_device *ndev, void *priv)
-- 
2.30.2

Re: [PATCH net-next 1/4] net: stmmac: qcom-ethqos: set serdes speed using serdes_speed

[Andrew Lunn]

On Sat, Apr 12, 2025 at 03:09:49PM +0100, Russell King (Oracle) wrote:
> ethqos->serdes_speed represents the current speed the serdes was
> configured for, which should be the same as ethqos->speed. Since we
> wish to remove ethqos->speed to simplify the code, switch to using the
> serdes_speed instead.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 2/4] net: stmmac: qcom-ethqos: remove ethqos->speed

[Russell King (Oracle)]

Rather than ethqos_fix_mac_speed() storing the speed in struct
qcom_ethqos and then functions that are only called from here reading
that speed, pass the speed to the called functions instead.

This removes all readers of this struct member, which then allows the
removal of the two places that set its value and the struct member.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 31 +++++++++----------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 5d8cd4336a8c..cd25aea3e48f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -106,12 +106,11 @@ struct qcom_ethqos {
 	struct platform_device *pdev;
 	void __iomem *rgmii_base;
 	void __iomem *mac_base;
-	int (*configure_func)(struct qcom_ethqos *ethqos);
+	int (*configure_func)(struct qcom_ethqos *ethqos, int speed);
 
 	unsigned int link_clk_rate;
 	struct clk *link_clk;
 	struct phy *serdes_phy;
-	int speed;
 	int serdes_speed;
 	phy_interface_t phy_mode;
 
@@ -385,7 +384,7 @@ static int ethqos_dll_configure(struct qcom_ethqos *ethqos)
 	return 0;
 }
 
-static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
+static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 {
 	struct device *dev = &ethqos->pdev->dev;
 	int phase_shift;
@@ -412,7 +411,7 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
 	rgmii_updatel(ethqos, RGMII_CONFIG_INTF_SEL,
 		      0, RGMII_IO_MACRO_CONFIG);
 
-	switch (ethqos->speed) {
+	switch (speed) {
 	case SPEED_1000:
 		rgmii_updatel(ethqos, RGMII_CONFIG_DDR_MODE,
 			      RGMII_CONFIG_DDR_MODE, RGMII_IO_MACRO_CONFIG);
@@ -532,14 +531,14 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
 			      loopback, RGMII_IO_MACRO_CONFIG);
 		break;
 	default:
-		dev_err(dev, "Invalid speed %d\n", ethqos->speed);
+		dev_err(dev, "Invalid speed %d\n", speed);
 		return -EINVAL;
 	}
 
 	return 0;
 }
 
-static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos)
+static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)
 {
 	struct device *dev = &ethqos->pdev->dev;
 	volatile unsigned int dll_lock;
@@ -562,7 +561,7 @@ static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos)
 		      SDCC_DLL_CONFIG_PDN, SDCC_HC_REG_DLL_CONFIG);
 
 	if (ethqos->has_emac_ge_3) {
-		if (ethqos->speed == SPEED_1000) {
+		if (speed == SPEED_1000) {
 			rgmii_writel(ethqos, 0x1800000, SDCC_TEST_CTL);
 			rgmii_writel(ethqos, 0x2C010800, SDCC_USR_CTL);
 			rgmii_writel(ethqos, 0xA001, SDCC_HC_REG_DLL_CONFIG2);
@@ -580,7 +579,7 @@ static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos)
 	rgmii_updatel(ethqos, SDCC_DLL_CONFIG_PDN, 0,
 		      SDCC_HC_REG_DLL_CONFIG);
 
-	if (ethqos->speed != SPEED_100 && ethqos->speed != SPEED_10) {
+	if (speed != SPEED_100 && speed != SPEED_10) {
 		/* Set DLL_EN */
 		rgmii_updatel(ethqos, SDCC_DLL_CONFIG_DLL_EN,
 			      SDCC_DLL_CONFIG_DLL_EN, SDCC_HC_REG_DLL_CONFIG);
@@ -607,10 +606,10 @@ static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos)
 			dev_err(dev, "Timeout while waiting for DLL lock\n");
 	}
 
-	if (ethqos->speed == SPEED_1000)
+	if (speed == SPEED_1000)
 		ethqos_dll_configure(ethqos);
 
-	ethqos_rgmii_macro_init(ethqos);
+	ethqos_rgmii_macro_init(ethqos, speed);
 
 	return 0;
 }
@@ -626,7 +625,7 @@ static void ethqos_set_serdes_speed(struct qcom_ethqos *ethqos, int speed)
 /* 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)
+static int 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);
@@ -634,7 +633,7 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos)
 
 	val = readl(ethqos->mac_base + MAC_CTRL_REG);
 
-	switch (ethqos->speed) {
+	switch (speed) {
 	case SPEED_2500:
 		val &= ~ETHQOS_MAC_CTRL_PORT_SEL;
 		rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
@@ -681,9 +680,9 @@ static void qcom_ethqos_speed_mode_2500(struct net_device *ndev, void *data)
 	priv->plat->phy_interface = PHY_INTERFACE_MODE_2500BASEX;
 }
 
-static int ethqos_configure(struct qcom_ethqos *ethqos)
+static int ethqos_configure(struct qcom_ethqos *ethqos, int speed)
 {
-	return ethqos->configure_func(ethqos);
+	return ethqos->configure_func(ethqos, speed);
 }
 
 static void ethqos_fix_mac_speed(void *priv, int speed, unsigned int mode)
@@ -691,9 +690,8 @@ 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->speed = speed;
 	ethqos_update_link_clk(ethqos, speed);
-	ethqos_configure(ethqos);
+	ethqos_configure(ethqos, speed);
 }
 
 static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
@@ -847,7 +845,6 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(ethqos->serdes_phy),
 				     "Failed to get serdes phy\n");
 
-	ethqos->speed = SPEED_1000;
 	ethqos->serdes_speed = SPEED_1000;
 	ethqos_update_link_clk(ethqos, SPEED_1000);
 	ethqos_set_func_clk_en(ethqos);
-- 
2.30.2

Re: [PATCH net-next 2/4] net: stmmac: qcom-ethqos: remove ethqos->speed

[Andrew Lunn]

On Sat, Apr 12, 2025 at 03:09:54PM +0100, Russell King (Oracle) wrote:
> Rather than ethqos_fix_mac_speed() storing the speed in struct
> qcom_ethqos and then functions that are only called from here reading
> that speed, pass the speed to the called functions instead.
> 
> This removes all readers of this struct member, which then allows the
> removal of the two places that set its value and the struct member.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 3/4] net: stmmac: qcom-ethqos: remove unnecessary setting max_speed

[Russell King (Oracle)]

Phylink will already limit the MAC speed according to the interface,
so if 2500BASE-X is selected, the maximum speed will be 2.5G. It is,
therefore, not necessary to set a speed limit. Remove setting
plat_dat->max_speed from this glue driver.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index cd25aea3e48f..e8d4925be21c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -676,7 +676,6 @@ static void qcom_ethqos_speed_mode_2500(struct net_device *ndev, void *data)
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
-	priv->plat->max_speed = 2500;
 	priv->plat->phy_interface = PHY_INTERFACE_MODE_2500BASEX;
 }
 
-- 
2.30.2

Re: [PATCH net-next 3/4] net: stmmac: qcom-ethqos: remove unnecessary setting max_speed

[Andrew Lunn]

On Sat, Apr 12, 2025 at 03:09:59PM +0100, Russell King (Oracle) wrote:
> Phylink will already limit the MAC speed according to the interface,
> so if 2500BASE-X is selected, the maximum speed will be 2.5G. It is,
> therefore, not necessary to set a speed limit. Remove setting
> plat_dat->max_speed from this glue driver.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 4/4] net: stmmac: qcom-ethqos: remove speed_mode_2500() method

[Russell King (Oracle)]

qcom-ethqos doesn't need to implement the speed_mode_2500() method as
it is only setting priv->plat->phy_interface to 2500BASE-X, which is
already a pre-condition for assigning speed_mode_2500 in
qcom_ethqos_probe(). So, qcom_ethqos_speed_mode_2500() has no effect.
Remove it.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index e8d4925be21c..e30bdf72331a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -672,13 +672,6 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)
 	return val;
 }
 
-static void qcom_ethqos_speed_mode_2500(struct net_device *ndev, void *data)
-{
-	struct stmmac_priv *priv = netdev_priv(ndev);
-
-	priv->plat->phy_interface = PHY_INTERFACE_MODE_2500BASEX;
-}
-
 static int ethqos_configure(struct qcom_ethqos *ethqos, int speed)
 {
 	return ethqos->configure_func(ethqos, speed);
@@ -800,8 +793,6 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 		ethqos->configure_func = ethqos_configure_rgmii;
 		break;
 	case PHY_INTERFACE_MODE_2500BASEX:
-		plat_dat->speed_mode_2500 = qcom_ethqos_speed_mode_2500;
-		fallthrough;
 	case PHY_INTERFACE_MODE_SGMII:
 		ethqos->configure_func = ethqos_configure_sgmii;
 		break;
-- 
2.30.2

Re: [PATCH net-next 4/4] net: stmmac: qcom-ethqos: remove speed_mode_2500() method

[Andrew Lunn]

On Sat, Apr 12, 2025 at 03:10:04PM +0100, Russell King (Oracle) wrote:
> qcom-ethqos doesn't need to implement the speed_mode_2500() method as
> it is only setting priv->plat->phy_interface to 2500BASE-X, which is
> already a pre-condition for assigning speed_mode_2500 in
> qcom_ethqos_probe(). So, qcom_ethqos_speed_mode_2500() has no effect.
> Remove it.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 0/4] net: stmmac: qcom-ethqos: simplifications

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 12 Apr 2025 15:09:51 +0100 you wrote:
> Remove unnecessary code from the qcom-ethqos glue driver.
> 
> Start by consistently using -> serdes_speed to set the speed of the
> serdes PHY rather than sometimes using ->serdes_speed and sometimes
> using ->speed.
> 
> This then allows the removal of ->speed in the second patch.
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] net: stmmac: qcom-ethqos: set serdes speed using serdes_speed
    https://git.kernel.org/netdev/net-next/c/b4589810082a
  - [net-next,2/4] net: stmmac: qcom-ethqos: remove ethqos->speed
    https://git.kernel.org/netdev/net-next/c/a3d54648ada2
  - [net-next,3/4] net: stmmac: qcom-ethqos: remove unnecessary setting max_speed
    https://git.kernel.org/netdev/net-next/c/4c30093f784e
  - [net-next,4/4] net: stmmac: qcom-ethqos: remove speed_mode_2500() method
    https://git.kernel.org/netdev/net-next/c/0d1c18a10dd1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html