Re: [PATCH] phy: add support for a reset-gpio specification

[Roger Quadros <rogerq@ti.com>]

On 12/05/16 17:02, Roger Quadros wrote:
> Hi,
> 
> On 12/05/16 16:50, Nishanth Menon wrote:
>> On 05/12/2016 05:00 AM, Uwe Kleine-König wrote:
>>> The framework only asserts (for now) that the reset gpio is not active.
>>>
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>> ---
>>>  Documentation/devicetree/bindings/net/phy.txt | 3 +++
>>>  drivers/net/phy/phy_device.c                  | 8 ++++++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
>>> index bc1c3c8bf8fa..c00a9a894547 100644
>>> --- a/Documentation/devicetree/bindings/net/phy.txt
>>> +++ b/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: Reference to a GPIO used to reset the phy.
>>> +
>>>  Example:
>>>  
>>>  ethernet-phy@0 {
>>> @@ -42,4 +44,5 @@ ethernet-phy@0 {
>>>  	interrupt-parent = <40000>;
>>>  	interrupts = <35 1>;
>>>  	reg = <0>;
>>> +	reset-gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
>>>  };
>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>> index e551f3a89cfd..7d666ab47271 100644
>>> --- a/drivers/net/phy/phy_device.c
>>> +++ b/drivers/net/phy/phy_device.c
>>> @@ -34,6 +34,7 @@
>>>  #include <linux/io.h>
>>>  #include <linux/uaccess.h>
>>>  #include <linux/of.h>
>>> +#include <linux/gpio/consumer.h>
>>>  
>>>  #include <asm/irq.h>
>>>  
>>> @@ -1569,9 +1570,16 @@ static int phy_probe(struct device *dev)
>>>  	struct device_driver *drv = phydev->mdio.dev.driver;
>>>  	struct phy_driver *phydrv = to_phy_driver(drv);
>>>  	int err = 0;
>>> +	struct gpio_descs *reset_gpios;
>>>  
>>>  	phydev->drv = phydrv;
>>>  
>>> +	/* take phy out of reset */
>>> +	reset_gpios = devm_gpiod_get_array_optional(dev, "reset",
>>> +						    GPIOD_OUT_LOW);
>>> +	if (IS_ERR(reset_gpios))
>>> +		return PTR_ERR(reset_gpios);
>>> +
>>>  	/* Disable the interrupt if the PHY doesn't support it
>>>  	 * but the interrupt is still a valid one
>>>  	 */
>>>
>>
>> This looks like the right approach to me at least: I see that TI EVMs
>> will also benefit with this approach.
>>
> 
> Agreed. Although on some of our boards we actually need a RESET pulse
> to get the PHY in a sane state. I can send a patch on top for that.
> 

I think I replied too early. Will phy_probe() be called at all if the
PHY is not detected on the MDIO bus?
If not then this approach is not sufficient for some of the TI boards.
e.g. am57xx-idk.
I think the reset needs to be done at MDIO bus level before it enumerates
the PHYs so that we know the PHY is in an enumerable state.

cheers,
-roger