From 900782a029e5ca57cc1c334e27f24697b5e47db7 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:45:39 +0000 Subject: [PATCH 1/9] net: stmmac: rename stmmac_disable_sw_eee_mode() stmmac_disable_sw_eee_mode() was not a good choice for this functions purpose - which is to stop transmitting LPI because we want to send a packet. Rename it to stmmac_stop_sw_lpi(). Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXIt1-000MAu-TE@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 58b013528dea1..8130b0f614d8e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -427,12 +427,11 @@ static int stmmac_enable_eee_mode(struct stmmac_priv *priv) } /** - * stmmac_disable_sw_eee_mode - disable and exit from LPI mode + * stmmac_stop_sw_lpi - stop transmitting LPI * @priv: driver private structure - * Description: this function is to exit and disable EEE in case of - * LPI state is true. This is called by the xmit. + * Description: When using software-controlled LPI, stop transmitting LPI state. */ -static void stmmac_disable_sw_eee_mode(struct stmmac_priv *priv) +static void stmmac_stop_sw_lpi(struct stmmac_priv *priv) { stmmac_reset_eee_mode(priv, priv->hw); del_timer_sync(&priv->eee_ctrl_timer); @@ -4497,7 +4496,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) first_tx = tx_q->cur_tx; if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en) - stmmac_disable_sw_eee_mode(priv); + stmmac_stop_sw_lpi(priv); /* Manage oversized TCP frames for GMAC4 device */ if (skb_is_gso(skb) && priv->tso) { From 4fe09a0d64d528cc4576ad5c4963836175f2d38d Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:45:45 +0000 Subject: [PATCH 2/9] net: stmmac: correct priv->eee_sw_timer_en setting If we are disabling EEE/LPI, then we should not be enabling software mode. The only time when we should is if EEE is active, and we are wanting to use software-timed EEE mode. Therefore, in the disable path of stmmac_eee_init(), ensure that priv->eee_sw_timer_en is set false as we are going to be calling del_timer_sync() on the timer. This will allow us to simplify some fast-path tests in later patches. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXIt7-000MB0-0W@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 8130b0f614d8e..f1e416b033491 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -478,7 +478,7 @@ static void stmmac_eee_init(struct stmmac_priv *priv, bool active) if (!priv->eee_active) { if (priv->eee_enabled) { netdev_dbg(priv->dev, "disable EEE\n"); - priv->eee_sw_timer_en = true; + priv->eee_sw_timer_en = false; stmmac_disable_hw_lpi_timer(priv); del_timer_sync(&priv->eee_ctrl_timer); stmmac_set_eee_timer(priv, priv->hw, 0, From bfa9e131c9b22506d21290494e4acd67a2adf3cd Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:45:50 +0000 Subject: [PATCH 3/9] net: stmmac: simplify TX cleanup decision for ending sw LPI mode As mentioned in "net: stmmac: correct priv->eee_sw_timer_en setting", we can simplify some fast-path tests. The transmit cleaning path checks whether EEE is enabled, the transmit path is not in LPI mode, and that we're using software timed mode. Since the above mentioned commit, checking whether EEE is enabled is no longer necessary as priv->eee_sw_timer_en will be false when EEE is disabled. Simplify this test. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXItC-000MB6-54@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index f1e416b033491..e8667848e0ee6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -2782,8 +2782,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue, xmits = budget; } - if (priv->eee_enabled && !priv->tx_path_in_lpi_mode && - priv->eee_sw_timer_en) { + if (priv->eee_sw_timer_en && !priv->tx_path_in_lpi_mode) { if (stmmac_enable_eee_mode(priv)) mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(priv->tx_lpi_timer)); } From c920e6402523ab43b035c913e14d7a580d815d83 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:45:55 +0000 Subject: [PATCH 4/9] net: stmmac: check priv->eee_sw_timer_en in suspend path The suspend path uses priv->eee_enabled when cleaning up the software timed LPI mode. Use priv->eee_sw_timer_en instead so we're consistently using a single control for software-based timer handling. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXItH-000MBC-8i@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index e8667848e0ee6..26ff1ded4e3db 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -7719,7 +7719,7 @@ int stmmac_suspend(struct device *dev) for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++) hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer); - if (priv->eee_enabled) { + if (priv->eee_sw_timer_en) { priv->tx_path_in_lpi_mode = false; del_timer_sync(&priv->eee_ctrl_timer); } From 0cf44bd0c118bcbbbb10f0cc740cd64db227496f Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:46:00 +0000 Subject: [PATCH 5/9] net: stmmac: add stmmac_try_to_start_sw_lpi() There are two places which call stmmac_enable_eee_mode() and follow it immediately by modifying the expiry of priv->eee_ctrl_timer. Both code paths are trying to enable LPI mode. Remove this duplication by providing a function for this. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXItM-000MBI-CX@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 26ff1ded4e3db..2bb61757e3207 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -426,6 +426,13 @@ static int stmmac_enable_eee_mode(struct stmmac_priv *priv) return 0; } +static void stmmac_try_to_start_sw_lpi(struct stmmac_priv *priv) +{ + if (stmmac_enable_eee_mode(priv)) + mod_timer(&priv->eee_ctrl_timer, + STMMAC_LPI_T(priv->tx_lpi_timer)); +} + /** * stmmac_stop_sw_lpi - stop transmitting LPI * @priv: driver private structure @@ -449,8 +456,7 @@ static void stmmac_eee_ctrl_timer(struct timer_list *t) { struct stmmac_priv *priv = from_timer(priv, t, eee_ctrl_timer); - if (stmmac_enable_eee_mode(priv)) - mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(priv->tx_lpi_timer)); + stmmac_try_to_start_sw_lpi(priv); } /** @@ -2782,10 +2788,8 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue, xmits = budget; } - if (priv->eee_sw_timer_en && !priv->tx_path_in_lpi_mode) { - if (stmmac_enable_eee_mode(priv)) - mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(priv->tx_lpi_timer)); - } + if (priv->eee_sw_timer_en && !priv->tx_path_in_lpi_mode) + stmmac_try_to_start_sw_lpi(priv); /* We still have pending packets, let's call for a new scheduling */ if (tx_q->dirty_tx != tx_q->cur_tx) From 82f2025dda761ec7193a03effaf9d48fee456618 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:46:05 +0000 Subject: [PATCH 6/9] net: stmmac: provide stmmac_eee_tx_busy() Extract the code which checks whether there's still work to do on any of the stmmac transmit queues. This will allow us to combine stmmac_enable_eee_mode() with stmmac_try_to_start_sw_lpi() in the next patch. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXItR-000MBO-GF@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 2bb61757e3207..ddbcbe3886c0e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -400,13 +400,7 @@ static void stmmac_enable_hw_lpi_timer(struct stmmac_priv *priv) stmmac_set_eee_lpi_timer(priv, priv->hw, priv->tx_lpi_timer); } -/** - * stmmac_enable_eee_mode - check and enter in LPI mode - * @priv: driver private structure - * Description: this function is to verify and enter in LPI mode in case of - * EEE. - */ -static int stmmac_enable_eee_mode(struct stmmac_priv *priv) +static bool stmmac_eee_tx_busy(struct stmmac_priv *priv) { u32 tx_cnt = priv->plat->tx_queues_to_use; u32 queue; @@ -416,9 +410,23 @@ static int stmmac_enable_eee_mode(struct stmmac_priv *priv) struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue]; if (tx_q->dirty_tx != tx_q->cur_tx) - return -EBUSY; /* still unfinished work */ + return true; /* still unfinished work */ } + return false; +} + +/** + * stmmac_enable_eee_mode - check and enter in LPI mode + * @priv: driver private structure + * Description: this function is to verify and enter in LPI mode in case of + * EEE. + */ +static int stmmac_enable_eee_mode(struct stmmac_priv *priv) +{ + if (stmmac_eee_tx_busy(priv)) + return -EBUSY; /* still unfinished work */ + /* Check and enter in LPI mode */ if (!priv->tx_path_in_lpi_mode) stmmac_set_eee_mode(priv, priv->hw, From af5dc22bdb5fe5a20ba0cdb18b90a995ba694a54 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:46:10 +0000 Subject: [PATCH 7/9] net: stmmac: provide function for restarting sw LPI timer Provide a function that encapsulates restarting the software LPI timer when we have determined that the transmit path is busy, or whether the EEE parameters have changed. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXItW-000MBU-KQ@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index ddbcbe3886c0e..677a2172a85ff 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -416,6 +416,11 @@ static bool stmmac_eee_tx_busy(struct stmmac_priv *priv) return false; } +static void stmmac_restart_sw_lpi_timer(struct stmmac_priv *priv) +{ + mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(priv->tx_lpi_timer)); +} + /** * stmmac_enable_eee_mode - check and enter in LPI mode * @priv: driver private structure @@ -437,8 +442,7 @@ static int stmmac_enable_eee_mode(struct stmmac_priv *priv) static void stmmac_try_to_start_sw_lpi(struct stmmac_priv *priv) { if (stmmac_enable_eee_mode(priv)) - mod_timer(&priv->eee_ctrl_timer, - STMMAC_LPI_T(priv->tx_lpi_timer)); + stmmac_restart_sw_lpi_timer(priv); } /** @@ -526,8 +530,7 @@ static void stmmac_eee_init(struct stmmac_priv *priv, bool active) /* Use software LPI mode */ priv->eee_sw_timer_en = true; stmmac_disable_hw_lpi_timer(priv); - mod_timer(&priv->eee_ctrl_timer, - STMMAC_LPI_T(priv->tx_lpi_timer)); + stmmac_restart_sw_lpi_timer(priv); } priv->eee_enabled = true; From ec8553673b1f441fa7503662679b93d0dd533985 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:46:15 +0000 Subject: [PATCH 8/9] net: stmmac: combine stmmac_enable_eee_mode() Combine stmmac_enable_eee_mode() with stmmac_try_to_start_sw_lpi() which makes the code easier to read and the flow more logical. We can now trivially see that if the transmit queues are busy, we (re-)start the eee_ctrl_timer. Otherwise, if the transmit path is not already in LPI mode, we ask the hardware to enter LPI mode. I believe that now we can see better what is going on here, this shows that there is a bug with the software LPI timer implementation. The LPI timer is supposed to define how long after the last transmittion completed before we start signalling LPI. However, this code structure shows that if all transmit queues are empty, and stmmac_try_to_start_sw_lpi() is called immediately after cleaning the transmit queue, we will instruct the hardware to start signalling LPI immediately. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXItb-000MBa-OU@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 677a2172a85ff..72f2700130862 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -422,27 +422,22 @@ static void stmmac_restart_sw_lpi_timer(struct stmmac_priv *priv) } /** - * stmmac_enable_eee_mode - check and enter in LPI mode + * stmmac_try_to_start_sw_lpi - check and enter in LPI mode * @priv: driver private structure * Description: this function is to verify and enter in LPI mode in case of * EEE. */ -static int stmmac_enable_eee_mode(struct stmmac_priv *priv) +static void stmmac_try_to_start_sw_lpi(struct stmmac_priv *priv) { - if (stmmac_eee_tx_busy(priv)) - return -EBUSY; /* still unfinished work */ + if (stmmac_eee_tx_busy(priv)) { + stmmac_restart_sw_lpi_timer(priv); + return; + } /* Check and enter in LPI mode */ if (!priv->tx_path_in_lpi_mode) stmmac_set_eee_mode(priv, priv->hw, priv->plat->flags & STMMAC_FLAG_EN_TX_LPI_CLOCKGATING); - return 0; -} - -static void stmmac_try_to_start_sw_lpi(struct stmmac_priv *priv) -{ - if (stmmac_enable_eee_mode(priv)) - stmmac_restart_sw_lpi_timer(priv); } /** From d28e89244978ef2a91e23e83c99b5e8985ff8db2 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 13 Jan 2025 11:46:20 +0000 Subject: [PATCH 9/9] net: stmmac: restart LPI timer after cleaning transmit descriptors Fix a bug in the LPI handling, where it is possible to immediately enter LPI mode after cleaning the transmit descriptors when all queues are empty rather than waiting for the LPI timeout to expire. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tXItg-000MBg-TW@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 72f2700130862..acd6994c17644 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -2795,7 +2795,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue, } if (priv->eee_sw_timer_en && !priv->tx_path_in_lpi_mode) - stmmac_try_to_start_sw_lpi(priv); + stmmac_restart_sw_lpi_timer(priv); /* We still have pending packets, let's call for a new scheduling */ if (tx_q->dirty_tx != tx_q->cur_tx)