* [PATCH net-next] net: ethernet: mediatek: ppe: fix busy wait loop
@ 2021-04-15 23:02 Ilya Lipnitskiy
2021-04-15 23:10 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Ilya Lipnitskiy @ 2021-04-15 23:02 UTC (permalink / raw)
To: Felix Fietkau, John Crispin, Sean Wang, Mark Lee,
David S. Miller, Jakub Kicinski, Matthias Brugger,
Pablo Neira Ayuso, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel
Cc: Ilya Lipnitskiy
The intention is for the loop to timeout if the body does not succeed.
The current logic calls time_is_before_jiffies(timeout) which is false
until after the timeout, so the loop body never executes.
time_is_after_jiffies(timeout) will return true until timeout is less
than jiffies, which is the intended behavior here.
Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE")
Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
Cc: Felix Fietkau <nbd@nbd.name>
---
drivers/net/ethernet/mediatek/mtk_ppe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c
index 71e1ccea6e72..af3c266297aa 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
@@ -46,7 +46,7 @@ static int mtk_ppe_wait_busy(struct mtk_ppe *ppe)
{
unsigned long timeout = jiffies + HZ;
- while (time_is_before_jiffies(timeout)) {
+ while (time_is_after_jiffies(timeout)) {
if (!(ppe_r32(ppe, MTK_PPE_GLO_CFG) & MTK_PPE_GLO_CFG_BUSY))
return 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next] net: ethernet: mediatek: ppe: fix busy wait loop 2021-04-15 23:02 [PATCH net-next] net: ethernet: mediatek: ppe: fix busy wait loop Ilya Lipnitskiy @ 2021-04-15 23:10 ` Andrew Lunn 2021-04-16 0:11 ` [PATCH net-next,v2] " Ilya Lipnitskiy 0 siblings, 1 reply; 6+ messages in thread From: Andrew Lunn @ 2021-04-15 23:10 UTC (permalink / raw) To: Ilya Lipnitskiy Cc: Felix Fietkau, John Crispin, Sean Wang, Mark Lee, David S. Miller, Jakub Kicinski, Matthias Brugger, Pablo Neira Ayuso, netdev, linux-arm-kernel, linux-mediatek, linux-kernel On Thu, Apr 15, 2021 at 04:02:34PM -0700, Ilya Lipnitskiy wrote: > The intention is for the loop to timeout if the body does not succeed. > The current logic calls time_is_before_jiffies(timeout) which is false > until after the timeout, so the loop body never executes. > > time_is_after_jiffies(timeout) will return true until timeout is less > than jiffies, which is the intended behavior here. > > Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE") > Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com> > Cc: Felix Fietkau <nbd@nbd.name> > --- > drivers/net/ethernet/mediatek/mtk_ppe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c > index 71e1ccea6e72..af3c266297aa 100644 > --- a/drivers/net/ethernet/mediatek/mtk_ppe.c > +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c > @@ -46,7 +46,7 @@ static int mtk_ppe_wait_busy(struct mtk_ppe *ppe) > { > unsigned long timeout = jiffies + HZ; > > - while (time_is_before_jiffies(timeout)) { > + while (time_is_after_jiffies(timeout)) { > if (!(ppe_r32(ppe, MTK_PPE_GLO_CFG) & MTK_PPE_GLO_CFG_BUSY)) > return 0; Maybe see is iopoll.h can be used. Andrew > > -- > 2.31.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next,v2] net: ethernet: mediatek: ppe: fix busy wait loop 2021-04-15 23:10 ` Andrew Lunn @ 2021-04-16 0:11 ` Ilya Lipnitskiy 2021-04-16 0:27 ` Andrew Lunn 0 siblings, 1 reply; 6+ messages in thread From: Ilya Lipnitskiy @ 2021-04-16 0:11 UTC (permalink / raw) To: Felix Fietkau, John Crispin, Sean Wang, Mark Lee, David S. Miller, Jakub Kicinski, Matthias Brugger, Pablo Neira Ayuso, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Andrew Lunn Cc: Ilya Lipnitskiy The intention is for the loop to timeout if the body does not succeed. The current logic calls time_is_before_jiffies(timeout) which is false until after the timeout, so the loop body never executes. Fix by using readl_poll_timeout as a more standard and less error-prone solution. Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE") Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com> Cc: Felix Fietkau <nbd@nbd.name> --- drivers/net/ethernet/mediatek/mtk_ppe.c | 18 +++++++++--------- drivers/net/ethernet/mediatek/mtk_ppe.h | 1 + 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c index 71e1ccea6e72..f4b3fc0eeb50 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe.c +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c @@ -5,6 +5,7 @@ #include <linux/jiffies.h> #include <linux/delay.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/etherdevice.h> #include <linux/platform_device.h> #include "mtk_ppe.h" @@ -44,18 +45,17 @@ static u32 ppe_clear(struct mtk_ppe *ppe, u32 reg, u32 val) static int mtk_ppe_wait_busy(struct mtk_ppe *ppe) { - unsigned long timeout = jiffies + HZ; - - while (time_is_before_jiffies(timeout)) { - if (!(ppe_r32(ppe, MTK_PPE_GLO_CFG) & MTK_PPE_GLO_CFG_BUSY)) - return 0; + int ret; + u32 val; - usleep_range(10, 20); - } + ret = readl_poll_timeout(ppe->base + MTK_PPE_GLO_CFG, val, + !(val & MTK_PPE_GLO_CFG_BUSY), + 20, MTK_PPE_WAIT_TIMEOUT_US); - dev_err(ppe->dev, "PPE table busy"); + if (ret) + dev_err(ppe->dev, "PPE table busy"); - return -ETIMEDOUT; + return ret; } static void mtk_ppe_cache_clear(struct mtk_ppe *ppe) diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h index 51bd5e75bbbd..242fb8f2ae65 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe.h +++ b/drivers/net/ethernet/mediatek/mtk_ppe.h @@ -12,6 +12,7 @@ #define MTK_PPE_ENTRIES_SHIFT 3 #define MTK_PPE_ENTRIES (1024 << MTK_PPE_ENTRIES_SHIFT) #define MTK_PPE_HASH_MASK (MTK_PPE_ENTRIES - 1) +#define MTK_PPE_WAIT_TIMEOUT_US 1000000 #define MTK_FOE_IB1_UNBIND_TIMESTAMP GENMASK(7, 0) #define MTK_FOE_IB1_UNBIND_PACKETS GENMASK(23, 8) -- 2.31.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next,v2] net: ethernet: mediatek: ppe: fix busy wait loop 2021-04-16 0:11 ` [PATCH net-next,v2] " Ilya Lipnitskiy @ 2021-04-16 0:27 ` Andrew Lunn 2021-04-16 0:37 ` [PATCH net-next,v3] " Ilya Lipnitskiy 0 siblings, 1 reply; 6+ messages in thread From: Andrew Lunn @ 2021-04-16 0:27 UTC (permalink / raw) To: Ilya Lipnitskiy Cc: Felix Fietkau, John Crispin, Sean Wang, Mark Lee, David S. Miller, Jakub Kicinski, Matthias Brugger, Pablo Neira Ayuso, netdev, linux-arm-kernel, linux-mediatek, linux-kernel On Thu, Apr 15, 2021 at 05:11:48PM -0700, Ilya Lipnitskiy wrote: > The intention is for the loop to timeout if the body does not succeed. > The current logic calls time_is_before_jiffies(timeout) which is false > until after the timeout, so the loop body never executes. > > Fix by using readl_poll_timeout as a more standard and less error-prone > solution. > > Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE") > Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com> > Cc: Felix Fietkau <nbd@nbd.name> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next,v3] net: ethernet: mediatek: ppe: fix busy wait loop 2021-04-16 0:27 ` Andrew Lunn @ 2021-04-16 0:37 ` Ilya Lipnitskiy 2021-04-16 22:30 ` patchwork-bot+netdevbpf 0 siblings, 1 reply; 6+ messages in thread From: Ilya Lipnitskiy @ 2021-04-16 0:37 UTC (permalink / raw) To: Felix Fietkau, John Crispin, Sean Wang, Mark Lee, David S. Miller, Jakub Kicinski, Matthias Brugger, Pablo Neira Ayuso, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Andrew Lunn Cc: Ilya Lipnitskiy The intention is for the loop to timeout if the body does not succeed. The current logic calls time_is_before_jiffies(timeout) which is false until after the timeout, so the loop body never executes. Fix by using readl_poll_timeout as a more standard and less error-prone solution. Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE") Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com> Cc: Felix Fietkau <nbd@nbd.name> Reviewed-by: Andrew Lunn <andrew@lunn.ch> --- v2: - Use readl_poll_timeout as suggested by Andrew Lunn v3: - Remove unused #include's drivers/net/ethernet/mediatek/mtk_ppe.c | 20 +++++++++----------- drivers/net/ethernet/mediatek/mtk_ppe.h | 1 + 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c index 71e1ccea6e72..3ad10c793308 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe.c +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c @@ -2,9 +2,8 @@ /* Copyright (C) 2020 Felix Fietkau <nbd@nbd.name> */ #include <linux/kernel.h> -#include <linux/jiffies.h> -#include <linux/delay.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/etherdevice.h> #include <linux/platform_device.h> #include "mtk_ppe.h" @@ -44,18 +43,17 @@ static u32 ppe_clear(struct mtk_ppe *ppe, u32 reg, u32 val) static int mtk_ppe_wait_busy(struct mtk_ppe *ppe) { - unsigned long timeout = jiffies + HZ; - - while (time_is_before_jiffies(timeout)) { - if (!(ppe_r32(ppe, MTK_PPE_GLO_CFG) & MTK_PPE_GLO_CFG_BUSY)) - return 0; + int ret; + u32 val; - usleep_range(10, 20); - } + ret = readl_poll_timeout(ppe->base + MTK_PPE_GLO_CFG, val, + !(val & MTK_PPE_GLO_CFG_BUSY), + 20, MTK_PPE_WAIT_TIMEOUT_US); - dev_err(ppe->dev, "PPE table busy"); + if (ret) + dev_err(ppe->dev, "PPE table busy"); - return -ETIMEDOUT; + return ret; } static void mtk_ppe_cache_clear(struct mtk_ppe *ppe) diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h index 51bd5e75bbbd..242fb8f2ae65 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe.h +++ b/drivers/net/ethernet/mediatek/mtk_ppe.h @@ -12,6 +12,7 @@ #define MTK_PPE_ENTRIES_SHIFT 3 #define MTK_PPE_ENTRIES (1024 << MTK_PPE_ENTRIES_SHIFT) #define MTK_PPE_HASH_MASK (MTK_PPE_ENTRIES - 1) +#define MTK_PPE_WAIT_TIMEOUT_US 1000000 #define MTK_FOE_IB1_UNBIND_TIMESTAMP GENMASK(7, 0) #define MTK_FOE_IB1_UNBIND_PACKETS GENMASK(23, 8) -- 2.31.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next,v3] net: ethernet: mediatek: ppe: fix busy wait loop 2021-04-16 0:37 ` [PATCH net-next,v3] " Ilya Lipnitskiy @ 2021-04-16 22:30 ` patchwork-bot+netdevbpf 0 siblings, 0 replies; 6+ messages in thread From: patchwork-bot+netdevbpf @ 2021-04-16 22:30 UTC (permalink / raw) To: Ilya Lipnitskiy Cc: nbd, john, sean.wang, Mark-MC.Lee, davem, kuba, matthias.bgg, pablo, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, andrew Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Thu, 15 Apr 2021 17:37:48 -0700 you wrote: > The intention is for the loop to timeout if the body does not succeed. > The current logic calls time_is_before_jiffies(timeout) which is false > until after the timeout, so the loop body never executes. > > Fix by using readl_poll_timeout as a more standard and less error-prone > solution. > > [...] Here is the summary with links: - [net-next,v3] net: ethernet: mediatek: ppe: fix busy wait loop https://git.kernel.org/netdev/net-next/c/c5d66587b890 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-16 22:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-15 23:02 [PATCH net-next] net: ethernet: mediatek: ppe: fix busy wait loop Ilya Lipnitskiy 2021-04-15 23:10 ` Andrew Lunn 2021-04-16 0:11 ` [PATCH net-next,v2] " Ilya Lipnitskiy 2021-04-16 0:27 ` Andrew Lunn 2021-04-16 0:37 ` [PATCH net-next,v3] " Ilya Lipnitskiy 2021-04-16 22:30 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).