* [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).