Hello.

On 05/13/2016 10:18 PM, Sergei Shtylyov wrote:

quoted

quoted

[we already talked about this patch in #armlinux, I'm now just
forwarding my comments on the list. Background was that I sent an easier
and less complete patch with the same idea. See
http://patchwork.ozlabs.org/patch/621418/]

[added Linus Walleij to Cc, there is a question for you/him below]

On Fri, Apr 29, 2016 at 01:12:54AM +0300, Sergei Shtylyov wrote:

quoted

--- net-next.orig/Documentation/devicetree/bindings/net/phy.txt
+++ net-next/Documentation/devicetree/bindings/net/phy.txt

@@ -35,6 +35,8 @@ Optional Properties:
 - broken-turn-around: If set, indicates the PHY device does not correctly
   release the turn around line low at the end of a MDIO transaction.

+- reset-gpios: The GPIO phandle and specifier for the PHY reset signal.
+
 Example:

 ethernet-phy@0 {

This is great.

quoted

Index: net-next/drivers/net/phy/at803x.c
===================================================================

--- net-next.orig/drivers/net/phy/at803x.c
+++ net-next/drivers/net/phy/at803x.c

@@ -65,7 +65,6 @@ MODULE_LICENSE("GPL");

[...]

My patch breaks this driver. I wasn't aware of it.

   I tried to be as careful as I could but still it looks that I didn't
succeed at that too...

   Hm, I'm starting to forget the vital details about my patch...

quoted

[...]

quoted

quoted

Index: net-next/drivers/net/phy/mdio_device.c
===================================================================

--- net-next.orig/drivers/net/phy/mdio_device.c
+++ net-next/drivers/net/phy/mdio_device.c

[...]

quoted

quoted

@@ -117,9 +126,16 @@ static int mdio_probe(struct device *dev
     struct mdio_driver *mdiodrv = to_mdio_driver(drv);
     int err = 0;

-    if (mdiodrv->probe)
+    if (mdiodrv->probe) {
+        /* Deassert the reset signal */
+        mdio_device_reset(mdiodev, 0);
+
         err = mdiodrv->probe(mdiodev);

+        /* Assert the reset signal */
+        mdio_device_reset(mdiodev, 1);

I wonder if it's safe to do this in general. What if ->probe does
something with the phy that is lost by resetting but that is relied on
later?

   Well, I thought that config_init() method is designed for that but indeed
the LXT driver writes to BMCR in its probe() method and hence is broken.
Thank you for noticing...

   It's broken even without my patch. The phylib will cause a PHY soft reset

    Only iff the config_init() method exists in the PHY driver...

when attaching to the PHY device, so all BMCR programming dpone in the probe()
method will be lost. My patch does make sense as is.

    No, actually it doesn't. :-(

Looks like I should alsolook into fixing lxt.c.

    It took me to actually do a patch to understand my fault. Sigh... :-/

Florian, what do you think?

    Florian, is phy_init_hw() logic correct?

MBR, Sergei