* [PATCH] net: fec: Adjust ENET MDIO timeouts
@ 2011-12-27 14:46 Fabio Estevam
2011-12-27 18:13 ` David Miller
2011-12-27 18:53 ` [PATCH v2] " Fabio Estevam
0 siblings, 2 replies; 4+ messages in thread
From: Fabio Estevam @ 2011-12-27 14:46 UTC (permalink / raw)
To: netdev; +Cc: davem, shawn.guo, kernel, Rogerio Pimentel, Fabio Estevam
On extensive NFS boots on a mx6qsabrelite board it was noted that "FEC: MDIO read timeout" were occuring,
which caused failure on loading the FEC driver.
The original FEC_MII_TIMEOUT was set to 1 ms, which is too low when passed to the usecs_to_jiffies macro.
On ARM one jiffy is 10ms, so use msecs_to_jiffies instead and use a timeout of 30ms, which corresponds to 3 jiffies.
After running extensive NFS boots, the MDIO timeouts do not occur anymore with this change.
Signed-off-by: Rogerio Pimentel <rogerio.pimentel@freescale.com>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index c136230..170a015 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -255,7 +255,7 @@ struct fec_enet_private {
#define FEC_MMFR_TA (2 << 16)
#define FEC_MMFR_DATA(v) (v & 0xffff)
-#define FEC_MII_TIMEOUT 1000 /* us */
+#define FEC_MII_TIMEOUT 30 /* ms */
/* Transmitter timeout */
#define TX_TIMEOUT (2 * HZ)
@@ -902,7 +902,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
/* wait for end of transfer */
time_left = wait_for_completion_timeout(&fep->mdio_done,
- usecs_to_jiffies(FEC_MII_TIMEOUT));
+ msecs_to_jiffies(FEC_MII_TIMEOUT));
if (time_left == 0) {
fep->mii_timeout = 1;
printk(KERN_ERR "FEC: MDIO read timeout\n");
@@ -930,7 +930,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
/* wait for end of transfer */
time_left = wait_for_completion_timeout(&fep->mdio_done,
- usecs_to_jiffies(FEC_MII_TIMEOUT));
+ msecs_to_jiffies(FEC_MII_TIMEOUT));
if (time_left == 0) {
fep->mii_timeout = 1;
printk(KERN_ERR "FEC: MDIO write timeout\n");
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: fec: Adjust ENET MDIO timeouts
2011-12-27 14:46 [PATCH] net: fec: Adjust ENET MDIO timeouts Fabio Estevam
@ 2011-12-27 18:13 ` David Miller
2011-12-27 18:53 ` [PATCH v2] " Fabio Estevam
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2011-12-27 18:13 UTC (permalink / raw)
To: festevam; +Cc: netdev, shawn.guo, kernel, rogerio.pimentel, fabio.estevam
From: Fabio Estevam <festevam@gmail.com>
Date: Tue, 27 Dec 2011 12:46:38 -0200
> -#define FEC_MII_TIMEOUT 1000 /* us */
> +#define FEC_MII_TIMEOUT 30 /* ms */
I don't see why you must use msecs_to_jiffies(), why not just change
FEC_MII_TIMEOUT to 30000?
That's a one-line, easy to validate, change.
Whereas if you also change the units of the timeout I have to go
through the entire driver to audit your patch, making sure none of the
FEC_MII_TIMEOUT uses in the driver have been missed.
When you make multiple changes at once, it leads to more work for the
patch reviewer, please avoid this whenever possible.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] net: fec: Adjust ENET MDIO timeouts
2011-12-27 14:46 [PATCH] net: fec: Adjust ENET MDIO timeouts Fabio Estevam
2011-12-27 18:13 ` David Miller
@ 2011-12-27 18:53 ` Fabio Estevam
2011-12-27 19:07 ` David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Fabio Estevam @ 2011-12-27 18:53 UTC (permalink / raw)
To: netdev; +Cc: davem, shawn.guo, kernel, Rogerio Pimentel, Fabio Estevam
On extensive NFS boots on a mx6qsabrelite board it was noted that "FEC: MDIO read timeout" were occuring,
which caused failure on loading the FEC driver.
The original FEC_MII_TIMEOUT was set to 1 ms, which is too low when passed to the usecs_to_jiffies macro.
On ARM one jiffy is 10ms, so use a timeout of 30ms, which corresponds to 3 jiffies.
After running extensive NFS boots, the MDIO timeouts do not occur anymore with this change.
Signed-off-by: Rogerio Pimentel <rogerio.pimentel@freescale.com>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Keep the timeout unit in us.
drivers/net/ethernet/freescale/fec.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index c136230..c54cc62 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -255,7 +255,7 @@ struct fec_enet_private {
#define FEC_MMFR_TA (2 << 16)
#define FEC_MMFR_DATA(v) (v & 0xffff)
-#define FEC_MII_TIMEOUT 1000 /* us */
+#define FEC_MII_TIMEOUT 30000 /* us */
/* Transmitter timeout */
#define TX_TIMEOUT (2 * HZ)
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] net: fec: Adjust ENET MDIO timeouts
2011-12-27 18:53 ` [PATCH v2] " Fabio Estevam
@ 2011-12-27 19:07 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2011-12-27 19:07 UTC (permalink / raw)
To: festevam; +Cc: netdev, shawn.guo, kernel, rogerio.pimentel, fabio.estevam
From: Fabio Estevam <festevam@gmail.com>
Date: Tue, 27 Dec 2011 16:53:41 -0200
> On extensive NFS boots on a mx6qsabrelite board it was noted that "FEC: MDIO read timeout" were occuring,
> which caused failure on loading the FEC driver.
>
> The original FEC_MII_TIMEOUT was set to 1 ms, which is too low when passed to the usecs_to_jiffies macro.
>
> On ARM one jiffy is 10ms, so use a timeout of 30ms, which corresponds to 3 jiffies.
>
> After running extensive NFS boots, the MDIO timeouts do not occur anymore with this change.
>
> Signed-off-by: Rogerio Pimentel <rogerio.pimentel@freescale.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-27 19:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-27 14:46 [PATCH] net: fec: Adjust ENET MDIO timeouts Fabio Estevam
2011-12-27 18:13 ` David Miller
2011-12-27 18:53 ` [PATCH v2] " Fabio Estevam
2011-12-27 19:07 ` David Miller
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).