Skip to contentSkip to conversation

net: pcs: xpcs: cleanups batch 1

PATCH Applied Russell King (Oracle) 5 participants 24 messages Oct 1, 2024 – Oct 4, 2024

Subsystems (9)
Networking Network drivers Arm STM32 Ethernet PHY +5 more subsystems
lore
Share & tools
lore

Russell King (Oracle)

author reviewer
#0 lore raw

cover net: pcs: xpcs: cleanups batch 1

Hi,

First, sorry for the bland series subject - this is the first in a number of cleanup series to the XPCS driver. This series has some functional changes beyond merely cleanups, notably the first patch.

This series starts off with a patch that moves the PCS reset from the xpcs_create*() family of calls to when phylink first configures the PHY. The motivation for this change is to get rid of the interface argument to the xpcs_create*() functions, which I see as unnecessary complexity. This patch should be tested on Wangxun and STMMAC drivers.

Patch 2 removes the now unnecessary interface argument from the internal xpcs_create() and xpcs_init_iface() functions. With this, xpcs_init_iface() becomes a misnamed function, but patch 3 removes this function, moving its now meager contents to xpcs_create().

Patch 4 adds xpcs_destroy_pcs() and xpcs_create_pcs_mdiodev() functions which return and take a phylink_pcs, allowing SJA1105 and Wangxun drivers to be converted to using the phylink_pcs structure internally.

Patches 5 through 8 convert both these drivers to that end.

Patch 9 drops the interface argument from the remaining xpcs_create*() functions, addressing the only remaining caller of these functions, that being the STMMAC driver.

As patch 7 removed the direct calls to the XPCS config/link-up functions, the last patch makes these functions static.

 drivers/net/dsa/sja1105/sja1105.h                 |  2 +-
 drivers/net/dsa/sja1105/sja1105_main.c            | 85 ++++++++++----------
 drivers/net/dsa/sja1105/sja1105_mdio.c            | 28 ++++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  7 +-
 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c    | 18 ++---
 drivers/net/ethernet/wangxun/txgbe/txgbe_type.h   |  2 +-
 drivers/net/pcs/pcs-xpcs.c                        | 92 ++++++++++++++---------
 include/linux/pcs/pcs-xpcs.h                      | 14 ++--
 8 files changed, 132 insertions(+), 116 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

PATCH 1/10 net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Move the PCS reset to .pcs_pre_config() rather than at creation time, which means we call the reset function with the interface that we're actually going to be using to talk to the downstream device.

Reviewed-by Vladimir Oltean <[email protected]>
Tested-by Vladimir Oltean <[email protected]> # sja1105
Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/pcs/pcs-xpcs.c   | 39 +++++++++++++++++++++++++++---------
 include/linux/pcs/pcs-xpcs.h |  1 +
 2 files changed, 30 insertions(+), 10 deletions(-)
drivers/net/pcs/pcs-xpcs.c +29 -10
index 82463f9d50c8..7c6c40ddf722 100644
@@ -659,6 +659,30 @@ int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns, int enable)
 }
 EXPORT_SYMBOL_GPL(xpcs_config_eee);
 
+static void xpcs_pre_config(struct phylink_pcs *pcs, phy_interface_t interface)
+{
+	struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
+	const struct dw_xpcs_compat *compat;
+	int ret;
+
+	if (!xpcs->need_reset)
+		return;
+
+	compat = xpcs_find_compat(xpcs->desc, interface);
+	if (!compat) {
+		dev_err(&xpcs->mdiodev->dev, "unsupported interface %s\n",
+			phy_modes(interface));
+		return;
+	}
+
+	ret = xpcs_soft_reset(xpcs, compat);
+	if (ret)
+		dev_err(&xpcs->mdiodev->dev, "soft reset failed: %pe\n",
+			ERR_PTR(ret));
+
+	xpcs->need_reset = false;
+}
+
 static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 				      unsigned int neg_mode)
 {

@@ -1365,6 +1389,7 @@ static const struct dw_xpcs_desc xpcs_desc_list[] = {
 
 static const struct phylink_pcs_ops xpcs_phylink_ops = {
 	.pcs_validate = xpcs_validate,
+	.pcs_pre_config = xpcs_pre_config,
 	.pcs_config = xpcs_config,
 	.pcs_get_state = xpcs_get_state,
 	.pcs_an_restart = xpcs_an_restart,

@@ -1460,18 +1485,12 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
 
 static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
 {
-	const struct dw_xpcs_compat *compat;
-
-	compat = xpcs_find_compat(xpcs->desc, interface);
-	if (!compat)
-		return -EINVAL;
-
-	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) {
+	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
 		xpcs->pcs.poll = false;
-		return 0;
-	}
+	else
+		xpcs->need_reset = true;
 
-	return xpcs_soft_reset(xpcs, compat);
+	return 0;
 }
 
 static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
include/linux/pcs/pcs-xpcs.h +1 -0
index b4a4eb6c8866..fd75d0605bb6 100644
@@ -61,6 +61,7 @@ struct dw_xpcs {
 	struct clk_bulk_data clks[DW_XPCS_NUM_CLKS];
 	struct phylink_pcs pcs;
 	phy_interface_t interface;
+	bool need_reset;
 };
 
 int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface);
Signature · 1 line
2.30.2
#2 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Hi Russell

Quoted 7 lines · Russell King (Oracle)

Move the PCS reset to .pcs_pre_config() rather than at creation time, which means we call the reset function with the interface that we're actually going to be using to talk to the downstream device.

Reviewed-by Vladimir Oltean <[email protected]>
Tested-by Vladimir Oltean <[email protected]> # sja1105
Signed-off-by Russell King (Oracle) <[email protected]>

Continuing the RFC discussion. As I mentioned here:
https://lore.kernel.org/netdev/mykeabksgikgk6otbub2i3ksfettbozuhqy3gt5vyezmemvttg@cpjn5bcfiwei/
The PCS-reset procedure actually can be converted to being independent from the PHY-interface. Thus you won't need to move the PCS resetting to the pre_config() method, and get rid from the pointer to dw_xpcs_compat utilization each time the reset is required.

So why not to merge in my patch to your series as a pre-requisite change and then this patch can be converted to just dropping the xpcs_find_compat() method call from the xpcs_init_iface() function? Alternatively the dropping can be just incorporated into my patch.

-Serge(y)

Quoted 87 lines
---
 drivers/net/pcs/pcs-xpcs.c   | 39 +++++++++++++++++++++++++++---------
 include/linux/pcs/pcs-xpcs.h |  1 +
 2 files changed, 30 insertions(+), 10 deletions(-)
drivers/net/pcs/pcs-xpcs.c +29 -10
index 82463f9d50c8..7c6c40ddf722 100644
@@ -659,6 +659,30 @@ int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns, int enable)
 }
 EXPORT_SYMBOL_GPL(xpcs_config_eee);
 
