* [PATCH net-next] drivers: net: davinci_mdio: prevent sprious timeout
@ 2018-05-08 8:26 ` Sekhar Nori
0 siblings, 0 replies; 5+ messages in thread
From: Sekhar Nori @ 2018-05-08 8:26 UTC (permalink / raw)
To: Grygorii Strashko; +Cc: David S . Miller, linux-omap, netdev
A well timed kernel preemption in the time_after() loop
in wait_for_idle() can result in a spurious timeout
error to be returned.
Fix it by checking for status of hardware before returning
timeout error.
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
The issue has not been personally observed by me, but has
been reported by users. Sending for next-next given the
non-critical nature. There is seems to be no easy way to
reproduce this.
drivers/net/ethernet/ti/davinci_mdio.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 3c33f4504d8e..4fbd04fd38cf 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -231,10 +231,16 @@ static inline int wait_for_idle(struct davinci_mdio_data *data)
while (time_after(timeout, jiffies)) {
if (__raw_readl(®s->control) & CONTROL_IDLE)
- return 0;
+ goto out;
}
- dev_err(data->dev, "timed out waiting for idle\n");
- return -ETIMEDOUT;
+
+ if (!(__raw_readl(®s->control) & CONTROL_IDLE)) {
+ dev_err(data->dev, "timed out waiting for idle\n");
+ return -ETIMEDOUT;
+ }
+
+out:
+ return 0;
}
static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
--
2.16.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next] drivers: net: davinci_mdio: prevent sprious timeout
@ 2018-05-08 8:26 ` Sekhar Nori
0 siblings, 0 replies; 5+ messages in thread
From: Sekhar Nori @ 2018-05-08 8:26 UTC (permalink / raw)
To: Grygorii Strashko; +Cc: David S . Miller, linux-omap, netdev
A well timed kernel preemption in the time_after() loop
in wait_for_idle() can result in a spurious timeout
error to be returned.
Fix it by checking for status of hardware before returning
timeout error.
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
The issue has not been personally observed by me, but has
been reported by users. Sending for next-next given the
non-critical nature. There is seems to be no easy way to
reproduce this.
drivers/net/ethernet/ti/davinci_mdio.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 3c33f4504d8e..4fbd04fd38cf 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -231,10 +231,16 @@ static inline int wait_for_idle(struct davinci_mdio_data *data)
while (time_after(timeout, jiffies)) {
if (__raw_readl(®s->control) & CONTROL_IDLE)
- return 0;
+ goto out;
}
- dev_err(data->dev, "timed out waiting for idle\n");
- return -ETIMEDOUT;
+
+ if (!(__raw_readl(®s->control) & CONTROL_IDLE)) {
+ dev_err(data->dev, "timed out waiting for idle\n");
+ return -ETIMEDOUT;
+ }
+
+out:
+ return 0;
}
static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
--
2.16.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] drivers: net: davinci_mdio: prevent sprious timeout
2018-05-08 8:26 ` Sekhar Nori
(?)
@ 2018-05-08 12:48 ` Andrew Lunn
2018-05-09 10:44 ` Sekhar Nori
-1 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2018-05-08 12:48 UTC (permalink / raw)
To: Sekhar Nori; +Cc: Grygorii Strashko, David S . Miller, linux-omap, netdev
On Tue, May 08, 2018 at 01:56:38PM +0530, Sekhar Nori wrote:
> A well timed kernel preemption in the time_after() loop
> in wait_for_idle() can result in a spurious timeout
> error to be returned.
>
> Fix it by checking for status of hardware before returning
> timeout error.
>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
I've seen this with other drivers as well.
I suggest you make use of readx_poll_timeout(), or one of its
cousins. They get this right.
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] drivers: net: davinci_mdio: prevent sprious timeout
2018-05-08 12:48 ` Andrew Lunn
@ 2018-05-09 10:44 ` Sekhar Nori
0 siblings, 0 replies; 5+ messages in thread
From: Sekhar Nori @ 2018-05-09 10:44 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Grygorii Strashko, David S . Miller, linux-omap, netdev
On Tuesday 08 May 2018 06:18 PM, Andrew Lunn wrote:
> On Tue, May 08, 2018 at 01:56:38PM +0530, Sekhar Nori wrote:
>> A well timed kernel preemption in the time_after() loop
>> in wait_for_idle() can result in a spurious timeout
>> error to be returned.
>>
>> Fix it by checking for status of hardware before returning
>> timeout error.
>>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>
> I've seen this with other drivers as well.
>
> I suggest you make use of readx_poll_timeout(), or one of its
> cousins. They get this right.
Thanks for pointing me to these. I somehow thought these helpers are
only available with regmap.
Sending a version using readl_poll_timeout(). I know __raw_readl() is
used elsewhere in the driver, but I think that should be cleaned up
sometime to use readl_relaxed() at least. So not sure if there is
benefit in persisting with existing accessor.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] drivers: net: davinci_mdio: prevent sprious timeout
@ 2018-05-09 10:44 ` Sekhar Nori
0 siblings, 0 replies; 5+ messages in thread
From: Sekhar Nori @ 2018-05-09 10:44 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Grygorii Strashko, David S . Miller, linux-omap, netdev
On Tuesday 08 May 2018 06:18 PM, Andrew Lunn wrote:
> On Tue, May 08, 2018 at 01:56:38PM +0530, Sekhar Nori wrote:
>> A well timed kernel preemption in the time_after() loop
>> in wait_for_idle() can result in a spurious timeout
>> error to be returned.
>>
>> Fix it by checking for status of hardware before returning
>> timeout error.
>>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>
> I've seen this with other drivers as well.
>
> I suggest you make use of readx_poll_timeout(), or one of its
> cousins. They get this right.
Thanks for pointing me to these. I somehow thought these helpers are
only available with regmap.
Sending a version using readl_poll_timeout(). I know __raw_readl() is
used elsewhere in the driver, but I think that should be cleaned up
sometime to use readl_relaxed() at least. So not sure if there is
benefit in persisting with existing accessor.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-09 10:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 8:26 [PATCH net-next] drivers: net: davinci_mdio: prevent sprious timeout Sekhar Nori
2018-05-08 8:26 ` Sekhar Nori
2018-05-08 12:48 ` Andrew Lunn
2018-05-09 10:44 ` Sekhar Nori
2018-05-09 10:44 ` Sekhar Nori
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.