Merge branch 'net-stmmac-clean-up-and-fix-eee-implementation'

Russell King says:

====================
net: stmmac: clean up and fix EEE implementation

This is a rework of stmmac's EEE support in light of the addition of EEE
management to phylib. It's slightly more than 15 patches, but I think it
makes sense to be so.

Patch 1 adds configuration of the receive clock phy_eee_rx_clock_stop()
(which was part of another series, but is necessary for this patch set.)

Patch 2 converts stmmac to use phylib's tracking of tx_lpi_timer.

Patch 3 corrects the data type used for things involving the LPI
timer. The user API uses u32, so stmmac should do too, rather than
blindly converting it to "int". eee_timer is left for patch 4.

Patch 4 (new) uses an unsigned int for eee_timer.

Patch 5 makes stmmac EEE state depend on phylib's enable_tx_lpi flag,
thus using phylib's resolution of EEE state.

Patch 6 removes redundant code from the ethtool EEE operations.

Patch 7 removes some redundant code in stmmac_disable_eee_mode()
and renames it to stmmac_disable_sw_eee_mode() to better reflect its
purpose.

Patch 8 removes the driver private tx_lpi_enabled, which is managed by
phylib since patch 4.

Patch 9 removes the dependence of EEE error statistics on the EEE
enable state, instead depending on whether EEE is supported by the
hardware.

Patch 10 removes phy_init_eee(), instead using phy_eee_rx_clock_stop()
to configure whether the PHY may stop the receive clock.

Patch 11 removes priv->eee_tw_timer, which is only ever set to one
value at probe time, effectively it is a constant. Hence this is
unnecessary complexity.

Patch 12 moves priv->eee_enabled into stmmac_eee_init(), and placing
it under the protection of priv->lock, except when EEE is not
supported (where it becomes constant-false.)

Patch 13 moves priv->eee_active also into stmmac_eee_init(), so
the indication whether EEE should be enabled or not is passed in
to this function.

Since both priv->eee_enabled and priv->eee_active are assigned
true/false values, they should be typed "bool". Make it sew in
patch 14. No Singer machine required.

Patch 15 moves the initialisation of priv->eee_ctrl_timer to the
probe function - it makes no sense to re-initialise the timer each
time we want to start using it.

Patch 16 removes the unnecessary EEE handling in the driver tear-down
method. The core net code will have brought the interface down
already, meaning EEE has already been disabled.

Patch 17 reorganises the code to split the hardware LPI timer
control paths from the software LPI timer paths.

Patch 18 works on this further by eliminating
stmmac_lpi_entry_timer_config() and making direct calls to the new
functions. This reveals a potential bug where priv->eee_sw_timer_en
is set true when EEE is disabled. This is not addressed in this
series, but will be in a future separate patch - so that if fixing
that causes a regression, it can be handled separately.
====================

Link: https://patch.msgid.link/Z36sHIlnExQBuFJE@shell.armlinux.org.uk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Jakub Kicinski 1 year ago 676cfca2 7b24f164

+85 -87

7 changed files

expand all

drivers

net

ethernet

stmicro

stmmac

dwmac4_core.c

hwif.h

stmmac.h

stmmac_ethtool.c

stmmac_main.c

phy

phy.c

include

linux

phy.h

+2 -2

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c

