net: stmmac: use msleep instead of udelay for gpio reset

PATCH In review Michael Trimarchi 2 participants 5 messages Apr 21, 2015 – Apr 22, 2015

Subsystems (4)

Networking Network drivers Arm STM32 stmmac Ethernet driver

lore

Share & tools

lore

Michael Trimarchi

author

2015-04-21 11:16 UTC #0 lore raw

PATCH v2 1/1 net: stmmac: use msleep instead of udelay for gpio reset

Reset delay values are expressed in microsecond but most of the time the delay is more then 2ms and up to 100ms. Use udelay is wrong for large sleep period. This function is not used in interrupt context according to the documentation.

Cc Fabio Estevam <[email protected]>

Signed-off-by Michael Trimarchi <[email protected]>

---
Changes v1 -> v2:
- remove unreleated changes

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +5 -5

index b735fa2..c843fbe 100644
@@ -161,11 +161,11 @@ int stmmac_mdio_reset(struct mii_bus *bus)
 
 		if (!gpio_request(reset_gpio, "mdio-reset")) {
 			gpio_direction_output(reset_gpio, active_low ? 1 : 0);
-			udelay(data->delays[0]);
-			gpio_set_value(reset_gpio, active_low ? 0 : 1);
-			udelay(data->delays[1]);
-			gpio_set_value(reset_gpio, active_low ? 1 : 0);
-			udelay(data->delays[2]);
+			msleep(max(1U, data->delays[0] / 1000));
+			gpio_set_value(reset_gpio, !active_low);
+			msleep(max(1U, data->delays[1] / 1000));
+			gpio_set_value(reset_gpio, active_low);
+			msleep(max(1U, data->delays[2] / 1000));
 		}
 	}
 #endif

Signature · 1 line

1.9.1

David Miller

maintainer

2015-04-21 21:35 UTC #1 lore raw

From: Michael Trimarchi <[email protected]>
Date: Tue, 21 Apr 2015 13:16:13 +0200

Quoted 1 line

drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +0 -1
-			udelay(data->delays[0]);

...

drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c int stmmac_mdio_reset(struct mii_bus *bus) · line 164

Quoted 1 line

drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +1 -0
+			msleep(max(1U, data->delays[0] / 1000));

That looks very ugly with that max() expression in there.

Please find some clean way to get rid of it if you want to make this conversion.

Thanks.

Michael Trimarchi

author

2015-04-21 23:14 UTC #2 lore raw

Quoted 9 lines · David Miller

From: Michael Trimarchi <[email protected]>
Date: Tue, 21 Apr 2015 13:16:13 +0200
Quoted 1 line
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +0 -1
-			udelay(data->delays[0]);
...
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c int stmmac_mdio_reset(struct mii_bus *bus) · line 164
Quoted 1 line
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +1 -0
+			msleep(max(1U, data->delays[0] / 1000));
That looks very ugly with that max() expression in there.

Is fine for you a DIV_ROUND_UP?

Quoted 3 lines

Please find some clean way to get rid of it if you want to make this conversion.

Agree, I will repost it

Michael

Quoted 1 line

Thanks.

Signature · 4 lines

| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

David Miller

maintainer

2015-04-22 00:31 UTC #3 lore raw

From: Michael Trimarchi <[email protected]>
Date: Wed, 22 Apr 2015 01:13:47 +0200

Quoted 6 lines

Hi
Quoted 3 lines · David Miller
From: Michael Trimarchi <[email protected]>
Date: Tue, 21 Apr 2015 13:16:13 +0200

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +0 -1
-			udelay(data->delays[0]);
...
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c int stmmac_mdio_reset(struct mii_bus *bus) · line 164
Quoted 1 line
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +1 -0
+			msleep(max(1U, data->delays[0] / 1000));
That looks very ugly with that max() expression in there.
Is fine for you a DIV_ROUND_UP?

Not inside of these simple msleep() calls, no.

How about adjusting the values either in the datastructure or in local variables instead? That wasn't so hard to come up with now, was it?

Michael Trimarchi

author

2015-04-22 00:54 UTC #4 lore raw

Quoted 16 lines · David Miller

From: Michael Trimarchi <[email protected]>
Date: Wed, 22 Apr 2015 01:13:47 +0200
Quoted 13 lines
Hi
Quoted 9 lines · David Miller
From: Michael Trimarchi <[email protected]>
Date: Tue, 21 Apr 2015 13:16:13 +0200
Quoted 1 line
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +0 -1
-			udelay(data->delays[0]);
...
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c int stmmac_mdio_reset(struct mii_bus *bus) · line 164
Quoted 1 line
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +1 -0
+			msleep(max(1U, data->delays[0] / 1000));
That looks very ugly with that max() expression in there.

Quoted 7 lines

Quoted 1 line
Is fine for you a DIV_ROUND_UP?
Not inside of these simple msleep() calls, no.
How about adjusting the values either in the datastructure or in local variables instead? That wasn't so hard to come up with now, was it?

Ok, it's easy no problem at all, I will post later today but I prefer local variables and use DIV_ROUND_UP

Michael