+static void xpcs_pre_config(struct phylink_pcs *pcs, phy_interface_t interface)
+{
+	struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
+	const struct dw_xpcs_compat *compat;
+	int ret;
+
+	if (!xpcs->need_reset)
+		return;
+
+	compat = xpcs_find_compat(xpcs->desc, interface);
+	if (!compat) {
+		dev_err(&xpcs->mdiodev->dev, "unsupported interface %s\n",
+			phy_modes(interface));
+		return;
+	}
+
+	ret = xpcs_soft_reset(xpcs, compat);
+	if (ret)
+		dev_err(&xpcs->mdiodev->dev, "soft reset failed: %pe\n",
+			ERR_PTR(ret));
+
+	xpcs->need_reset = false;
+}
+
 static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 				      unsigned int neg_mode)
 {

@@ -1365,6 +1389,7 @@ static const struct dw_xpcs_desc xpcs_desc_list[] = {
 
 static const struct phylink_pcs_ops xpcs_phylink_ops = {
 	.pcs_validate = xpcs_validate,
+	.pcs_pre_config = xpcs_pre_config,
 	.pcs_config = xpcs_config,
 	.pcs_get_state = xpcs_get_state,
 	.pcs_an_restart = xpcs_an_restart,

@@ -1460,18 +1485,12 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
 
 static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
 {
-	const struct dw_xpcs_compat *compat;
-
-	compat = xpcs_find_compat(xpcs->desc, interface);
-	if (!compat)
-		return -EINVAL;
-
-	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) {
+	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
 		xpcs->pcs.poll = false;
-		return 0;
-	}
+	else
+		xpcs->need_reset = true;
 
-	return xpcs_soft_reset(xpcs, compat);
+	return 0;
 }
 
 static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
include/linux/pcs/pcs-xpcs.h +1 -0
index b4a4eb6c8866..fd75d0605bb6 100644
@@ -61,6 +61,7 @@ struct dw_xpcs {
 	struct clk_bulk_data clks[DW_XPCS_NUM_CLKS];
 	struct phylink_pcs pcs;
 	phy_interface_t interface;
+	bool need_reset;
 };
 
 int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface);
Signature · 1 line
2.30.2

From 7e36cef5d954cc17586194b8e0b3c58fe0dfe592 Mon Sep 17 00:00:00 2001
From: Serge Semin <[email protected]>
Date: Tue, 4 Jul 2023 12:39:29 +0300
Subject: [PATCH] net: pcs: xpcs: Drop compat arg from soft-reset method
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It's very much inconvenient to have the soft-reset method requiring the xpcs_compat structure instance passed. The later one is found based on the PHY-interface type which isn't always available. Such design makes an ordinary reset-method context depended and unnecessary limits its usage area. Indeed based on [1,2] all Soft-RST flags exported by the PMA/PMD, PCS, AN or MII MMDs are _shared_. It means it resets all the DWX_xpcs internal blocks including CSRs, but except the Management Interface (MDIO, MCI, APB). Thus it doesn't really matter which MMDs soft-reset flag is set, the result will be the same. So the AN-mode-depended code can be freely dropped from the soft-reset method. But depending on the DW XPCS device capabilities (basically it depends on the IP-core synthesize parameters) it can lack some of the MMDs. In order to solve that difficulty the Vendor-Specific 1 MMD can be utilized. It is also called as Control MMD and exports some generic device info about the device including a list of the available MMDs: PMA/PMD, XS/PCS, AN or MII. This MMD persists on all the DW XPCS device [3]. Thus it can be freely utilize to cross-platformly determine actual MMD to perform the soft-reset.

[1] DesignWare® Cores Ethernet PCS, Version 3.11b, June 2015, p.111.
[2] DesignWare® Cores Ethernet PCS, Version 3.11b, June 2015, p.268.
[3] DesignWare® Cores Ethernet PCS, Version 3.11b, June 2015, p.269.

Signed-off-by Serge Semin <[email protected]>
---
 drivers/net/pcs/pcs-xpcs.c   | 31 ++++++++++++++++---------------
 drivers/net/pcs/pcs-xpcs.h   |  7 +++++++
 include/linux/pcs/pcs-xpcs.h |  1 +
 3 files changed, 24 insertions(+), 15 deletions(-)
drivers/net/pcs/pcs-xpcs.c +16 -15
index 014ca2b067f4..81c166726636 100644
@@ -271,24 +271,18 @@ static int xpcs_poll_reset(struct dw_xpcs *xpcs, int dev)
 	return (ret & MDIO_CTRL1_RESET) ? -ETIMEDOUT : 0;
 }
 
