[net-next,v4,1/4] phylib: Add device reset delay support

Message ID 20171207144358.3351-2-dev@g0hl1n.net
State Superseded
Headers show
Series
  • net: fec: fix refclk enable for SMSC LAN8710/20
Related show

Commit Message

Richard Leitner Dec. 7, 2017, 2:43 p.m.
From: Richard Leitner <richard.leitner@skidata.com>

Some PHYs need a minimum time after the reset gpio was asserted and/or
deasserted. To ensure we meet these timing requirements add two new
optional devicetree parameters for the phy: reset-delay-us and
reset-post-delay-us.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/net/phy.txt | 10 ++++++++++
 drivers/net/phy/mdio_device.c                 | 13 +++++++++++--
 drivers/of/of_mdio.c                          |  4 ++++
 include/linux/mdio.h                          |  2 ++
 4 files changed, 27 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven Dec. 7, 2017, 2:52 p.m. | #1
Hi Richard,

On Thu, Dec 7, 2017 at 3:43 PM, Richard Leitner <dev@g0hl1n.net> wrote:
> --- a/drivers/net/phy/mdio_device.c
> +++ b/drivers/net/phy/mdio_device.c
> @@ -24,6 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/string.h>
>  #include <linux/unistd.h>
> +#include <linux/delay.h>
>
>  void mdio_device_free(struct mdio_device *mdiodev)
>  {
> @@ -118,8 +119,16 @@ EXPORT_SYMBOL(mdio_device_remove);
>
>  void mdio_device_reset(struct mdio_device *mdiodev, int value)
>  {
> -       if (mdiodev->reset)
> -               gpiod_set_value(mdiodev->reset, value);
> +       unsigned int d;
> +
> +       if (!mdiodev->reset)
> +               return;
> +
> +       gpiod_set_value(mdiodev->reset, value);
> +
> +       d = value ? mdiodev->reset_delay : mdiodev->reset_post_delay;
> +       if (d)
> +               usleep_range(d, d + min_t(unsigned int, d / 10, 100));

Oops, I meant "max_t", not "min_t", else the upper limit can be "d + 0",
which is not what we want.

Sorry, my fault.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Leitner Dec. 7, 2017, 3:01 p.m. | #2
Hi Geert,

On 12/07/2017 03:52 PM, Geert Uytterhoeven wrote:
> Hi Richard,
> 
> On Thu, Dec 7, 2017 at 3:43 PM, Richard Leitner <dev@g0hl1n.net> wrote:
>> --- a/drivers/net/phy/mdio_device.c
>> +++ b/drivers/net/phy/mdio_device.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/string.h>
>>  #include <linux/unistd.h>
>> +#include <linux/delay.h>
>>
>>  void mdio_device_free(struct mdio_device *mdiodev)
>>  {
>> @@ -118,8 +119,16 @@ EXPORT_SYMBOL(mdio_device_remove);
>>
>>  void mdio_device_reset(struct mdio_device *mdiodev, int value)
>>  {
>> -       if (mdiodev->reset)
>> -               gpiod_set_value(mdiodev->reset, value);
>> +       unsigned int d;
>> +
>> +       if (!mdiodev->reset)
>> +               return;
>> +
>> +       gpiod_set_value(mdiodev->reset, value);
>> +
>> +       d = value ? mdiodev->reset_delay : mdiodev->reset_post_delay;
>> +       if (d)
>> +               usleep_range(d, d + min_t(unsigned int, d / 10, 100));
> 
> Oops, I meant "max_t", not "min_t", else the upper limit can be "d + 0",
> which is not what we want.

You're right...

> Sorry, my fault.

I just copied it over from you suggestion without thinking about it...
So it's definitely my fault too ;-)

I'll wait for some more comments and send a new version next week.

regards;Richard.L
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index c05479f5ac7c..72860ce7f610 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -55,6 +55,12 @@  Optional Properties:
 
 - reset-gpios: The GPIO phandle and specifier for the PHY reset signal.
 
+- reset-delay-us: Delay after the reset was asserted in microseconds.
+  If this property is missing the delay will be skipped.
+
+- reset-post-delay-us: Delay after the reset was deasserted in microseconds.
+  If this property is missing the delay will be skipped.
+
 Example:
 
 ethernet-phy@0 {
@@ -62,4 +68,8 @@  ethernet-phy@0 {
 	interrupt-parent = <&PIC>;
 	interrupts = <35 IRQ_TYPE_EDGE_RISING>;
 	reg = <0>;
+
+	reset-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
+	reset-delay-us = <1000>;
+	reset-post-delay-us = <2000>;
 };
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index 75d97dd9fb28..0423280c88fe 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -24,6 +24,7 @@ 
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/unistd.h>
+#include <linux/delay.h>
 
 void mdio_device_free(struct mdio_device *mdiodev)
 {
@@ -118,8 +119,16 @@  EXPORT_SYMBOL(mdio_device_remove);
 
 void mdio_device_reset(struct mdio_device *mdiodev, int value)
 {
-	if (mdiodev->reset)
-		gpiod_set_value(mdiodev->reset, value);
+	unsigned int d;
+
+	if (!mdiodev->reset)
+		return;
+
+	gpiod_set_value(mdiodev->reset, value);
+
+	d = value ? mdiodev->reset_delay : mdiodev->reset_post_delay;
+	if (d)
+		usleep_range(d, d + min_t(unsigned int, d / 10, 100));
 }
 EXPORT_SYMBOL(mdio_device_reset);
 
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 98258583abb0..7c8767176315 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -77,6 +77,10 @@  static int of_mdiobus_register_phy(struct mii_bus *mdio,
 	if (of_property_read_bool(child, "broken-turn-around"))
 		mdio->phy_ignore_ta_mask |= 1 << addr;
 
+	of_property_read_u32(child, "reset-delay-us", &phy->mdio.reset_delay);
+	of_property_read_u32(child, "reset-post-delay-us",
+			     &phy->mdio.reset_post_delay);
+
 	/* Associate the OF node with the device structure so it
 	 * can be looked up later */
 	of_node_get(child);
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 92d4e55ffe67..e37c21d8eb19 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -41,6 +41,8 @@  struct mdio_device {
 	int addr;
 	int flags;
 	struct gpio_desc *reset;
+	unsigned int reset_delay;
+	unsigned int reset_post_delay;
 };
 #define to_mdio_device(d) container_of(d, struct mdio_device, dev)