··· 420 420 writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS); 421 421 } 422 422 423 - static void dwmac4_set_eee_lpi_entry_timer(struct mac_device_info *hw, int et) 423 + static void dwmac4_set_eee_lpi_entry_timer(struct mac_device_info *hw, u32 et) 424 424 { 425 425 void __iomem *ioaddr = hw->pcsr; 426 - int value = et & STMMAC_ET_MAX; 426 + u32 value = et & STMMAC_ET_MAX; 427 427 int regval; 428 428 429 429 /* Program LPI entry timer value into register */

+1 -1

drivers/net/ethernet/stmicro/stmmac/hwif.h

··· 363 363 void (*set_eee_mode)(struct mac_device_info *hw, 364 364 bool en_tx_lpi_clockgating); 365 365 void (*reset_eee_mode)(struct mac_device_info *hw); 366 - void (*set_eee_lpi_entry_timer)(struct mac_device_info *hw, int et); 366 + void (*set_eee_lpi_entry_timer)(struct mac_device_info *hw, u32 et); 367 367 void (*set_eee_timer)(struct mac_device_info *hw, int ls, int tw); 368 368 void (*set_eee_pls)(struct mac_device_info *hw, int link); 369 369 void (*debug)(struct stmmac_priv *priv, void __iomem *ioaddr,

+3 -7

drivers/net/ethernet/stmicro/stmmac/stmmac.h

··· 305 305 int clk_csr; 306 306 struct timer_list eee_ctrl_timer; 307 307 int lpi_irq; 308 - int eee_enabled; 309 - int eee_active; 310 - int tx_lpi_timer; 311 - int tx_lpi_enabled; 312 - int eee_tw_timer; 308 + u32 tx_lpi_timer; 309 + bool eee_enabled; 310 + bool eee_active; 313 311 bool eee_sw_timer_en; 314 312 unsigned int mode; 315 313 unsigned int chain_mode; ··· 403 405 int stmmac_dvr_probe(struct device *device, 404 406 struct plat_stmmacenet_data *plat_dat, 405 407 struct stmmac_resources *res); 406 - void stmmac_disable_eee_mode(struct stmmac_priv *priv); 407 - bool stmmac_eee_init(struct stmmac_priv *priv); 408 408 int stmmac_reinit_queues(struct net_device *dev, u32 rx_cnt, u32 tx_cnt); 409 409 int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size); 410 410 int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled);

+2 -23

drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c

··· 654 654 (*(u32 *)p); 655 655 } 656 656 } 657 - if (priv->eee_enabled) { 657 + if (priv->dma_cap.eee) { 658 658 int val = phylink_get_eee_err(priv->phylink); 659 659 if (val) 660 660 priv->xstats.phy_eee_wakeup_error_n = val; ··· 898 898 if (!priv->dma_cap.eee) 899 899 return -EOPNOTSUPP; 900 900 901 - edata->tx_lpi_timer = priv->tx_lpi_timer; 902 - edata->tx_lpi_enabled = priv->tx_lpi_enabled; 903 - 904 901 return phylink_ethtool_get_eee(priv->phylink, edata); 905 902 } 906 903 ··· 905 908 struct ethtool_keee *edata) 906 909 { 907 910 struct stmmac_priv *priv = netdev_priv(dev); 908 - int ret; 909 911 910 912 if (!priv->dma_cap.eee) 911 913 return -EOPNOTSUPP; 912 914 913 - if (priv->tx_lpi_enabled != edata->tx_lpi_enabled) 914 - netdev_warn(priv->dev, 915 - "Setting EEE tx-lpi is not supported\n"); 916 - 917 - if (!edata->eee_enabled) 918 - stmmac_disable_eee_mode(priv); 919 - 920 - ret = phylink_ethtool_set_eee(priv->phylink, edata); 921 - if (ret) 922 - return ret; 923 - 924 - if (edata->eee_enabled && 925 - priv->tx_lpi_timer != edata->tx_lpi_timer) { 926 - priv->tx_lpi_timer = edata->tx_lpi_timer; 927 - stmmac_eee_init(priv); 928 - } 929 - 930 - return 0; 915 + return phylink_ethtool_set_eee(priv->phylink, edata); 931 916 } 932 917 933 918 static u32 stmmac_usec2riwt(u32 usec, struct stmmac_priv *priv)

+54 -49

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