-static int xpcs_soft_reset(struct dw_xpcs *xpcs,
-			   const struct dw_xpcs_compat *compat)
+static int xpcs_soft_reset(struct dw_xpcs *xpcs)
 {
 	int ret, dev;
 
-	switch (compat->an_mode) {
-	case DW_AN_C73:
-	case DW_10GBASER:
-		dev = MDIO_MMD_PCS;
-		break;
-	case DW_AN_C37_SGMII:
-	case DW_2500BASEX:
-	case DW_AN_C37_1000BASEX:
+	if (xpcs->mmd_ctrl & DW_SR_CTRL_MII_MMD_EN)
 		dev = MDIO_MMD_VEND2;
-		break;
-	default:
+	else if (xpcs->mmd_ctrl & DW_SR_CTRL_PCS_XS_MMD_EN)
+		dev = MDIO_MMD_PCS;
+	else if (xpcs->mmd_ctrl & DW_SR_CTRL_PMA_MMD_EN)
+		dev = MDIO_MMD_PMAPMD;
+	else
 		return -EINVAL;
-	}
 
 	ret = xpcs_write(xpcs, dev, MDIO_CTRL1, MDIO_CTRL1_RESET);
 	if (ret < 0)

@@ -935,7 +929,7 @@ static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
 	/* ... and then we check the faults. */
 	ret = xpcs_read_fault_c73(xpcs, state, pcs_stat1);
 	if (ret) {
-		ret = xpcs_soft_reset(xpcs, compat);
+		ret = xpcs_soft_reset(xpcs);
 		if (ret)
 			return ret;
 

@@ -1485,17 +1479,24 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
 static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
 {
 	const struct dw_xpcs_compat *compat;
+	int ret;
 
 	compat = xpcs_find_compat(xpcs->desc, interface);
 	if (!compat)
 		return -EINVAL;
 
+	ret = xpcs_read(xpcs, MDIO_MMD_VEND1, DW_SR_CTRL_MMD_CTRL);
+	if (ret < 0)
+		return ret;
+
+	xpcs->mmd_ctrl = ret;
+
 	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) {
 		xpcs->pcs.poll = false;
 		return 0;
 	}
 
-	return xpcs_soft_reset(xpcs, compat);
+	return xpcs_soft_reset(xpcs);
 }
 
 static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
drivers/net/pcs/pcs-xpcs.h +7 -0
index fa05adfae220..774b71801cc0 100644
@@ -52,6 +52,13 @@
 #define DW_C73_2500KX			BIT(0)
 #define DW_C73_5000KR			BIT(1)
 
+/* VR_CTRL_MMD */
+#define DW_SR_CTRL_MMD_CTRL		0x0009
+#define DW_SR_CTRL_AN_MMD_EN		BIT(0)
+#define DW_SR_CTRL_PCS_XS_MMD_EN	BIT(1)
+#define DW_SR_CTRL_MII_MMD_EN		BIT(2)
+#define DW_SR_CTRL_PMA_MMD_EN		BIT(3)
+
 /* Clause 37 Defines */
 /* VR MII MMD registers offsets */
 #define DW_VR_MII_MMD_CTRL		0x0000
include/linux/pcs/pcs-xpcs.h +1 -0
index b4a4eb6c8866..241a1a959406 100644
@@ -59,6 +59,7 @@ struct dw_xpcs {
 	const struct dw_xpcs_desc *desc;
 	struct mdio_device *mdiodev;
 	struct clk_bulk_data clks[DW_XPCS_NUM_CLKS];
+	u16 mmd_ctrl;
 	struct phylink_pcs pcs;
 	phy_interface_t interface;
 };
Signature · 1 line
2.46.1
author reviewer
#3 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Quoted 18 lines · Serge Semin

Hi Russell

Quoted 7 lines · Russell King (Oracle)

Move the PCS reset to .pcs_pre_config() rather than at creation time, which means we call the reset function with the interface that we're actually going to be using to talk to the downstream device.

Reviewed-by Vladimir Oltean <[email protected]>
Tested-by Vladimir Oltean <[email protected]> # sja1105
Signed-off-by Russell King (Oracle) <[email protected]>

Continuing the RFC discussion. As I mentioned here:
https://lore.kernel.org/netdev/mykeabksgikgk6otbub2i3ksfettbozuhqy3gt5vyezmemvttg@cpjn5bcfiwei/
The PCS-reset procedure actually can be converted to being independent from the PHY-interface. Thus you won't need to move the PCS resetting to the pre_config() method, and get rid from the pointer to dw_xpcs_compat utilization each time the reset is required.

Quoted 4 lines

So why not to merge in my patch to your series as a pre-requisite change and then this patch can be converted to just dropping the xpcs_find_compat() method call from the xpcs_init_iface() function? Alternatively the dropping can be just incorporated into my patch.

I'm wondering why we seem to be having a communication issue here.

I'm not sure which part of "keeping the functional changes to a minimum for a cleanup series" you're not understanding. This is one of the basics for kernel development... and given that you're effectively maintaining stmmac, it's something you _should_ know.

So no, I'm going to outright refuse to merge your patch in to this series, because as I see it, it would be wrong to do so. This is a _cleanup_ series, not a functional change series, and what you're proposing _changes_ the _way_ reset happens in this driver beyond the minimum that is required for this cleanup. It's introducing a completely _new_ way of writing to the devices registers to do the reset that's different.

The more differences there are, the more chances there are of regressions.

So, again, no..

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!
maintainer
#4 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Quoted 7 lines

I'm wondering why we seem to be having a communication issue here.

I'm not sure which part of "keeping the functional changes to a minimum for a cleanup series" you're not understanding. This is one of the basics for kernel development... and given that you're effectively maintaining stmmac, it's something you _should_ know.

Quoted 7 lines

So no, I'm going to outright refuse to merge your patch in to this series, because as I see it, it would be wrong to do so. This is a _cleanup_ series, not a functional change series, and what you're proposing _changes_ the _way_ reset happens in this driver beyond the minimum that is required for this cleanup. It's introducing a completely _new_ way of writing to the devices registers to do the reset that's different.

I have to agree with Russell. Cleanups should be as simple as possible, and hopefully obviously correct. They should be low risk.

Lets do all the simple cleanups first. Later we can consider more invasive and risky changes.

Andrew

#5 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Quoted 1 line · Andrew Lunn
Quoted 1 line

I'm wondering why we seem to be having a communication issue here.

No communication issue. I just didn't find the discussion over with all the aspects clarified. That's why I've got back to the topic here.

Quoted 14 lines
Quoted 6 lines

I'm not sure which part of "keeping the functional changes to a minimum for a cleanup series" you're not understanding. This is one of the basics for kernel development... and given that you're effectively maintaining stmmac, it's something you _should_ know.

Quoted 7 lines

So no, I'm going to outright refuse to merge your patch in to this series, because as I see it, it would be wrong to do so. This is a _cleanup_ series, not a functional change series, and what you're proposing _changes_ the _way_ reset happens in this driver beyond the minimum that is required for this cleanup. It's introducing a completely _new_ way of writing to the devices registers to do the reset that's different.

Quoted 2 lines

I have to agree with Russell. Cleanups should be as simple as possible, and hopefully obviously correct. They should be low risk.

In general as a thing in itself with no better option to improve the code logic I agree, it should be kept simple. But since the cleanups normally land to net-next, and seeing the patch set still implies some level of the functional change, I don't see much problem with adding a one more change to simplify the driver logic, decrease the level of cohesions (by eliminating the PHY-interface passing to the soft-reset method) and avoid some unneeded change in this patch set. Yes, my patch adds some amount of functional change, but is that that a big problem if both this series and my patch (set) are going to land in net-next anyway, and probably with a little time-lag?

Here what we'll see in the commits-tree if my patch is applied as a pre-requisite one of this series:

1.0 Serge: net: pcs: xpcs: Drop compat arg from soft-reset method
- 1.1 Russell: net: pcs: xpcs: move PCS reset to .pcs_pre_config()
* This patch won't be needed since the PHY-interface will be no longer used for the soft-reset to be performed.
1.2 Russell: net: pcs: xpcs: drop interface argument from internal functions
- 1.3 net: pcs: xpcs: get rid of xpcs_init_iface()
* This patch won't be applicable since the xpcs_init_iface() method will be still utilized for the basic dw_xpcs initializations and the controller soft-resetting.
...
1.1x Serge: my series rebased onto the Russell' patch set

Here is what we'll see in the git-tree if my patch left omitted in this patch set:

2.1 Russell: net: pcs: xpcs: move PCS reset to .pcs_pre_config() 2.2 Russell: net: pcs: xpcs: drop interface argument from internal functions 2.3 Russell: net: pcs: xpcs: get rid of xpcs_init_iface()
...
2.1x Serge: net: pcs: xpcs: Drop compat arg from soft-reset method
+ 2.1y Serge: net: pcs: xpcs: Get back xpcs_init_iface()
* Since the PHY-interface is no longer required for the XPCS soft-resetting I'll move the basic dw_xpcs initializations to the xpcs_init_iface() in order to simplify the driver logic by consolidating the initial setups at the early XPCS-setup stage. This will basically mean to revert the Russell' patches 2.1 and 2.3.
2.1z Serge: the rest of my series rebased onto the Russell' patch set

Quoted 3 lines

Lets do all the simple cleanups first. Later we can consider more invasive and risky changes.

Based on all the considerations above I still think that option 1. described above looks better since it decreases the changes volume in general and decreases the number of patches (by three actually), conserves the changes linearity.

But if my reasoning haven't been persuasive enough anyway, then fine by me. I'll just add a new patch (as described in 2.1y) to my series. But please be ready that it will look as a reversion of the Russell' patches 2.1 and 2.3.

-Serge(y)

Quoted 2 lines

Andrew

maintainer
#6 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Quoted 4 lines

But if my reasoning haven't been persuasive enough anyway, then fine by me. I'll just add a new patch (as described in 2.1y) to my series. But please be ready that it will look as a reversion of the Russell' patches 2.1 and 2.3.

Note what Russell said in patch 0/X:

Quoted 2 lines

First, sorry for the bland series subject - this is the first in a number of cleanup series to the XPCS driver.

I suspect you need to wait until all the series have landed before your patches can be applied on top.

Andrew

#7 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Quoted 5 lines · Andrew Lunn
Quoted 4 lines

But if my reasoning haven't been persuasive enough anyway, then fine by me. I'll just add a new patch (as described in 2.1y) to my series. But please be ready that it will look as a reversion of the Russell' patches 2.1 and 2.3.

Quoted 8 lines

Note what Russell said in patch 0/X:

Quoted 2 lines

First, sorry for the bland series subject - this is the first in a number of cleanup series to the XPCS driver.

I suspect you need to wait until all the series have landed before your patches can be applied on top.

Of course I have no intention to needlessly over-complicate the review/maintenance process by submitting a new series interfering with the already sent work. That's what I mentioned on the RFC-stage of this series a few days ago:
https://lore.kernel.org/netdev/mykeabksgikgk6otbub2i3ksfettbozuhqy3gt5vyezmemvttg@cpjn5bcfiwei/

But for the reason that I've already done some improvements too, why not to use some of them to simplify the Russell' and further changes if they concern the same functionality?.. That's why I originally suggested my patch as a pre-requisite change.

Anyway the Russell' patch set in general looks good to me. I have no more comments other than regarding the soft-reset change I described in my previous message.

-Serge(y)

Quoted 2 lines

Andrew

maintainer
#8 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Quoted 3 lines

Anyway the Russell' patch set in general looks good to me. I have no more comments other than regarding the soft-reset change I described in my previous message.

Sorry, i've not been keeping track. Have you sent reviewed-by: and
Tested-by: for them?

Andrew

author reviewer
#9 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

Quoted 6 lines · Andrew Lunn
Quoted 3 lines

Anyway the Russell' patch set in general looks good to me. I have no more comments other than regarding the soft-reset change I described in my previous message.

Sorry, i've not been keeping track. Have you sent reviewed-by: and
Tested-by: for them?

Of course Serge hasn't. He hasn't even said he's tested them. He's more concerned with the soft-reset change to do anything else other than whinge about that.

After the previous debacle over the stmmac PCS cleanup (that I've given up with) I decided later in the series of XPCS cleanups I have to touch stmmac as little as possible because I don't want to interact with Serge anymore. This has now been reinforced further, to the extent that I'm now going to ask Serge to _remove_ all usage of phylink from stmmac for this very reason - I do not wish to interact further with Serge.

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!
#10 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

In reply to Andrew Lunn
Quoted 6 lines · Andrew Lunn
Quoted 3 lines

Anyway the Russell' patch set in general looks good to me. I have no more comments other than regarding the soft-reset change I described in my previous message.

Sorry, i've not been keeping track. Have you sent reviewed-by: and
Tested-by: for them?

I have reviewed the series twice on the RFC and v1 stages. But I haven't sent the Rb-tag for the series, just the standard phrase above. I was and still am going to test it out today, when I get to reach my hardware treasury (alas, I couldn't do that earlier this week due to my time-table).

Regarding the tags, sorry for not explicitly submitting them. I was going to send them after testing the series out. Seeing the patch set has already been merged in it won't much of importance to do that now though, but I'll send at least Tb-tag anyway after the testing.

-Serge(y)

Quoted 2 lines

Andrew

author reviewer
#11 lore raw

Re: [PATCH net-next 01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

In reply to Serge Semin
Quoted 74 lines · Serge Semin
Quoted 1 line · Andrew Lunn
Quoted 1 line

I'm wondering why we seem to be having a communication issue here.

No communication issue. I just didn't find the discussion over with all the aspects clarified. That's why I've got back to the topic here.

Quoted 14 lines
Quoted 6 lines

I'm not sure which part of "keeping the functional changes to a minimum for a cleanup series" you're not understanding. This is one of the basics for kernel development... and given that you're effectively maintaining stmmac, it's something you _should_ know.

Quoted 7 lines

So no, I'm going to outright refuse to merge your patch in to this series, because as I see it, it would be wrong to do so. This is a _cleanup_ series, not a functional change series, and what you're proposing _changes_ the _way_ reset happens in this driver beyond the minimum that is required for this cleanup. It's introducing a completely _new_ way of writing to the devices registers to do the reset that's different.

Quoted 2 lines

I have to agree with Russell. Cleanups should be as simple as possible, and hopefully obviously correct. They should be low risk.

In general as a thing in itself with no better option to improve the code logic I agree, it should be kept simple. But since the cleanups normally land to net-next, and seeing the patch set still implies some level of the functional change, I don't see much problem with adding a one more change to simplify the driver logic, decrease the level of cohesions (by eliminating the PHY-interface passing to the soft-reset method) and avoid some unneeded change in this patch set. Yes, my patch adds some amount of functional change, but is that that a big problem if both this series and my patch (set) are going to land in net-next anyway, and probably with a little time-lag?

Here what we'll see in the commits-tree if my patch is applied as a pre-requisite one of this series:

1.0 Serge: net: pcs: xpcs: Drop compat arg from soft-reset method
- 1.1 Russell: net: pcs: xpcs: move PCS reset to .pcs_pre_config()
* This patch won't be needed since the PHY-interface will be no longer used for the soft-reset to be performed.
1.2 Russell: net: pcs: xpcs: drop interface argument from internal functions
- 1.3 net: pcs: xpcs: get rid of xpcs_init_iface()
* This patch won't be applicable since the xpcs_init_iface() method will be still utilized for the basic dw_xpcs initializations and the controller soft-resetting.
...
1.1x Serge: my series rebased onto the Russell' patch set

Here is what we'll see in the git-tree if my patch left omitted in this patch set:

2.1 Russell: net: pcs: xpcs: move PCS reset to .pcs_pre_config() 2.2 Russell: net: pcs: xpcs: drop interface argument from internal functions 2.3 Russell: net: pcs: xpcs: get rid of xpcs_init_iface()
...
2.1x Serge: net: pcs: xpcs: Drop compat arg from soft-reset method
+ 2.1y Serge: net: pcs: xpcs: Get back xpcs_init_iface()
* Since the PHY-interface is no longer required for the XPCS soft-resetting I'll move the basic dw_xpcs initializations to the xpcs_init_iface() in order to simplify the driver logic by consolidating the initial setups at the early XPCS-setup stage. This will basically mean to revert the Russell' patches 2.1 and 2.3.
2.1z Serge: the rest of my series rebased onto the Russell' patch set

Quoted 3 lines

Lets do all the simple cleanups first. Later we can consider more invasive and risky changes.

Based on all the considerations above I still think that option 1. described above looks better since it decreases the changes volume in general and decreases the number of patches (by three actually), conserves the changes linearity.

Quoted 4 lines

But if my reasoning haven't been persuasive enough anyway, then fine by me. I'll just add a new patch (as described in 2.1y) to my series. But please be ready that it will look as a reversion of the Russell' patches 2.1 and 2.3.

Oh, sod it. Do whatever you bloody well want. I don't care. You're constantly arguing against me, and I've had enough of this.

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!
#12 lore raw

PATCH 2/10 net: pcs: xpcs: drop interface argument from internal functions

In reply to Russell King (Oracle)

Now that we no longer use the "interface" argument when creating the XPCS sub-driver, remove it from xpcs_create() and xpcs_init_iface().

Reviewed-by Vladimir Oltean <[email protected]>
Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/pcs/pcs-xpcs.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/net/pcs/pcs-xpcs.c +5 -6
index 7c6c40ddf722..2d8cc3959b4c 100644
@@ -1483,7 +1483,7 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
 	return 0;
 }
 
-static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
+static int xpcs_init_iface(struct dw_xpcs *xpcs)
 {
 	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
 		xpcs->pcs.poll = false;

@@ -1493,8 +1493,7 @@ static int xpcs_init_iface(struct dw_xpcs *xpcs, phy_interface_t interface)
 	return 0;
 }
 
-static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
-				   phy_interface_t interface)
+static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev)
 {
 	struct dw_xpcs *xpcs;
 	int ret;

@@ -1511,7 +1510,7 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
 	if (ret)
 		goto out_clear_clks;
 
-	ret = xpcs_init_iface(xpcs, interface);
+	ret = xpcs_init_iface(xpcs);
 	if (ret)
 		goto out_clear_clks;
 

@@ -1546,7 +1545,7 @@ struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
 	if (IS_ERR(mdiodev))
 		return ERR_CAST(mdiodev);
 
-	xpcs = xpcs_create(mdiodev, interface);
+	xpcs = xpcs_create(mdiodev);
 
 	/* xpcs_create() has taken a refcount on the mdiodev if it was
 	 * successful. If xpcs_create() fails, this will free the mdio

@@ -1584,7 +1583,7 @@ struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
 	if (!mdiodev)
 		return ERR_PTR(-EPROBE_DEFER);
 
-	xpcs = xpcs_create(mdiodev, interface);
+	xpcs = xpcs_create(mdiodev);
 
 	/* xpcs_create() has taken a refcount on the mdiodev if it was
 	 * successful. If xpcs_create() fails, this will free the mdio
Signature · 1 line
2.30.2
#13 lore raw

PATCH 3/10 net: pcs: xpcs: get rid of xpcs_init_iface()

In reply to Russell King (Oracle)

xpcs_init_iface() no longer does anything with the interface mode, and now merely does configuration related to the PMA ID. Move this back into xpcs_create() as it doesn't warrant being a separate function anymore.

Reviewed-by Vladimir Oltean <[email protected]>
Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/pcs/pcs-xpcs.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)
drivers/net/pcs/pcs-xpcs.c +4 -13
index 2d8cc3959b4c..8765b01c0b5d 100644
@@ -1483,16 +1483,6 @@ static int xpcs_init_id(struct dw_xpcs *xpcs)
 	return 0;
 }
 
-static int xpcs_init_iface(struct dw_xpcs *xpcs)
-{
-	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
-		xpcs->pcs.poll = false;
-	else
-		xpcs->need_reset = true;
-
-	return 0;
-}
-
 static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev)
 {
 	struct dw_xpcs *xpcs;

@@ -1510,9 +1500,10 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev)
 	if (ret)
 		goto out_clear_clks;
 
-	ret = xpcs_init_iface(xpcs);
-	if (ret)
-		goto out_clear_clks;
+	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
+		xpcs->pcs.poll = false;
+	else
+		xpcs->need_reset = true;
 
 	return xpcs;
Signature · 1 line
2.30.2
#14 lore raw

PATCH 4/10 net: pcs: xpcs: add xpcs_destroy_pcs() and xpcs_create_pcs_mdiodev()

In reply to Russell King (Oracle)

Provide xpcs create/destroy functions that return and take a phylink_pcs pointer instead of an xpcs pointer. This will be used by drivers that have been converted to use phylink_pcs pointers internally, rather than dw_xpcs pointers.

As xpcs_create_mdiodev() no longer makes use of its interface argument, pass PHY_INTERFACE_MODE_NA into xpcs_create_mdiodev() until it is removed later in the series.

Reviewed-by Vladimir Oltean <[email protected]>
Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/pcs/pcs-xpcs.c   | 18 ++++++++++++++++++
 include/linux/pcs/pcs-xpcs.h |  3 +++
 2 files changed, 21 insertions(+)
drivers/net/pcs/pcs-xpcs.c +18 -0
index 8765b01c0b5d..9b61f97222b9 100644
@@ -1550,6 +1550,18 @@ struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
 }
 EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
 
+struct phylink_pcs *xpcs_create_pcs_mdiodev(struct mii_bus *bus, int addr)
+{
+	struct dw_xpcs *xpcs;
+
+	xpcs = xpcs_create_mdiodev(bus, addr, PHY_INTERFACE_MODE_NA);
+	if (IS_ERR(xpcs))
+		return ERR_CAST(xpcs);
+
+	return &xpcs->pcs;
+}
+EXPORT_SYMBOL_GPL(xpcs_create_pcs_mdiodev);
+
 /**
  * xpcs_create_fwnode() - Create a DW xPCS instance from @fwnode
  * @fwnode: fwnode handle poining to the DW XPCS device

@@ -1599,5 +1611,11 @@ void xpcs_destroy(struct dw_xpcs *xpcs)
 }
 EXPORT_SYMBOL_GPL(xpcs_destroy);
 
+void xpcs_destroy_pcs(struct phylink_pcs *pcs)
+{
+	xpcs_destroy(phylink_pcs_to_xpcs(pcs));
+}
+EXPORT_SYMBOL_GPL(xpcs_destroy_pcs);
+
 MODULE_DESCRIPTION("Synopsys DesignWare XPCS library");
 MODULE_LICENSE("GPL v2");
include/linux/pcs/pcs-xpcs.h +3 -0
index fd75d0605bb6..a4e2243ce647 100644
@@ -78,4 +78,7 @@ struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
 				   phy_interface_t interface);
 void xpcs_destroy(struct dw_xpcs *xpcs);
 
+struct phylink_pcs *xpcs_create_pcs_mdiodev(struct mii_bus *bus, int addr);
+void xpcs_destroy_pcs(struct phylink_pcs *pcs);
+
 #endif /* __LINUX_PCS_XPCS_H */
Signature · 1 line
2.30.2
#15 lore raw

PATCH 5/10 net: wangxun: txgbe: use phylink_pcs internally

In reply to Russell King (Oracle)

Use xpcs_create_pcs_mdiodev() to create the XPCS instance, storing and using the phylink_pcs pointer internally, rather than dw_xpcs. Use xpcs_destroy_pcs() to destroy the XPCS instance when we've finished with it.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 18 +++++++++---------
 .../net/ethernet/wangxun/txgbe/txgbe_type.h    |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)
drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c +9 -9
index 67b61afdde96..3dd89dafe7c7 100644
@@ -122,7 +122,7 @@ static int txgbe_pcs_write(struct mii_bus *bus, int addr, int devnum, int regnum
 static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
 {
 	struct mii_bus *mii_bus;
-	struct dw_xpcs *xpcs;
+	struct phylink_pcs *pcs;
 	struct pci_dev *pdev;
 	struct wx *wx;
 	int ret = 0;

@@ -147,11 +147,11 @@ static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
 	if (ret)
 		return ret;
 
-	xpcs = xpcs_create_mdiodev(mii_bus, 0, PHY_INTERFACE_MODE_10GBASER);
-	if (IS_ERR(xpcs))
-		return PTR_ERR(xpcs);
+	pcs = xpcs_create_pcs_mdiodev(mii_bus, 0);
+	if (IS_ERR(pcs))
+		return PTR_ERR(pcs);
 
-	txgbe->xpcs = xpcs;
+	txgbe->pcs = pcs;
 
 	return 0;
 }

@@ -163,7 +163,7 @@ static struct phylink_pcs *txgbe_phylink_mac_select(struct phylink_config *confi
 	struct txgbe *txgbe = wx->priv;
 
 	if (interface == PHY_INTERFACE_MODE_10GBASER)
-		return &txgbe->xpcs->pcs;
+		return txgbe->pcs;
 
 	return NULL;
 }

@@ -302,7 +302,7 @@ irqreturn_t txgbe_link_irq_handler(int irq, void *data)
 	status = rd32(wx, TXGBE_CFG_PORT_ST);
 	up = !!(status & TXGBE_CFG_PORT_ST_LINK_UP);
 
-	phylink_pcs_change(&txgbe->xpcs->pcs, up);
+	phylink_pcs_change(txgbe->pcs, up);
 
 	return IRQ_HANDLED;
 }

