linux-kernel - Re: [PATCH v4 net-next] mdio_bus: Issue GPIO RESET to PHYs.

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]

Message-ID: <551ee7a0-d153-3a36-e025-2c1f70f866b7@ti.com>
Date:   Mon, 24 Apr 2017 12:04:14 +0300
From:   Roger Quadros <rogerq@...com>
To:     Andrew Lunn <andrew@...n.ch>, Lars-Peter Clausen <lars@...afoo.de>
CC:     <davem@...emloft.net>, Florian Fainelli <f.fainelli@...il.com>,
        <tony@...mide.com>, <nsekhar@...com>, <jsarha@...com>,
        <netdev@...r.kernel.org>, <linux-omap@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 net-next] mdio_bus: Issue GPIO RESET to PHYs.

On 24/04/17 02:35, Andrew Lunn wrote:
> On Fri, Apr 21, 2017 at 03:31:09PM +0200, Lars-Peter Clausen wrote:
>> On 04/21/2017 03:15 PM, Roger Quadros wrote:
>>> diff --git a/Documentation/devicetree/bindings/net/mdio.txt b/Documentation/devicetree/bindings/net/mdio.txt
>>> new file mode 100644
>>> index 0000000..4ffbbac
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/mdio.txt
>>> @@ -0,0 +1,33 @@
>>> +Common MDIO bus properties.
>>> +
>>> +These are generic properties that can apply to any MDIO bus.
>>> +
>>> +Optional properties:
>>> +- reset-gpios: List of one or more GPIOs that control the RESET lines
>>> +  of the PHYs on that MDIO bus.
>>> +- reset-delay-us: RESET pulse width in microseconds as per PHY datasheet.
>>> +
>>> +A list of child nodes, one per device on the bus is expected. These
>>> +should follow the generic phy.txt, or a device specific binding document.
>>> +
>>> +Example :
>>> +This example shows these optional properties, plus other properties
>>> +required for the TI Davinci MDIO driver.
>>> +
>>> +	davinci_mdio: ethernet@...c030000 {
>>> +		compatible = "ti,davinci_mdio";
>>> +		reg = <0x5c030000 0x1000>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
>>> +		reset-delay-us = <2>;   /* PHY datasheet states 1us min */
>>
>> If this is the reset line of the PHY shouldn't it be a property of the PHY
>> node rather than of the MDIO controller node (which might have a reset on
>> its own)?
>>> +
>>> +		ethphy0: ethernet-phy@1 {
>>> +			reg = <1>;
>>> +		};
>>> +
>>> +		ethphy1: ethernet-phy@3 {
>>> +			reg = <3>;
>>> +		};
> 
> Hi Lars-Peter
> 
> We discussed this when the first proposal was made. There are two
> cases, to consider.
> 
> 1) Here, one GPIO line resets all PHYs on the same MDIO bus. In this
> example, two PHYs.
> 
> 2) There is one GPIO line per PHY. That is a separate case, and as you
> say, the reset line should probably be considered a PHY property, not
> an MDIO property. However, it can be messy, since in order to probe
> the MDIO bus, you probably need to take the PHY out of reset.
> 
> Anyway, this patch addresses the first case, so should be accepted. If
> anybody wants to address the second case, they are free to do so.

Thanks for the explanation Andrew.

For the second case, even if the RESET GPIO property is specified
in the PHY node, the RESET *will* have to be done by the MDIO bus driver
else the PHY might not be probed at all.

Whether we need additional code to just to make the DT look prettier is
questionable and if required can come as a separate patch.

cheers,
-roger