··· 111 111 NETIF_MSG_IFDOWN | NETIF_MSG_TIMER); 112 112 113 113 #define STMMAC_DEFAULT_LPI_TIMER 1000 114 - static int eee_timer = STMMAC_DEFAULT_LPI_TIMER; 115 - module_param(eee_timer, int, 0644); 114 + static unsigned int eee_timer = STMMAC_DEFAULT_LPI_TIMER; 115 + module_param(eee_timer, uint, 0644); 116 116 MODULE_PARM_DESC(eee_timer, "LPI tx expiration time in msec"); 117 117 #define STMMAC_LPI_T(x) (jiffies + usecs_to_jiffies(x)) 118 118 ··· 194 194 flow_ctrl = FLOW_OFF; 195 195 if (unlikely((pause < 0) || (pause > 0xffff))) 196 196 pause = PAUSE_TIME; 197 - if (eee_timer < 0) 198 - eee_timer = STMMAC_DEFAULT_LPI_TIMER; 199 197 } 200 198 201 199 static void __stmmac_disable_all_queues(struct stmmac_priv *priv) ··· 390 392 return dirty; 391 393 } 392 394 393 - static void stmmac_lpi_entry_timer_config(struct stmmac_priv *priv, bool en) 395 + static void stmmac_disable_hw_lpi_timer(struct stmmac_priv *priv) 394 396 { 395 - int tx_lpi_timer; 397 + stmmac_set_eee_lpi_timer(priv, priv->hw, 0); 398 + } 396 399 397 - /* Clear/set the SW EEE timer flag based on LPI ET enablement */ 398 - priv->eee_sw_timer_en = en ? 0 : 1; 399 - tx_lpi_timer = en ? priv->tx_lpi_timer : 0; 400 - stmmac_set_eee_lpi_timer(priv, priv->hw, tx_lpi_timer); 400 + static void stmmac_enable_hw_lpi_timer(struct stmmac_priv *priv) 401 + { 402 + stmmac_set_eee_lpi_timer(priv, priv->hw, priv->tx_lpi_timer); 401 403 } 402 404 403 405 /** ··· 427 429 } 428 430 429 431 /** 430 - * stmmac_disable_eee_mode - disable and exit from LPI mode 432 + * stmmac_disable_sw_eee_mode - disable and exit from LPI mode 431 433 * @priv: driver private structure 432 434 * Description: this function is to exit and disable EEE in case of 433 435 * LPI state is true. This is called by the xmit. 434 436 */ 435 - void stmmac_disable_eee_mode(struct stmmac_priv *priv) 437 + static void stmmac_disable_sw_eee_mode(struct stmmac_priv *priv) 436 438 { 437 - if (!priv->eee_sw_timer_en) { 438 - stmmac_lpi_entry_timer_config(priv, 0); 439 - return; 440 - } 441 - 442 439 stmmac_reset_eee_mode(priv, priv->hw); 443 440 del_timer_sync(&priv->eee_ctrl_timer); 444 441 priv->tx_path_in_lpi_mode = false; ··· 457 464 /** 458 465 * stmmac_eee_init - init EEE 459 466 * @priv: driver private structure 467 + * @active: indicates whether EEE should be enabled. 460 468 * Description: 461 469 * if the GMAC supports the EEE (from the HW cap reg) and the phy device 462 470 * can also manage EEE, this function enable the LPI state and start related 463 471 * timer. 464 472 */ 465 - bool stmmac_eee_init(struct stmmac_priv *priv) 473 + static void stmmac_eee_init(struct stmmac_priv *priv, bool active) 466 474 { 467 - int eee_tw_timer = priv->eee_tw_timer; 475 + priv->eee_active = active; 468 476 469 477 /* Check if MAC core supports the EEE feature. */ 470 - if (!priv->dma_cap.eee) 471 - return false; 478 + if (!priv->dma_cap.eee) { 479 + priv->eee_enabled = false; 480 + return; 481 + } 472 482 473 483 mutex_lock(&priv->lock); 474 484 ··· 479 483 if (!priv->eee_active) { 480 484 if (priv->eee_enabled) { 481 485 netdev_dbg(priv->dev, "disable EEE\n"); 482 - stmmac_lpi_entry_timer_config(priv, 0); 486 + priv->eee_sw_timer_en = true; 487 + stmmac_disable_hw_lpi_timer(priv); 483 488 del_timer_sync(&priv->eee_ctrl_timer); 484 - stmmac_set_eee_timer(priv, priv->hw, 0, eee_tw_timer); 489 + stmmac_set_eee_timer(priv, priv->hw, 0, 490 + STMMAC_DEFAULT_TWT_LS); 485 491 if (priv->hw->xpcs) 486 492 xpcs_config_eee(priv->hw->xpcs, 487 493 priv->plat->mult_fact_100ns, 488 494 false); 489 495 } 496 + priv->eee_enabled = false; 490 497 mutex_unlock(&priv->lock); 491 - return false; 498 + return; 492 499 } 493 500 494 501 if (priv->eee_active && !priv->eee_enabled) { 495 - timer_setup(&priv->eee_ctrl_timer, stmmac_eee_ctrl_timer, 0); 496 502 stmmac_set_eee_timer(priv, priv->hw, STMMAC_DEFAULT_LIT_LS, 497 - eee_tw_timer); 503 + STMMAC_DEFAULT_TWT_LS); 498 504 if (priv->hw->xpcs) 499 505 xpcs_config_eee(priv->hw->xpcs, 500 506 priv->plat->mult_fact_100ns, ··· 504 506 } 505 507 506 508 if (priv->plat->has_gmac4 && priv->tx_lpi_timer <= STMMAC_ET_MAX) { 509 + /* Use hardware LPI mode */ 507 510 del_timer_sync(&priv->eee_ctrl_timer); 508 511 priv->tx_path_in_lpi_mode = false; 509 - stmmac_lpi_entry_timer_config(priv, 1); 512 + priv->eee_sw_timer_en = false; 513 + stmmac_enable_hw_lpi_timer(priv); 510 514 } else { 511 - stmmac_lpi_entry_timer_config(priv, 0); 515 + /* Use software LPI mode */ 516 + priv->eee_sw_timer_en = true; 517 + stmmac_disable_hw_lpi_timer(priv); 512 518 mod_timer(&priv->eee_ctrl_timer, 513 519 STMMAC_LPI_T(priv->tx_lpi_timer)); 514 520 } 515 521 522 + priv->eee_enabled = true; 523 + 516 524 mutex_unlock(&priv->lock); 517 525 netdev_dbg(priv->dev, "Energy-Efficient Ethernet initialized\n"); 518 - return true; 519 526 } 520 527 521 528 /* stmmac_get_tx_hwtstamp - get HW TX timestamps ··· 977 974 struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); 978 975 979 976 stmmac_mac_set(priv, priv->ioaddr, false); 980 - priv->eee_active = false; 981 - priv->tx_lpi_enabled = false; 982 - priv->eee_enabled = stmmac_eee_init(priv); 977 + stmmac_eee_init(priv, false); 983 978 stmmac_set_eee_pls(priv, priv->hw, false); 984 979 985 980 if (stmmac_fpe_supported(priv)) ··· 1086 1085 1087 1086 stmmac_mac_set(priv, priv->ioaddr, true); 1088 1087 if (phy && priv->dma_cap.eee) { 1089 - priv->eee_active = 1090 - phy_init_eee(phy, !(priv->plat->flags & 1091 - STMMAC_FLAG_RX_CLK_RUNS_IN_LPI)) >= 0; 1092 - priv->eee_enabled = stmmac_eee_init(priv); 1093 - priv->tx_lpi_enabled = priv->eee_enabled; 1088 + phy_eee_rx_clock_stop(phy, !(priv->plat->flags & 1089 + STMMAC_FLAG_RX_CLK_RUNS_IN_LPI)); 1090 + priv->tx_lpi_timer = phy->eee_cfg.tx_lpi_timer; 1091 + stmmac_eee_init(priv, phy->enable_tx_lpi); 1094 1092 stmmac_set_eee_pls(priv, priv->hw, true); 1095 1093 } 1096 1094 ··· 1185 1185 } else { 1186 1186 fwnode_handle_put(phy_fwnode); 1187 1187 ret = phylink_fwnode_phy_connect(priv->phylink, fwnode, 0); 1188 + } 1189 + 1190 + if (ret == 0) { 1191 + struct ethtool_keee eee; 1192 + 1193 + /* Configure phylib's copy of the LPI timer */ 1194 + if (!phylink_ethtool_get_eee(priv->phylink, &eee)) { 1195 + eee.tx_lpi_timer = priv->tx_lpi_timer; 1196 + phylink_ethtool_set_eee(priv->phylink, &eee); 1197 + } 1188 1198 } 1189 1199 1190 1200 if (!priv->plat->pmt) { ··· 3462 3452 else if (ptp_register) 3463 3453 stmmac_ptp_register(priv); 3464 3454 3465 - priv->eee_tw_timer = STMMAC_DEFAULT_TWT_LS; 3466 - 3467 - /* Convert the timer from msec to usec */ 3468 - if (!priv->tx_lpi_timer) 3469 - priv->tx_lpi_timer = eee_timer * 1000; 3470 - 3471 3455 if (priv->use_riwt) { 3472 3456 u32 queue; 3473 3457 ··· 3928 3924 u32 chan; 3929 3925 int ret; 3930 3926 3927 + /* Initialise the tx lpi timer, converting from msec to usec */ 3928 + if (!priv->tx_lpi_timer) 3929 + priv->tx_lpi_timer = eee_timer * 1000; 3930 + 3931 3931 ret = pm_runtime_resume_and_get(priv->device); 3932 3932 if (ret < 0) 3933 3933 return ret; ··· 4045 4037 4046 4038 /* Free the IRQ lines */ 4047 4039 stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0); 4048 - 4049 - if (priv->eee_enabled) { 4050 - priv->tx_path_in_lpi_mode = false; 4051 - del_timer_sync(&priv->eee_ctrl_timer); 4052 - } 4053 4040 4054 4041 /* Stop TX/RX DMA and clear the descriptors */ 4055 4042 stmmac_stop_all_dma(priv); ··· 4497 4494 first_tx = tx_q->cur_tx; 4498 4495 4499 4496 if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en) 4500 - stmmac_disable_eee_mode(priv); 4497 + stmmac_disable_sw_eee_mode(priv); 4501 4498 4502 4499 /* Manage oversized TCP frames for GMAC4 device */ 4503 4500 if (skb_is_gso(skb) && priv->tso) { ··· 7411 7408 } 7412 7409 7413 7410 INIT_WORK(&priv->service_task, stmmac_service_task); 7411 + 7412 + timer_setup(&priv->eee_ctrl_timer, stmmac_eee_ctrl_timer, 0); 7414 7413 7415 7414 /* Override with kernel parameters if supplied XXX CRS XXX 7416 7415 * this needs to have multiple instances

