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 at rock-chips.com> > --- > > 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). [...] MBR, Sergei