tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'net-improve-stmmac-resume-rx-clocking'

Russell King says:

====================
net: improve stmmac resume rx clocking

stmmac has had a long history of problems with resuming, illustrated by
reset failure due to the receive clock not running.

Several attempts have been attempted over the years to address this
issue, such as moving phylink_start() (now phylink_resume()) super
early in stmmac_resume() in commit 90702dcd19c0 ("net: stmmac: fix MAC
not working when system resume back with WoL a ctive.") However, this
has the downside that stmmac_mac_link_up() can (and demonstrably is)
called before or during the driver initialisation in another thread.
This can cause issues as packets could begin to be queued, and the
transmit/receive enable bits will be set before any initialisation has
been done.

Another attempt is used by dwmac-socfpga.c in commit 2d871aa07136 ("net:
stmmac: add platform init/exit for Altera's ARM socfpga") which
pre-dates the above commit.

Neither of these two approaches consider the effect of EEE with a PHY
that supports receive clock-stop and has that feature enabled (which
the stmmac driver does enable). If the link is up, then there is the
possibility for the receive path to be in low-power mode, and the PHY
may stop its receive clock.

This series addresses these issues by (each is not necessarily a
separate patch):

1) introducing phylink_prepare_resume(), which can be used by MAC
drivers to ensure that the PHY is resumed prior to doing any
re-initialisation work. This call is added to stmmac_resume().

2) moving phylink_resume() after all re-initialisation has completed,
thereby ensuring that the hardware is ready to be enabled for
packet reception/transmission.

3) with (1) and (2) addressed, the need for socfpga to have a private
work-around is no longer necessary, so it is removed.

4) introducing phylink functions to block/unblock the receive clock-
stop at the PHY. As these require PHY access over the MDIO bus,
they can sleep, so are not suitable for atomic access.

5) the stmmac hardware requires the receive clock to be running for
reset to complete. Depending on synthesis options, this requirement
may also extend to writing various registers as well, e.g. setting
the MAC address, writing some of the vlan registers, etc. Full
details are in the databook.

We add blocking/unblocking of the PHY receive clock-stop around
parts of the main stmmac driver where we have a context that we
can sleep. These are wrapped with the new phylink functions.

However, depending on synthesis options, there could be other
places where the net core calls the driver with a BH-disabled
context where we can't sleep, and thus can't block the PHY from
disabling its receive clock. These are documented with FIXME
comments.

Given the last paragraph above, I am wondering whether a better
approach would be to ensure that receive clock-stop is always disabled
at the PHY with stmmac. From what I can see, implementations do not
document to this level of detail, which makes it difficult to tell
which registers require the receive clock to be running to behave
correctly.

This patch series has been tested on the Tegra194 Jetson Xavier NX
board kindly donated by NVidia, with two additional patches that are
pending in patchwork - the first is required to have EEE's LPI mode
passed through to the MAC on this platform to allow testing under
PHY clock-stop scenarios. The second is a bug fix for PHYLIB and
makes "eee off" functional, but should not affect this series.

All patches on top of net-next commit f749448ce9f1 ("Merge branch
'net-mlx5-hw-steering-cleanups'")

https://patchwork.kernel.org/project/netdevbpf/patch/E1ttnHW-00785s-Uq@rmk-PC.armlinux.org.uk/
https://patchwork.kernel.org/project/netdevbpf/patch/E1ttmWN-0077Mb-Q6@rmk-PC.armlinux.org.uk/
====================

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

Jakub Kicinski 73ed6f53 23746210

+129 -23
-18
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
··· 523 523 524 524 dwmac_priv->ops->set_phy_mode(priv->plat->bsp_priv); 525 525 526 - /* Before the enet controller is suspended, the phy is suspended. 527 - * This causes the phy clock to be gated. The enet controller is 528 - * resumed before the phy, so the clock is still gated "off" when 529 - * the enet controller is resumed. This code makes sure the phy 530 - * is "resumed" before reinitializing the enet controller since 531 - * the enet controller depends on an active phy clock to complete 532 - * a DMA reset. A DMA reset will "time out" if executed 533 - * with no phy clock input on the Synopsys enet controller. 534 - * Verified through Synopsys Case #8000711656. 535 - * 536 - * Note that the phy clock is also gated when the phy is isolated. 537 - * Phy "suspend" and "isolate" controls are located in phy basic 538 - * control register 0, and can be modified by the phy driver 539 - * framework. 540 - */ 541 - if (ndev->phydev) 542 - phy_resume(ndev->phydev); 543 - 544 526 return stmmac_resume(dev); 545 527 } 546 528 #endif /* CONFIG_PM_SLEEP */
+41 -5
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
··· 3484 3484 if (priv->hw->phylink_pcs) 3485 3485 phylink_pcs_pre_init(priv->phylink, priv->hw->phylink_pcs); 3486 3486 3487 + /* Note that clk_rx_i must be running for reset to complete. This 3488 + * clock may also be required when setting the MAC address. 3489 + * 3490 + * Block the receive clock stop for LPI mode at the PHY in case 3491 + * the link is established with EEE mode active. 3492 + */ 3493 + phylink_rx_clk_stop_block(priv->phylink); 3494 + 3487 3495 /* DMA initialization and SW reset */ 3488 3496 ret = stmmac_init_dma_engine(priv); 3489 3497 if (ret < 0) { 3498 + phylink_rx_clk_stop_unblock(priv->phylink); 3490 3499 netdev_err(priv->dev, "%s: DMA engine initialization failed\n", 3491 3500 __func__); 3492 3501 return ret; ··· 3503 3494 3504 3495 /* Copy the MAC addr into the HW */ 3505 3496 stmmac_set_umac_addr(priv, priv->hw, dev->dev_addr, 0); 3497 + phylink_rx_clk_stop_unblock(priv->phylink); 3506 3498 3507 3499 /* PS and related bits will be programmed according to the speed */ 3508 3500 if (priv->hw->pcs) { ··· 3614 3604 /* Start the ball rolling... */ 3615 3605 stmmac_start_all_dma(priv); 3616 3606 3607 + phylink_rx_clk_stop_block(priv->phylink); 3617 3608 stmmac_set_hw_vlan_mode(priv, priv->hw); 3609 + phylink_rx_clk_stop_unblock(priv->phylink); 3618 3610 3619 3611 return 0; 3620 3612 } ··· 5900 5888 * whenever multicast addresses must be enabled/disabled. 5901 5889 * Return value: 5902 5890 * void. 5891 + * 5892 + * FIXME: This may need RXC to be running, but it may be called with BH 5893 + * disabled, which means we can't call phylink_rx_clk_stop*(). 5903 5894 */ 5904 5895 static void stmmac_set_rx_mode(struct net_device *dev) 5905 5896 { ··· 6035 6020 else 6036 6021 priv->hw->hw_vlan_en = false; 6037 6022 6023 + phylink_rx_clk_stop_block(priv->phylink); 6038 6024 stmmac_set_hw_vlan_mode(priv, priv->hw); 6025 + phylink_rx_clk_stop_unblock(priv->phylink); 6039 6026 6040 6027 return 0; 6041 6028 } ··· 6321 6304 if (ret) 6322 6305 goto set_mac_error; 6323 6306 6307 + phylink_rx_clk_stop_block(priv->phylink); 6324 6308 stmmac_set_umac_addr(priv, priv->hw, ndev->dev_addr, 0); 6309 + phylink_rx_clk_stop_unblock(priv->phylink); 6325 6310 6326 6311 set_mac_error: 6327 6312 pm_runtime_put(priv->device); ··· 6679 6660 return stmmac_update_vlan_hash(priv, priv->hw, hash, pmatch, is_double); 6680 6661 } 6681 6662 6663 + /* FIXME: This may need RXC to be running, but it may be called with BH 6664 + * disabled, which means we can't call phylink_rx_clk_stop*(). 6665 + */ 6682 6666 static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid) 6683 6667 { 6684 6668 struct stmmac_priv *priv = netdev_priv(ndev); ··· 6713 6691 return ret; 6714 6692 } 6715 6693 6694 + /* FIXME: This may need RXC to be running, but it may be called with BH 6695 + * disabled, which means we can't call phylink_rx_clk_stop*(). 6696 + */ 6716 6697 static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vid) 6717 6698 { 6718 6699 struct stmmac_priv *priv = netdev_priv(ndev); ··· 7961 7936 } 7962 7937 7963 7938 rtnl_lock(); 7964 - phylink_resume(priv->phylink); 7965 - if (device_may_wakeup(priv->device) && !priv->plat->pmt) 7966 - phylink_speed_up(priv->phylink); 7967 - rtnl_unlock(); 7968 7939 7969 - rtnl_lock(); 7940 + /* Prepare the PHY to resume, ensuring that its clocks which are 7941 + * necessary for the MAC DMA reset to complete are running 7942 + */ 7943 + phylink_prepare_resume(priv->phylink); 7944 + 7970 7945 mutex_lock(&priv->lock); 7971 7946 7972 7947 stmmac_reset_queues_param(priv); ··· 7976 7951 7977 7952 stmmac_hw_setup(ndev, false); 7978 7953 stmmac_init_coalesce(priv); 7954 + phylink_rx_clk_stop_block(priv->phylink); 7979 7955 stmmac_set_rx_mode(ndev); 7980 7956 7981 7957 stmmac_restore_hw_vlan_rx_fltr(priv, ndev, priv->hw); 7958 + phylink_rx_clk_stop_unblock(priv->phylink); 7982 7959 7983 7960 stmmac_enable_all_queues(priv); 7984 7961 stmmac_enable_all_dma_irq(priv); 7985 7962 7986 7963 mutex_unlock(&priv->lock); 7964 + 7965 + /* phylink_resume() must be called after the hardware has been 7966 + * initialised because it may bring the link up immediately in a 7967 + * workqueue thread, which will race with initialisation. 7968 + */ 7969 + phylink_resume(priv->phylink); 7970 + if (device_may_wakeup(priv->device) && !priv->plat->pmt) 7971 + phylink_speed_up(priv->phylink); 7972 + 7987 7973 rtnl_unlock(); 7988 7974 7989 7975 netif_device_attach(ndev);
+84
drivers/net/phy/phylink.c
··· 87 87 bool mac_enable_tx_lpi; 88 88 bool mac_tx_clk_stop; 89 89 u32 mac_tx_lpi_timer; 90 + u8 mac_rx_clk_stop_blocked; 90 91 91 92 struct sfp_bus *sfp_bus; 92 93 bool sfp_may_have_phy; ··· 2437 2436 EXPORT_SYMBOL_GPL(phylink_stop); 2438 2437 2439 2438 /** 2439 + * phylink_rx_clk_stop_block() - block PHY ability to stop receive clock in LPI 2440 + * @pl: a pointer to a &struct phylink returned from phylink_create() 2441 + * 2442 + * Disable the PHY's ability to stop the receive clock while the receive path 2443 + * is in EEE LPI state, until the number of calls to phylink_rx_clk_stop_block() 2444 + * are balanced by calls to phylink_rx_clk_stop_unblock(). 2445 + */ 2446 + void phylink_rx_clk_stop_block(struct phylink *pl) 2447 + { 2448 + ASSERT_RTNL(); 2449 + 2450 + if (pl->mac_rx_clk_stop_blocked == U8_MAX) { 2451 + phylink_warn(pl, "%s called too many times - ignoring\n", 2452 + __func__); 2453 + dump_stack(); 2454 + return; 2455 + } 2456 + 2457 + /* Disable PHY receive clock stop if this is the first time this 2458 + * function has been called and clock-stop was previously enabled. 2459 + */ 2460 + if (pl->mac_rx_clk_stop_blocked++ == 0 && 2461 + pl->mac_supports_eee_ops && pl->phydev && 2462 + pl->config->eee_rx_clk_stop_enable) 2463 + phy_eee_rx_clock_stop(pl->phydev, false); 2464 + } 2465 + EXPORT_SYMBOL_GPL(phylink_rx_clk_stop_block); 2466 + 2467 + /** 2468 + * phylink_rx_clk_stop_unblock() - unblock PHY ability to stop receive clock 2469 + * @pl: a pointer to a &struct phylink returned from phylink_create() 2470 + * 2471 + * All calls to phylink_rx_clk_stop_block() must be balanced with a 2472 + * corresponding call to phylink_rx_clk_stop_unblock() to restore the PHYs 2473 + * ability to stop the receive clock when the receive path is in EEE LPI mode. 2474 + */ 2475 + void phylink_rx_clk_stop_unblock(struct phylink *pl) 2476 + { 2477 + ASSERT_RTNL(); 2478 + 2479 + if (pl->mac_rx_clk_stop_blocked == 0) { 2480 + phylink_warn(pl, "%s called too many times - ignoring\n", 2481 + __func__); 2482 + dump_stack(); 2483 + return; 2484 + } 2485 + 2486 + /* Re-enable PHY receive clock stop if the number of unblocks matches 2487 + * the number of calls to the block function above. 2488 + */ 2489 + if (--pl->mac_rx_clk_stop_blocked == 0 && 2490 + pl->mac_supports_eee_ops && pl->phydev && 2491 + pl->config->eee_rx_clk_stop_enable) 2492 + phy_eee_rx_clock_stop(pl->phydev, true); 2493 + } 2494 + EXPORT_SYMBOL_GPL(phylink_rx_clk_stop_unblock); 2495 + 2496 + /** 2440 2497 * phylink_suspend() - handle a network device suspend event 2441 2498 * @pl: a pointer to a &struct phylink returned from phylink_create() 2442 2499 * @mac_wol: true if the MAC needs to receive packets for Wake-on-Lan ··· 2537 2478 } 2538 2479 } 2539 2480 EXPORT_SYMBOL_GPL(phylink_suspend); 2481 + 2482 + /** 2483 + * phylink_prepare_resume() - prepare to resume a network device 2484 + * @pl: a pointer to a &struct phylink returned from phylink_create() 2485 + * 2486 + * Optional, but if called must be called prior to phylink_resume(). 2487 + * 2488 + * Prepare to resume a network device, preparing the PHY as necessary. 2489 + */ 2490 + void phylink_prepare_resume(struct phylink *pl) 2491 + { 2492 + struct phy_device *phydev = pl->phydev; 2493 + 2494 + ASSERT_RTNL(); 2495 + 2496 + /* IEEE 802.3 22.2.4.1.5 allows PHYs to stop their receive clock 2497 + * when PDOWN is set. However, some MACs require RXC to be running 2498 + * in order to resume. If the MAC requires RXC, and we have a PHY, 2499 + * then resume the PHY. Note that 802.3 allows PHYs 500ms before 2500 + * the clock meets requirements. We do not implement this delay. 2501 + */ 2502 + if (pl->config->mac_requires_rxc && phydev && phydev->suspended) 2503 + phy_resume(phydev); 2504 + } 2505 + EXPORT_SYMBOL_GPL(phylink_prepare_resume); 2540 2506 2541 2507 /** 2542 2508 * phylink_resume() - handle a network device resume event
+4
include/linux/phylink.h
··· 706 706 void phylink_start(struct phylink *); 707 707 void phylink_stop(struct phylink *); 708 708 709 + void phylink_rx_clk_stop_block(struct phylink *); 710 + void phylink_rx_clk_stop_unblock(struct phylink *); 711 + 709 712 void phylink_suspend(struct phylink *pl, bool mac_wol); 713 + void phylink_prepare_resume(struct phylink *pl); 710 714 void phylink_resume(struct phylink *pl); 711 715 712 716 void phylink_ethtool_get_wol(struct phylink *, struct ethtool_wolinfo *);