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] [day] [month] [year] [list]
Date:   Wed, 3 Nov 2021 10:47:41 +0000
From:   Radhey Shyam Pandey <radheys@...inx.com>
To:     Russell King <linux@...linux.org.uk>
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

> -----Original Message-----
> From: Russell King <linux@...linux.org.uk>
> Sent: Wednesday, November 3, 2021 2:32 PM
> To: Radhey Shyam Pandey <radheys@...inx.com>
> Cc: Florian Fainelli <f.fainelli@...il.com>; 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.

Thanks! Using ethernet-phy-id compatible property seems to work.
I think we are good for now, will get back if there is any followup
discussion required.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!