Re: [PATCH net-next v3 1/4] phylib: Add device reset delay support
flat view
From: Geert Uytterhoeven <hidden>
Date: 2017-12-05 13:54:11
Also in:
lkml, netdev
Hi Richard, On Tue, Dec 5, 2017 at 2:25 PM, Richard Leitner [off-list ref] wrote:
From: Richard Leitner <redacted> 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.
Thanks for your patch!
This patch depends on the "phylib: Add device reset GPIO support" patch
submitted by Geert Uytterhoeven/Sergei Shtylyov, see:
https://patchwork.kernel.org/patch/10090149/The above paragraph belongs under the "---" line below, as it is not intended to be preserved in the eternal git history.
Signed-off-by: Richard Leitner <redacted>
Reviewed-by: Geert Uytterhoeven <redacted> Although I have a few suggestions below:
quoted hunk ↗ jump to hunk
--- a/drivers/net/phy/mdio_device.c +++ b/drivers/net/phy/mdio_device.c@@ -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); + if (!mdiodev->reset) + return; + + gpiod_set_value(mdiodev->reset, value); + + if (value && mdiodev->reset_delay) + usleep_range(mdiodev->reset_delay, mdiodev->reset_delay + 100); + else if (!value && mdiodev->reset_post_delay) + usleep_range(mdiodev->reset_post_delay, + mdiodev->reset_post_delay + 100);
I think this can be written simpler using e.g.:
unsigned int delay;
...
delay = value ? mdiodev->reset_delay : mdiodev->reset_post_delay;
if (delay)
usleep_range(delay, delay + 100);
Perhaps the range extension should be relative, e.g.
"delay + min(delay / 10, 100)"?
quoted hunk ↗ jump to hunk
--- a/drivers/of/of_mdio.c +++ b/drivers/of/of_mdio.c@@ -77,6 +77,14 @@ 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; + if (of_property_read_u32(child, "reset-delay-us", + &phy->mdio.reset_delay)) + phy->mdio.reset_delay = 0; + + if (of_property_read_u32(child, "reset-post-delay-us", + &phy->mdio.reset_post_delay)) + phy->mdio.reset_post_delay = 0;
If of_property_read_u32() fails, it doesn't write to its output parameter.
As the structure should be zeroed during allocation, you can just write:
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);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html