| Message ID | E1tKeg8-006SNJ-4Q@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: add phylink managed EEE support | expand |
> + adapter->phylink_config.lpi_capabilities = MAC_100FD | MAC_1000FD;
Is EEE not defined for 10Mbps?
Andrew
On Tue, Dec 10, 2024 at 04:37:27AM +0100, Andrew Lunn wrote: > > + adapter->phylink_config.lpi_capabilities = MAC_100FD | MAC_1000FD; > > Is EEE not defined for 10Mbps? According to 802.3, 10BASE-Te supports it. Phylib only has support for 10BASE-T1L (not mentioned in my 802.3 copy) using it, and I don't think LAN743x is used with such PHYs.
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-mdio-add-definition-for-clock-stop-capable-bit/20241210-022608
base: net-next/main
patch link: https://lore.kernel.org/r/E1tKeg8-006SNJ-4Q%40rmk-PC.armlinux.org.uk
patch subject: [PATCH net-next 10/10] net: lan743x: convert to phylink managed EEE
config: powerpc-randconfig-002-20241210 (https://download.01.org/0day-ci/archive/20241210/202412102203.FVir20i4-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241210/202412102203.FVir20i4-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412102203.FVir20i4-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/net/ethernet/microchip/lan743x_main.c:5:
In file included from include/linux/pci.h:38:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/powerpc/include/asm/io.h:24:
In file included from include/linux/mm.h:2223:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/net/ethernet/microchip/lan743x_main.c:3078:25: error: use of undeclared identifier 'adapter'
3078 | lan743x_mac_eee_enable(adapter, false);
| ^
drivers/net/ethernet/microchip/lan743x_main.c:3089:20: error: use of undeclared identifier 'adapter'
3089 | lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, timer);
| ^
drivers/net/ethernet/microchip/lan743x_main.c:3090:25: error: use of undeclared identifier 'adapter'
3090 | lan743x_mac_eee_enable(adapter, true);
| ^
>> drivers/net/ethernet/microchip/lan743x_main.c:3097:24: error: use of undeclared identifier 'lan743x_mac_disable_tx_lpi'; did you mean 'mac_disable_tx_lpi'?
3097 | .mac_disable_tx_lpi = lan743x_mac_disable_tx_lpi,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
| mac_disable_tx_lpi
drivers/net/ethernet/microchip/lan743x_main.c:3076:13: note: 'mac_disable_tx_lpi' declared here
3076 | static void mac_disable_tx_lpi(struct phylink_config *config)
| ^
>> drivers/net/ethernet/microchip/lan743x_main.c:3098:23: error: use of undeclared identifier 'lan743x_mac_enable_tx_lpi'; did you mean 'mac_enable_tx_lpi'?
3098 | .mac_enable_tx_lpi = lan743x_mac_enable_tx_lpi,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| mac_enable_tx_lpi
drivers/net/ethernet/microchip/lan743x_main.c:3081:13: note: 'mac_enable_tx_lpi' declared here
3081 | static void mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
| ^
>> drivers/net/ethernet/microchip/lan743x_main.c:3113:26: error: no member named 'lpi_timer_max' in 'struct phylink_config'
3113 | adapter->phylink_config.lpi_timer_max = U32_MAX;
| ~~~~~~~~~~~~~~~~~~~~~~~ ^
1 warning and 6 errors generated.
vim +/adapter +3078 drivers/net/ethernet/microchip/lan743x_main.c
3075
3076 static void mac_disable_tx_lpi(struct phylink_config *config)
3077 {
> 3078 lan743x_mac_eee_enable(adapter, false);
3079 }
3080
3081 static void mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
3082 bool tx_clk_stop)
3083 {
3084 /* Software should only change this field when Energy Efficient
3085 * Ethernet Enable (EEEEN) is cleared. We ensure that by clearing
3086 * EEEEN during probe, and phylink itself guarantees that
3087 * mac_disable_tx_lpi() will have been previously called.
3088 */
3089 lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, timer);
3090 lan743x_mac_eee_enable(adapter, true);
3091 }
3092
3093 static const struct phylink_mac_ops lan743x_phylink_mac_ops = {
3094 .mac_config = lan743x_phylink_mac_config,
3095 .mac_link_down = lan743x_phylink_mac_link_down,
3096 .mac_link_up = lan743x_phylink_mac_link_up,
> 3097 .mac_disable_tx_lpi = lan743x_mac_disable_tx_lpi,
> 3098 .mac_enable_tx_lpi = lan743x_mac_enable_tx_lpi,
3099 };
3100
3101 static int lan743x_phylink_create(struct lan743x_adapter *adapter)
3102 {
3103 struct net_device *netdev = adapter->netdev;
3104 struct phylink *pl;
3105
3106 adapter->phylink_config.dev = &netdev->dev;
3107 adapter->phylink_config.type = PHYLINK_NETDEV;
3108 adapter->phylink_config.mac_managed_pm = false;
3109
3110 adapter->phylink_config.mac_capabilities = MAC_ASYM_PAUSE |
3111 MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000FD;
3112 adapter->phylink_config.lpi_capabilities = MAC_100FD | MAC_1000FD;
> 3113 adapter->phylink_config.lpi_timer_max = U32_MAX;
3114 adapter->phylink_config.lpi_timer_default =
3115 lan743x_csr_read(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT);
3116
3117 lan743x_phy_interface_select(adapter);
3118
3119 switch (adapter->phy_interface) {
3120 case PHY_INTERFACE_MODE_SGMII:
3121 __set_bit(PHY_INTERFACE_MODE_SGMII,
3122 adapter->phylink_config.supported_interfaces);
3123 __set_bit(PHY_INTERFACE_MODE_1000BASEX,
3124 adapter->phylink_config.supported_interfaces);
3125 __set_bit(PHY_INTERFACE_MODE_2500BASEX,
3126 adapter->phylink_config.supported_interfaces);
3127 adapter->phylink_config.mac_capabilities |= MAC_2500FD;
3128 break;
3129 case PHY_INTERFACE_MODE_GMII:
3130 __set_bit(PHY_INTERFACE_MODE_GMII,
3131 adapter->phylink_config.supported_interfaces);
3132 break;
3133 case PHY_INTERFACE_MODE_MII:
3134 __set_bit(PHY_INTERFACE_MODE_MII,
3135 adapter->phylink_config.supported_interfaces);
3136 break;
3137 default:
3138 phy_interface_set_rgmii(adapter->phylink_config.supported_interfaces);
3139 }
3140
3141 memcpy(adapter->phylink_config.lpi_interfaces,
3142 adapter->phylink_config.supported_interfaces,
3143 sizeof(adapter->phylink_config.lpi_interfaces));
3144
3145 pl = phylink_create(&adapter->phylink_config, NULL,
3146 adapter->phy_interface, &lan743x_phylink_mac_ops);
3147
3148 if (IS_ERR(pl)) {
3149 netdev_err(netdev, "Could not create phylink (%pe)\n", pl);
3150 return PTR_ERR(pl);
3151 }
3152
3153 adapter->phylink = pl;
3154 netdev_dbg(netdev, "lan743x phylink created");
3155
3156 return 0;
3157 }
3158
Hi Russell,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Russell-King-Oracle/net-mdio-add-definition-for-clock-stop-capable-bit/20241210-022608
base: net-next/main
patch link: https://lore.kernel.org/r/E1tKeg8-006SNJ-4Q%40rmk-PC.armlinux.org.uk
patch subject: [PATCH net-next 10/10] net: lan743x: convert to phylink managed EEE
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20241212/202412120900.DYyJpYUw-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241212/202412120900.DYyJpYUw-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412120900.DYyJpYUw-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/ethernet/microchip/lan743x_main.c: In function 'mac_disable_tx_lpi':
drivers/net/ethernet/microchip/lan743x_main.c:3078:32: error: 'adapter' undeclared (first use in this function)
3078 | lan743x_mac_eee_enable(adapter, false);
| ^~~~~~~
drivers/net/ethernet/microchip/lan743x_main.c:3078:32: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/microchip/lan743x_main.c: In function 'mac_enable_tx_lpi':
drivers/net/ethernet/microchip/lan743x_main.c:3089:27: error: 'adapter' undeclared (first use in this function)
3089 | lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, timer);
| ^~~~~~~
drivers/net/ethernet/microchip/lan743x_main.c: At top level:
drivers/net/ethernet/microchip/lan743x_main.c:3097:31: error: 'lan743x_mac_disable_tx_lpi' undeclared here (not in a function); did you mean 'mac_disable_tx_lpi'?
3097 | .mac_disable_tx_lpi = lan743x_mac_disable_tx_lpi,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
| mac_disable_tx_lpi
drivers/net/ethernet/microchip/lan743x_main.c:3098:30: error: 'lan743x_mac_enable_tx_lpi' undeclared here (not in a function); did you mean 'mac_enable_tx_lpi'?
3098 | .mac_enable_tx_lpi = lan743x_mac_enable_tx_lpi,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| mac_enable_tx_lpi
drivers/net/ethernet/microchip/lan743x_main.c: In function 'lan743x_phylink_create':
drivers/net/ethernet/microchip/lan743x_main.c:3113:33: error: 'struct phylink_config' has no member named 'lpi_timer_max'; did you mean 'lpi_timer_default'?
3113 | adapter->phylink_config.lpi_timer_max = U32_MAX;
| ^~~~~~~~~~~~~
| lpi_timer_default
drivers/net/ethernet/microchip/lan743x_main.c: At top level:
>> drivers/net/ethernet/microchip/lan743x_main.c:3081:13: warning: 'mac_enable_tx_lpi' defined but not used [-Wunused-function]
3081 | static void mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
| ^~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/microchip/lan743x_main.c:3076:13: warning: 'mac_disable_tx_lpi' defined but not used [-Wunused-function]
3076 | static void mac_disable_tx_lpi(struct phylink_config *config)
| ^~~~~~~~~~~~~~~~~~
vim +/mac_enable_tx_lpi +3081 drivers/net/ethernet/microchip/lan743x_main.c
3075
> 3076 static void mac_disable_tx_lpi(struct phylink_config *config)
3077 {
3078 lan743x_mac_eee_enable(adapter, false);
3079 }
3080
> 3081 static void mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
3082 bool tx_clk_stop)
3083 {
3084 /* Software should only change this field when Energy Efficient
3085 * Ethernet Enable (EEEEN) is cleared. We ensure that by clearing
3086 * EEEEN during probe, and phylink itself guarantees that
3087 * mac_disable_tx_lpi() will have been previously called.
3088 */
3089 lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, timer);
3090 lan743x_mac_eee_enable(adapter, true);
3091 }
3092
diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c index 1a1cbd034eda..1459acfb1e61 100644 --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c @@ -1055,9 +1055,6 @@ static int lan743x_ethtool_get_eee(struct net_device *netdev, { struct lan743x_adapter *adapter = netdev_priv(netdev); - eee->tx_lpi_timer = lan743x_csr_read(adapter, - MAC_EEE_TX_LPI_REQ_DLY_CNT); - return phylink_ethtool_get_eee(adapter->phylink, eee); } @@ -1065,24 +1062,6 @@ static int lan743x_ethtool_set_eee(struct net_device *netdev, struct ethtool_keee *eee) { struct lan743x_adapter *adapter = netdev_priv(netdev); - u32 tx_lpi_timer; - - tx_lpi_timer = lan743x_csr_read(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT); - if (tx_lpi_timer != eee->tx_lpi_timer) { - u32 mac_cr = lan743x_csr_read(adapter, MAC_CR); - - /* Software should only change this field when Energy Efficient - * Ethernet Enable (EEEEN) is cleared. - * This function will trigger an autonegotiation restart and - * eee will be reenabled during link up if eee was negotiated. - */ - lan743x_mac_eee_enable(adapter, false); - lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, - eee->tx_lpi_timer); - - if (mac_cr & MAC_CR_EEE_EN_) - lan743x_mac_eee_enable(adapter, true); - } return phylink_ethtool_set_eee(adapter->phylink, eee); } diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c index 8d7ad021ac70..25d37a2cb4a6 100644 --- a/drivers/net/ethernet/microchip/lan743x_main.c +++ b/drivers/net/ethernet/microchip/lan743x_main.c @@ -2966,7 +2966,7 @@ static int lan743x_phylink_2500basex_config(struct lan743x_adapter *adapter) return lan743x_pcs_power_reset(adapter); } -void lan743x_mac_eee_enable(struct lan743x_adapter *adapter, bool enable) +static void lan743x_mac_eee_enable(struct lan743x_adapter *adapter, bool enable) { u32 mac_cr; @@ -3027,10 +3027,8 @@ static void lan743x_phylink_mac_link_down(struct phylink_config *config, phy_interface_t interface) { struct net_device *netdev = to_net_dev(config->dev); - struct lan743x_adapter *adapter = netdev_priv(netdev); netif_tx_stop_all_queues(netdev); - lan743x_mac_eee_enable(adapter, false); } static void lan743x_phylink_mac_link_up(struct phylink_config *config, @@ -3072,16 +3070,32 @@ static void lan743x_phylink_mac_link_up(struct phylink_config *config, cap & FLOW_CTRL_TX, cap & FLOW_CTRL_RX); - if (phydev) - lan743x_mac_eee_enable(adapter, phydev->enable_tx_lpi); - netif_tx_wake_all_queues(netdev); } +static void mac_disable_tx_lpi(struct phylink_config *config) +{ + lan743x_mac_eee_enable(adapter, false); +} + +static void mac_enable_tx_lpi(struct phylink_config *config, u32 timer, + bool tx_clk_stop) +{ + /* Software should only change this field when Energy Efficient + * Ethernet Enable (EEEEN) is cleared. We ensure that by clearing + * EEEEN during probe, and phylink itself guarantees that + * mac_disable_tx_lpi() will have been previously called. + */ + lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, timer); + lan743x_mac_eee_enable(adapter, true); +} + static const struct phylink_mac_ops lan743x_phylink_mac_ops = { .mac_config = lan743x_phylink_mac_config, .mac_link_down = lan743x_phylink_mac_link_down, .mac_link_up = lan743x_phylink_mac_link_up, + .mac_disable_tx_lpi = lan743x_mac_disable_tx_lpi, + .mac_enable_tx_lpi = lan743x_mac_enable_tx_lpi, }; static int lan743x_phylink_create(struct lan743x_adapter *adapter) @@ -3095,6 +3109,10 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter) adapter->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000FD; + adapter->phylink_config.lpi_capabilities = MAC_100FD | MAC_1000FD; + adapter->phylink_config.lpi_timer_max = U32_MAX; + adapter->phylink_config.lpi_timer_default = + lan743x_csr_read(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT); lan743x_phy_interface_select(adapter); @@ -3120,6 +3138,10 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter) phy_interface_set_rgmii(adapter->phylink_config.supported_interfaces); } + memcpy(adapter->phylink_config.lpi_interfaces, + adapter->phylink_config.supported_interfaces, + sizeof(adapter->phylink_config.lpi_interfaces)); + pl = phylink_create(&adapter->phylink_config, NULL, adapter->phy_interface, &lan743x_phylink_mac_ops); @@ -3517,6 +3539,9 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter, spin_lock_init(&tx->ring_lock); } + /* Ensure EEEEN is clear */ + lan743x_mac_eee_enable(adapter, false); + return 0; } diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h index 8ef897c114d3..7f73d66854be 100644 --- a/drivers/net/ethernet/microchip/lan743x_main.h +++ b/drivers/net/ethernet/microchip/lan743x_main.h @@ -1206,6 +1206,5 @@ void lan743x_hs_syslock_release(struct lan743x_adapter *adapter); void lan743x_mac_flow_ctrl_set_enables(struct lan743x_adapter *adapter, bool tx_enable, bool rx_enable); int lan743x_sgmii_read(struct lan743x_adapter *adapter, u8 mmd, u16 addr); -void lan743x_mac_eee_enable(struct lan743x_adapter *adapter, bool enable); #endif /* _LAN743X_H */
Convert lan743x to phylink managed EEE: - Set the lpi_capabilties. - Move the call to lan743x_mac_eee_enable() into the enable/disable tx_lpi functions. - Ensure that EEEEN is clear during probe. - Move the setting of the LPI timer into mac_enable_tx_lpi(). - Move reading of LPI timer to phylink initialisation to set the default timer value. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- .../net/ethernet/microchip/lan743x_ethtool.c | 21 ----------- drivers/net/ethernet/microchip/lan743x_main.c | 37 ++++++++++++++++--- drivers/net/ethernet/microchip/lan743x_main.h | 1 - 3 files changed, 31 insertions(+), 28 deletions(-)