+22 -5

drivers/net/phy/phy.c

··· 1641 1641 EXPORT_SYMBOL(phy_mac_interrupt); 1642 1642 1643 1643 /** 1644 + * phy_eee_rx_clock_stop() - configure PHY receive clock in LPI 1645 + * @phydev: target phy_device struct 1646 + * @clk_stop_enable: flag to indicate whether the clock can be stopped 1647 + * 1648 + * Configure whether the PHY can disable its receive clock during LPI mode, 1649 + * See IEEE 802.3 sections 22.2.2.2, 35.2.2.10, and 45.2.3.1.4. 1650 + * 1651 + * Returns: 0 or negative error. 1652 + */ 1653 + int phy_eee_rx_clock_stop(struct phy_device *phydev, bool clk_stop_enable) 1654 + { 1655 + /* Configure the PHY to stop receiving xMII 1656 + * clock while it is signaling LPI. 1657 + */ 1658 + return phy_modify_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1, 1659 + MDIO_PCS_CTRL1_CLKSTOP_EN, 1660 + clk_stop_enable ? MDIO_PCS_CTRL1_CLKSTOP_EN : 0); 1661 + } 1662 + EXPORT_SYMBOL_GPL(phy_eee_rx_clock_stop); 1663 + 1664 + /** 1644 1665 * phy_init_eee - init and check the EEE feature 1645 1666 * @phydev: target phy_device struct 1646 1667 * @clk_stop_enable: PHY may stop the clock during LPI ··· 1685 1664 return -EPROTONOSUPPORT; 1686 1665 1687 1666 if (clk_stop_enable) 1688 - /* Configure the PHY to stop receiving xMII 1689 - * clock while it is signaling LPI. 1690 - */ 1691 - ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1, 1692 - MDIO_PCS_CTRL1_CLKSTOP_EN); 1667 + ret = phy_eee_rx_clock_stop(phydev, true); 1693 1668 1694 1669 return ret < 0 ? ret : 0; 1695 1670 }

include/linux/phy.h

··· 2096 2096 int phy_unregister_fixup_for_id(const char *bus_id); 2097 2097 int phy_unregister_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask); 2098 2098 2099 + int phy_eee_rx_clock_stop(struct phy_device *phydev, bool clk_stop_enable); 2099 2100 int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable); 2100 2101 int phy_get_eee_err(struct phy_device *phydev); 2101 2102 int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_keee *data);

Configure Feed

Configure Feed