netdev - RE: mdio: separate gpio-reset property per child phy usecase

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 3 Nov 2021 08:50:30 +0000
From:   Radhey Shyam Pandey <radheys@...inx.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>
CC:     Michal Simek <michals@...inx.com>,
        Harini Katakam <harinik@...inx.com>
Subject: RE: mdio: separate gpio-reset property per child phy usecase

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@...il.com>
> Sent: Wednesday, October 27, 2021 10:48 PM
> To: Radhey Shyam Pandey <radheys@...inx.com>; netdev@...r.kernel.org;
> Andrew Lunn <andrew@...n.ch>; Heiner Kallweit <hkallweit1@...il.com>;
> Russell King <linux@...linux.org.uk>
> Cc: Michal Simek <michals@...inx.com>; Harini Katakam <harinik@...inx.com>
> Subject: Re: mdio: separate gpio-reset property per child phy usecase
> 
> +PHY library maintainers,
> 
> On 10/27/21 5:58 AM, Radhey Shyam Pandey wrote:
> > Hi all,
> >
> > In a xilinx internal board we have shared GEM MDIO configuration with
> > TI DP83867 phy and for proper phy detection both PHYs need prior
> > separate GPIO-reset.
> >
> > Description:
> > There are two GEM ethernet IPs instances GEM0 and GEM1. GEM0 and GEM1
> > used shared MDIO driven by GEM1.
> >
> > TI PHYs need prior reset (RESET_B) for PHY detection at defined address.
> > However with current framework limitation " one reset line per PHY
> > present on the MDIO bus" the other PHY get detected at incorrect
> > address and later having child PHY node reset property will also not help.
> >
> > In order to fix this one possible solution is to allow reset-gpios
> > property to have PHY reset GPIO tuple for each phy. If this approach
> > looks fine we can make changes and send out a RFC.
> 
> I don't think your proposed solution would work because there is no way to
> disambiguate which 'reset-gpios' property applies to which PHY, unless you use
> a 'reset-gpio-names' property which encodes the phy address in there. But
> even doing so, if the 'reset-gpios' property is placed within the MDIO controller
> node then it applies within its scope which is the MDIO controller. The other
> reason why it is wrong is because the MDIO bus itself may need multiple resets
> to be toggled to be put in a functional state. This is probably uncommon for
> MDIO, but it is not for other types of peripherals with complex asynchronous
> reset circuits (the things you love to hate).
> 
> The MDIO bus layer supports something like this which is much more accurate
> in describing the reset GPIOs pertaining to each PHY device:
> 
> 	mdio {
> 		..
> 		phy0: ethernet-phy@0 {
> 			reg = <0>;
> 			reset-gpios = <&slg7xl45106 5 GPIO_ACTIVE_HIGH>;
> 		};
> 		phy1: ethernet-phy@8 {
> 			reg = <8>;
> 			reset-gpios = <&slg7xl45106 6 GPIO_ACTIVE_HIGH>;
> 		};
> 	};
> 
> The code that will parse that property is in drivers/net/phy/mdio_bus.c under
> mdiobus_register_gpiod()/mdiobus_register_reset() and then
> mdio_device_reset() called by phy_device_reset() will pulse the per-PHY device
> reset line/GPIO.
> 
> Are you saying that you tried that and this did not work somehow? Can you
> describe in more details how the timing of the reset pulsing affects the way
> each TI PHY is going to gets its MDIO address assigned?

Yes, having reset-gpios in PHY node is not working.  Just to highlight - We are
using external strap configuration for PHY Address configuration. The strap
pin configuration is set by sw stack at a later stage. PHY address on 
power on is configured based on sampled values at strap pins which is not
PHY address mentioned in DT. (It could be any PHY Address depending on
strap pins default input). For PHY detect to happen at proper PHY Address
we have call PHY reset (RESET_B) after strap pins are configured otherwise 
probe (of_mdiobus_phy_device_register) fails and we see below error:

mdio_bus ff0c0000.ethernet-ffffffff: MDIO device at address 8 is missing.

Thanks,
Radhey

References:
8.5.4PHYAddressConfiguration [DP83867E/IS/CS datasheet]
 
> 
> Thanks
> --
> Florian