Re: [PATCH RFT 1/2] phylib: add device reset GPIO support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hello.

On 05/13/2016 12:07 PM, Roger Quadros wrote:

[...]

+}
+EXPORT_SYMBOL(mdio_device_reset);
+
 /**
  * mdio_probe - probe an MDIO device
  * @dev: device to probe
@@ -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?

mdio_probe is called for non PHY devices only, right?

   Yes, those also can have a reset signal.

I'm a bit lost as to why we're de-asserting reset at multiple places. i.e.
mdio_probe(), phy_device_register(), phy_init_hw(), phy_probe(), of_mdiobus_register_phy().

Isn't it simpler to just de-assert it once at the topmost level?

I wasn't after simplicity here. The intent was to save some power putting the device at reset when it's not needed. Florian Fainelly (the phylib maintainer) actually wanted me to go even further with that and assert reset in phy_suspend() but it was too much for me.

i.e. of_mdiobus_register_device() f and of_mdiobus_register_phy()?

Also, how about issuing a reset pulse instead of just de-asserting it.

If it's already held at reset, my assumption is that it's enough time passed already.

This would tackle the case where PHY is in invalid state with reset already
de-asserted.

    Good question. I haven't yet met such cases though.

Another issue is that on some boards we have one reset line tied to
multiple PHYs.How do we prevent multiple resets being taking place when each of
the PHYs are registered?

My patch just doesn't address this case -- it's about the individual resets only.

Do we just specify the reset line only for one PHY in
the DT or can we have the reset gpio in the mdio_bus node for such case?

I think there's mii_bus::reset() method for that case. Some Ethernet drivers even use it
(mostly instead of the code being suggested here).

cheers,
-roger

MBR, Sergei

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux