netdev - Re: [PATCH 2/4] phy: s32g: Add serdes subsystem phy

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] [day] [month] [year] [list]
Message-ID: <aXzFH09AeIRawCwU@shell.armlinux.org.uk>
Date: Fri, 30 Jan 2026 14:50:07 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vinod Koul <vkoul@...nel.org>
Cc: Vincent Guittot <vincent.guittot@...aro.org>, neil.armstrong@...aro.org,
	Simon Horman <horms@...nel.org>, krzk+dt@...nel.org,
	conor+dt@...nel.org, ciprianmarian.costea@....nxp.com, s32@....com,
	p.zabel@...gutronix.de, ghennadi.procopciuc@....com,
	Ionut.Vicovan@....com, linux-phy@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
	Frank.li@....com
Subject: Re: [PATCH 2/4] phy: s32g: Add serdes subsystem phy

On Thu, Jan 29, 2026 at 02:36:46PM +0000, Russell King (Oracle) wrote:
> On Thu, Jan 29, 2026 at 08:00:38PM +0530, Vinod Koul wrote:
> > On 29-01-26, 13:51, Russell King (Oracle) wrote:
> > > On Thu, Jan 29, 2026 at 02:36:01PM +0100, Vincent Guittot wrote:
> > > > On Thu, 29 Jan 2026 at 14:23, Russell King (Oracle)
> > > > <linux@...linux.org.uk> wrote:
> > > > >
> > > > > On Thu, Jan 29, 2026 at 02:01:13PM +0100, Vincent Guittot wrote:
> > > > > > yes, the usual pattern is :
> > > > > > - phy_set_mode_ext()
> > > > > > - then phy_power_on()
> > > > > > but I can add an additional check
> > > > >
> > > > > Please read Documentation/driver-api/phy/phy.rst section "Order of API
> > > > > calls" which suggests phy_set_mode_ext() after phy_power_on().
> > > > 
> > > > Fair enough.
> > > > That being said, all pcie drivers  that use phy_set_mode_ext(), call
> > > > it before phy_power_on()
> > > 
> > > It looks like many ethernet drivers do the same, so I think maybe the
> > > generic PHY documentation is incorrect or misleading, or is expressing
> > > a preference that almost no one follows. Something for the generic PHY
> > > maintainers to look at and/or comment on.
> > 
> > I would feel it makes sense to configure the mode first and then power
> > the phy up. As commented above yes it looks like apart from one tegra
> > driver rest seem to do it this way.
> > 
> > Lets update the documentation
> 
> Please also indicate in the documentation whether changing the submode
> of the serdes (particularly for ethernet) is permitted without doing a
> phy_power_down()..phy_power_up() dance around the phy_set_mode_ext()
> call.

Maybe something like this, which simply alters the documentation to
indicate that phy_set_mode*() is permissible prior to phy_power_on(),
and should be used at that point where drivers know the mode which
will be used.

Leaving the existing phy_set_mode*() in the sequence also indicates
that it's permissible to call this while the PHY is still powered
on.

For drivers such as stmmac, it will be important that details such as
whether phy_est_mode*() can be called with the PHY powered on are
riveted down and not left up to the generic PHY driver author - without
that, generic PHYs basically aren't usable from SoC/platform
independent code, and stmmac has bazillions of platform specific glue
already because of (a) bad code structuring and (b) lack of
generalisation through standardised interfaces that abstract platform
differences.

I want to be able for core stmmac code, or even phylink code (which
is even more platform generic) to be able to make use of generic PHY
stuff, but if the calls that can be made into generic PHY are platform
dependent, that is a blocking issue against that, and makes me question
why we have the generic PHY subsystem... it's not very generic if it
exposes the differences of each implementation to users of its
interfaces.

I think generic PHY has had the idea that its interfaces will only be
used from platform specific code that knows about the behaviour of it's
generic PHY driver, but as can be seen above, this will not remain the
case given that we have hardware designs where the core of the driver
is one vendor's IP that gets re-used across many different platforms,
but the SerDes PHY is one of many other vendor's IP.

diff --git a/Documentation/driver-api/phy/phy.rst b/Documentation/driver-api/phy/phy.rst
index 719a2b3fd2ab..cf73e4fb0951 100644
--- a/Documentation/driver-api/phy/phy.rst
+++ b/Documentation/driver-api/phy/phy.rst
@@ -142,6 +142,7 @@ Order of API calls
 
     [devm_][of_]phy_get()
     phy_init()
+    [phy_set_mode[_ext]()]
     phy_power_on()
     [phy_set_mode[_ext]()]
     ...
@@ -154,7 +155,7 @@ but controllers should always call these functions to be compatible with other
 PHYs. Some PHYs may require :c:func:`phy_set_mode <phy_set_mode_ext>`, while
 others may use a default mode (typically configured via devicetree or other
 firmware). For compatibility, you should always call this function if you know
-what mode you will be using. Generally, this function should be called after
+what mode you will be using. Generally, this function should be called before
 :c:func:`phy_power_on`, although some PHY drivers may allow it at any time.
 
 Releasing a reference to the PHY

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