| Message ID | 20241215-qca8k-jiffies-v1-1-5a4d313c76ea@lunn.ch (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5a49edec44f638952da8dc8d754e76f462c19034 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: qca8k: Fix inconsistent use of jiffies vs milliseconds | expand |
On Sun, Dec 15, 2024 at 05:43:55PM +0000, Andrew Lunn wrote: > wait_for_complete_timeout() expects a timeout in jiffies. With the > driver, some call sites converted QCA8K_ETHERNET_TIMEOUT to jiffies, > others did not. Make the code consistent by changes the #define to > include a call to msecs_to_jiffies, and remove all other calls to > msecs_to_jiffies. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- If my calculations are correct, for CONFIG_HZ=100, 5 jiffies last 50 ms. So, assuming that configuration, the patch would be _decreasing_ the timeout from 50 ms to 5 ms. The change should be tested to confirm it's enough. Christian, could you do that?
On Mon, Dec 16, 2024 at 01:13:34AM +0200, Vladimir Oltean wrote: > On Sun, Dec 15, 2024 at 05:43:55PM +0000, Andrew Lunn wrote: > > wait_for_complete_timeout() expects a timeout in jiffies. With the > > driver, some call sites converted QCA8K_ETHERNET_TIMEOUT to jiffies, > > others did not. Make the code consistent by changes the #define to > > include a call to msecs_to_jiffies, and remove all other calls to > > msecs_to_jiffies. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > If my calculations are correct, for CONFIG_HZ=100, 5 jiffies last 50 ms. > So, assuming that configuration, the patch would be _decreasing_ the timeout > from 50 ms to 5 ms. The change should be tested to confirm it's enough. > Christian, could you do that? I've have an qca8k system now, and have tested this patch. However, a Tested-by: from Christian would be very welcome. Andrew
On Mon, Dec 16, 2024 at 10:21:12AM +0100, Andrew Lunn wrote: > On Mon, Dec 16, 2024 at 01:13:34AM +0200, Vladimir Oltean wrote: > > On Sun, Dec 15, 2024 at 05:43:55PM +0000, Andrew Lunn wrote: > > > wait_for_complete_timeout() expects a timeout in jiffies. With the > > > driver, some call sites converted QCA8K_ETHERNET_TIMEOUT to jiffies, > > > others did not. Make the code consistent by changes the #define to > > > include a call to msecs_to_jiffies, and remove all other calls to > > > msecs_to_jiffies. > > > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > > --- > > > > If my calculations are correct, for CONFIG_HZ=100, 5 jiffies last 50 ms. > > So, assuming that configuration, the patch would be _decreasing_ the timeout > > from 50 ms to 5 ms. The change should be tested to confirm it's enough. > > Christian, could you do that? > > I've have an qca8k system now, and have tested this patch. However, a > Tested-by: from Christian would be very welcome. > Hi need 1-2 days to test this, hope that is O.K.
On Mon, Dec 16, 2024 at 08:55:13PM +0100, Christian Marangi wrote: > On Mon, Dec 16, 2024 at 10:21:12AM +0100, Andrew Lunn wrote: > > On Mon, Dec 16, 2024 at 01:13:34AM +0200, Vladimir Oltean wrote: > > > On Sun, Dec 15, 2024 at 05:43:55PM +0000, Andrew Lunn wrote: > > > > wait_for_complete_timeout() expects a timeout in jiffies. With the > > > > driver, some call sites converted QCA8K_ETHERNET_TIMEOUT to jiffies, > > > > others did not. Make the code consistent by changes the #define to > > > > include a call to msecs_to_jiffies, and remove all other calls to > > > > msecs_to_jiffies. > > > > > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > > > --- > > > > > > If my calculations are correct, for CONFIG_HZ=100, 5 jiffies last 50 ms. > > > So, assuming that configuration, the patch would be _decreasing_ the timeout > > > from 50 ms to 5 ms. The change should be tested to confirm it's enough. > > > Christian, could you do that? > > > > I've have an qca8k system now, and have tested this patch. However, a > > Tested-by: from Christian would be very welcome. > > > > Hi need 1-2 days to test this, hope that is O.K. That is fine, I don't really expect the remaining patches will go anywhere until next year, with net-next closing soon. Andrew
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Sun, 15 Dec 2024 17:43:55 +0000 you wrote: > wait_for_complete_timeout() expects a timeout in jiffies. With the > driver, some call sites converted QCA8K_ETHERNET_TIMEOUT to jiffies, > others did not. Make the code consistent by changes the #define to > include a call to msecs_to_jiffies, and remove all other calls to > msecs_to_jiffies. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > [...] Here is the summary with links: - [net-next] net: dsa: qca8k: Fix inconsistent use of jiffies vs milliseconds https://git.kernel.org/netdev/net-next/c/5a49edec44f6 You are awesome, thank you!
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c index ec74e3c2b0e9b90b29de4442f056783043fac69f..90e24bc00b99cca1f65ed1df8b06713d7002e029 100644 --- a/drivers/net/dsa/qca/qca8k-8xxx.c +++ b/drivers/net/dsa/qca/qca8k-8xxx.c @@ -342,7 +342,7 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) dev_queue_xmit(skb); ret = wait_for_completion_timeout(&mgmt_eth_data->rw_done, - msecs_to_jiffies(QCA8K_ETHERNET_TIMEOUT)); + QCA8K_ETHERNET_TIMEOUT); *val = mgmt_eth_data->data[0]; if (len > QCA_HDR_MGMT_DATA1_LEN) @@ -394,7 +394,7 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) dev_queue_xmit(skb); ret = wait_for_completion_timeout(&mgmt_eth_data->rw_done, - msecs_to_jiffies(QCA8K_ETHERNET_TIMEOUT)); + QCA8K_ETHERNET_TIMEOUT); ack = mgmt_eth_data->ack; diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h index 3664a2e2f1f641d2d5533b767f709bf34eec3753..24962a395754c1f12de58d11cdc97e5fe5f0d046 100644 --- a/drivers/net/dsa/qca/qca8k.h +++ b/drivers/net/dsa/qca/qca8k.h @@ -16,7 +16,7 @@ #define QCA8K_ETHERNET_MDIO_PRIORITY 7 #define QCA8K_ETHERNET_PHY_PRIORITY 6 -#define QCA8K_ETHERNET_TIMEOUT 5 +#define QCA8K_ETHERNET_TIMEOUT msecs_to_jiffies(5) #define QCA8K_NUM_PORTS 7 #define QCA8K_NUM_CPU_PORTS 2
wait_for_complete_timeout() expects a timeout in jiffies. With the driver, some call sites converted QCA8K_ETHERNET_TIMEOUT to jiffies, others did not. Make the code consistent by changes the #define to include a call to msecs_to_jiffies, and remove all other calls to msecs_to_jiffies. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/dsa/qca/qca8k-8xxx.c | 4 ++-- drivers/net/dsa/qca/qca8k.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) --- base-commit: 2c2b61d2138f472e50b5531ec0cb4a1485837e21 change-id: 20241215-qca8k-jiffies-01e8c5dd5099 Best regards,