@@ -778,7 +778,7 @@ int txgbe_init_phy(struct txgbe *txgbe)
 err_destroy_phylink:
 	phylink_destroy(wx->phylink);
 err_destroy_xpcs:
-	xpcs_destroy(txgbe->xpcs);
+	xpcs_destroy_pcs(txgbe->pcs);
 err_unregister_swnode:
 	software_node_unregister_node_group(txgbe->nodes.group);
 

@@ -798,6 +798,6 @@ void txgbe_remove_phy(struct txgbe *txgbe)
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
 	phylink_destroy(txgbe->wx->phylink);
-	xpcs_destroy(txgbe->xpcs);
+	xpcs_destroy_pcs(txgbe->pcs);
 	software_node_unregister_node_group(txgbe->nodes.group);
 }
drivers/net/ethernet/wangxun/txgbe/txgbe_type.h +1 -1
index 959102c4c379..cc3a7b62fe9e 100644
@@ -329,7 +329,7 @@ struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
 	struct txgbe_irq misc;
-	struct dw_xpcs *xpcs;
+	struct phylink_pcs *pcs;
 	struct platform_device *sfp_dev;
 	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
Signature · 1 line
2.30.2
maintainer Reviewed-by on v1 5/10
#16 lore raw

Re: [PATCH net-next 05/10] net: wangxun: txgbe: use phylink_pcs internally

Quoted 6 lines · Russell King (Oracle)

Use xpcs_create_pcs_mdiodev() to create the XPCS instance, storing and using the phylink_pcs pointer internally, rather than dw_xpcs. Use xpcs_destroy_pcs() to destroy the XPCS instance when we've finished with it.

Signed-off-by Russell King (Oracle) <[email protected]>

It would be good if the net-swift.com people gave a Reviewed-by: or a Tested-by.

To me, this looks O.K.

Reviewed-by Andrew Lunn <[email protected]>

Andrew

#17 lore raw

PATCH 6/10 net: dsa: sja1105: simplify static configuration reload

In reply to Russell King (Oracle)

The static configuration reload saves the port speed in the static configuration tables by first converting it from the internal respresentation to the SPEED_xxx ethtool representation, and then converts it back to restore the setting. This is because sja1105_adjust_port_config() takes the speed as SPEED_xxx.

However, this is unnecessarily complex. If we split sja1105_adjust_port_config() up, we can simply save and restore the mac[port].speed member in the static configuration tables.

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since rfc:
- added () to function name
- fixed local variable order

 drivers/net/dsa/sja1105/sja1105_main.c | 65 ++++++++++++++------------
 1 file changed, 34 insertions(+), 31 deletions(-)
drivers/net/dsa/sja1105/sja1105_main.c +34 -31
index bc7e50dcb57c..5481ccb921df 100644
@@ -1257,29 +1257,11 @@ static int sja1105_parse_dt(struct sja1105_private *priv)
 	return rc;
 }
 
