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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YYJQFt0mDpcOcxGd@shell.armlinux.org.uk>
Date:   Wed, 3 Nov 2021 09:02:14 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Radhey Shyam Pandey <radheys@...inx.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Michal Simek <michals@...inx.com>,
        Harini Katakam <harinik@...inx.com>
Subject: Re: mdio: separate gpio-reset property per child phy usecase

On Wed, Nov 03, 2021 at 08:50:30AM +0000, Radhey Shyam Pandey wrote:
> > -----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.

This is a well-known problem with placing resets in the PHY node. In
this case, you must add a compatible property as well that matches
"ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}" so that phylib knows the
contents of the ID registers.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!