Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

From: Caesar Wang
Date: Sat Mar 12 2016 - 22:58:18 EST


Hi Sergei,

å 2016å03æ12æ 02:35, Sergei Shtylyov åé:
On 03/11/2016 01:55 PM, Caesar Wang wrote:

This patch adds to support the emac phy reset.

1) phy-reset-gpios:
The phy-reset-gpios is an optional property for arc emac device tree boot.
Change the binding document to match the driver code.

The binding document is apparently changed in another patch. Not sure what you wanted to say here...

2) phy-reset-duration:
Different boards may require different phy reset duration. Add property
phy-reset-duration for device tree probe, so that the boards that need
a longer reset duration can specify it in their device tree.

3) phy-reset-active-high:
We need that for a custom hardware that needs the reverse reset sequence.

No, we don't really need that, "phy-reset-gpio" prop can contain this data.

Of course, this patch will fix the issue on
https://patchwork.kernel.org/patch/8186801/.

In some cases, the emac couldn't work if you don't have reset the phy.
Let's add it to happy work.

Signed-off-by: Caesar Wang <wxt@xxxxxxxxxxxxxx>
---

drivers/net/ethernet/arc/emac_main.c | 41 ++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index 6446af1..42384f6a 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -764,6 +764,45 @@ static const struct net_device_ops arc_emac_netdev_ops = {
#endif
};

+#ifdef CONFIG_OF
+static void emac_reset_phy(struct net_device *pdev)
+{
+ int err, phy_reset;
+ bool active_high = false;
+ int msec = 10;
+ struct device *dev = pdev->dev.parent;
+ struct device_node *np = dev->of_node;
+
+ if (!np)
+ return;
+
+ of_property_read_u32(np, "phy-reset-duration", &msec);
+ /* A sane reset duration should not be longer than 1s */
+ if (msec > 1000)
+ msec = 1;
+
+ phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);

Forgot to say that these old integer-base GPIO APIs shouldn't be used by the new code, there's new 'struct gpio_desc' based APIs, like devm_gpiod_get() etc.

+ if (!gpio_is_valid(phy_reset))
+ return;
+
+ active_high = of_property_read_bool(np, "phy-reset-active-high");

Well, I still don't understand why this prop is needed, while the GPIO sense is transparently handled by the gpiolib (at least when using DT).

Okay, I have to admit that reference the exist way to do...


wxt@nb:~/kernel/drivers/net/ethernet$ ag reset-gpios
micrel/ks8851.c
1427: gpio = of_get_named_gpio_flags(spi->dev.of_node, "reset-gpios",

arc/emac_main.c
787: phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
797: dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);

arc/emac_main.c~
784: phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
794: dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);

davicom/dm9000.c
1451: reset_gpios = of_get_named_gpio_flags(dev->of_node, "reset-gpios", 0,

freescale/fec_main.c
3206: phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
3216: dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);

cadence/macb.c
2958: int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0);

...

Yep, the gpiolib that can include the prop to work with the same ways.




[...]

MBR, Sergei


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip


--
Thanks,
Caesar