-/* Convert link speed from SJA1105 to ethtool encoding */
-static int sja1105_port_speed_to_ethtool(struct sja1105_private *priv,
-					 u64 speed)
-{
-	if (speed == priv->info->port_speed[SJA1105_SPEED_10MBPS])
-		return SPEED_10;
-	if (speed == priv->info->port_speed[SJA1105_SPEED_100MBPS])
-		return SPEED_100;
-	if (speed == priv->info->port_speed[SJA1105_SPEED_1000MBPS])
-		return SPEED_1000;
-	if (speed == priv->info->port_speed[SJA1105_SPEED_2500MBPS])
-		return SPEED_2500;
-	return SPEED_UNKNOWN;
-}
-
-/* Set link speed in the MAC configuration for a specific port. */
-static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
-				      int speed_mbps)
+static int sja1105_set_port_speed(struct sja1105_private *priv, int port,
+				  int speed_mbps)
 {
 	struct sja1105_mac_config_entry *mac;
-	struct device *dev = priv->ds->dev;
 	u64 speed;
-	int rc;
 
 	/* On P/Q/R/S, one can read from the device via the MAC reconfiguration
 	 * tables. On E/T, MAC reconfig tables are not readable, only writable.

@@ -1313,7 +1295,7 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
 		speed = priv->info->port_speed[SJA1105_SPEED_2500MBPS];
 		break;
 	default:
-		dev_err(dev, "Invalid speed %iMbps\n", speed_mbps);
+		dev_err(priv->ds->dev, "Invalid speed %iMbps\n", speed_mbps);
 		return -EINVAL;
 	}
 

@@ -1325,11 +1307,31 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
 	 * we need to configure the PCS only (if even that).
 	 */
 	if (priv->phy_mode[port] == PHY_INTERFACE_MODE_SGMII)
-		mac[port].speed = priv->info->port_speed[SJA1105_SPEED_1000MBPS];
+		speed = priv->info->port_speed[SJA1105_SPEED_1000MBPS];
 	else if (priv->phy_mode[port] == PHY_INTERFACE_MODE_2500BASEX)
-		mac[port].speed = priv->info->port_speed[SJA1105_SPEED_2500MBPS];
-	else
-		mac[port].speed = speed;
+		speed = priv->info->port_speed[SJA1105_SPEED_2500MBPS];
+
+	mac[port].speed = speed;
+
+	return 0;
+}
+
+/* Write the MAC Configuration Table entry and, if necessary, the CGU settings,
+ * after a link speedchange for this port.
+ */
+static int sja1105_set_port_config(struct sja1105_private *priv, int port)
+{
+	struct sja1105_mac_config_entry *mac;
+	struct device *dev = priv->ds->dev;
+	int rc;
+
+	/* On P/Q/R/S, one can read from the device via the MAC reconfiguration
+	 * tables. On E/T, MAC reconfig tables are not readable, only writable.
+	 * We have to *know* what the MAC looks like.  For the sake of keeping
+	 * the code common, we'll use the static configuration tables as a
+	 * reasonable approximation for both E/T and P/Q/R/S.
+	 */
+	mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
 
 	/* Write to the dynamic reconfiguration tables */
 	rc = sja1105_dynamic_config_write(priv, BLK_IDX_MAC_CONFIG, port,

@@ -1390,7 +1392,8 @@ static void sja1105_mac_link_up(struct phylink_config *config,
 	struct sja1105_private *priv = dp->ds->priv;
 	int port = dp->index;
 
-	sja1105_adjust_port_config(priv, port, speed);
+	if (!sja1105_set_port_speed(priv, port, speed))
+		sja1105_set_port_config(priv, port);
 
 	sja1105_inhibit_tx(priv, BIT(port), false);
 }

@@ -2293,8 +2296,8 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 {
 	struct ptp_system_timestamp ptp_sts_before;
 	struct ptp_system_timestamp ptp_sts_after;
-	int speed_mbps[SJA1105_MAX_NUM_PORTS];
 	u16 bmcr[SJA1105_MAX_NUM_PORTS] = {0};
+	u64 mac_speed[SJA1105_MAX_NUM_PORTS];
 	struct sja1105_mac_config_entry *mac;
 	struct dsa_switch *ds = priv->ds;
 	s64 t1, t2, t3, t4;

@@ -2307,14 +2310,13 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 
 	mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
 
-	/* Back up the dynamic link speed changed by sja1105_adjust_port_config
+	/* Back up the dynamic link speed changed by sja1105_set_port_speed()
 	 * in order to temporarily restore it to SJA1105_SPEED_AUTO - which the
 	 * switch wants to see in the static config in order to allow us to
 	 * change it through the dynamic interface later.
 	 */
 	for (i = 0; i < ds->num_ports; i++) {
-		speed_mbps[i] = sja1105_port_speed_to_ethtool(priv,
-							      mac[i].speed);
+		mac_speed[i] = mac[i].speed;
 		mac[i].speed = priv->info->port_speed[SJA1105_SPEED_AUTO];
 
 		if (priv->xpcs[i])

@@ -2377,7 +2379,8 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 		struct dw_xpcs *xpcs = priv->xpcs[i];
 		unsigned int neg_mode;
 
-		rc = sja1105_adjust_port_config(priv, i, speed_mbps[i]);
+		mac[i].speed = mac_speed[i];
+		rc = sja1105_set_port_config(priv, i);
 		if (rc < 0)
 			goto out;
Signature · 1 line
2.30.2
#18 lore raw

PATCH 7/10 net: dsa: sja1105: call PCS config/link_up via pcs_ops structure

In reply to Russell King (Oracle)

Call the PCS operations through the ops structure, which avoids needing to export xpcs internal functions.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/net/dsa/sja1105/sja1105_main.c +7 -3
index 5481ccb921df..e5918ac862eb 100644
@@ -2377,6 +2377,7 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 
 	for (i = 0; i < ds->num_ports; i++) {
 		struct dw_xpcs *xpcs = priv->xpcs[i];
+		struct phylink_pcs *pcs;
 		unsigned int neg_mode;
 
 		mac[i].speed = mac_speed[i];

@@ -2387,12 +2388,15 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 		if (!xpcs)
 			continue;
 
+		pcs = &xpcs->pcs;
+
 		if (bmcr[i] & BMCR_ANENABLE)
 			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
 		else
 			neg_mode = PHYLINK_PCS_NEG_OUTBAND;
 
-		rc = xpcs_do_config(xpcs, priv->phy_mode[i], NULL, neg_mode);
+		rc = pcs->ops->pcs_config(pcs, neg_mode, priv->phy_mode[i],
+					  NULL, true);
 		if (rc < 0)
 			goto out;
 

@@ -2408,8 +2412,8 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 			else
 				speed = SPEED_10;
 
-			xpcs_link_up(&xpcs->pcs, neg_mode, priv->phy_mode[i],
-				     speed, DUPLEX_FULL);
+			pcs->ops->pcs_link_up(pcs, neg_mode, priv->phy_mode[i],
+					      speed, DUPLEX_FULL);
 		}
 	}
Signature · 1 line
2.30.2
#19 lore raw

PATCH 8/10 net: dsa: sja1105: use phylink_pcs internally

In reply to Russell King (Oracle)

Use xpcs_create_pcs_mdiodev() to create the XPCS instance, storing and using the phylink_pcs pointer internally, rather than dw_xpcs. Use xpcs_destroy_pcs() to destroy the XPCS instance when we've finished with it.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/dsa/sja1105/sja1105.h      |  2 +-
 drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++-----------
 drivers/net/dsa/sja1105/sja1105_mdio.c | 28 ++++++++++++--------------
 3 files changed, 18 insertions(+), 28 deletions(-)
drivers/net/dsa/sja1105/sja1105.h +1 -1
index 8c66d3bf61f0..dceb96ae9c83 100644
@@ -278,7 +278,7 @@ struct sja1105_private {
 	struct mii_bus *mdio_base_t1;
 	struct mii_bus *mdio_base_tx;
 	struct mii_bus *mdio_pcs;
-	struct dw_xpcs *xpcs[SJA1105_MAX_NUM_PORTS];
+	struct phylink_pcs *pcs[SJA1105_MAX_NUM_PORTS];
 	struct sja1105_ptp_data ptp_data;
 	struct sja1105_tas_data tas_data;
 };
drivers/net/dsa/sja1105/sja1105_main.c +4 -12
index e5918ac862eb..af38b8959d8d 100644
@@ -15,7 +15,6 @@
 #include <linux/of.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
-#include <linux/pcs/pcs-xpcs.h>
 #include <linux/netdev_features.h>
 #include <linux/netdevice.h>
 #include <linux/if_bridge.h>

@@ -1358,12 +1357,8 @@ sja1105_mac_select_pcs(struct phylink_config *config, phy_interface_t iface)
 {
 	struct dsa_port *dp = dsa_phylink_to_port(config);
 	struct sja1105_private *priv = dp->ds->priv;
-	struct dw_xpcs *xpcs = priv->xpcs[dp->index];
 
-	if (xpcs)
-		return &xpcs->pcs;
-
-	return NULL;
+	return priv->pcs[dp->index];
 }
 
 static void sja1105_mac_config(struct phylink_config *config,

@@ -2319,7 +2314,7 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 		mac_speed[i] = mac[i].speed;
 		mac[i].speed = priv->info->port_speed[SJA1105_SPEED_AUTO];
 
-		if (priv->xpcs[i])
+		if (priv->pcs[i])
 			bmcr[i] = mdiobus_c45_read(priv->mdio_pcs, i,
 						   MDIO_MMD_VEND2, MDIO_CTRL1);
 	}

@@ -2376,8 +2371,7 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 	}
 
 	for (i = 0; i < ds->num_ports; i++) {
-		struct dw_xpcs *xpcs = priv->xpcs[i];
-		struct phylink_pcs *pcs;
+		struct phylink_pcs *pcs = priv->pcs[i];
 		unsigned int neg_mode;
 
 		mac[i].speed = mac_speed[i];

@@ -2385,11 +2379,9 @@ int sja1105_static_config_reload(struct sja1105_private *priv,
 		if (rc < 0)
 			goto out;
 
-		if (!xpcs)
+		if (!pcs)
 			continue;
 
-		pcs = &xpcs->pcs;
-
 		if (bmcr[i] & BMCR_ANENABLE)
 			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
 		else
drivers/net/dsa/sja1105/sja1105_mdio.c +13 -15
index 52ddb4ef259e..84b7169f2974 100644
@@ -400,7 +400,7 @@ static int sja1105_mdiobus_pcs_register(struct sja1105_private *priv)
 	}
 
 	for (port = 0; port < ds->num_ports; port++) {
-		struct dw_xpcs *xpcs;
+		struct phylink_pcs *pcs;
 
 		if (dsa_is_unused_port(ds, port))
 			continue;

@@ -409,13 +409,13 @@ static int sja1105_mdiobus_pcs_register(struct sja1105_private *priv)
 		    priv->phy_mode[port] != PHY_INTERFACE_MODE_2500BASEX)
 			continue;
 
-		xpcs = xpcs_create_mdiodev(bus, port, priv->phy_mode[port]);
-		if (IS_ERR(xpcs)) {
-			rc = PTR_ERR(xpcs);
+		pcs = xpcs_create_pcs_mdiodev(bus, port);
+		if (IS_ERR(pcs)) {
+			rc = PTR_ERR(pcs);
 			goto out_pcs_free;
 		}
 
-		priv->xpcs[port] = xpcs;
+		priv->pcs[port] = pcs;
 	}
 
 	priv->mdio_pcs = bus;

@@ -424,11 +424,10 @@ static int sja1105_mdiobus_pcs_register(struct sja1105_private *priv)
 
 out_pcs_free:
 	for (port = 0; port < ds->num_ports; port++) {
-		if (!priv->xpcs[port])
-			continue;
-
-		xpcs_destroy(priv->xpcs[port]);
-		priv->xpcs[port] = NULL;
+		if (priv->pcs[port]) {
+			xpcs_destroy_pcs(priv->pcs[port]);
+			priv->pcs[port] = NULL;
+		}
 	}
 
 	mdiobus_unregister(bus);

@@ -446,11 +445,10 @@ static void sja1105_mdiobus_pcs_unregister(struct sja1105_private *priv)
 		return;
 
 	for (port = 0; port < ds->num_ports; port++) {
-		if (!priv->xpcs[port])
-			continue;
-
-		xpcs_destroy(priv->xpcs[port]);
-		priv->xpcs[port] = NULL;
+		if (priv->pcs[port]) {
+			xpcs_destroy_pcs(priv->pcs[port]);
+			priv->pcs[port] = NULL;
+		}
 	}
 
 	mdiobus_unregister(priv->mdio_pcs);
Signature · 1 line
2.30.2
#20 lore raw

PATCH 9/10 net: pcs: xpcs: drop interface argument from xpcs_create*()

In reply to Russell King (Oracle)

The XPCS sub-driver no longer uses the "interface" argument to the xpcs_create_mdiodev() and xpcs_create_fwnode() functions. Remove this now unnecessary argument, updating the stmmac driver appropriately.

Reviewed-by Vladimir Oltean <[email protected]>
Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  7 +++----
 drivers/net/pcs/pcs-xpcs.c                        | 10 +++-------
 include/linux/pcs/pcs-xpcs.h                      |  6 ++----
 3 files changed, 8 insertions(+), 15 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +3 -4
index 03f90676b3ad..0c7d81ddd440 100644
@@ -500,23 +500,22 @@ int stmmac_pcs_setup(struct net_device *ndev)
 	struct fwnode_handle *devnode, *pcsnode;
 	struct dw_xpcs *xpcs = NULL;
 	struct stmmac_priv *priv;
-	int addr, mode, ret;
+	int addr, ret;
 
 	priv = netdev_priv(ndev);
-	mode = priv->plat->phy_interface;
 	devnode = priv->plat->port_node;
 
 	if (priv->plat->pcs_init) {
 		ret = priv->plat->pcs_init(priv);
 	} else if (fwnode_property_present(devnode, "pcs-handle")) {
 		pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
-		xpcs = xpcs_create_fwnode(pcsnode, mode);
+		xpcs = xpcs_create_fwnode(pcsnode);
 		fwnode_handle_put(pcsnode);
 		ret = PTR_ERR_OR_ZERO(xpcs);
 	} else if (priv->plat->mdio_bus_data &&
 		   priv->plat->mdio_bus_data->pcs_mask) {
 		addr = ffs(priv->plat->mdio_bus_data->pcs_mask) - 1;
-		xpcs = xpcs_create_mdiodev(priv->mii, addr, mode);
+		xpcs = xpcs_create_mdiodev(priv->mii, addr);
 		ret = PTR_ERR_OR_ZERO(xpcs);
 	} else {
 		return 0;
drivers/net/pcs/pcs-xpcs.c +3 -7
index 9b61f97222b9..f25e7afdfdf5 100644
@@ -1520,14 +1520,12 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev)
  * xpcs_create_mdiodev() - create a DW xPCS instance with the MDIO @addr
  * @bus: pointer to the MDIO-bus descriptor for the device to be looked at
  * @addr: device MDIO-bus ID
- * @interface: requested PHY interface
  *
  * Return: a pointer to the DW XPCS handle if successful, otherwise -ENODEV if
  * the PCS device couldn't be found on the bus and other negative errno related
  * to the data allocation and MDIO-bus communications.
  */
-struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
-				    phy_interface_t interface)
+struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr)
 {
 	struct mdio_device *mdiodev;
 	struct dw_xpcs *xpcs;

@@ -1554,7 +1552,7 @@ struct phylink_pcs *xpcs_create_pcs_mdiodev(struct mii_bus *bus, int addr)
 {
 	struct dw_xpcs *xpcs;
 
-	xpcs = xpcs_create_mdiodev(bus, addr, PHY_INTERFACE_MODE_NA);
+	xpcs = xpcs_create_mdiodev(bus, addr);
 	if (IS_ERR(xpcs))
 		return ERR_CAST(xpcs);
 

@@ -1565,7 +1563,6 @@ EXPORT_SYMBOL_GPL(xpcs_create_pcs_mdiodev);
 /**
  * xpcs_create_fwnode() - Create a DW xPCS instance from @fwnode
  * @fwnode: fwnode handle poining to the DW XPCS device
- * @interface: requested PHY interface
  *
  * Return: a pointer to the DW XPCS handle if successful, otherwise -ENODEV if
  * the fwnode device is unavailable or the PCS device couldn't be found on the

@@ -1573,8 +1570,7 @@ EXPORT_SYMBOL_GPL(xpcs_create_pcs_mdiodev);
  * other negative errno related to the data allocations and MDIO-bus
  * communications.
  */
-struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
-				   phy_interface_t interface)
+struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode)
 {
 	struct mdio_device *mdiodev;
 	struct dw_xpcs *xpcs;
include/linux/pcs/pcs-xpcs.h +2 -4
index a4e2243ce647..758daabb76c7 100644
@@ -72,10 +72,8 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
 void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces);
 int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns,
 		    int enable);
-struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
-				    phy_interface_t interface);
-struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode,
-				   phy_interface_t interface);
+struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr);
+struct dw_xpcs *xpcs_create_fwnode(struct fwnode_handle *fwnode);
 void xpcs_destroy(struct dw_xpcs *xpcs);
 
 struct phylink_pcs *xpcs_create_pcs_mdiodev(struct mii_bus *bus, int addr);
Signature · 1 line
2.30.2
#21 lore raw

PATCH 10/10 net: pcs: xpcs: make xpcs_do_config() and xpcs_link_up() internal

In reply to Russell King (Oracle)

As nothing outside pcs-xpcs.c calls neither xpcs_do_config() nor xpcs_link_up(), remove their exports and prototypes.

Reviewed-by Vladimir Oltean <[email protected]>
Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/pcs/pcs-xpcs.c   | 11 +++++------
 include/linux/pcs/pcs-xpcs.h |  4 ----
 2 files changed, 5 insertions(+), 10 deletions(-)
drivers/net/pcs/pcs-xpcs.c +5 -6
index f25e7afdfdf5..0a01c552f591 100644
@@ -851,8 +851,9 @@ static int xpcs_config_2500basex(struct dw_xpcs *xpcs)
 	return xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL, ret);
 }
 
-int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
-		   const unsigned long *advertising, unsigned int neg_mode)
+static int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
+			  const unsigned long *advertising,
+			  unsigned int neg_mode)
 {
 	const struct dw_xpcs_compat *compat;
 	int ret;

@@ -905,7 +906,6 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(xpcs_do_config);
 
 static int xpcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 		       phy_interface_t interface,

@@ -1207,8 +1207,8 @@ static void xpcs_link_up_1000basex(struct dw_xpcs *xpcs, unsigned int neg_mode,
 		pr_err("%s: xpcs_write returned %pe\n", __func__, ERR_PTR(ret));
 }
 
-void xpcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
-		  phy_interface_t interface, int speed, int duplex)
+static void xpcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
+			 phy_interface_t interface, int speed, int duplex)
 {
 	struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
 

@@ -1219,7 +1219,6 @@ void xpcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
 	if (interface == PHY_INTERFACE_MODE_1000BASEX)
 		return xpcs_link_up_1000basex(xpcs, neg_mode, speed, duplex);
 }
