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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 May 2016 17:25:09 +0300
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Roger Quadros <rogerq@...com>, Nishanth Menon <nm@...com>,
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
	netdev@...r.kernel.org, devicetree@...r.kernel.org,
	Florian Fainelli <f.fainelli@...il.com>
Cc:	kernel@...gutronix.de, "Andrew F . Davis" <afd@...com>,
	Sekhar Nori <nsekhar@...com>, Suman Anna <s-anna@...com>
Subject: Re: [PATCH] phy: add support for a reset-gpio specification

On 5/12/2016 5:16 PM, Roger Quadros wrote:

>>>> The framework only asserts (for now) that the reset gpio is not active.
>>>>
>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.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?

    No. What Uwe haven't said is that the PHY ID needs to be specified in the 
device tree for his patch to actually work. ;-)

> 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.

    Have you seen my patch, [1]?

[1] http://patchwork.ozlabs.org/patch/616495/

> cheers,
> -roger

MBR, Seregi