netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver
@ 2022-11-14 17:07 Vladimir Oltean
  2022-11-14 17:07 ` [PATCH v2 net-next 1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs Vladimir Oltean
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Vladimir Oltean @ 2022-11-14 17:07 UTC (permalink / raw)
  To: netdev
  Cc: Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Russell King, Sean Anderson, Colin Foster,
	Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni

v1->v2: leave dsa_port_phylink_validate() for now, just remove
        ds->ops->phylink_validate()

The Felix DSA driver still uses its own phylink_validate() procedure
rather than the (relatively newly introduced) phylink_generic_validate()
because the latter did not cater for the case where a PHY provides rate
matching between the Ethernet cable side speed and the SERDES side
speed (and does not advertise other speeds except for the SERDES speed).

This changed with Sean Anderson's generic support for rate matching PHYs
in phylib and phylink:
https://patchwork.kernel.org/project/netdevbpf/cover/20220920221235.1487501-1-sean.anderson@seco.com/

Building upon that support, this patch set makes Linux understand that
the PHYs used in combination with the Felix DSA driver (SCH-30841 riser
card with AQR412 PHY, used with SERDES protocol 0x7777 - 4x2500base-x,
plugged into LS1028A-QDS) do support PAUSE rate matching. This requires
Aquantia PHY driver support for new PHY IDs.

To activate the rate matching support in phylink, config->mac_capabilities
must be populated. Coincidentally, this also opts the Felix driver into
the generic phylink validation.

Next, code that is no longer necessary is eliminated. This includes the
Felix driver validation procedures for VSC9959 and VSC9953, the
workaround in the Ocelot switch library to leave RX flow control always
enabled, as well as DSA plumbing necessary for a custom phylink
validation procedure to be propagated to the hardware driver level.

Many thanks go to Sean Anderson for providing generic support for rate
matching.

Vladimir Oltean (4):
  net: phy: aquantia: add AQR112 and AQR412 PHY IDs
  net: dsa: felix: use phylink_generic_validate()
  net: mscc: ocelot: drop workaround for forcing RX flow control
  net: dsa: remove phylink_validate() method

 drivers/net/dsa/ocelot/felix.c           | 16 +++-------
 drivers/net/dsa/ocelot/felix.h           |  3 --
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 30 ------------------
 drivers/net/dsa/ocelot/seville_vsc9953.c | 27 ----------------
 drivers/net/ethernet/mscc/ocelot.c       |  6 ++--
 drivers/net/phy/aquantia_main.c          | 40 ++++++++++++++++++++++++
 include/net/dsa.h                        |  3 --
 net/dsa/port.c                           | 18 +++++------
 8 files changed, 54 insertions(+), 89 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 net-next 1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs
  2022-11-14 17:07 [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver Vladimir Oltean
@ 2022-11-14 17:07 ` Vladimir Oltean
  2022-11-14 17:07 ` [PATCH v2 net-next 2/4] net: dsa: felix: use phylink_generic_validate() Vladimir Oltean
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Vladimir Oltean @ 2022-11-14 17:07 UTC (permalink / raw)
  To: netdev
  Cc: Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Russell King, Sean Anderson, Colin Foster,
	Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni

These are Gen3 Aquantia N-BASET PHYs which support 5GBASE-T,
2.5GBASE-T, 1000BASE-T and 100BASE-TX (not 10G); also EEE, Sync-E,
PTP, PoE.

The 112 is a single PHY package, the 412 is a quad PHY package.

The system-side SERDES interface of these PHYs selects its protocol
depending on the negotiated media side link speed. That protocol can be
1000BASE-X, 2500BASE-X, 10GBASE-R, SGMII, USXGMII.

The configuration of which SERDES protocol to use for which link speed
is made by firmware; even though it could be overwritten over MDIO by
Linux, we assume that the firmware provisioning is ok for the board on
which the driver probes.

For cases when the system side runs at a fixed rate, we want phylib/phylink
to detect the PAUSE rate matching ability of these PHYs, so we need to
use the Aquantia rather than the generic C45 driver. This needs
aqr107_read_status() -> aqr107_read_rate() to set phydev->rate_matching,
as well as the aqr107_get_rate_matching() method.

I am a bit unsure about the naming convention in the driver. Since
AQR107 is a Gen2 PHY, I assume all functions prefixed with "aqr107_"
rather than "aqr_" mean Gen2+ features. So I've reused this naming
convention.

I've tested PHY "SGMII" statistics as well as the .link_change_notify
method, which prints:

Aquantia AQR412 mdio_mux-0.4:00: Link partner is Aquantia PHY, FW 4.3, fast-retrain downshift advertised, fast reframe advertised

Tested SERDES protocols are usxgmii and 2500base-x (the latter with
PAUSE rate matching). Tested link modes are 100/1000/2500 Base-T
(with Aquantia link partner and with other link partners). No notable
events observed.

The placement of these PHY IDs in the driver is right before AQR113C,
a Gen4 PHY.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: none

 drivers/net/phy/aquantia_main.c | 40 +++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index 47a76df36b74..334a6904ca5a 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c
@@ -22,6 +22,8 @@
 #define PHY_ID_AQR107	0x03a1b4e0
 #define PHY_ID_AQCS109	0x03a1b5c2
 #define PHY_ID_AQR405	0x03a1b4b0
+#define PHY_ID_AQR112	0x03a1b662
+#define PHY_ID_AQR412	0x03a1b712
 #define PHY_ID_AQR113C	0x31c31c12
 
 #define MDIO_PHYXS_VEND_IF_STATUS		0xe812
@@ -800,6 +802,42 @@ static struct phy_driver aqr_driver[] = {
 	.handle_interrupt = aqr_handle_interrupt,
 	.read_status	= aqr_read_status,
 },
+{
+	PHY_ID_MATCH_MODEL(PHY_ID_AQR112),
+	.name		= "Aquantia AQR112",
+	.probe		= aqr107_probe,
+	.config_aneg    = aqr_config_aneg,
+	.config_intr	= aqr_config_intr,
+	.handle_interrupt = aqr_handle_interrupt,
+	.get_tunable    = aqr107_get_tunable,
+	.set_tunable    = aqr107_set_tunable,
+	.suspend	= aqr107_suspend,
+	.resume		= aqr107_resume,
+	.read_status	= aqr107_read_status,
+	.get_rate_matching = aqr107_get_rate_matching,
+	.get_sset_count = aqr107_get_sset_count,
+	.get_strings	= aqr107_get_strings,
+	.get_stats	= aqr107_get_stats,
+	.link_change_notify = aqr107_link_change_notify,
+},
+{
+	PHY_ID_MATCH_MODEL(PHY_ID_AQR412),
+	.name		= "Aquantia AQR412",
+	.probe		= aqr107_probe,
+	.config_aneg    = aqr_config_aneg,
+	.config_intr	= aqr_config_intr,
+	.handle_interrupt = aqr_handle_interrupt,
+	.get_tunable    = aqr107_get_tunable,
+	.set_tunable    = aqr107_set_tunable,
+	.suspend	= aqr107_suspend,
+	.resume		= aqr107_resume,
+	.read_status	= aqr107_read_status,
+	.get_rate_matching = aqr107_get_rate_matching,
+	.get_sset_count = aqr107_get_sset_count,
+	.get_strings	= aqr107_get_strings,
+	.get_stats	= aqr107_get_stats,
+	.link_change_notify = aqr107_link_change_notify,
+},
 {
 	PHY_ID_MATCH_MODEL(PHY_ID_AQR113C),
 	.name           = "Aquantia AQR113C",
@@ -831,6 +869,8 @@ static struct mdio_device_id __maybe_unused aqr_tbl[] = {
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR107) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQCS109) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR405) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR112) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR412) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_AQR113C) },
 	{ }
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 net-next 2/4] net: dsa: felix: use phylink_generic_validate()
  2022-11-14 17:07 [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver Vladimir Oltean
  2022-11-14 17:07 ` [PATCH v2 net-next 1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs Vladimir Oltean
@ 2022-11-14 17:07 ` Vladimir Oltean
  2022-11-14 17:09   ` Russell King (Oracle)
  2022-11-14 17:07 ` [PATCH v2 net-next 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control Vladimir Oltean
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Vladimir Oltean @ 2022-11-14 17:07 UTC (permalink / raw)
  To: netdev
  Cc: Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Russell King, Sean Anderson, Colin Foster,
	Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni

Drop the custom implementation of phylink_validate() in favor of the
generic one, which requires config->mac_capabilities to be set.

This was used up until now because of the possibility of being paired
with Aquantia PHYs with support for rate matching. The phylink framework
gained generic support for these, and knows to advertise all 10/100/1000
lower speed link modes when our SERDES protocol is 2500base-x
(fixed speed).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: none

 drivers/net/dsa/ocelot/felix.c           | 16 ++++---------
 drivers/net/dsa/ocelot/felix.h           |  3 ---
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 30 ------------------------
 drivers/net/dsa/ocelot/seville_vsc9953.c | 27 ---------------------
 4 files changed, 4 insertions(+), 72 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index dd3a18cc89dd..44e160f32067 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1048,21 +1048,14 @@ static void felix_phylink_get_caps(struct dsa_switch *ds, int port,
 	 */
 	config->legacy_pre_march2020 = false;
 
+	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
+				   MAC_10 | MAC_100 | MAC_1000FD |
+				   MAC_2500FD;
+
 	__set_bit(ocelot->ports[port]->phy_mode,
 		  config->supported_interfaces);
 }
 
-static void felix_phylink_validate(struct dsa_switch *ds, int port,
-				   unsigned long *supported,
-				   struct phylink_link_state *state)
-{
-	struct ocelot *ocelot = ds->priv;
-	struct felix *felix = ocelot_to_felix(ocelot);
-
-	if (felix->info->phylink_validate)
-		felix->info->phylink_validate(ocelot, port, supported, state);
-}
-
 static struct phylink_pcs *felix_phylink_mac_select_pcs(struct dsa_switch *ds,
 							int port,
 							phy_interface_t iface)
@@ -2050,7 +2043,6 @@ const struct dsa_switch_ops felix_switch_ops = {
 	.get_sset_count			= felix_get_sset_count,
 	.get_ts_info			= felix_get_ts_info,
 	.phylink_get_caps		= felix_phylink_get_caps,
-	.phylink_validate		= felix_phylink_validate,
 	.phylink_mac_select_pcs		= felix_phylink_mac_select_pcs,
 	.phylink_mac_link_down		= felix_phylink_mac_link_down,
 	.phylink_mac_link_up		= felix_phylink_mac_link_up,
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index c9c29999c336..42338116eed0 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -52,9 +52,6 @@ struct felix_info {
 
 	int	(*mdio_bus_alloc)(struct ocelot *ocelot);
 	void	(*mdio_bus_free)(struct ocelot *ocelot);
-	void	(*phylink_validate)(struct ocelot *ocelot, int port,
-				    unsigned long *supported,
-				    struct phylink_link_state *state);
 	int	(*port_setup_tc)(struct dsa_switch *ds, int port,
 				 enum tc_setup_type type, void *type_data);
 	void	(*tas_guard_bands_update)(struct ocelot *ocelot, int port);
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 26a35ae322d1..b0ae8d6156f6 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -885,35 +885,6 @@ static int vsc9959_reset(struct ocelot *ocelot)
 	return 0;
 }
 
-static void vsc9959_phylink_validate(struct ocelot *ocelot, int port,
-				     unsigned long *supported,
-				     struct phylink_link_state *state)
-{
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
-
-	phylink_set_port_modes(mask);
-	phylink_set(mask, Autoneg);
-	phylink_set(mask, Pause);
-	phylink_set(mask, Asym_Pause);
-	phylink_set(mask, 10baseT_Half);
-	phylink_set(mask, 10baseT_Full);
-	phylink_set(mask, 100baseT_Half);
-	phylink_set(mask, 100baseT_Full);
-	phylink_set(mask, 1000baseT_Half);
-	phylink_set(mask, 1000baseT_Full);
-	phylink_set(mask, 1000baseX_Full);
-
-	if (state->interface == PHY_INTERFACE_MODE_INTERNAL ||
-	    state->interface == PHY_INTERFACE_MODE_2500BASEX ||
-	    state->interface == PHY_INTERFACE_MODE_USXGMII) {
-		phylink_set(mask, 2500baseT_Full);
-		phylink_set(mask, 2500baseX_Full);
-	}
-
-	linkmode_and(supported, supported, mask);
-	linkmode_and(state->advertising, state->advertising, mask);
-}
-
 /* Watermark encode
  * Bit 8:   Unit; 0:1, 1:16
  * Bit 7-0: Value to be multiplied with unit
@@ -2588,7 +2559,6 @@ static const struct felix_info felix_info_vsc9959 = {
 	.ptp_caps		= &vsc9959_ptp_caps,
 	.mdio_bus_alloc		= vsc9959_mdio_bus_alloc,
 	.mdio_bus_free		= vsc9959_mdio_bus_free,
-	.phylink_validate	= vsc9959_phylink_validate,
 	.port_modes		= vsc9959_port_modes,
 	.port_setup_tc		= vsc9959_port_setup_tc,
 	.port_sched_speed_set	= vsc9959_sched_speed_set,
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 7af33b2c685d..6500c1697dd6 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -840,32 +840,6 @@ static int vsc9953_reset(struct ocelot *ocelot)
 	return 0;
 }
 
-static void vsc9953_phylink_validate(struct ocelot *ocelot, int port,
-				     unsigned long *supported,
-				     struct phylink_link_state *state)
-{
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
-
-	phylink_set_port_modes(mask);
-	phylink_set(mask, Autoneg);
-	phylink_set(mask, Pause);
-	phylink_set(mask, Asym_Pause);
-	phylink_set(mask, 10baseT_Full);
-	phylink_set(mask, 10baseT_Half);
-	phylink_set(mask, 100baseT_Full);
-	phylink_set(mask, 100baseT_Half);
-	phylink_set(mask, 1000baseT_Full);
-	phylink_set(mask, 1000baseX_Full);
-
-	if (state->interface == PHY_INTERFACE_MODE_INTERNAL) {
-		phylink_set(mask, 2500baseT_Full);
-		phylink_set(mask, 2500baseX_Full);
-	}
-
-	linkmode_and(supported, supported, mask);
-	linkmode_and(state->advertising, state->advertising, mask);
-}
-
 /* Watermark encode
  * Bit 9:   Unit; 0:1, 1:16
  * Bit 8-0: Value to be multiplied with unit
@@ -1007,7 +981,6 @@ static const struct felix_info seville_info_vsc9953 = {
 	.num_tx_queues		= OCELOT_NUM_TC,
 	.mdio_bus_alloc		= vsc9953_mdio_bus_alloc,
 	.mdio_bus_free		= vsc9953_mdio_bus_free,
-	.phylink_validate	= vsc9953_phylink_validate,
 	.port_modes		= vsc9953_port_modes,
 };
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 net-next 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control
  2022-11-14 17:07 [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver Vladimir Oltean
  2022-11-14 17:07 ` [PATCH v2 net-next 1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs Vladimir Oltean
  2022-11-14 17:07 ` [PATCH v2 net-next 2/4] net: dsa: felix: use phylink_generic_validate() Vladimir Oltean
@ 2022-11-14 17:07 ` Vladimir Oltean
  2022-11-14 17:10   ` Russell King (Oracle)
  2022-11-14 17:07 ` [PATCH v2 net-next 4/4] net: dsa: remove phylink_validate() method Vladimir Oltean
  2022-11-16  5:00 ` [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver patchwork-bot+netdevbpf
  4 siblings, 1 reply; 9+ messages in thread
From: Vladimir Oltean @ 2022-11-14 17:07 UTC (permalink / raw)
  To: netdev
  Cc: Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Russell King, Sean Anderson, Colin Foster,
	Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni

As phylink gained generic support for PHYs with rate matching via PAUSE
frames, the phylink_mac_link_up() method will be called with the maximum
speed and with rx_pause=true if rate matching is in use. This means that
setups with 2500base-x as the SERDES protocol between the MAC/PCS and
the PHY now work with no need for the driver to do anything special.

Tested with fsl-ls1028a-qds-7777.dts.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: none

 drivers/net/ethernet/mscc/ocelot.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 13b14110a060..da56f9bfeaf0 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -872,10 +872,8 @@ void ocelot_phylink_mac_link_up(struct ocelot *ocelot, int port,
 		return;
 	}
 
-	/* Handle RX pause in all cases, with 2500base-X this is used for rate
-	 * adaptation.
-	 */
-	mac_fc_cfg |= SYS_MAC_FC_CFG_RX_FC_ENA;
+	if (rx_pause)
+		mac_fc_cfg |= SYS_MAC_FC_CFG_RX_FC_ENA;
 
 	if (tx_pause)
 		mac_fc_cfg |= SYS_MAC_FC_CFG_TX_FC_ENA |
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 net-next 4/4] net: dsa: remove phylink_validate() method
  2022-11-14 17:07 [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver Vladimir Oltean
                   ` (2 preceding siblings ...)
  2022-11-14 17:07 ` [PATCH v2 net-next 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control Vladimir Oltean
@ 2022-11-14 17:07 ` Vladimir Oltean
  2022-11-14 17:11   ` Russell King (Oracle)
  2022-11-16  5:00 ` [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver patchwork-bot+netdevbpf
  4 siblings, 1 reply; 9+ messages in thread
From: Vladimir Oltean @ 2022-11-14 17:07 UTC (permalink / raw)
  To: netdev
  Cc: Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Russell King, Sean Anderson, Colin Foster,
	Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni

As of now, no DSA driver uses a custom link mode validation procedure
anymore. So remove this DSA operation and let phylink determine what is
supported based on config->mac_capabilities (if provided by the driver).
Leave a comment why we left the code that we did, and that there is more
work to do.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2:
remove ds->ops->phylink_validate() but keep dsa_port_phylink_validate()
as a no-op shim for those drivers which do not populate mac_capabilities.
Eventually this will have to go away too, but it's outside the scope of
the current series.

 include/net/dsa.h |  3 ---
 net/dsa/port.c    | 18 ++++++++----------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ee369670e20e..dde364688739 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -880,9 +880,6 @@ struct dsa_switch_ops {
 	 */
 	void	(*phylink_get_caps)(struct dsa_switch *ds, int port,
 				    struct phylink_config *config);
-	void	(*phylink_validate)(struct dsa_switch *ds, int port,
-				    unsigned long *supported,
-				    struct phylink_link_state *state);
 	struct phylink_pcs *(*phylink_mac_select_pcs)(struct dsa_switch *ds,
 						      int port,
 						      phy_interface_t iface);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 208168276995..48c9eaa74aee 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1536,16 +1536,14 @@ static void dsa_port_phylink_validate(struct phylink_config *config,
 				      unsigned long *supported,
 				      struct phylink_link_state *state)
 {
-	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct dsa_switch *ds = dp->ds;
-
-	if (!ds->ops->phylink_validate) {
-		if (config->mac_capabilities)
-			phylink_generic_validate(config, supported, state);
-		return;
-	}
-
-	ds->ops->phylink_validate(ds, dp->index, supported, state);
+	/* Skip call for drivers which don't yet set mac_capabilities,
+	 * since validating in that case would mean their PHY will advertise
+	 * nothing. In turn, skipping validation makes them advertise
+	 * everything that the PHY supports, so those drivers should be
+	 * converted ASAP.
+	 */
+	if (config->mac_capabilities)
+		phylink_generic_validate(config, supported, state);
 }
 
 static void dsa_port_phylink_mac_pcs_get_state(struct phylink_config *config,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 net-next 2/4] net: dsa: felix: use phylink_generic_validate()
  2022-11-14 17:07 ` [PATCH v2 net-next 2/4] net: dsa: felix: use phylink_generic_validate() Vladimir Oltean
@ 2022-11-14 17:09   ` Russell King (Oracle)
  0 siblings, 0 replies; 9+ messages in thread
From: Russell King (Oracle) @ 2022-11-14 17:09 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Sean Anderson, Colin Foster, Andrew Lunn,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Mon, Nov 14, 2022 at 07:07:28PM +0200, Vladimir Oltean wrote:
> Drop the custom implementation of phylink_validate() in favor of the
> generic one, which requires config->mac_capabilities to be set.
> 
> This was used up until now because of the possibility of being paired
> with Aquantia PHYs with support for rate matching. The phylink framework
> gained generic support for these, and knows to advertise all 10/100/1000
> lower speed link modes when our SERDES protocol is 2500base-x
> (fixed speed).
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

LGTM.

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 net-next 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control
  2022-11-14 17:07 ` [PATCH v2 net-next 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control Vladimir Oltean
@ 2022-11-14 17:10   ` Russell King (Oracle)
  0 siblings, 0 replies; 9+ messages in thread
From: Russell King (Oracle) @ 2022-11-14 17:10 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Sean Anderson, Colin Foster, Andrew Lunn,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Mon, Nov 14, 2022 at 07:07:29PM +0200, Vladimir Oltean wrote:
> As phylink gained generic support for PHYs with rate matching via PAUSE
> frames, the phylink_mac_link_up() method will be called with the maximum
> speed and with rx_pause=true if rate matching is in use. This means that
> setups with 2500base-x as the SERDES protocol between the MAC/PCS and
> the PHY now work with no need for the driver to do anything special.
> 
> Tested with fsl-ls1028a-qds-7777.dts.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

LGTM.

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 net-next 4/4] net: dsa: remove phylink_validate() method
  2022-11-14 17:07 ` [PATCH v2 net-next 4/4] net: dsa: remove phylink_validate() method Vladimir Oltean
@ 2022-11-14 17:11   ` Russell King (Oracle)
  0 siblings, 0 replies; 9+ messages in thread
From: Russell King (Oracle) @ 2022-11-14 17:11 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Claudiu Manoil, Alexandre Belloni, UNGLinuxDriver,
	Heiner Kallweit, Sean Anderson, Colin Foster, Andrew Lunn,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Mon, Nov 14, 2022 at 07:07:30PM +0200, Vladimir Oltean wrote:
> As of now, no DSA driver uses a custom link mode validation procedure
> anymore. So remove this DSA operation and let phylink determine what is
> supported based on config->mac_capabilities (if provided by the driver).
> Leave a comment why we left the code that we did, and that there is more
> work to do.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Yay!

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver
  2022-11-14 17:07 [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver Vladimir Oltean
                   ` (3 preceding siblings ...)
  2022-11-14 17:07 ` [PATCH v2 net-next 4/4] net: dsa: remove phylink_validate() method Vladimir Oltean
@ 2022-11-16  5:00 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-11-16  5:00 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, claudiu.manoil, alexandre.belloni, UNGLinuxDriver,
	hkallweit1, linux, sean.anderson, colin.foster, andrew,
	f.fainelli, olteanv, davem, edumazet, kuba, pabeni

Hello:

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

On Mon, 14 Nov 2022 19:07:26 +0200 you wrote:
> v1->v2: leave dsa_port_phylink_validate() for now, just remove
>         ds->ops->phylink_validate()
> 
> The Felix DSA driver still uses its own phylink_validate() procedure
> rather than the (relatively newly introduced) phylink_generic_validate()
> because the latter did not cater for the case where a PHY provides rate
> matching between the Ethernet cable side speed and the SERDES side
> speed (and does not advertise other speeds except for the SERDES speed).
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs
    https://git.kernel.org/netdev/net-next/c/973fbe68df39
  - [v2,net-next,2/4] net: dsa: felix: use phylink_generic_validate()
    https://git.kernel.org/netdev/net-next/c/3e7e783291b4
  - [v2,net-next,3/4] net: mscc: ocelot: drop workaround for forcing RX flow control
    https://git.kernel.org/netdev/net-next/c/de8586ed4311
  - [v2,net-next,4/4] net: dsa: remove phylink_validate() method
    https://git.kernel.org/netdev/net-next/c/53d04b981110

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-11-16  5:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-14 17:07 [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver Vladimir Oltean
2022-11-14 17:07 ` [PATCH v2 net-next 1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs Vladimir Oltean
2022-11-14 17:07 ` [PATCH v2 net-next 2/4] net: dsa: felix: use phylink_generic_validate() Vladimir Oltean
2022-11-14 17:09   ` Russell King (Oracle)
2022-11-14 17:07 ` [PATCH v2 net-next 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control Vladimir Oltean
2022-11-14 17:10   ` Russell King (Oracle)
2022-11-14 17:07 ` [PATCH v2 net-next 4/4] net: dsa: remove phylink_validate() method Vladimir Oltean
2022-11-14 17:11   ` Russell King (Oracle)
2022-11-16  5:00 ` [PATCH v2 net-next 0/4] Remove phylink_validate() from Felix DSA driver patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).