-EXPORT_SYMBOL_GPL(xpcs_link_up);
 
 static void xpcs_an_restart(struct phylink_pcs *pcs)
 {
include/linux/pcs/pcs-xpcs.h +0 -4
index 758daabb76c7..abda475111d1 100644
@@ -65,10 +65,6 @@ struct dw_xpcs {
 };
 
 int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface);
-void xpcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
-		  phy_interface_t interface, int speed, int duplex);
-int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
-		   const unsigned long *advertising, unsigned int neg_mode);
 void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces);
 int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns,
 		    int enable);
Signature · 1 line
2.30.2
CI Applied
#22 lore raw
In reply to Russell King (Oracle)
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 6 lines · On Tue, 1 Oct 2024 17:02:48 +0100 you wrote:

Hi,

First, sorry for the bland series subject - this is the first in a number of cleanup series to the XPCS driver. This series has some functional changes beyond merely cleanups, notably the first patch.

Quoted 8 lines

This series starts off with a patch that moves the PCS reset from the xpcs_create*() family of calls to when phylink first configures the PHY. The motivation for this change is to get rid of the interface argument to the xpcs_create*() functions, which I see as unnecessary complexity. This patch should be tested on Wangxun and STMMAC drivers.

[...]

Here is the summary with links:
- [net-next,01/10] net: pcs: xpcs: move PCS reset to .pcs_pre_config()

    https://git.kernel.org/netdev/net-next/c/277b339c4ba5
  - [net-next,02/10] net: pcs: xpcs: drop interface argument from internal functions
    https://git.kernel.org/netdev/net-next/c/92fb8986083a
  - [net-next,03/10] net: pcs: xpcs: get rid of xpcs_init_iface()
    https://git.kernel.org/netdev/net-next/c/a487c9e7cfc4
  - [net-next,04/10] net: pcs: xpcs: add xpcs_destroy_pcs() and xpcs_create_pcs_mdiodev()
    https://git.kernel.org/netdev/net-next/c/bedea1539acb
  - [net-next,05/10] net: wangxun: txgbe: use phylink_pcs internally
    https://git.kernel.org/netdev/net-next/c/155c499ffd1d
  - [net-next,06/10] net: dsa: sja1105: simplify static configuration reload
    https://git.kernel.org/netdev/net-next/c/a18891b55703
  - [net-next,07/10] net: dsa: sja1105: call PCS config/link_up via pcs_ops structure
    https://git.kernel.org/netdev/net-next/c/907476c66d73
  - [net-next,08/10] net: dsa: sja1105: use phylink_pcs internally
    https://git.kernel.org/netdev/net-next/c/41bf58314b17
  - [net-next,09/10] net: pcs: xpcs: drop interface argument from xpcs_create*()
    https://git.kernel.org/netdev/net-next/c/bf5a61645bb2
  - [net-next,10/10] net: pcs: xpcs: make xpcs_do_config() and xpcs_link_up() internal
    https://git.kernel.org/netdev/net-next/c/faefc9730d07

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Tested-by on the v1 series
#23 lore raw
In reply to Russell King (Oracle)
Quoted 23 lines · Russell King (Oracle)

Hi,

First, sorry for the bland series subject - this is the first in a number of cleanup series to the XPCS driver. This series has some functional changes beyond merely cleanups, notably the first patch.

This series starts off with a patch that moves the PCS reset from the xpcs_create*() family of calls to when phylink first configures the PHY. The motivation for this change is to get rid of the interface argument to the xpcs_create*() functions, which I see as unnecessary complexity. This patch should be tested on Wangxun and STMMAC drivers.

Patch 2 removes the now unnecessary interface argument from the internal xpcs_create() and xpcs_init_iface() functions. With this, xpcs_init_iface() becomes a misnamed function, but patch 3 removes this function, moving its now meager contents to xpcs_create().

Patch 4 adds xpcs_destroy_pcs() and xpcs_create_pcs_mdiodev() functions which return and take a phylink_pcs, allowing SJA1105 and Wangxun drivers to be converted to using the phylink_pcs structure internally.

Quoted 5 lines

Patches 5 through 8 convert both these drivers to that end.

Patch 9 drops the interface argument from the remaining xpcs_create*() functions, addressing the only remaining caller of these functions, that being the STMMAC driver.

Better later than never. Just reached my hardware treasury and managed to test the series out on the next setup:

DW XGMAC <-(XGMII)-> DW XPCS <-(10Gbase-R)-> Marvell 88x2222 <-(10gbase-R)->
SFP+ fiber SFP+
<-(10gbase-r)->
Marvell 88x2222 <-(10gbase-r)-> DW XPCS <-(XGMII)-> DW XGMAC

DW XGMAC was working under the STMMAC driver control.

No problem has been spotted. So

Tested-by Serge Semin <[email protected]>

-Serge(y)

Quoted 18 lines

As patch 7 removed the direct calls to the XPCS config/link-up functions, the last patch makes these functions static.

 drivers/net/dsa/sja1105/sja1105.h                 |  2 +-
 drivers/net/dsa/sja1105/sja1105_main.c            | 85 ++++++++++----------
 drivers/net/dsa/sja1105/sja1105_mdio.c            | 28 ++++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  7 +-
 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c    | 18 ++---
 drivers/net/ethernet/wangxun/txgbe/txgbe_type.h   |  2 +-
 drivers/net/pcs/pcs-xpcs.c                        | 92 ++++++++++++++---------
 include/linux/pcs/pcs-xpcs.h                      | 14 ++--
 8 files changed, 132 insertions(+), 116 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!