net: stmmac: SerDes, PCS, BASE-X, and inband goodies
Subsystems (7)
cover net: stmmac: SerDes, PCS, BASE-X, and inband goodies
Mentioned previously, I've been trying to sort out the PCS support in stmmac, and this series represents the current state of play.
Previous posted patches centred around merely getting autonegotiation to be configured correctly, to a point where the manual configuration can be removed from the qcom-ethqos driver. The qcom-ethqos driver uses both SGMII and 2500BASE-X, manually configuring the dwmac's integrated PCS appropriately.
This *untested* series attempts to take this further. The patches:
- clean up qcom-ethqos only-written mac_base member.
- convert qcom-ethqos to use the set_clk_tx_rate() method for setting the link clock rate.
- add support for phy_set_mode_ext() to the qcom "SGMII" ethernet SerDes driver (which is really only what it needs. Note that phy_set_mode_ext() is an expected call to be made, where as phy_set_speed() is optional and not. See PHY documentation.)
- add platform-glue independent SerDes support to the stmmac core driver. Currently, only qcom-ethqos will make use of this, and I suspect as we haven't had this, it's going to be difficult to convert other platform glue to use this - but had this existed earlier, we could've pushed people to use PHY to abstract some of the platform glue differences. Adding it now makes it available for future platform glue.
- convert qcom-ethqos to use this core SerDes support.
- arrange for stmmac_pcs.c to supply the phy_intf_sel field value if the integrated PCS will be used. (PHY_INTF_SEL_SGMII requires the integrated PCS rather than an external PCS.)
- add BASE-X support to the integrated PCS driver, and use it for BASE-X modes. This fully supports in-band mode, including reading the link partner advertisement.
- add in-band support for SGMII, reading the state from the RGSMII status field.
As we leave qcom-ethqos' manual configuration of the PCS in place at the moment, the last patch adds reporting of any changes in its configuration that the qcom-ethqos driver does beyond what phylink requested, thus providing a path to debug and eventually remove qcom-ethqos' manual configuration.
One patch is not included in this set - which adds a phy_intf_sel value for external PCS (using PHY_INTF_SEL_GMII_MII). I believe all external PCS use this mode when connected to a MAC capable of up to 2.5G. However, no platform glue that provides the mac_select_pcs() method also provide the set_phy_intf_sel() method, so we can safely ignore this for now.
I would like to get this into net-next before the next merge window, so testing would be appreciated. If there are issues with these patches applied, please check whether the issue exists without these patches and only report regressions caused by this patch set. For example, I'm aware that qcom-ethqos has issues with 10Mbps mode due to an AQR PHY being insanely provisioned to use SGMII in 1000M mode but with rate matching with 10M media. This is not an issue that is relevant to this patch series, but a problem with the PHY provisioning.
drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +- drivers/net/ethernet/stmicro/stmmac/common.h | 1 - .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 74 ++----- .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 9 +- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 8 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 69 +++++-- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c | 218 +++++++++++++++++++-- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 53 ++--- .../net/ethernet/stmicro/stmmac/stmmac_serdes.c | 111 +++++++++++ .../net/ethernet/stmicro/stmmac/stmmac_serdes.h | 16 ++ drivers/phy/qualcomm/phy-qcom-sgmii-eth.c | 42 ++++ include/linux/stmmac.h | 2 + 12 files changed, 480 insertions(+), 125 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/14 net: stmmac: qcom-ethqos: remove mac_base
Since the blamed commit, ethqos->mac_base is only written, never read. Let's remove it.
--- drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 3 --- 1 file changed, 3 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +0 -3
@@ -100,7 +100,6 @@ struct ethqos_emac_driver_data {struct qcom_ethqos {struct platform_device *pdev;void __iomem *rgmii_base; -void __iomem *mac_base;int (*configure_func)(struct qcom_ethqos *ethqos,int speed);unsigned int link_clk_rate;
@@ -772,8 +771,6 @@ static int qcom_ethqos_probe(struct platform_device *pdev)return dev_err_probe (dev,PTR_ERR (ethqos->rgmii_base),"Failed to map rgmii resource\n" ); - ethqos->mac_base = stmmac_res.addr; - data =of_device_get_match_data (dev); ethqos->por = data->por; ethqos->num_por = data->num_por;
Signature · 1 line
2.47.3
Re: [PATCH net-next 01/14] net: stmmac: qcom-ethqos: remove mac_base
Quoted 6 lines · Russell King (Oracle)
Since the blamed commit,
ethqos->mac_baseis only written, never read. Let's remove it.Fixes 9b443e58a896 ("net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification")Signed-off-by Russell King (Oracle) <[email protected]>---
I think we reserve the use of Fixes: tags for user-visible issues, not cleanup of dangling variables. You can move the sha1sum and its commit title in the main commit message body and delete that tag.
PATCH 2/14 net: stmmac: qcom-ethqos: convert to set_clk_tx_rate() method
In reply to Russell King (Oracle)Set the RGMII link clock using the set_clk_tx_rate() method rather than coding it into the .fix_mac_speed() method. This simplifies ethqos's ethqos_fix_mac_speed().
--- .../stmicro/stmmac/dwmac-qcom-ethqos.c | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +11 -10
@@ -102,7 +102,6 @@ struct qcom_ethqos {void __iomem *rgmii_base;int (*configure_func)(struct qcom_ethqos *ethqos,int speed); -unsigned int link_clk_rate;struct clk *link_clk;struct phy *serdes_phy;int serdes_speed;
@@ -174,19 +173,20 @@ static void rgmii_dump(void *priv) rgmii_readl(ethqos, EMAC_SYSTEM_LOW_POWER_DEBUG)); } -static void -ethqos_update_link_clk (struct qcom_ethqos *ethqos,int speed) +static int ethqos_set_clk_tx_rate (void *bsp_priv,struct clk *clk_tx_i, +phy_interface_t interface,int speed) { +struct qcom_ethqos *ethqos = bsp_priv;long rate; -if (!phy_interface_mode_is_rgmii (ethqos->phy_mode)) -return ; +if (!phy_interface_mode_is_rgmii (interface)) +return 0 ; rate =rgmii_clock (speed); -if (rate >0 ) - ethqos->link_clk_rate = rate *2 ; +if (rate <0 ) +return rate; -clk_set_rate (ethqos->link_clk, ethqos->link_clk_rate); +return clk_set_rate (ethqos->link_clk, rate *2 ); } static void
@@ -645,7 +645,6 @@ static void ethqos_fix_mac_speed(void *priv, int speed, unsigned int mode)struct qcom_ethqos *ethqos = priv;qcom_ethqos_set_sgmii_loopback (ethqos, false); -ethqos_update_link_clk (ethqos, speed);ethqos_configure (ethqos, speed); }
@@ -797,10 +796,12 @@ static int qcom_ethqos_probe(struct platform_device *pdev) "Failed to get serdes phy\n"); ethqos->serdes_speed =SPEED_1000 ; -ethqos_update_link_clk (ethqos,SPEED_1000 ); +ethqos_set_clk_tx_rate (ethqos,NULL , plat_dat->phy_interface, +SPEED_1000 );ethqos_set_func_clk_en (ethqos); plat_dat->bsp_priv = ethqos; + plat_dat->set_clk_tx_rate = ethqos_set_clk_tx_rate; plat_dat->fix_mac_speed = ethqos_fix_mac_speed; plat_dat->dump_debug_regs = rgmii_dump; plat_dat->ptp_clk_freq_config = ethqos_ptp_clk_freq_config;
Signature · 1 line
2.47.3
PATCH 3/14 phy: qcom-sgmii-eth: add .set_mode() and .validate() methods
In reply to Russell King (Oracle)qcom-sgmii-eth is an Ethernet SerDes supporting only Ethernet mode using SGMII, 1000BASE-X and 2500BASE-X.
Add an implementation of the .set_mode() method, which can be used instead of or as well as the .set_speed() method. The Ethernet interface modes mentioned above all have a fixed data rate, so setting the mode is sufficient to fully specify the operating parameters.
Add an implementation of the .validate() method, which will be necessary to allow discovery of the SerDes capabilities for platform independent SerDes support in the stmmac netowrk driver.
--- drivers/phy/qualcomm/phy-qcom-sgmii-eth.c | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+)
drivers/phy/qualcomm/phy-qcom-sgmii-eth.c +42 -0
@@ -286,6 +286,37 @@ static int qcom_dwmac_sgmii_phy_power_off(struct phy *phy)return 0 ; } +static int qcom_dwmac_sgmii_phy_speed (enum phy_mode mode,int submode) +{ +if (mode !=PHY_MODE_ETHERNET ) +return -EINVAL ; + +if (submode ==PHY_INTERFACE_MODE_SGMII || + submode ==PHY_INTERFACE_MODE_1000BASEX ) +return SPEED_1000 ; + +if (submode ==PHY_INTERFACE_MODE_2500BASEX ) +return SPEED_2500 ; + +return -EINVAL ; +} + +static int qcom_dwmac_sgmii_phy_set_mode (struct phy *phy,enum phy_mode mode, +int submode) +{ +struct qcom_dwmac_sgmii_phy_data *data =phy_get_drvdata (phy); +int speed; + + speed =qcom_dwmac_sgmii_phy_speed (mode, submode); +if (speed <0 ) +return speed; + +if (speed != data->speed) + data->speed = speed; + +return qcom_dwmac_sgmii_phy_calibrate (phy); +} +static int qcom_dwmac_sgmii_phy_set_speed (struct phy *phy,int speed) {struct qcom_dwmac_sgmii_phy_data *data =phy_get_drvdata (phy);
@@ -296,10 +327,21 @@ static int qcom_dwmac_sgmii_phy_set_speed(struct phy *phy, int speed)return qcom_dwmac_sgmii_phy_calibrate (phy); } +static int qcom_dwmac_sgmii_phy_validate (struct phy *phy,enum phy_mode mode, +int submode, +union phy_configure_opts *opts) +{ +int ret =qcom_dwmac_sgmii_phy_speed (mode, submode); + +return ret <0 ? ret :0 ; +} +static const struct phy_ops qcom_dwmac_sgmii_phy_ops = { .power_on = qcom_dwmac_sgmii_phy_power_on, .power_off = qcom_dwmac_sgmii_phy_power_off, + .set_mode = qcom_dwmac_sgmii_phy_set_mode, .set_speed = qcom_dwmac_sgmii_phy_set_speed, + .validate = qcom_dwmac_sgmii_phy_validate, .calibrate = qcom_dwmac_sgmii_phy_calibrate, .owner =THIS_MODULE , };
Signature · 1 line
2.47.3
Re: [PATCH net-next 03/14] phy: qcom-sgmii-eth: add .set_mode() and .validate() methods
Quoted 9 lines · Russell King (Oracle)
qcom-sgmii-eth is an Ethernet SerDes supporting only Ethernet mode using SGMII, 1000BASE-X and 2500BASE-X.
Add an implementation of the .set_mode() method, which can be used instead of or as well as the .set_speed() method. The Ethernet interface modes mentioned above all have a fixed data rate, so setting the mode is sufficient to fully specify the operating parameters.
Quoted 3 lines
Add an implementation of the .validate() method, which will be necessary to allow discovery of the SerDes capabilities for platform independent SerDes support in the stmmac netowrk driver.
s/netowrk/network/
Quoted 3 lines
Signed-off-by Russell King (Oracle) <[email protected]>---
Re: [PATCH net-next 03/14] phy: qcom-sgmii-eth: add .set_mode() and .validate() methods
Quoted 14 lines · Vladimir Oltean
Quoted 9 lines · Russell King (Oracle)
qcom-sgmii-eth is an Ethernet SerDes supporting only Ethernet mode using SGMII, 1000BASE-X and 2500BASE-X.
Add an implementation of the .set_mode() method, which can be used instead of or as well as the .set_speed() method. The Ethernet interface modes mentioned above all have a fixed data rate, so setting the mode is sufficient to fully specify the operating parameters.
Quoted 3 lines
Add an implementation of the .validate() method, which will be necessary to allow discovery of the SerDes capabilities for platform independent SerDes support in the stmmac netowrk driver.
Quoted 7 lines
s/netowrk/network/
Quoted 3 lines
Signed-off-by Russell King (Oracle) <[email protected]>---
Reviewed-by Vladimir Oltean <[email protected]>
According to patchwork (I forgot the RFC tag on the patches) it needs linux/phy.h included. Plesae let me know if you'd like me to retain your r-b. Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 03/14] phy: qcom-sgmii-eth: add .set_mode() and .validate() methods
Quoted 18 lines · Russell King (Oracle)
Quoted 17 lines · Vladimir Oltean
Quoted 9 lines · Russell King (Oracle)
qcom-sgmii-eth is an Ethernet SerDes supporting only Ethernet mode using SGMII, 1000BASE-X and 2500BASE-X.
Add an implementation of the .set_mode() method, which can be used instead of or as well as the .set_speed() method. The Ethernet interface modes mentioned above all have a fixed data rate, so setting the mode is sufficient to fully specify the operating parameters.
Quoted 3 lines
Add an implementation of the .validate() method, which will be necessary to allow discovery of the SerDes capabilities for platform independent SerDes support in the stmmac netowrk driver.
s/netowrk/network/
Quoted 1 line
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Signed-off-by Russell King (Oracle) <[email protected]>---
Reviewed-by Vladimir Oltean <[email protected]>According to patchwork (I forgot the RFC tag on the patches) it needs
linux/phy.hincluded. Plesae let me know if you'd like me to retain your r-b. Thanks.
Ah, ok, I thought that the SPEED_* macros are also phylib-specific like PHY_INTERFACE_MODE_*, and hence, the phylib header would already be included, but it seems the SPEED_* macros are provided by include/uapi/linux/ethtool.h. Go figure...
Please keep the review tag.
kernel test robot
build botRe: [PATCH net-next 03/14] phy: qcom-sgmii-eth: add .set_mode() and .validate() methods
In reply to Russell King (Oracle)kernel test robot noticed the following build errors:
Hi Russell,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Russell-King-Oracle/net-stmmac-qcom-ethqos-remove-mac_base/20260115-054728 base: net-next/main patch link: https://lore.kernel.org/r/E1vg4vs-00000003SFt-1Fje%40rmk-PC.armlinux.org.uk patch subject: [PATCH net-next 03/14] phy: qcom-sgmii-eth: add .set_mode() and .validate() methods config: microblaze-randconfig-r073-20260115 (https://download.01.org/0day-ci/archive/20260115/[email protected]/config) compiler: microblaze-linux-gcc (GCC) 14.3.0 smatch version: v0.5.0-8985-g2614ff1a reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260115/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
drivers/phy/qualcomm/phy-qcom-sgmii-eth.c: In function 'qcom_dwmac_sgmii_phy_speed':
>> drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:294:24: error: 'PHY_INTERFACE_MODE_SGMII' undeclared (first use in this function)
294 | if (submode == PHY_INTERFACE_MODE_SGMII ||
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:294:24: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:295:24: error: 'PHY_INTERFACE_MODE_1000BASEX' undeclared (first use in this function)
295 | submode == PHY_INTERFACE_MODE_1000BASEX)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:298:24: error: 'PHY_INTERFACE_MODE_2500BASEX' undeclared (first use in this function)
298 | if (submode == PHY_INTERFACE_MODE_2500BASEX)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~vim +/PHY_INTERFACE_MODE_SGMII +294 drivers/phy/qualcomm/phy-qcom-sgmii-eth.c
288
289 static int qcom_dwmac_sgmii_phy_speed(enum phy_mode mode, int submode)
290 {
291 if (mode != PHY_MODE_ETHERNET)
292 return -EINVAL;
293
> 294 if (submode == PHY_INTERFACE_MODE_SGMII || > 295 submode == PHY_INTERFACE_MODE_1000BASEX) 296 return SPEED_1000; 297 > 298 if (submode == PHY_INTERFACE_MODE_2500BASEX)
299 return SPEED_2500;
300
301 return -EINVAL;
302 }
303
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH net-next 03/14] phy: qcom-sgmii-eth: add .set_mode() and .validate() methods
In reply to Russell King (Oracle)kernel test robot noticed the following build errors:
Hi Russell,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Russell-King-Oracle/net-stmmac-qcom-ethqos-remove-mac_base/20260115-054728 base: net-next/main patch link: https://lore.kernel.org/r/E1vg4vs-00000003SFt-1Fje%40rmk-PC.armlinux.org.uk patch subject: [PATCH net-next 03/14] phy: qcom-sgmii-eth: add .set_mode() and .validate() methods config: powerpc-randconfig-002-20260115 (https://download.01.org/0day-ci/archive/20260115/[email protected]/config) compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260115/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:294:17: error: use of undeclared identifier 'PHY_INTERFACE_MODE_SGMII'
294 | if (submode == PHY_INTERFACE_MODE_SGMII ||
| ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:295:17: error: use of undeclared identifier 'PHY_INTERFACE_MODE_1000BASEX'
295 | submode == PHY_INTERFACE_MODE_1000BASEX)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:298:17: error: use of undeclared identifier 'PHY_INTERFACE_MODE_2500BASEX'
298 | if (submode == PHY_INTERFACE_MODE_2500BASEX)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~3 errors generated.
vim +/PHY_INTERFACE_MODE_SGMII +294 drivers/phy/qualcomm/phy-qcom-sgmii-eth.c
288
289 static int qcom_dwmac_sgmii_phy_speed(enum phy_mode mode, int submode)
290 {
291 if (mode != PHY_MODE_ETHERNET)
292 return -EINVAL;
293
> 294 if (submode == PHY_INTERFACE_MODE_SGMII || > 295 submode == PHY_INTERFACE_MODE_1000BASEX) 296 return SPEED_1000; 297 > 298 if (submode == PHY_INTERFACE_MODE_2500BASEX)
299 return SPEED_2500;
300
301 return -EINVAL;
302 }
303
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
PATCH 4/14 net: stmmac: wrap phylink's rx_clk_stop functions
In reply to Russell King (Oracle)With generic SerDes support, stmmac will need to do more work to ensure that clk_rx_i is running in all configurations. Rather than turn each site that calls phylink_rx_clk_stop_xxx() into a list of functions, move these to their own pair of functions so that they can be augmented at a single location.
--- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +21 -11
@@ -3547,6 +3547,16 @@ static void stmmac_safety_feat_configuration(struct stmmac_priv *priv) } } +static void stmmac_clk_rx_i_require (struct stmmac_priv *priv) +{ +phylink_rx_clk_stop_block (priv->phylink); +} + +static void stmmac_clk_rx_i_release (struct stmmac_priv *priv) +{ +phylink_rx_clk_stop_unblock (priv->phylink); +} +/** * stmmac_hw_setup - setup mac in a usable state. * @dev : pointer to the device structure.
@@ -3578,12 +3588,12 @@ static int stmmac_hw_setup(struct net_device *dev)* Block the receive clock stop for LPI mode at the PHY in case * the link is established with EEE mode active. */ -phylink_rx_clk_stop_block (priv->phylink); +stmmac_clk_rx_i_require (priv);/* DMA initialization and SW reset */ ret =stmmac_init_dma_engine (priv);if (ret <0 ) { -phylink_rx_clk_stop_unblock (priv->phylink); +stmmac_clk_rx_i_release (priv);netdev_err (priv->dev,"%s: DMA engine initialization failed\n" , __func__);return ret;
@@ -3591,7 +3601,7 @@ static int stmmac_hw_setup(struct net_device *dev)/* Copy the MAC addr into the HW */ stmmac_set_umac_addr (priv, priv->hw, dev->dev_addr,0 ); -phylink_rx_clk_stop_unblock (priv->phylink); +stmmac_clk_rx_i_release (priv);/* Initialize the MAC Core */ stmmac_core_init (priv, priv->hw, dev);
@@ -3670,9 +3680,9 @@ static int stmmac_hw_setup(struct net_device *dev)/* Start the ball rolling... */ stmmac_start_all_dma (priv); -phylink_rx_clk_stop_block (priv->phylink); +stmmac_clk_rx_i_require (priv);stmmac_set_hw_vlan_mode (priv, priv->hw); -phylink_rx_clk_stop_unblock (priv->phylink); +stmmac_clk_rx_i_release (priv);return 0 ; }
@@ -6107,9 +6117,9 @@ static int stmmac_set_features(struct net_device *netdev,else priv->hw->hw_vlan_en = false; -phylink_rx_clk_stop_block (priv->phylink); +stmmac_clk_rx_i_require (priv);stmmac_set_hw_vlan_mode (priv, priv->hw); -phylink_rx_clk_stop_unblock (priv->phylink); +stmmac_clk_rx_i_release (priv);return 0 ; }
@@ -6378,9 +6388,9 @@ static int stmmac_set_mac_address(struct net_device *ndev, void *addr)if (ret) goto set_mac_error; -phylink_rx_clk_stop_block (priv->phylink); +stmmac_clk_rx_i_require (priv);stmmac_set_umac_addr (priv, priv->hw, ndev->dev_addr,0 ); -phylink_rx_clk_stop_unblock (priv->phylink); +stmmac_clk_rx_i_release (priv); set_mac_error:pm_runtime_put (priv->device);
@@ -8192,11 +8202,11 @@ int stmmac_resume(struct device *dev)stmmac_init_timestamping (priv);stmmac_init_coalesce (priv); -phylink_rx_clk_stop_block (priv->phylink); +stmmac_clk_rx_i_require (priv);stmmac_set_rx_mode (ndev);stmmac_restore_hw_vlan_rx_fltr (priv, ndev, priv->hw); -phylink_rx_clk_stop_unblock (priv->phylink); +stmmac_clk_rx_i_release (priv);stmmac_enable_all_queues (priv);stmmac_enable_all_dma_irq (priv);
Signature · 1 line
2.47.3
Re: [PATCH net-next 04/14] net: stmmac: wrap phylink's rx_clk_stop functions
Hi Russell,
Quoted 7 lines · Russell King (Oracle)
With generic SerDes support, stmmac will need to do more work to ensure that
clk_rx_iis running in all configurations. Rather than turn each site that callsphylink_rx_clk_stop_xxx()into a list of functions, move these to their own pair of functions so that they can be augmented at a single location.Signed-off-by Russell King (Oracle) <[email protected]>
Maxime
PATCH 5/14 net: stmmac: add stmmac core serdes support
In reply to Russell King (Oracle)Rather than having platform glue implement SerDes PHY support, add it to the core driver, specifically to the stmmac integrated PCS driver as the SerDes is connected to the integrated PCS.
Platforms using external PCS can also populate plat->serdes, and the core driver will call phy_init() and phy_exit() when the administrative state of the interface changes, but the other phy methods will not be called.
--- drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 38 +++++- .../net/ethernet/stmicro/stmmac/stmmac_pcs.h | 1 + .../ethernet/stmicro/stmmac/stmmac_serdes.c | 111 ++++++++++++++++++ .../ethernet/stmicro/stmmac/stmmac_serdes.h | 16 +++ include/linux/stmmac.h | 2 + 7 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.h
drivers/net/ethernet/stmicro/stmmac/Makefile +1 -1
@@ -7,7 +7,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \ dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \ stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \ stmmac_xdp.o stmmac_est.o stmmac_fpe.o stmmac_vlan.o \ - stmmac_pcs.o $(stmmac-y) + stmmac_pcs.o stmmac_serdes.o $(stmmac-y) stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +13 -1
@@ -48,6 +48,7 @@#include "stmmac_fpe.h" #include "stmmac.h" #include "stmmac_pcs.h" +#include "stmmac_serdes.h" #include "stmmac_xdp.h" #include <linux/reset.h> #include <linux/of_mdio.h>
@@ -3549,12 +3550,16 @@ static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)static void stmmac_clk_rx_i_require (struct stmmac_priv *priv) { +dwmac_serdes_power_on (priv); +/* Only sets the SerDes mode if it wasn't already configured. */ +dwmac_serdes_init_mode (priv, priv->plat->phy_interface);phylink_rx_clk_stop_block (priv->phylink); }static void stmmac_clk_rx_i_release (struct stmmac_priv *priv) {phylink_rx_clk_stop_unblock (priv->phylink); +dwmac_serdes_power_off (priv); }/**
@@ -4152,10 +4157,14 @@ static int stmmac_open(struct net_device *dev)if (ret) goto err_runtime_pm; - ret =__stmmac_open (dev, dma_conf); + ret =dwmac_serdes_init (priv);if (ret) goto err_disconnect_phy; + ret =__stmmac_open (dev, dma_conf); +if (ret) + goto err_serdes; +kfree (dma_conf);/* We may have called phylink_speed_down before */
@@ -4163,6 +4172,8 @@ static int stmmac_open(struct net_device *dev)return ret; +err_serdes: +dwmac_serdes_exit (priv); err_disconnect_phy:phylink_disconnect_phy (priv->phylink); err_runtime_pm:
@@ -4226,6 +4237,7 @@ static int stmmac_release(struct net_device *dev)__stmmac_release (dev); +dwmac_serdes_exit (priv);phylink_disconnect_phy (priv->phylink);pm_runtime_put (priv->device);
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +36 -2
@@ -1,12 +1,25 @@// SPDX-License-Identifier: GPL-2.0-only #include "stmmac.h" #include "stmmac_pcs.h" +#include "stmmac_serdes.h" static int dwmac_integrated_pcs_enable (struct phylink_pcs *pcs) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +struct stmmac_priv *priv = spcs->priv; +int ret; -stmmac_mac_irq_modify (spcs->priv,0 , spcs->int_mask); + ret =dwmac_serdes_power_on (priv); +if (ret) +return ret; + +if (spcs->interface !=PHY_INTERFACE_MODE_NA ) { + ret =dwmac_serdes_set_mode (priv, spcs->interface); +if (ret) +return ret; + } + +stmmac_mac_irq_modify (priv,0 , spcs->int_mask);return 0 ; }
@@ -14,8 +27,11 @@ static int dwmac_integrated_pcs_enable(struct phylink_pcs *pcs)static void dwmac_integrated_pcs_disable (struct phylink_pcs *pcs) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +struct stmmac_priv *priv = spcs->priv; -stmmac_mac_irq_modify (spcs->priv, spcs->int_mask,0 ); +stmmac_mac_irq_modify (priv, spcs->int_mask,0 ); + +dwmac_serdes_power_off (priv); } static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs,
@@ -32,6 +48,15 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs, bool permit_pause_to_mac) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +int ret; + +if (spcs->interface != interface) { + ret =dwmac_serdes_set_mode (spcs->priv, interface); +if (ret) +return ret; + + spcs->interface = interface; + }dwmac_ctrl_ane (spcs->base,0 ,1 , spcs->priv->hw->reverse_sgmii_enable);
@@ -71,6 +96,7 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, u32 int_mask) {struct stmmac_pcs *spcs; +int ret; spcs =devm_kzalloc (priv->device,sizeof (*spcs),GFP_KERNEL );if (!spcs)
@@ -81,6 +107,14 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, spcs->int_mask = int_mask; spcs->pcs.ops = &dwmac_integrated_pcs_ops; +if (priv->plat->serdes) { + ret =dwmac_serdes_validate (priv,PHY_INTERFACE_MODE_SGMII ); +if (ret) +dev_warn (priv->device, +"serdes does not support SGMII: %pe\n" , +ERR_PTR (ret)); + } +__set_bit (PHY_INTERFACE_MODE_SGMII , spcs->pcs.supported_interfaces); priv->integrated_pcs = spcs;
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +1 -0
@@ -53,6 +53,7 @@ struct stmmac_pcs {struct stmmac_priv *priv;void __iomem *base;u32 int_mask; +phy_interface_t interface;struct phylink_pcs pcs; };
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c new file +111 -0
111 lines not on this page · raw message
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.h new file +16 -0
@@ -0,0 +1,16 @@ +#ifndef STMMAC_SERDES_H +#define STMMAC_SERDES_H + +#include <linux/phy.h> + +struct stmmac_priv ; + +int dwmac_serdes_validate (struct stmmac_priv *priv,phy_interface_t interface); +int dwmac_serdes_init (struct stmmac_priv *priv); +int dwmac_serdes_power_on (struct stmmac_priv *priv); +int dwmac_serdes_init_mode (struct stmmac_priv *priv,phy_interface_t interface); +int dwmac_serdes_set_mode (struct stmmac_priv *priv,phy_interface_t interface); +void dwmac_serdes_power_off (struct stmmac_priv *priv); +void dwmac_serdes_exit (struct stmmac_priv *priv); + +#endif
include/linux/stmmac.h +2 -0
@@ -193,6 +193,7 @@ enum dwmac_core_type {#define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY BIT(13)struct mac_device_info ; +struct phy ;struct plat_stmmacenet_data {enum dwmac_core_type core_type;
@@ -222,6 +223,7 @@ struct plat_stmmacenet_data {* that phylink uses. */ phy_interface_t phy_interface; +struct phy *serdes;struct stmmac_mdio_bus_data *mdio_bus_data;struct device_node *phy_node;struct fwnode_handle *port_node;
Signature · 1 line
2.47.3
Re: [PATCH net-next 05/14] net: stmmac: add stmmac core serdes support
Hi Russell,
Quoted 10 lines · Russell King (Oracle)
Rather than having platform glue implement SerDes PHY support, add it to the core driver, specifically to the stmmac integrated PCS driver as the SerDes is connected to the integrated PCS.
Platforms using external PCS can also populate
plat->serdes, and the core driver will callphy_init()andphy_exit()when the administrative state of the interface changes, but the other phy methods will not be called.Signed-off-by Russell King (Oracle) <[email protected]>
Unfortunately I have no way to test. But still,
Maxime
Quoted 341 lines
--- drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 38 +++++- .../net/ethernet/stmicro/stmmac/stmmac_pcs.h | 1 + .../ethernet/stmicro/stmmac/stmmac_serdes.c | 111 ++++++++++++++++++ .../ethernet/stmicro/stmmac/stmmac_serdes.h | 16 +++ include/linux/stmmac.h | 2 + 7 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.hdrivers/net/ethernet/stmicro/stmmac/Makefile +1
-1@@ -7,7 +7,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \ dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \ stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \ stmmac_xdp.o stmmac_est.o stmmac_fpe.o stmmac_vlan.o \ - stmmac_pcs.o $(stmmac-y) + stmmac_pcs.o stmmac_serdes.o $(stmmac-y) stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.odrivers/net/ethernet/stmicro/stmmac/stmmac_main.c +13
-1@@ -48,6 +48,7 @@#include "stmmac_fpe.h" #include "stmmac.h" #include "stmmac_pcs.h" +#include "stmmac_serdes.h" #include "stmmac_xdp.h" #include <linux/reset.h> #include <linux/of_mdio.h> @@ -3549,12 +3550,16 @@ static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)static void stmmac_clk_rx_i_require (struct stmmac_priv *priv) { +dwmac_serdes_power_on (priv); +/* Only sets the SerDes mode if it wasn't already configured. */ +dwmac_serdes_init_mode (priv, priv->plat->phy_interface);phylink_rx_clk_stop_block (priv->phylink); }static void stmmac_clk_rx_i_release (struct stmmac_priv *priv) {phylink_rx_clk_stop_unblock (priv->phylink); +dwmac_serdes_power_off (priv); }/** @@ -4152,10 +4157,14 @@ static int stmmac_open(struct net_device *dev)if (ret) goto err_runtime_pm; - ret =__stmmac_open (dev, dma_conf); + ret =dwmac_serdes_init (priv);if (ret) goto err_disconnect_phy; + ret =__stmmac_open (dev, dma_conf); +if (ret) + goto err_serdes; +kfree (dma_conf);/* We may have called phylink_speed_down before */ @@ -4163,6 +4172,8 @@ static int stmmac_open(struct net_device *dev)return ret; +err_serdes: +dwmac_serdes_exit (priv); err_disconnect_phy:phylink_disconnect_phy (priv->phylink); err_runtime_pm:@@ -4226,6 +4237,7 @@ static int stmmac_release(struct net_device *dev)__stmmac_release (dev); +dwmac_serdes_exit (priv);phylink_disconnect_phy (priv->phylink);pm_runtime_put (priv->device);drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +36
-2@@ -1,12 +1,25 @@// SPDX-License-Identifier: GPL-2.0-only #include "stmmac.h" #include "stmmac_pcs.h" +#include "stmmac_serdes.h" static int dwmac_integrated_pcs_enable (struct phylink_pcs *pcs) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +struct stmmac_priv *priv = spcs->priv; +int ret; -stmmac_mac_irq_modify (spcs->priv,0 , spcs->int_mask); + ret =dwmac_serdes_power_on (priv); +if (ret) +return ret; + +if (spcs->interface !=PHY_INTERFACE_MODE_NA ) { + ret =dwmac_serdes_set_mode (priv, spcs->interface); +if (ret) +return ret; + } + +stmmac_mac_irq_modify (priv,0 , spcs->int_mask);return 0 ; }@@ -14,8 +27,11 @@ static int dwmac_integrated_pcs_enable(struct phylink_pcs *pcs)static void dwmac_integrated_pcs_disable (struct phylink_pcs *pcs) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +struct stmmac_priv *priv = spcs->priv; -stmmac_mac_irq_modify (spcs->priv, spcs->int_mask,0 ); +stmmac_mac_irq_modify (priv, spcs->int_mask,0 ); + +dwmac_serdes_power_off (priv); } static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs,@@ -32,6 +48,15 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs, bool permit_pause_to_mac) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +int ret; + +if (spcs->interface != interface) { + ret =dwmac_serdes_set_mode (spcs->priv, interface); +if (ret) +return ret; + + spcs->interface = interface; + }dwmac_ctrl_ane (spcs->base,0 ,1 , spcs->priv->hw->reverse_sgmii_enable);@@ -71,6 +96,7 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, u32 int_mask) {struct stmmac_pcs *spcs; +int ret; spcs =devm_kzalloc (priv->device,sizeof (*spcs),GFP_KERNEL );if (!spcs)@@ -81,6 +107,14 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, spcs->int_mask = int_mask; spcs->pcs.ops = &dwmac_integrated_pcs_ops; +if (priv->plat->serdes) { + ret =dwmac_serdes_validate (priv,PHY_INTERFACE_MODE_SGMII ); +if (ret) +dev_warn (priv->device, +"serdes does not support SGMII: %pe\n" , +ERR_PTR (ret)); + } +__set_bit (PHY_INTERFACE_MODE_SGMII , spcs->pcs.supported_interfaces); priv->integrated_pcs = spcs;drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +1
-0@@ -53,6 +53,7 @@ struct stmmac_pcs {struct stmmac_priv *priv;void __iomem *base;u32 int_mask; +phy_interface_t interface;struct phylink_pcs pcs; };drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c new file +111
-0@@ -0,0 +1,111 @@ +#include <linux/phy/phy.h> + +#include "stmmac.h" +#include "stmmac_serdes.h" + +static phy_interface_t dwmac_serdes_phy_modes[] = { +PHY_INTERFACE_MODE_SGMII , +PHY_INTERFACE_MODE_1000BASEX , +PHY_INTERFACE_MODE_2500BASEX +}; + +int dwmac_serdes_validate (struct stmmac_priv *priv,phy_interface_t interface) +{ +return phy_validate (priv->plat->serdes,PHY_MODE_ETHERNET , interface, +NULL ); +} + +int dwmac_serdes_init (struct stmmac_priv *priv) +{ +size_t i; +int ret; + +if (!priv->plat->serdes) +return 0 ; + +/* Encourage good implementation of the SerDes PHY driver, so that +* we can discover which Ethernet modes the SerDes supports. +* Unfortunately, some implementations are noisy (bad), others +* require phy_set_speed() to select the correct speed first +* (which then reprograms the SerDes, negating the whole point of +* phy_validate().) Weed out these incompatible implementations. +*/ +for (i =0 ; i <ARRAY_SIZE (dwmac_serdes_phy_modes); i++) { + ret =phy_validate (priv->plat->serdes,PHY_MODE_ETHERNET , + dwmac_serdes_phy_modes[i],NULL ); +if (ret ==0 || ret == -EOPNOTSUPP ) +break ; + } + +if (ret == -EOPNOTSUPP ) +dev_warn (priv->device, +"SerDes driver does not implement phy_validate()\n" ); +if (ret) { +/* The SerDes PHY failed validation, refuse to use it. */ +dev_warn (priv->device, +"SerDes driver fails to validate SGMII, 1000BASE-X nor 2500BASE-X\n" ); +return -EINVAL ; + } + + ret =phy_init (priv->plat->serdes); +if (ret) +dev_err (priv->device,"failed to initialize SerDes: %pe\n" , +ERR_PTR (ret)); + +return ret; +} + +int dwmac_serdes_power_on (struct stmmac_priv *priv) +{ +int ret; + + ret =phy_power_on (priv->plat->serdes); +if (ret) +dev_err (priv->device,"failed to power on SerDes: %pe\n" , +ERR_PTR (ret)); + +return ret; +} + +int dwmac_serdes_init_mode (struct stmmac_priv *priv,phy_interface_t interface) +{ +struct phy *serdes = priv->plat->serdes; + +if (phy_get_mode (serdes) ==PHY_MODE_ETHERNET ) +return 0 ; + +return dwmac_serdes_set_mode (priv, interface); +} + +int dwmac_serdes_set_mode (struct stmmac_priv *priv,phy_interface_t interface) +{ +struct phy *serdes = priv->plat->serdes; +int ret; + + ret =phy_set_mode_ext (serdes,PHY_MODE_ETHERNET , interface); +if (ret) +dev_err (priv->device, +"failed to set SerDes mode %s: %pe\n" , +phy_modes (interface),ERR_PTR (ret)); + +return ret; +} + +void dwmac_serdes_power_off (struct stmmac_priv *priv) +{ +int ret; + + ret =phy_power_off (priv->plat->serdes); +if (ret) +dev_err (priv->device,"failed to power off SerDes: %pe\n" , +ERR_PTR (ret)); +} + +void dwmac_serdes_exit (struct stmmac_priv *priv) +{ +int ret =phy_exit (priv->plat->serdes); + +if (ret) +dev_err (priv->device,"failed to shutdown SerDes: %pe\n" , +ERR_PTR (ret)); +}drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.h new file +16
-0@@ -0,0 +1,16 @@ +#ifndef STMMAC_SERDES_H +#define STMMAC_SERDES_H + +#include <linux/phy.h> + +struct stmmac_priv ; + +int dwmac_serdes_validate (struct stmmac_priv *priv,phy_interface_t interface); +int dwmac_serdes_init (struct stmmac_priv *priv); +int dwmac_serdes_power_on (struct stmmac_priv *priv); +int dwmac_serdes_init_mode (struct stmmac_priv *priv,phy_interface_t interface); +int dwmac_serdes_set_mode (struct stmmac_priv *priv,phy_interface_t interface); +void dwmac_serdes_power_off (struct stmmac_priv *priv); +void dwmac_serdes_exit (struct stmmac_priv *priv); + +#endif include/linux/stmmac.h +2
-0@@ -193,6 +193,7 @@ enum dwmac_core_type {#define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY BIT(13)struct mac_device_info ; +struct phy ;struct plat_stmmacenet_data {enum dwmac_core_type core_type;@@ -222,6 +223,7 @@ struct plat_stmmacenet_data {* that phylink uses. */ phy_interface_t phy_interface; +struct phy *serdes;struct stmmac_mdio_bus_data *mdio_bus_data;struct device_node *phy_node;struct fwnode_handle *port_node;
Re: [PATCH net-next 05/14] net: stmmac: add stmmac core serdes support
Quoted 7 lines · Maxime Chevallier
Hi Russell,
Quoted 4 lines · Russell King (Oracle)
Rather than having platform glue implement SerDes PHY support, add it to the core driver, specifically to the stmmac integrated PCS driver as the SerDes is connected to the integrated PCS.
Quoted 8 lines
Quoted 6 lines
Platforms using external PCS can also populate
plat->serdes, and the core driver will callphy_init()andphy_exit()when the administrative state of the interface changes, but the other phy methods will not be called.Signed-off-by Russell King (Oracle) <[email protected]>Unfortunately I have no way to test. But still,
I am hoping Mohd Ayaan Anwar will be able to do at least some testing on the qcom-ethqos hardware.
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!
Jakub Kicinski
maintainerRe: [net-next,05/14] net: stmmac: add stmmac core serdes support
In reply to Russell King (Oracle)I haven't investigated too closely if this is legit or too defensive TBH but since I'm sending the second one I'll include this as well..
This is an AI-generated review of your patch.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: add stmmac core serdes support
This commit adds SerDes PHY support to the stmmac core driver, moving it from platform glue into the integrated PCS driver. The serdes field in platform data is optional and NULL when not provided by platforms.
Quoted 4 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0
-0
[ ... ]
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_safety_feat_configuration(struct stmmac_priv *priv) · line 3555
Quoted 7 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3
-0@@ -3549,12 +3550,16 @@ static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)static void stmmac_clk_rx_i_require (struct stmmac_priv *priv) { +dwmac_serdes_power_on (priv); +/* Only sets the SerDes mode if it wasn't already configured. */ +dwmac_serdes_init_mode (priv, priv->plat->phy_interface);
When priv->plat->serdes is NULL, dwmac_serdes_init_mode() will call phy_get_mode(NULL). Unlike other phy_* functions (phy_power_on, phy_power_off, phy_set_mode_ext, phy_init, phy_exit) which handle NULL by returning early, phy_get_mode() is an inline that directly dereferences the pointer:
static inline enum phy_mode phy_get_mode(struct phy *phy)
{
return phy->attrs.mode;
}Can this NULL pointer dereference occur when a platform does not populate plat->serdes? The call path stmmac_open() -> dwmac_serdes_init() returns 0 (success) when serdes is NULL, allowing __stmmac_open() -> stmmac_hw_setup() -> stmmac_clk_rx_i_require() to proceed.
Quoted 2 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1
-0+phylink_rx_clk_stop_block (priv->phylink); }
[ ... ]
Quoted 5 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c new file +0
-0
[ ... ]
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c line 24
Quoted 7 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c new file +7
-0+int dwmac_serdes_init (struct stmmac_priv *priv) +{ +size_t i; +int ret; + +if (!priv->plat->serdes) +return 0 ;
This returns success when serdes is NULL...
[ ... ]
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c line 74
Quoted 5 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c new file +5
-0+int dwmac_serdes_init_mode (struct stmmac_priv *priv,phy_interface_t interface) +{ +struct phy *serdes = priv->plat->serdes; + +if (phy_get_mode (serdes) ==PHY_MODE_ETHERNET )
^^^^^^
...but dwmac_serdes_init_mode() has no NULL check before calling phy_get_mode(), which will crash when serdes is NULL. Should this function also check for NULL serdes before calling phy_get_mode()?
Quoted 4 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_serdes.c new file +4
-0+return 0 ; + +return dwmac_serdes_set_mode (priv, interface); +}
PATCH 6/14 net: stmmac: qcom-ethqos: convert to dwmac generic SerDes support
In reply to Russell King (Oracle)Convert qcom-ethqos to use the dwmac core's generic SerDes support, which will handle SerDes initialisation, powering, and mode setting.
Note that generic support requires the SerDes to support phy_validate() in order to probe which PHY interface modes are supported, and phy_set_mode_ext() to configure the appropriate PHY interface mode (and thus the speed.)
--- .../stmicro/stmmac/dwmac-qcom-ethqos.c | 50 ++----------------- 1 file changed, 3 insertions(+), 47 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +3 -47
@@ -103,8 +103,6 @@ struct qcom_ethqos {int (*configure_func)(struct qcom_ethqos *ethqos,int speed);struct clk *link_clk; -struct phy *serdes_phy; -int serdes_speed;phy_interface_t phy_mode;const struct ethqos_emac_por *por;
@@ -584,14 +582,6 @@ static int ethqos_configure_rgmii(struct qcom_ethqos *ethqos, int speed)return 0 ; } -static void ethqos_set_serdes_speed (struct qcom_ethqos *ethqos,int speed) -{ -if (ethqos->serdes_speed != speed) { -phy_set_speed (ethqos->serdes_phy, speed); - ethqos->serdes_speed = speed; - } -} -static void ethqos_pcs_set_inband (struct stmmac_priv *priv,bool enable) {stmmac_pcs_ctrl_ane (priv, enable,0 );
@@ -609,17 +599,14 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)case SPEED_2500 :rgmii_setmask (ethqos,RGMII_CONFIG2_RGMII_CLK_SEL_CFG ,RGMII_IO_MACRO_CONFIG2 ); -ethqos_set_serdes_speed (ethqos,SPEED_2500 );ethqos_pcs_set_inband (priv, false);break ;case SPEED_1000 :rgmii_setmask (ethqos,RGMII_CONFIG2_RGMII_CLK_SEL_CFG ,RGMII_IO_MACRO_CONFIG2 ); -ethqos_set_serdes_speed (ethqos,SPEED_1000 );ethqos_pcs_set_inband (priv, true);break ;case SPEED_100 : -ethqos_set_serdes_speed (ethqos,SPEED_1000 );ethqos_pcs_set_inband (priv, true);break ;case SPEED_10 :
@@ -627,7 +614,6 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed)FIELD_PREP (RGMII_CONFIG_SGMII_CLK_DVDR ,SGMII_10M_RX_CLK_DVDR ), RGMII_IO_MACRO_CONFIG); -ethqos_set_serdes_speed (ethqos,SPEED_1000 );ethqos_pcs_set_inband (priv, true);break ; }
@@ -648,30 +634,6 @@ static void ethqos_fix_mac_speed(void *priv, int speed, unsigned int mode)ethqos_configure (ethqos, speed); } -static int qcom_ethqos_serdes_powerup (struct net_device *ndev,void *priv) -{ -struct qcom_ethqos *ethqos = priv; -int ret; - - ret =phy_init (ethqos->serdes_phy); -if (ret) -return ret; - - ret =phy_power_on (ethqos->serdes_phy); -if (ret) -return ret; - -return phy_set_speed (ethqos->serdes_phy, ethqos->serdes_speed); -} - -static void qcom_ethqos_serdes_powerdown (struct net_device *ndev,void *priv) -{ -struct qcom_ethqos *ethqos = priv; - -phy_power_off (ethqos->serdes_phy); -phy_exit (ethqos->serdes_phy); -} -static int ethqos_clks_config (void *priv,bool enabled) {struct qcom_ethqos *ethqos = priv;
@@ -790,12 +752,11 @@ static int qcom_ethqos_probe(struct platform_device *pdev)if (ret)return ret; - ethqos->serdes_phy =devm_phy_optional_get (dev,"serdes" ); -if (IS_ERR (ethqos->serdes_phy)) -return dev_err_probe (dev,PTR_ERR (ethqos->serdes_phy), + plat_dat->serdes =devm_phy_optional_get (dev,"serdes" ); +if (IS_ERR (plat_dat->serdes)) +return dev_err_probe (dev,PTR_ERR (plat_dat->serdes),"Failed to get serdes phy\n" ); - ethqos->serdes_speed =SPEED_1000 ;ethqos_set_clk_tx_rate (ethqos,NULL , plat_dat->phy_interface,SPEED_1000 );ethqos_set_func_clk_en (ethqos);
@@ -816,11 +777,6 @@ static int qcom_ethqos_probe(struct platform_device *pdev)if (data->dma_addr_width) plat_dat->host_dma_width = data->dma_addr_width; -if (ethqos->serdes_phy) { - plat_dat->serdes_powerup = qcom_ethqos_serdes_powerup; - plat_dat->serdes_powerdown = qcom_ethqos_serdes_powerdown; - } -/* Enable TSO on queue0 and enable TBS on rest of the queues */ for (i =1 ; i < plat_dat->tx_queues_to_use; i++) plat_dat->tx_queues_cfg[i].tbs_en =1 ;
Signature · 1 line
2.47.3
PATCH 7/14 net: stmmac: move most PCS register definitions to stmmac_pcs.c
In reply to Russell King (Oracle)Move most of the PCS register offset definitions to stmmac_pcs.c. Since stmmac_pcs.c only ever passes zero into the register offset macros, remove that ability, making them simple constant integer definitions.
Add appropriate descriptions of the registers, pointing out their similarity with their IEEE 802.3 counterparts. Make use of the BMSR definitions for the GMAC_AN_STATUS register and remove the driver private versions.
Note that BMSR_LSTATUS is non-low-latching, unlike it's 802.3z counterpart.
--- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 22 +++++++++++++++---- .../net/ethernet/stmicro/stmmac/stmmac_pcs.h | 22 ------------------- 2 files changed, 18 insertions(+), 26 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +18 -4
@@ -3,6 +3,20 @@#include "stmmac_pcs.h" #include "stmmac_serdes.h" +/* +* GMAC_AN_STATUS is equivalent to MII_BMSR +* GMAC_ANE_ADV is equivalent to 802.3z MII_ADVERTISE +* GMAC_ANE_LPA is equivalent to 802.3z MII_LPA +* GMAC_ANE_EXP is equivalent to MII_EXPANSION +* GMAC_TBI is equivalent to MII_ESTATUS +* +* ADV, LPA and EXP are only available for the TBI and RTBI modes. +*/ +#define GMAC_AN_STATUS 0x04/* AN status */ +#define GMAC_ANE_ADV 0x08/* ANE Advertisement */ +#define GMAC_ANE_LPA 0x0c/* ANE link partener ability */ +#define GMAC_TBI 0x14/* TBI extend status */ +static int dwmac_integrated_pcs_enable (struct phylink_pcs *pcs) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs);
@@ -74,11 +88,11 @@ void stmmac_integrated_pcs_irq(struct stmmac_priv *priv, u32 status, struct stmmac_extra_stats *x) {struct stmmac_pcs *spcs = priv->integrated_pcs; -u32 val =readl (spcs->base +GMAC_AN_STATUS (0 )); +u32 val =readl (spcs->base +GMAC_AN_STATUS );if (status &PCS_ANE_IRQ ) { x->irq_pcs_ane_n++; -if (val &GMAC_AN_STATUS_ANC ) +if (val &BMSR_ANEGCOMPLETE )dev_info (priv->device,"PCS ANE process completed\n" ); }
@@ -86,9 +100,9 @@ void stmmac_integrated_pcs_irq(struct stmmac_priv *priv, u32 status,if (status &PCS_LINK_IRQ ) { x->irq_pcs_link_n++;dev_info (priv->device,"PCS Link %s\n" , - val &GMAC_AN_STATUS_LS ?"Up" :"Down" ); + val &BMSR_LSTATUS ?"Up" :"Down" ); -phylink_pcs_change (&spcs->pcs, val &GMAC_AN_STATUS_LS ); +phylink_pcs_change (&spcs->pcs, val &BMSR_LSTATUS ); } }
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +0 -22
@@ -16,13 +16,6 @@/* PCS registers (AN/TBI/SGMII/RGMII) offsets */ #define GMAC_AN_CTRL (x) (x)/* AN control */ -#define GMAC_AN_STATUS (x) (x + 0x4)/* AN status */ - -/* ADV, LPA and EXP are only available for the TBI and RTBI interfaces */ -#define GMAC_ANE_ADV (x) (x + 0x8)/* ANE Advertisement */ -#define GMAC_ANE_LPA (x) (x + 0xc)/* ANE link partener ability */ -#define GMAC_ANE_EXP (x) (x + 0x10)/* ANE expansion */ -#define GMAC_TBI (x) (x + 0x14)/* TBI extend status */ /* AN Configuration defines */ #define GMAC_AN_CTRL_RAN BIT_U32(9)/* Restart Auto-Negotiation */
@@ -32,21 +25,6 @@#define GMAC_AN_CTRL_LR BIT_U32(17)/* Lock to Reference */ #define GMAC_AN_CTRL_SGMRAL BIT_U32(18)/* SGMII RAL Control */ -/* AN Status defines */ -#define GMAC_AN_STATUS_LS BIT_U32(2)/* Link Status 0:down 1:up */ -#define GMAC_AN_STATUS_ANA BIT_U32(3)/* Auto-Negotiation Ability */ -#define GMAC_AN_STATUS_ANC BIT_U32(5)/* Auto-Negotiation Complete */ -#define GMAC_AN_STATUS_ES BIT_U32(8)/* Extended Status */ - -/* ADV and LPA defines */ -#define GMAC_ANE_FD BIT_U32(5) -#define GMAC_ANE_HD BIT_U32(6) -#define GMAC_ANE_PSE GENMASK_U32(8, 7) -#define GMAC_ANE_PSE_SHIFT 7 -#define GMAC_ANE_RFE GENMASK_U32(13, 12) -#define GMAC_ANE_RFE_SHIFT 12 -#define GMAC_ANE_ACK BIT_U32(14) -struct stmmac_priv ;struct stmmac_pcs {
Signature · 1 line
2.47.3
PATCH 8/14 net: stmmac: handle integrated PCS phy_intf_sel separately
In reply to Russell King (Oracle)The dwmac core has no support for SGMII without using its integrated PCS. Thus, PHY_INTF_SEL_SGMII is only supported when this block is present, and it makes no sense for stmmac_get_phy_intf_sel() to decode this.
None of the platform glue users that use stmmac_get_phy_intf_sel() directly accept PHY_INTF_SEL_SGMII as a valid mode.
Check whether a PCS will be used by the driver for the interface mode, and if it is the integrated PCS, query the integrated PCS for the phy_intf_sel_i value to use.
--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 15 ++++++++++++--- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c | 9 +++++++++ drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 2 ++ 3 files changed, 23 insertions(+), 3 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -3
@@ -3135,8 +3135,6 @@ int stmmac_get_phy_intf_sel(phy_interface_t interface) phy_intf_sel =PHY_INTF_SEL_GMII_MII ; else if (phy_interface_mode_is_rgmii(interface)) phy_intf_sel =PHY_INTF_SEL_RGMII ; - else if (interface == PHY_INTERFACE_MODE_SGMII) - phy_intf_sel =PHY_INTF_SEL_SGMII ; else if (interface == PHY_INTERFACE_MODE_RMII) phy_intf_sel =PHY_INTF_SEL_RMII ; else if (interface == PHY_INTERFACE_MODE_REVMII)
@@ -3150,13 +3148,24 @@ static int stmmac_prereset_configure(struct stmmac_priv *priv) {struct plat_stmmacenet_data *plat_dat = priv->plat;phy_interface_t interface; +struct phylink_pcs *pcs;int phy_intf_sel, ret;if (!plat_dat->set_phy_intf_sel)return 0 ; interface = plat_dat->phy_interface; - phy_intf_sel =stmmac_get_phy_intf_sel (interface); + +/* Check whether this mode uses a PCS */ + pcs =stmmac_mac_select_pcs (&priv->phylink_config, interface); +if (priv->integrated_pcs && pcs == &priv->integrated_pcs->pcs) { +/* Request the phy_intf_sel from the integrated PCS */ + phy_intf_sel =stmmac_integrated_pcs_get_phy_intf_sel (priv, + interface); + }else { + phy_intf_sel =stmmac_get_phy_intf_sel (interface); + } +if (phy_intf_sel <0 ) { netdev_err(priv->dev, "failed to get phy_intf_sel for %s: %pe\n",
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +9 -0
@@ -106,6 +106,15 @@ void stmmac_integrated_pcs_irq(struct stmmac_priv *priv, u32 status, } } +int stmmac_integrated_pcs_get_phy_intf_sel (struct stmmac_priv *priv, +phy_interface_t interface) +{ +if (interface ==PHY_INTERFACE_MODE_SGMII ) +return PHY_INTF_SEL_SGMII ; + +return -EINVAL ; +} +int stmmac_integrated_pcs_init (struct stmmac_priv *priv,unsigned int offset,u32 int_mask) {
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +2 -0
@@ -43,6 +43,8 @@ phylink_pcs_to_stmmac_pcs(struct phylink_pcs *pcs)void stmmac_integrated_pcs_irq (struct stmmac_priv *priv,u32 status,struct stmmac_extra_stats *x); +int stmmac_integrated_pcs_get_phy_intf_sel (struct stmmac_priv *priv, +phy_interface_t interface);int stmmac_integrated_pcs_init (struct stmmac_priv *priv,unsigned int offset,u32 int_mask);
Signature · 1 line
2.47.3
PATCH 9/14 net: stmmac: add BASE-X support to integrated PCS
In reply to Russell King (Oracle)The integrated PCS supports 802.3z (BASE-X) modes when the Synopsys IP is coupled with an appropriate SerDes to provide the electrical interface. The PCS presents a TBI interface to the SerDes for this. Thus, the BASE-X related registers are only present when TBI mode is supported.
dwmac-qcom-ethqos added support for using 2.5G with the integrated PCS by calling dwmac_ctrl_ane() directly.
Add support for 1000BASE-X mode to the integrated PCS support if the PCS supports TBI, and 2500BASE-X if we have a SerDes that supports this mode.
--- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 95 ++++++++++++++++++- 1 file changed, 92 insertions(+), 3 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +92 -3
@@ -17,6 +17,50 @@#define GMAC_ANE_LPA 0x0c/* ANE link partener ability */ #define GMAC_TBI 0x14/* TBI extend status */ +static enum ethtool_link_mode_bit_indices dwmac_hd_mode_bits[] = { + ETHTOOL_LINK_MODE_10baseT_Half_BIT, + ETHTOOL_LINK_MODE_100baseT_Half_BIT, + ETHTOOL_LINK_MODE_1000baseT_Half_BIT, + ETHTOOL_LINK_MODE_100baseFX_Half_BIT, + ETHTOOL_LINK_MODE_10baseT1S_Half_BIT, + ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT, +}; + +static int dwmac_integrated_pcs_validate (struct phylink_pcs *pcs, +unsigned long *supported, +const struct phylink_link_state *state) +{ +struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +size_t i; +u32 val; + +if (phy_interface_mode_is_8023z (state->interface)) { +/* ESTATUS_1000_XFULL is always set, so full duplex is +* supported. ESTATUS_1000_XHALF depends on core configuration. +*/ + val =readl (spcs->base +GMAC_TBI ); +if (~val &ESTATUS_1000_XHALF ) +for (i =0 ; i <ARRAY_SIZE (dwmac_hd_mode_bits); i++) +linkmode_clear_bit (dwmac_hd_mode_bits[i], + supported); + +return 0 ; + }else if (state->interface ==PHY_INTERFACE_MODE_SGMII ) { +return 0 ; + } + +return -EINVAL ; +} + +static unsigned int dwmac_integrated_pcs_inband_caps (struct phylink_pcs *pcs, +phy_interface_t interface) +{ +if (phy_interface_mode_is_8023z (interface)) +return LINK_INBAND_ENABLE |LINK_INBAND_DISABLE ; + +return 0 ; +} +static int dwmac_integrated_pcs_enable (struct phylink_pcs *pcs) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs);
@@ -52,7 +96,23 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs,unsigned int neg_mode, struct phylink_link_state *state) { - state->link = false; + struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs); +u32 status, lpa; + + status =readl (spcs->base +GMAC_AN_STATUS ); + +if (phy_interface_mode_is_8023z (state->interface)) { +/* For 802.3z modes, the PCS block supports the advertisement +* and link partner advertisement registers using standard +* 802.3 format. The status register also has the link status +* and AN complete bits in the same bit location. +*/ + lpa =readl (spcs->base +GMAC_ANE_LPA ); + +phylink_mii_c22_pcs_decode_state (state, neg_mode, status, lpa); + }else { + state->link = false; + } } static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs,
@@ -62,6 +122,8 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs, bool permit_pause_to_mac) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); +bool changed = false, ane = true; +u32 adv;int ret;if (spcs->interface != interface) {
@@ -72,12 +134,25 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs, spcs->interface = interface; } -dwmac_ctrl_ane (spcs->base,0 ,1 , spcs->priv->hw->reverse_sgmii_enable); +if (phy_interface_mode_is_8023z (interface)) { + adv =phylink_mii_c22_pcs_encode_advertisement (interface, + advertising); +if (readl (spcs->base +GMAC_ANE_ADV ) != adv) + changed = true; +writel (adv, spcs->base +GMAC_ANE_ADV ); -return 0 ; + ane = neg_mode ==PHYLINK_PCS_NEG_INBAND_ENABLED ; + } + +dwmac_ctrl_ane (spcs->base,0 , ane, + spcs->priv->hw->reverse_sgmii_enable); + +return changed; }static const struct phylink_pcs_ops dwmac_integrated_pcs_ops = { + .pcs_validate = dwmac_integrated_pcs_validate, + .pcs_inband_caps = dwmac_integrated_pcs_inband_caps, .pcs_enable = dwmac_integrated_pcs_enable, .pcs_disable = dwmac_integrated_pcs_disable, .pcs_get_state = dwmac_integrated_pcs_get_state,
@@ -112,6 +187,9 @@ int stmmac_integrated_pcs_get_phy_intf_sel(struct stmmac_priv *priv,if (interface ==PHY_INTERFACE_MODE_SGMII )return PHY_INTF_SEL_SGMII ; +if (phy_interface_mode_is_8023z (interface)) +return PHY_INTF_SEL_TBI ; +return -EINVAL ; }
@@ -140,6 +218,17 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset,__set_bit (PHY_INTERFACE_MODE_SGMII , spcs->pcs.supported_interfaces); +if (readl (spcs->base +GMAC_AN_STATUS ) &BMSR_ESTATEN ) { +__set_bit (PHY_INTERFACE_MODE_1000BASEX , + spcs->pcs.supported_interfaces); + +/* Only allow 2500Base-X if the SerDes has support. */ + ret =dwmac_serdes_validate (priv,PHY_INTERFACE_MODE_2500BASEX ); +if (ret ==0 ) +__set_bit (PHY_INTERFACE_MODE_2500BASEX , + spcs->pcs.supported_interfaces); + } + priv->integrated_pcs = spcs;return 0 ;
Signature · 1 line
2.47.3
PATCH 10/14 net: stmmac: use integrated PCS for BASE-X modes
In reply to Russell King (Oracle)dwmac-qcom-ethqos supports SGMII and 2500BASE-X using the integrated PCS, so we need to expand the PCS support to include support for BASE-X modes.
Add support to the prereset configuration to detect 2500BASE-X, and arrange for stmmac_mac_select_pcs() to return the integrated PCS if its supported_interfaces bitmap reports support for the interface mode.
This results in priv->hw->pcs now being write-only, so remove it.
--- drivers/net/ethernet/stmicro/stmmac/common.h | 1 - drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++------ 2 files changed, 2 insertions(+), 7 deletions(-)
drivers/net/ethernet/stmicro/stmmac/common.h +0 -1
@@ -629,7 +629,6 @@ struct mac_device_info {unsigned int unicast_filter_entries;unsigned int mcast_bits_log2;unsigned int rx_csum; -unsigned int pcs;unsigned int xlgmac;unsigned int num_vlan;u32 vlan_filter[32 ];
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2 -6
@@ -911,11 +911,8 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,return pcs; } -/* The PCS control register is only relevant for SGMII, TBI and RTBI -* modes. We no longer support TBI or RTBI, so only configure this -* register when operating in SGMII mode with the integrated PCS. -*/ -if (priv->hw->pcs &STMMAC_PCS_SGMII && priv->integrated_pcs) +if (priv->integrated_pcs && +test_bit (interface, priv->integrated_pcs->pcs.supported_interfaces))return &priv->integrated_pcs->pcs;return NULL ;
@@ -1173,7 +1170,6 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv)if (priv->dma_cap.pcs && interface ==PHY_INTERFACE_MODE_SGMII ) {netdev_dbg (priv->dev,"PCS SGMII support enabled\n" ); - priv->hw->pcs =STMMAC_PCS_SGMII ;switch (speed) {case SPEED_10 :
Signature · 1 line
2.47.3
PATCH 11/14 net: stmmac: add struct stmmac_pcs_info
In reply to Russell King (Oracle)We need to describe one more register (offset and field bitmask) to the PCS code. Move the existing PCS offset and interrupt enable bits to a new struct and pass that in to stmmac_integrated_pcs_init().
--- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 9 ++++++--- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 8 ++++++-- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c | 8 ++++---- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 9 +++++++-- 4 files changed, 23 insertions(+), 11 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +6 -3
@@ -22,14 +22,17 @@#include "stmmac_ptp.h" #include "dwmac1000.h" +static const struct stmmac_pcs_info dwmac1000_pcs_info = { + .pcs_offset =GMAC_PCS_BASE , + .int_mask =GMAC_INT_DISABLE_PCSLINK |GMAC_INT_DISABLE_PCSAN , +}; +static int dwmac1000_pcs_init (struct stmmac_priv *priv) {if (!priv->dma_cap.pcs)return 0 ; -return stmmac_integrated_pcs_init (priv,GMAC_PCS_BASE , -GMAC_INT_DISABLE_PCSLINK | -GMAC_INT_DISABLE_PCSAN ); +return stmmac_integrated_pcs_init (priv, &dwmac1000_pcs_info); } static void dwmac1000_core_init(struct mac_device_info *hw,
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +6 -2
@@ -22,13 +22,17 @@#include "dwmac4.h" #include "dwmac5.h" +static const struct stmmac_pcs_info dwmac4_pcs_info = { + .pcs_offset =GMAC_PCS_BASE , + .int_mask =GMAC_INT_PCS_LINK |GMAC_INT_PCS_ANE , +}; +static int dwmac4_pcs_init (struct stmmac_priv *priv) {if (!priv->dma_cap.pcs)return 0 ; -return stmmac_integrated_pcs_init (priv,GMAC_PCS_BASE , -GMAC_INT_PCS_LINK |GMAC_INT_PCS_ANE ); +return stmmac_integrated_pcs_init (priv, &dwmac4_pcs_info); } static void dwmac4_core_init(struct mac_device_info *hw,
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +4 -4
@@ -193,8 +193,8 @@ int stmmac_integrated_pcs_get_phy_intf_sel(struct stmmac_priv *priv,return -EINVAL ; } -int stmmac_integrated_pcs_init (struct stmmac_priv *priv,unsigned int offset, -u32 int_mask) +int stmmac_integrated_pcs_init (struct stmmac_priv *priv, +const struct stmmac_pcs_info *pcs_info) {struct stmmac_pcs *spcs;int ret;
@@ -204,8 +204,8 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset,return -ENOMEM ; spcs->priv = priv; - spcs->base = priv->ioaddr + offset; - spcs->int_mask = int_mask; + spcs->base = priv->ioaddr + pcs_info->pcs_offset; + spcs->int_mask = pcs_info->int_mask; spcs->pcs.ops = &dwmac_integrated_pcs_ops;if (priv->plat->serdes) {
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +7 -2
@@ -27,6 +27,11 @@struct stmmac_priv ; +struct stmmac_pcs_info { +unsigned int pcs_offset; +u32 int_mask; +}; +struct stmmac_pcs {struct stmmac_priv *priv;void __iomem *base;
@@ -45,8 +50,8 @@ void stmmac_integrated_pcs_irq(struct stmmac_priv *priv, u32 status, struct stmmac_extra_stats *x);int stmmac_integrated_pcs_get_phy_intf_sel (struct stmmac_priv *priv,phy_interface_t interface); -int stmmac_integrated_pcs_init (struct stmmac_priv *priv,unsigned int offset, -u32 int_mask); +int stmmac_integrated_pcs_init (struct stmmac_priv *priv, +const struct stmmac_pcs_info *pcs_info);/** * dwmac_ctrl_ane - To program the AN Control Register.
Signature · 1 line
2.47.3
PATCH 12/14 net: stmmac: add support for reading inband SGMII status
In reply to Russell King (Oracle)Report the link, speed and duplex for SGMII links, read from the SGMII, RGMII and SMII status and control register.
--- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 44 ++++++++++++++++++- .../net/ethernet/stmicro/stmmac/stmmac_pcs.h | 4 ++ 2 files changed, 46 insertions(+), 2 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +42 -2
@@ -17,6 +17,16 @@#define GMAC_ANE_LPA 0x0c/* ANE link partener ability */ #define GMAC_TBI 0x14/* TBI extend status */ +/* +* RGSMII status bitfield definitions. +*/ +#define GMAC_RGSMIII_LNKMOD BIT(0) +#define GMAC_RGSMIII_SPEED_MASK GENMASK(2, 1) +#define GMAC_RGSMIII_SPEED_125 2 +#define GMAC_RGSMIII_SPEED_25 1 +#define GMAC_RGSMIII_SPEED_2_5 0 +#define GMAC_RGSMIII_LNKSTS BIT(3) +static enum ethtool_link_mode_bit_indices dwmac_hd_mode_bits[] = { ETHTOOL_LINK_MODE_10baseT_Half_BIT, ETHTOOL_LINK_MODE_100baseT_Half_BIT,
@@ -97,7 +107,7 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs, struct phylink_link_state *state) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); -u32 status, lpa; +u32 status, lpa, rgsmii; status =readl (spcs->base +GMAC_AN_STATUS );
@@ -111,7 +121,35 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs,phylink_mii_c22_pcs_decode_state (state, neg_mode, status, lpa); }else { - state->link = false; + rgsmii =field_get (spcs->rgsmii_status_mask, +readl (spcs->rgsmii)); + state->link = !!(status &GMAC_RGSMIII_LNKSTS ); + +if (state->link && neg_mode ==PHYLINK_PCS_NEG_INBAND_ENABLED ) { +/* FIXME: fill in speed and duplex. This requires the +* contents of the dwmac1000 GMAC_RGSMIIIS or dwmac4 +* GMAC_PHYIF_CONTROL_STATUS register. +*/ + state->duplex = rgsmii &GMAC_RGSMIII_LNKMOD ? +DUPLEX_FULL :DUPLEX_HALF ; +switch (FIELD_GET (GMAC_RGSMIII_SPEED_MASK , rgsmii)) { +case GMAC_RGSMIII_SPEED_2_5 : + state->speed =SPEED_10 ; +break ; + +case GMAC_RGSMIII_SPEED_25 : + state->speed =SPEED_100 ; +break ; + +case GMAC_RGSMIII_SPEED_125 : + state->speed =SPEED_1000 ; +break ; + +default : + state->link = false; +break ; + } + } } }
@@ -205,6 +243,8 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, spcs->priv = priv; spcs->base = priv->ioaddr + pcs_info->pcs_offset; + spcs->rgsmii = priv->ioaddr + pcs_info->rgsmii_offset; + spcs->rgsmii_status_mask = pcs_info->rgsmii_status_mask; spcs->int_mask = pcs_info->int_mask; spcs->pcs.ops = &dwmac_integrated_pcs_ops;
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +4 -0
@@ -29,12 +29,16 @@ struct stmmac_priv;struct stmmac_pcs_info {unsigned int pcs_offset; +unsigned int rgsmii_offset; +u32 rgsmii_status_mask;u32 int_mask; };struct stmmac_pcs {struct stmmac_priv *priv;void __iomem *base; +void __iomem *rgsmii; +u32 rgsmii_status_mask;u32 int_mask;phy_interface_t interface;struct phylink_pcs pcs;
Signature · 1 line
2.47.3
Jakub Kicinski
maintainerRe: [net-next,12/14] net: stmmac: add support for reading inband SGMII status
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: add support for reading inband SGMII status
This commit adds support for reading link status, speed, and duplex for SGMII links from the RGSMII status register.
Quoted 4 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +0
-0
[ ... ]
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +1
-1@@ -97,7 +107,7 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs, struct phylink_link_state *state) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); -u32 status, lpa; +u32 status, lpa, rgsmii; status =readl (spcs->base +GMAC_AN_STATUS );
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs, · line 126
Quoted 7 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +3
-1@@ -111,7 +121,35 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs,phylink_mii_c22_pcs_decode_state (state, neg_mode, status, lpa); }else { - state->link = false; + rgsmii =field_get (spcs->rgsmii_status_mask, +readl (spcs->rgsmii)); + state->link = !!(status &GMAC_RGSMIII_LNKSTS );
^^^^^^
Should this be checking rgsmii instead of status here? The status variable contains the GMAC_AN_STATUS register which is equivalent to MII_BMSR according to the comment at line 7. In BMSR, the link status bit is BMSR_LSTATUS (bit 2), not bit 3.
GMAC_RGSMIII_LNKSTS is defined as BIT(3), which is the correct bit position for the RGSMII status register (matching GMAC_RGSMIIIS_LNKSTS in dwmac1000.h), but it appears to be applied to the wrong register variable.
The code later uses rgsmii correctly for speed and duplex extraction. Was the intent to use rgsmii for the link status check as well?
Quoted 2 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +2
-0+ +if (state->link && neg_mode ==PHYLINK_PCS_NEG_INBAND_ENABLED ) {
[ ... ]
PATCH 13/14 net: stmmac: configure SGMII AN control according to phylink
In reply to Russell King (Oracle)Provide phylink with the integrated PCS autonegotiation capabilities, and configure the PCS's AN settings according to phylink's requested requirements.
This may cause regressions.
--- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +13 -7
@@ -65,7 +65,8 @@ static int dwmac_integrated_pcs_validate(struct phylink_pcs *pcs,static unsigned int dwmac_integrated_pcs_inband_caps (struct phylink_pcs *pcs,phy_interface_t interface) { -if (phy_interface_mode_is_8023z (interface)) +if (phy_interface_mode_is_8023z (interface) || + interface ==PHY_INTERFACE_MODE_SGMII )return LINK_INBAND_ENABLE |LINK_INBAND_DISABLE ;return 0 ;
@@ -160,8 +161,9 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs, bool permit_pause_to_mac) {struct stmmac_pcs *spcs =phylink_pcs_to_stmmac_pcs (pcs); -bool changed = false, ane = true; -u32 adv; +void __iomem *an_control = spcs->base +GMAC_AN_CTRL (0 ); +bool changed = false; +u32 adv, ctrl;int ret;if (spcs->interface != interface) {
@@ -178,12 +180,16 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs,if (readl (spcs->base +GMAC_ANE_ADV ) != adv) changed = true;writel (adv, spcs->base +GMAC_ANE_ADV ); - - ane = neg_mode ==PHYLINK_PCS_NEG_INBAND_ENABLED ; } -dwmac_ctrl_ane (spcs->base,0 , ane, - spcs->priv->hw->reverse_sgmii_enable); + ctrl =readl (an_control) & ~(GMAC_AN_CTRL_ANE |GMAC_AN_CTRL_SGMRAL ); +if (spcs->priv->hw->reverse_sgmii_enable) + ctrl |=GMAC_AN_CTRL_SGMRAL |GMAC_AN_CTRL_ANE ; +else if (neg_mode ==PHYLINK_PCS_NEG_INBAND_ENABLED ) + ctrl |=GMAC_AN_CTRL_ANE ; +else + ctrl |=GMAC_AN_CTRL_SGMRAL ; +writel (ctrl, an_control);return changed; }
Signature · 1 line
2.47.3
PATCH 14/14 net: stmmac: report PCS configuration changes
In reply to Russell King (Oracle)Report if/when qcom-ethqos changes the PCS configuration. With phylink now setting the PCS configuration, there should be no need for drivers to change this.
--- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +15 -0
@@ -71,6 +71,7 @@ static inline void dwmac_ctrl_ane(void __iomem *ioaddr, u32 reg, bool ane, bool srgmi_ral) {u32 value =readl (ioaddr +GMAC_AN_CTRL (reg)); +u32 old = value, diff;/* Enable and restart the Auto-Negotiation */ if (ane)
@@ -84,6 +85,20 @@ static inline void dwmac_ctrl_ane(void __iomem *ioaddr, u32 reg, bool ane,if (srgmi_ral) value |=GMAC_AN_CTRL_SGMRAL ; + diff = old ^ value; +if (diff & ~GMAC_AN_CTRL_RAN ) { +pr_warn ("dwmac: PCS configuration changed from phylink by glue, please report: 0x%08x -> 0x%08x\n" , + old & ~GMAC_AN_CTRL_RAN , value & ~GMAC_AN_CTRL_RAN ); +#define REPORT_BIT (x) \ + if (diff & GMAC_AN_CTRL_##x) \ + pr_warn("dwmac: %8s %u -> %u\n", #x, \ + !!(old & GMAC_AN_CTRL_##x), \ + !!(value & GMAC_AN_CTRL_##x)) +REPORT_BIT (ANE ); +REPORT_BIT (SGMRAL ); +#undef REPORT_BIT + } + writel(value, ioaddr + GMAC_AN_CTRL(reg)); }#endif /* __STMMAC_PCS_H__ */
Signature · 1 line
2.47.3