net: phylink: rearrange ovr_an_inband support
Subsystems (7)
Russell King (Oracle)
maintainercover net: phylink: rearrange ovr_an_inband support
Hi,
This series addresses the use of the ovr_an_inband flag, which is used by two drivers to indicate to phylink that they wish to use inband mode without firmware specifying inband mode.
The issue with ovr_an_inband is that it overrides not only PHY mode, but also fixed-link mode. Both of the drivers that set this flag contain code to detect when fixed-link mode will be used, and then either avoid setting it or explicitly clear the flag. This is wasteful when phylink already knows this.
Therefore, the approach taken in this patch set is to replace the ovr_an_inband flag with a default_an_inband flag which means that phylink defaults to MLO_AN_INBAND instead of MLO_AN_PHY, and will allow that default to be overriden if firmware specifies a fixed-link. This allows users of ovr_an_inband to be simplified.
What's more is this requires minimal changes in phylink to allow this new mode of operation.
This series changes phylink, and also updates the two drivers (fman_memac and stmmac), and then removes the unnecessary complexity from the drivers.
This series may depend on the stmmac cleanup series I've posted earlier - this is something I have not checked, but I currently have these patches on top of that series.
drivers/net/ethernet/freescale/fman/fman_memac.c | 16 ++++++---------- drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 15 ++------------- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++-- drivers/net/phy/phylink.c | 11 ++++++++--- include/linux/phylink.h | 5 +++-- include/linux/stmmac.h | 2 +- 6 files changed, 22 insertions(+), 31 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!
PATCH 1/6 net: phylink: rearrange phylink_parse_mode()
Of the two users of phylink_config->ovr_an_inband, both manually check for a fixed link before setting this flag (or clearing it if they find a fixed link.) This is unnecessary complication.
Rearrange phylink_parse_mode() a little so we can change how phylink_config->ovr_an_inband works. This will allow the flag to be tested before checking for the fixed link properties in the next patch.
--- drivers/net/phy/phylink.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/net/phy/phylink.c +4 -1
@@ -899,12 +899,15 @@ static int phylink_parse_mode(struct phylink *pl,return -EINVAL ; } + pl->cfg_link_an_mode =MLO_AN_INBAND ; + } + +if (pl->cfg_link_an_mode ==MLO_AN_INBAND ) {linkmode_zero (pl->supported);phylink_set (pl->supported,MII );phylink_set (pl->supported, Autoneg);phylink_set (pl->supported, Asym_Pause);phylink_set (pl->supported, Pause); - pl->cfg_link_an_mode =MLO_AN_INBAND ;switch (pl->link_config.interface) {case PHY_INTERFACE_MODE_SGMII :
Signature · 1 line
2.30.2
PATCH 2/6 net: phylink: move test for ovr_an_inband
In reply to Russell King (Oracle)Of the two users of phylink_config->ovr_an_inband, both manually check for a fixed link before setting this flag (or clearing it if they find a fixed link.) This is unnecessary complication.
Test ovr_an_inband before checking for the fixed-link properties, which will allow ovr_an_inband to be overriden by a fixed link specification.
--- drivers/net/phy/phylink.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/phy/phylink.c +4 -2
@@ -885,14 +885,16 @@ static int phylink_parse_mode(struct phylink *pl,const char *managed;unsigned long caps; +if (pl->config->ovr_an_inband) + pl->cfg_link_an_mode =MLO_AN_INBAND ; + dn =fwnode_get_named_child_node (fwnode,"fixed-link" );if (dn ||fwnode_property_present (fwnode,"fixed-link" )) pl->cfg_link_an_mode =MLO_AN_FIXED ;fwnode_handle_put (dn);if ((fwnode_property_read_string (fwnode,"managed" , &managed) ==0 && -strcmp (managed,"in-band-status" ) ==0 ) || - pl->config->ovr_an_inband) { +strcmp (managed,"in-band-status" ) ==0 )) {if (pl->cfg_link_an_mode ==MLO_AN_FIXED ) {phylink_err (pl,"can't use both fixed-link and in-band-status\n" );
Signature · 1 line
2.30.2
PATCH 3/6 net: phylink: rename ovr_an_inband to default_an_inband
In reply to Russell King (Oracle)Since ovr_an_inband no longer overrides every MLO_AN_xxx mode, rename it to reflect what it now does - it changes the default mode from MLO_AN_PHY to MLO_AN_INBAND. Fix up the two users of this.
--- drivers/net/ethernet/freescale/fman/fman_memac.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- drivers/net/phy/phylink.c | 2 +- include/linux/phylink.h | 5 +++-- 4 files changed, 6 insertions(+), 5 deletions(-)
drivers/net/ethernet/freescale/fman/fman_memac.c +1 -1
@@ -1232,7 +1232,7 @@ int memac_initialization(struct mac_device *mac_dev, !of_property_read_bool (mac_node,"managed" ) && mac_dev->phy_if !=PHY_INTERFACE_MODE_MII && !phy_interface_mode_is_rgmii(mac_dev->phy_if)) - mac_dev->phylink_config.ovr_an_inband = true; + mac_dev->phylink_config.default_an_inband = true;of_node_put (fixed); err =memac_init (mac_dev->fman_mac);
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1 -1
@@ -1221,7 +1221,7 @@ static int stmmac_phy_setup(struct stmmac_priv *priv) mdio_bus_data = priv->plat->mdio_bus_data;if (mdio_bus_data) - priv->phylink_config.ovr_an_inband = + priv->phylink_config.default_an_inband = mdio_bus_data->xpcs_an_inband;/* Set the platform/firmware specified interface mode. Note, phylink
drivers/net/phy/phylink.c +1 -1
@@ -885,7 +885,7 @@ static int phylink_parse_mode(struct phylink *pl,const char *managed;unsigned long caps; -if (pl->config->ovr_an_inband) +if (pl->config->default_an_inband) pl->cfg_link_an_mode =MLO_AN_INBAND ; dn =fwnode_get_named_child_node (fwnode,"fixed-link" );
include/linux/phylink.h +3 -2
@@ -141,7 +141,8 @@ enum phylink_op_type {* @mac_requires_rxc: if true, the MAC always requires a receive clock from PHY. * The PHY driver should start the clock signal as soon as * possible and avoid stopping it during suspend events. -* @ovr_an_inband: if true, override PCS to MLO_AN_INBAND +* @default_an_inband: if true, defaults to MLO_AN_INBAND rather than +* MLO_AN_PHY. A fixed-link specification will override. * @get_fixed_state: callback to execute to determine the fixed link state, * if MAC link is at %MLO_AN_FIXED mode. * @supported_interfaces: bitmap describing which PHY_INTERFACE_MODE_xxx
@@ -154,7 +155,7 @@ struct phylink_config {bool poll_fixed_state;bool mac_managed_pm;bool mac_requires_rxc; -bool ovr_an_inband; +bool default_an_inband;void (*get_fixed_state)(struct phylink_config *config,struct phylink_link_state *state);DECLARE_PHY_INTERFACE_MASK (supported_interfaces);
Signature · 1 line
2.30.2
PATCH 4/6 net: fman_memac: remove the now unnecessary checking for fixed-link
In reply to Russell King (Oracle)Since default_an_inband can be overriden by a fixed-link specification, there is no need for memac to be checking for this before setting default_an_inband. Remove this code and update the comment.
--- drivers/net/ethernet/freescale/fman/fman_memac.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
drivers/net/ethernet/freescale/fman/fman_memac.c +5 -9
@@ -1066,7 +1066,6 @@ int memac_initialization(struct mac_device *mac_dev, struct fman_mac_params *params) {int err; -struct device_node *fixed;struct phylink_pcs *pcs;struct fman_mac *memac;unsigned long capabilities;
@@ -1222,18 +1221,15 @@ int memac_initialization(struct mac_device *mac_dev, memac->rgmii_no_half_duplex = true;/* Most boards should use MLO_AN_INBAND, but existing boards don't have -* a managed property. Default to MLO_AN_INBAND if nothing else is -* specified. We need to be careful and not enable this if we have a -* fixed link or if we are using MII or RGMII, since those -* configurations modes don't use in-band autonegotiation. +* a managed property. Default to MLO_AN_INBAND rather than MLO_AN_PHY. +* Phylink will allow this to be overriden by a fixed link. We need to +* be careful and not enable this if we are using MII or RGMII, since +* those configurations modes don't use in-band autonegotiation. */ - fixed =of_get_child_by_name (mac_node,"fixed-link" ); -if (!fixed && !of_property_read_bool (mac_node,"fixed-link" ) && - !of_property_read_bool (mac_node,"managed" ) && +if (!of_property_read_bool (mac_node,"managed" ) && mac_dev->phy_if !=PHY_INTERFACE_MODE_MII && !phy_interface_mode_is_rgmii (mac_dev->phy_if)) mac_dev->phylink_config.default_an_inband = true; -of_node_put (fixed); err =memac_init (mac_dev->fman_mac);if (err <0 )
Signature · 1 line
2.30.2
PATCH 5/6 net: stmmac: rename xpcs_an_inband to default_an_inband
In reply to Russell King (Oracle)Rename xpcs_an_inband to default_an_inband to reflect the change in phylink and its changed functionality.
--- drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 8 ++++---- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- include/linux/stmmac.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c +4 -4
@@ -248,7 +248,7 @@ static void intel_speed_mode_2500(struct net_device *ndev, void *intel_data)dev_info (priv->device,"Link Speed Mode: 2.5Gbps\n" ); priv->plat->max_speed =2500 ; priv->plat->phy_interface =PHY_INTERFACE_MODE_2500BASEX ; - priv->plat->mdio_bus_data->xpcs_an_inband = false; + priv->plat->mdio_bus_data->default_an_inband = false; }else { priv->plat->max_speed =1000 ; }
@@ -586,16 +586,16 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,if (plat->phy_interface ==PHY_INTERFACE_MODE_SGMII || plat->phy_interface ==PHY_INTERFACE_MODE_1000BASEX ) { plat->mdio_bus_data->has_xpcs = true; - plat->mdio_bus_data->xpcs_an_inband = true; + plat->mdio_bus_data->default_an_inband = true; } -/* For fixed-link setup, we clear xpcs_an_inband */ +/* For fixed-link setup, we clear default_an_inband */ if (fwnode) {struct fwnode_handle *fixed_node; fixed_node =fwnode_get_named_child_node (fwnode,"fixed-link" );if (fixed_node) - plat->mdio_bus_data->xpcs_an_inband = false; + plat->mdio_bus_data->default_an_inband = false;fwnode_handle_put (fixed_node); }
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1 -1
@@ -1222,7 +1222,7 @@ static int stmmac_phy_setup(struct stmmac_priv *priv) mdio_bus_data = priv->plat->mdio_bus_data;if (mdio_bus_data) priv->phylink_config.default_an_inband = - mdio_bus_data->xpcs_an_inband; + mdio_bus_data->default_an_inband;/* Set the platform/firmware specified interface mode. Note, phylink * deals with the PHY interface mode, not the MAC interface mode.
include/linux/stmmac.h +1 -1
@@ -83,7 +83,7 @@ struct stmmac_priv;struct stmmac_mdio_bus_data {unsigned int phy_mask;unsigned int has_xpcs; -unsigned int xpcs_an_inband; +unsigned int default_an_inband;int *irqs;int probed_phy_irq;bool needs_reset;
Signature · 1 line
2.30.2
PATCH 6/6 net: stmmac: dwmac-intel: remove checking for fixed link
In reply to Russell King (Oracle)With the new default_an_inband functionality in phylink, there is no need to check for a fixed link when this flag is set, since a fixed link will now override default_an_inband.
--- drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 11 ----------- 1 file changed, 11 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c +0 -11
@@ -589,17 +589,6 @@ static int intel_mgbe_common_data(struct pci_dev *pdev, plat->mdio_bus_data->default_an_inband = true; } -/* For fixed-link setup, we clear default_an_inband */ -if (fwnode) { -struct fwnode_handle *fixed_node; - - fixed_node =fwnode_get_named_child_node (fwnode,"fixed-link" ); -if (fixed_node) - plat->mdio_bus_data->default_an_inband = false; - -fwnode_handle_put (fixed_node); - } -/* Ensure mdio bus scan skips intel serdes and pcs-xpcs */ plat->mdio_bus_data->phy_mask =1 <<INTEL_MGBE_ADHOC_ADDR ; plat->mdio_bus_data->phy_mask |=1 <<INTEL_MGBE_XPCS_ADDR ;
Signature · 1 line
2.30.2
Quoted 29 lines · Russell King (Oracle)
Hi,
This series addresses the use of the
ovr_an_inbandflag, which is used by two drivers to indicate to phylink that they wish to use inband mode without firmware specifying inband mode.The issue with
ovr_an_inbandis that it overrides not only PHY mode, but also fixed-link mode. Both of the drivers that set this flag contain code to detect when fixed-link mode will be used, and then either avoid setting it or explicitly clear the flag. This is wasteful when phylink already knows this.Therefore, the approach taken in this patch set is to replace the
ovr_an_inbandflag with adefault_an_inbandflag which means that phylink defaults toMLO_AN_INBANDinstead ofMLO_AN_PHY, and will allow that default to be overriden if firmware specifies a fixed-link. This allows users ofovr_an_inbandto be simplified.What's more is this requires minimal changes in phylink to allow this new mode of operation.
This series changes phylink, and also updates the two drivers (
fman_memacand stmmac), and then removes the unnecessary complexity from the drivers.This series may depend on the stmmac cleanup series I've posted earlier - this is something I have not checked, but I currently have these patches on top of that series.
Quoted 12 lines
drivers/net/ethernet/freescale/fman/fman_memac.c | 16 ++++++---------- drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 15 ++------------- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++-- drivers/net/phy/phylink.c | 11 ++++++++--- include/linux/phylink.h | 5 +++-- include/linux/stmmac.h | 2 +- 6 files changed, 22 insertions(+), 31 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!
This all seems more clear to me now for what it is worth:
Re: [PATCH net-next 4/6] net: fman_memac: remove the now unnecessary checking for fixed-link
In reply to Russell King (Oracle)Quoted 35 lines · Russell King (Oracle)
Since
default_an_inbandcan be overriden by a fixed-link specification, there is no need for memac to be checking for this before settingdefault_an_inband. Remove this code and update the comment.Signed-off-by Russell King (Oracle) <[email protected]>--- drivers/net/ethernet/freescale/fman/fman_memac.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)drivers/net/ethernet/freescale/fman/fman_memac.c +4
-6@@ -1066,7 +1066,6 @@ int memac_initialization(struct mac_device *mac_dev, struct fman_mac_params *params) {int err; -struct device_node *fixed;struct phylink_pcs *pcs;struct fman_mac *memac;unsigned long capabilities;@@ -1222,18 +1221,15 @@ int memac_initialization(struct mac_device *mac_dev, memac->rgmii_no_half_duplex = true;/* Most boards should use MLO_AN_INBAND, but existing boards don't have -* a managed property. Default to MLO_AN_INBAND if nothing else is -* specified. We need to be careful and not enable this if we have a -* fixed link or if we are using MII or RGMII, since those -* configurations modes don't use in-band autonegotiation. +* a managed property. Default to MLO_AN_INBAND rather than MLO_AN_PHY. +* Phylink will allow this to be overriden by a fixed link. We need to +* be careful and not enable this if we are using MII or RGMII, since +* those configurations modes don't use in-band autonegotiation. */ - fixed =of_get_child_by_name (mac_node,"fixed-link" );
drivers/net/ethernet/freescale/fman/fman_memac.c int memac_initialization(struct mac_device *mac_dev, · line 1235
Quoted 10 lines
drivers/net/ethernet/freescale/fman/fman_memac.c +1
-3-if (!fixed && !of_property_read_bool (mac_node,"fixed-link" ) && - !of_property_read_bool (mac_node,"managed" ) && +if (!of_property_read_bool (mac_node,"managed" ) && mac_dev->phy_if !=PHY_INTERFACE_MODE_MII && !phy_interface_mode_is_rgmii (mac_dev->phy_if)) mac_dev->phylink_config.default_an_inband = true; -of_node_put (fixed); err =memac_init (mac_dev->fman_mac);if (err <0 )
[Clea Astarte Google Cloud, SECO SpA]<https://clea.ai/astarte-on-google-cloud>
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 Wed, 29 May 2024 14:28:42 +0100 you wrote:
Hi,
This series addresses the use of the
ovr_an_inbandflag, which is used by two drivers to indicate to phylink that they wish to use inband mode without firmware specifying inband mode.
Quoted 7 lines
The issue with
ovr_an_inbandis that it overrides not only PHY mode, but also fixed-link mode. Both of the drivers that set this flag contain code to detect when fixed-link mode will be used, and then either avoid setting it or explicitly clear the flag. This is wasteful when phylink already knows this.[...]
Here is the summary with links:
- [net-next,1/6] net: phylink: rearrange phylink_parse_mode()
https://git.kernel.org/netdev/net-next/c/75518b0dc9d6 - [net-next,2/6] net: phylink: move test for ovr_an_inband https://git.kernel.org/netdev/net-next/c/fea49f065c1c - [net-next,3/6] net: phylink: rename ovr_an_inband to default_an_inband https://git.kernel.org/netdev/net-next/c/02d00dc73d8d - [net-next,4/6] net: fman_memac: remove the now unnecessary checking for fixed-link https://git.kernel.org/netdev/net-next/c/5e332954e760 - [net-next,5/6] net: stmmac: rename xpcs_an_inband to default_an_inband https://git.kernel.org/netdev/net-next/c/83f55b01dd90 - [net-next,6/6] net: stmmac: dwmac-intel: remove checking for fixed link https://git.kernel.org/netdev/net-next/c/ab77c7aa9388
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html