[PATCH net-next] doc: generic phy: update generic PHY documentation

[PATCH net-next] doc: generic phy: update generic PHY documentation

[Russell King (Oracle)]

Update the generic PHY documentation as a result of the discussion for
the s32g submission.

Link: https://lore.kernel.org/r/aXtvDn_-pCuKPrnf@vaman
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
I didn't get any replies to my follow-up question to Vinod:

  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.

I also didn't get any response to:

  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.

 Documentation/driver-api/phy/phy.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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
-- 
2.47.3


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Vladimir Oltean]

On Thu, Feb 05, 2026 at 02:56:15PM +0000, Russell King (Oracle) wrote:
> Update the generic PHY documentation as a result of the discussion for
> the s32g submission.
> 
> Link: https://lore.kernel.org/r/aXtvDn_-pCuKPrnf@vaman
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
> I didn't get any replies to my follow-up question to Vinod:
> 
>   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.

There's certainly nothing *not* permitting that call sequence.

> I also didn't get any response to:
> 
>   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.

It may well be that the "one consumer, multiple PHY providers, requiring
unified behaviour" situation you bring up is sufficiently unique here,
with stmmac/phylink, that local conventions are not sufficient.

I'm not sure that precise answers exist for your questions. Rather,
it's best if you could lay out common sense requirements, and generic
PHY reviewers will make sure they are followed.

As far as the framework is concerned, power and mode are fundamentally
decoupled, so I think the documentation is actually muddying the waters,
more than anything, by talking about a preferred calling order.
If there's any connection between power and mode, it should be handled
internally by the PHY provider driver.

>  Documentation/driver-api/phy/phy.rst | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> 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.

My 2 cents: I would actually remove any reference to any sort of preferred
call order. There's nothing in the framework to back up such a concept.
Just say that it is recommended for PHY provider drivers to not rely on
a particular calling order, such that PHY consumers have the freedom to
choose depending on what suits them best.

>  
>  Releasing a reference to the PHY
> -- 
> 2.47.3
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Russell King (Oracle)]

On Wed, Feb 11, 2026 at 05:48:39PM +0200, Vladimir Oltean wrote:
> My 2 cents: I would actually remove any reference to any sort of preferred
> call order. There's nothing in the framework to back up such a concept.
> Just say that it is recommended for PHY provider drivers to not rely on
> a particular calling order, such that PHY consumers have the freedom to
> choose depending on what suits them best.

Sending out this patch was a last ditch attempt to get a response to
improve the "generic" PHY subsystem, However, as the issue is now
almost two weeks old, and the current patch series causes a regression
according to Mohd's testing, I've rewritten the series to be a finer
set of smaller incremental changes.

This has meant dropping the idea of using the "generic" PHY subsystem
in generic code, because as "generic" PHY drivers are currently
written, that's just impossible given the current state of "generic"
PHY.

There are "generic" PHY drivers that require to be powered up for
any of the phy_set_*() functions to not error out. There are also
"generic" PHY drivers that require the PHY to be powered down
before calling phy_set_*() before the new setting taking effect
at PHY power up time. In this group there are drivers that error
out if phy_set_*() is called while the PHY is powered, and there
are drivers that silently accept the call, returning success, but
do not change the PHY mode.

This makes it pretty much impossible for platform independent code to
know the correct order to call the functions, and what to do if an
error or success is returned from any particular API call.

In other words, it's a trainwreck as currently implemented, and this
was my attempt to try and get some consistency.

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Vladimir Oltean]

On Wed, Feb 11, 2026 at 06:15:11PM +0000, Russell King (Oracle) wrote:
> On Wed, Feb 11, 2026 at 05:48:39PM +0200, Vladimir Oltean wrote:
> > My 2 cents: I would actually remove any reference to any sort of preferred
> > call order. There's nothing in the framework to back up such a concept.
> > Just say that it is recommended for PHY provider drivers to not rely on
> > a particular calling order, such that PHY consumers have the freedom to
> > choose depending on what suits them best.
> 
> Sending out this patch was a last ditch attempt to get a response to
> improve the "generic" PHY subsystem, However, as the issue is now
> almost two weeks old, and the current patch series causes a regression
> according to Mohd's testing, I've rewritten the series to be a finer
> set of smaller incremental changes.
> 
> This has meant dropping the idea of using the "generic" PHY subsystem
> in generic code, because as "generic" PHY drivers are currently
> written, that's just impossible given the current state of "generic"
> PHY.
> 
> There are "generic" PHY drivers that require to be powered up for
> any of the phy_set_*() functions to not error out. There are also
> "generic" PHY drivers that require the PHY to be powered down
> before calling phy_set_*() before the new setting taking effect
> at PHY power up time. In this group there are drivers that error
> out if phy_set_*() is called while the PHY is powered, and there
> are drivers that silently accept the call, returning success, but
> do not change the PHY mode.

Are we talking about PHY_MODE_ETHERNET generic PHYs here, or in general?

> This makes it pretty much impossible for platform independent code to
> know the correct order to call the functions, and what to do if an
> error or success is returned from any particular API call.
> 
> In other words, it's a trainwreck as currently implemented, and this
> was my attempt to try and get some consistency.

Do you have a list of such PHYs relevant to phylink? We can still
document the expectation, and modify them.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Vladimir Oltean]

On Wed, Feb 11, 2026 at 09:30:06PM +0200, Vladimir Oltean wrote:
> On Wed, Feb 11, 2026 at 06:15:11PM +0000, Russell King (Oracle) wrote:
> > On Wed, Feb 11, 2026 at 05:48:39PM +0200, Vladimir Oltean wrote:
> > > My 2 cents: I would actually remove any reference to any sort of preferred
> > > call order. There's nothing in the framework to back up such a concept.
> > > Just say that it is recommended for PHY provider drivers to not rely on
> > > a particular calling order, such that PHY consumers have the freedom to
> > > choose depending on what suits them best.
> > 
> > Sending out this patch was a last ditch attempt to get a response to
> > improve the "generic" PHY subsystem, However, as the issue is now
> > almost two weeks old, and the current patch series causes a regression
> > according to Mohd's testing, I've rewritten the series to be a finer
> > set of smaller incremental changes.
> > 
> > This has meant dropping the idea of using the "generic" PHY subsystem
> > in generic code, because as "generic" PHY drivers are currently
> > written, that's just impossible given the current state of "generic"
> > PHY.
> > 
> > There are "generic" PHY drivers that require to be powered up for
> > any of the phy_set_*() functions to not error out. There are also
> > "generic" PHY drivers that require the PHY to be powered down
> > before calling phy_set_*() before the new setting taking effect
> > at PHY power up time. In this group there are drivers that error
> > out if phy_set_*() is called while the PHY is powered, and there
> > are drivers that silently accept the call, returning success, but
> > do not change the PHY mode.
> 
> Are we talking about PHY_MODE_ETHERNET generic PHYs here, or in general?
> 
> > This makes it pretty much impossible for platform independent code to
> > know the correct order to call the functions, and what to do if an
> > error or success is returned from any particular API call.
> > 
> > In other words, it's a trainwreck as currently implemented, and this
> > was my attempt to try and get some consistency.
> 
> Do you have a list of such PHYs relevant to phylink? We can still
> document the expectation, and modify them.

To (partially) answer my own question, here is what I found so far:

commit 5cb630925b49dd13bf3aa43f299b11615c8fe9cd
Author: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date:   Wed Feb 1 22:14:53 2023 +0900

    net: renesas: rswitch: Add phy_power_{on,off}() calling

    Some Ethernet PHYs (like marvell10g) will decide the host interface
    mode by the media-side speed. So, the rswitch driver needs to
    initialize one of the Ethernet SERDES (r8a779f0-eth-serdes) ports
    after linked the Ethernet PHY up. The r8a779f0-eth-serdes driver has
    .init() for initializing all ports and .power_on() for initializing
    each port. So, add phy_power_{on,off} calling for it.

    Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

This needs to be changed, for sure.

I'll take a closer look tomorrow at the rest and see how bad the
situation is overall.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Russell King (Oracle)]

On Wed, Feb 11, 2026 at 09:30:06PM +0200, Vladimir Oltean wrote:
> On Wed, Feb 11, 2026 at 06:15:11PM +0000, Russell King (Oracle) wrote:
> > On Wed, Feb 11, 2026 at 05:48:39PM +0200, Vladimir Oltean wrote:
> > > My 2 cents: I would actually remove any reference to any sort of preferred
> > > call order. There's nothing in the framework to back up such a concept.
> > > Just say that it is recommended for PHY provider drivers to not rely on
> > > a particular calling order, such that PHY consumers have the freedom to
> > > choose depending on what suits them best.
> > 
> > Sending out this patch was a last ditch attempt to get a response to
> > improve the "generic" PHY subsystem, However, as the issue is now
> > almost two weeks old, and the current patch series causes a regression
> > according to Mohd's testing, I've rewritten the series to be a finer
> > set of smaller incremental changes.
> > 
> > This has meant dropping the idea of using the "generic" PHY subsystem
> > in generic code, because as "generic" PHY drivers are currently
> > written, that's just impossible given the current state of "generic"
> > PHY.
> > 
> > There are "generic" PHY drivers that require to be powered up for
> > any of the phy_set_*() functions to not error out. There are also
> > "generic" PHY drivers that require the PHY to be powered down
> > before calling phy_set_*() before the new setting taking effect
> > at PHY power up time. In this group there are drivers that error
> > out if phy_set_*() is called while the PHY is powered, and there
> > are drivers that silently accept the call, returning success, but
> > do not change the PHY mode.
> 
> Are we talking about PHY_MODE_ETHERNET generic PHYs here, or in general?

PHY_MODE_ETHERNET PHYs.

> > This makes it pretty much impossible for platform independent code to
> > know the correct order to call the functions, and what to do if an
> > error or success is returned from any particular API call.
> > 
> > In other words, it's a trainwreck as currently implemented, and this
> > was my attempt to try and get some consistency.
> 
> Do you have a list of such PHYs relevant to phylink? We can still
> document the expectation, and modify them.

These are the generic PHYs used for ethernet, so they are potential
cases for phylink to drive. They were found via:

grep ETHERNET drivers/phy -rl | xargs grep -l power_on



r8a779f0-ether-serdes: r8a779f0_eth_serdes_set_mode() records the
state, returning success, with no hardware programming. All hardware
programming for the desired mode is done via phy_power_on() call.

This driver requires the PHY to be powered down before phy_set_mode*()
otherwise it will accept the new mode but not reconfigure the hardware.


phy-mvebu-cp110-comphy: mvebu_comphy_set_mode() records the state in
struct members and does nothing more, returning success. This
requires the PHY to be powered down before phy_set_mode().

This driver requires the PHY to be powered down before phy_set_mode*()
otherwise it will accept the new mode but not reconfigure the hardware.


phy-mvebu-a3700-comphy: mvebu_a3700_comphy_set_mode() errors out if
mode is changed with PHY powered up:

        /* Mode cannot be changed while the PHY is powered on */
        if (phy->power_count &&
            (lane->mode != mode || lane->submode != submode))
                return -EBUSY;

This driver requires the PHY to be powered down before phy_set_mode*()
otherwise it will error out.


phy-fsl-lynx-28g: lynx_28g_set_mode() powers down the PHY if necessary,
sets the mode, before powering it back up.

This is the only driver that will tolerate phy_power_*() vs
phy_set_mode*() in any order.


phy-qcom-sgmii-eth: doesn't implement phy_set_mode*() but instead
implements phy_set_speed() and phy_calibrate(). phy_set_speed()
sets the mode and immediately calibrates, which presumably requires
the clocks enabled by qcom_dwmac_sgmii_phy_power_on(). No check
for PHY being powered, so my guess is that calling phy_set_speed()
with the PHY powered down will cause the status polling in
qcom_dwmac_sgmii_phy_calibrate() to time out.

This driver requires phy_power_on() before phy_set_speed().


sparx5_serdes: phy_power_*() controls power down mode, no check
in sparx5_serdes_set_mode() whether powered up, but config function
checks for e.g. PLL lock, which probably won't be locked if powered
down. Failures aren't propagated to the API caller.

This driver requires phy_power_on() before phy_set_mode*().


phy-mtk-xfi-tphy: while phy_power_*() controls clocks, it isn't
obvious whether these are required before mtk_xfi_tphy_setup() is
invoked. There are calls to usleep() implying a need to wait for
things to happen before the next register write, which presumably
require the clock to be running.

My guess is, this driver requires phy_power_on() before phy_set_mode*().


Categorisation of drivers:
drivers requiring powered down PHY before phy_set_mode*(), but are
silent if not: 2

drivers requiring powered down PHY before phy_set_mode*(), but error
out if not: 1

drivers that allow any power state before phy_set_mode*(): 1

drivers that do not implement .power_*() methods, allowing any
power state, but presumably require the phy_set_mode*() call to
setup the hardware: 5

drivers requiring powered up PHY before phy_set_mode*(): 3

So, the summary is... only _one_ driver allows the calls in any order.
The other 11 drivers are... stick a pin in the donkey and depending
on where the pin ends up determines the appropriate order that the
generic PHY APIs must be called for the driver to work correctly.

I think you'll agree that this is a very sorry state of affairs.

This is exactly why I think there needs to be better documentation.
I'm not sure that some kind of test code to validate the API would
work, because you'd need some way to test whether the PHY were in a
functional state, and that's difficult without the help of the
ethernet MAC.

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Vinod Koul]

On 11-02-26, 17:48, Vladimir Oltean wrote:
> On Thu, Feb 05, 2026 at 02:56:15PM +0000, Russell King (Oracle) wrote:
> > Update the generic PHY documentation as a result of the discussion for
> > the s32g submission.
> > 
> > Link: https://lore.kernel.org/r/aXtvDn_-pCuKPrnf@vaman
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> > I didn't get any replies to my follow-up question to Vinod:

Sorry to have missed that...

> >   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.
> 
> There's certainly nothing *not* permitting that call sequence.

I agree on that. The documentation can be update to reflect that. Common
logic would say that we should set things up before powering up, that
should be the guidance here as well.
> 
> > I also didn't get any response to:
> > 
> >   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.
> 
> It may well be that the "one consumer, multiple PHY providers, requiring
> unified behaviour" situation you bring up is sufficiently unique here,
> with stmmac/phylink, that local conventions are not sufficient.
> 
> I'm not sure that precise answers exist for your questions. Rather,
> it's best if you could lay out common sense requirements, and generic
> PHY reviewers will make sure they are followed.
> 
> As far as the framework is concerned, power and mode are fundamentally
> decoupled, so I think the documentation is actually muddying the waters,
> more than anything, by talking about a preferred calling order.
> If there's any connection between power and mode, it should be handled
> internally by the PHY provider driver.

Agree, setting mode and powering up are not coupled anywhere in the API.
framework does not care about it. There might be provider limitations
which should be handled internally.

> 
> >  Documentation/driver-api/phy/phy.rst | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > 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.
> 
> My 2 cents: I would actually remove any reference to any sort of preferred
> call order. There's nothing in the framework to back up such a concept.
> Just say that it is recommended for PHY provider drivers to not rely on
> a particular calling order, such that PHY consumers have the freedom to
> choose depending on what suits them best.

Yes but I would say rather than order, a guidance would help users and
providers. Set things up and power up would be a good logic to follow.

If the provider supports setting/switching mode up when running, we
should not limit that. I dont see a reason for limiting that from an
framework.

-- 
~Vinod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Vinod Koul]

On 11-02-26, 20:31, Russell King (Oracle) wrote:
> On Wed, Feb 11, 2026 at 09:30:06PM +0200, Vladimir Oltean wrote:
> > On Wed, Feb 11, 2026 at 06:15:11PM +0000, Russell King (Oracle) wrote:
> > > On Wed, Feb 11, 2026 at 05:48:39PM +0200, Vladimir Oltean wrote:
> > > > My 2 cents: I would actually remove any reference to any sort of preferred
> > > > call order. There's nothing in the framework to back up such a concept.
> > > > Just say that it is recommended for PHY provider drivers to not rely on
> > > > a particular calling order, such that PHY consumers have the freedom to
> > > > choose depending on what suits them best.
> > > 
> > > Sending out this patch was a last ditch attempt to get a response to
> > > improve the "generic" PHY subsystem, However, as the issue is now
> > > almost two weeks old, and the current patch series causes a regression
> > > according to Mohd's testing, I've rewritten the series to be a finer
> > > set of smaller incremental changes.
> > > 
> > > This has meant dropping the idea of using the "generic" PHY subsystem
> > > in generic code, because as "generic" PHY drivers are currently
> > > written, that's just impossible given the current state of "generic"
> > > PHY.
> > > 
> > > There are "generic" PHY drivers that require to be powered up for
> > > any of the phy_set_*() functions to not error out. There are also
> > > "generic" PHY drivers that require the PHY to be powered down
> > > before calling phy_set_*() before the new setting taking effect
> > > at PHY power up time. In this group there are drivers that error
> > > out if phy_set_*() is called while the PHY is powered, and there
> > > are drivers that silently accept the call, returning success, but
> > > do not change the PHY mode.
> > 
> > Are we talking about PHY_MODE_ETHERNET generic PHYs here, or in general?
> 
> PHY_MODE_ETHERNET PHYs.
> 
> > > This makes it pretty much impossible for platform independent code to
> > > know the correct order to call the functions, and what to do if an
> > > error or success is returned from any particular API call.
> > > 
> > > In other words, it's a trainwreck as currently implemented, and this
> > > was my attempt to try and get some consistency.
> > 
> > Do you have a list of such PHYs relevant to phylink? We can still
> > document the expectation, and modify them.
> 
> These are the generic PHYs used for ethernet, so they are potential
> cases for phylink to drive. They were found via:
> 
> grep ETHERNET drivers/phy -rl | xargs grep -l power_on
> 
> 
> 
> r8a779f0-ether-serdes: r8a779f0_eth_serdes_set_mode() records the
> state, returning success, with no hardware programming. All hardware
> programming for the desired mode is done via phy_power_on() call.
> 
> This driver requires the PHY to be powered down before phy_set_mode*()
> otherwise it will accept the new mode but not reconfigure the hardware.
> 
> 
> phy-mvebu-cp110-comphy: mvebu_comphy_set_mode() records the state in
> struct members and does nothing more, returning success. This
> requires the PHY to be powered down before phy_set_mode().
> 
> This driver requires the PHY to be powered down before phy_set_mode*()
> otherwise it will accept the new mode but not reconfigure the hardware.
> 
> 
> phy-mvebu-a3700-comphy: mvebu_a3700_comphy_set_mode() errors out if
> mode is changed with PHY powered up:
> 
>         /* Mode cannot be changed while the PHY is powered on */
>         if (phy->power_count &&
>             (lane->mode != mode || lane->submode != submode))
>                 return -EBUSY;
> 
> This driver requires the PHY to be powered down before phy_set_mode*()
> otherwise it will error out.
> 
> 
> phy-fsl-lynx-28g: lynx_28g_set_mode() powers down the PHY if necessary,
> sets the mode, before powering it back up.
> 
> This is the only driver that will tolerate phy_power_*() vs
> phy_set_mode*() in any order.
> 
> 
> phy-qcom-sgmii-eth: doesn't implement phy_set_mode*() but instead
> implements phy_set_speed() and phy_calibrate(). phy_set_speed()
> sets the mode and immediately calibrates, which presumably requires
> the clocks enabled by qcom_dwmac_sgmii_phy_power_on(). No check
> for PHY being powered, so my guess is that calling phy_set_speed()
> with the PHY powered down will cause the status polling in
> qcom_dwmac_sgmii_phy_calibrate() to time out.
> 
> This driver requires phy_power_on() before phy_set_speed().
> 
> 
> sparx5_serdes: phy_power_*() controls power down mode, no check
> in sparx5_serdes_set_mode() whether powered up, but config function
> checks for e.g. PLL lock, which probably won't be locked if powered
> down. Failures aren't propagated to the API caller.
> 
> This driver requires phy_power_on() before phy_set_mode*().
> 
> 
> phy-mtk-xfi-tphy: while phy_power_*() controls clocks, it isn't
> obvious whether these are required before mtk_xfi_tphy_setup() is
> invoked. There are calls to usleep() implying a need to wait for
> things to happen before the next register write, which presumably
> require the clock to be running.
> 
> My guess is, this driver requires phy_power_on() before phy_set_mode*().
> 
> 
> Categorisation of drivers:
> drivers requiring powered down PHY before phy_set_mode*(), but are
> silent if not: 2
> 
> drivers requiring powered down PHY before phy_set_mode*(), but error
> out if not: 1
> 
> drivers that allow any power state before phy_set_mode*(): 1
> 
> drivers that do not implement .power_*() methods, allowing any
> power state, but presumably require the phy_set_mode*() call to
> setup the hardware: 5
> 
> drivers requiring powered up PHY before phy_set_mode*(): 3
> 
> So, the summary is... only _one_ driver allows the calls in any order.
> The other 11 drivers are... stick a pin in the donkey and depending
> on where the pin ends up determines the appropriate order that the
> generic PHY APIs must be called for the driver to work correctly.
> 
> I think you'll agree that this is a very sorry state of affairs.

Yes. It is lazy state from provider implementation. If hardware requires
you to power down and set mode, it could have been handled internally in
the driver. I think we should fix these providers.

> This is exactly why I think there needs to be better documentation.
> I'm not sure that some kind of test code to validate the API would
> work, because you'd need some way to test whether the PHY were in a
> functional state, and that's difficult without the help of the
> ethernet MAC.

Lets document that call order is immaterial and driver is expected to
work both ways? As I said earlier logically people would set things up
and power up, and on the fly mode changes can be handled internally in
the driver by doing off-set-on dance.

Thinking out loud, we can also move this into framework and ensure when
modes are set, we do off-set-on dance so that onus on providers is
removed. Moving into fwk might expose some bugs in drivers though...

One thing I agree is that we should have consistency. How we drive that
can be agreed upon.

Thanks
-- 
~Vinod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Vladimir Oltean]

On Thu, Feb 12, 2026 at 10:44:27AM +0530, Vinod Koul wrote:
> Lets document that call order is immaterial and driver is expected to
> work both ways? As I said earlier logically people would set things up
> and power up, and on the fly mode changes can be handled internally in
> the driver by doing off-set-on dance.

FWIW, I already started telling people during review to not rely on call
order:
https://lore.kernel.org/linux-phy/20260210193516.temrg46yozxma7xb@skbuf

I don't mind continuing to scan for this in new submissions. Then, only
the topic of existing drivers remains to be resolved.

> Thinking out loud, we can also move this into framework and ensure when
> modes are set, we do off-set-on dance so that onus on providers is
> removed. Moving into fwk might expose some bugs in drivers though...
> 
> One thing I agree is that we should have consistency. How we drive that
> can be agreed upon.
> 
> Thanks
> -- 
> ~Vinod

I kind of like the fact that the framework doesn't have power vs mode
assumptions built in.

Also thinking out loud, we could do something else - introduce something
similar in spirit to CONFIG_DEBUG_TEST_DRIVER_REMOVE, which would be a
debug option that sees what power state the PHY is in during the
phy_set_mode_ext() call, flips it before calling ->set_mode() (calling
either ->power_on() or ->power_off()), and restores it after the call.

Having this option should also give PHY provider developers a quick way
of testing both calling orders without modifying the consumers.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Russell King (Oracle)]

On Thu, Feb 12, 2026 at 11:13:32AM +0200, Vladimir Oltean wrote:
> Also thinking out loud, we could do something else - introduce something
> similar in spirit to CONFIG_DEBUG_TEST_DRIVER_REMOVE, which would be a
> debug option that sees what power state the PHY is in during the
> phy_set_mode_ext() call, flips it before calling ->set_mode() (calling
> either ->power_on() or ->power_off()), and restores it after the call.
> 
> Having this option should also give PHY provider developers a quick way
> of testing both calling orders without modifying the consumers.

I don't think anyone would enable that option, beause clearly what
happens is they develop their generic PHY driver, and also develop
the consumer of that generic PHY driver. Once it works, they say
"job done" and submit it.

I was thinking that maybe some automated testing is needed, but
that runs into other problems:

1. any test code doesn't have any way to determine what a PHY
   driver supports, because phy_validate() is optional. So it has
   no way to know whether e.g. PHY_MODE_ETHERNET is supported or
   not. Calling phy_set_mode() isn't sufficient, if ->set_mode()
   isn't implemented, this is effectively a no-op.

2. drivers that just return success for ->set_mode() irrespective
   of the PHY power state but don't program the hardware would be
   undetectable.

I'm also going to point out that phy-core allows ->set_mode() to be
unimplemented, yet the phy_mode is stored. It looks to me like this is
intentional part of the API, which means that phy_set_mode*() is not
expected to always result in the hardware being programmed. That
brings up the obvious question: if phy_set_mode() is not expected to
always reprogram the hardware, then what phy API call should follow
this to ensure the hardware is reprogrammed.

On the other hand, if the API intention was that ->set_mode() must be
implemented if phy_set_mode*() is to be accepted, then surely
phy_set_mode_ext() should be checking that phy->ops->set_mode exists,
and returning -EOPNOTSUPP if it doesn't.

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Russell King (Oracle)]

On Thu, Feb 12, 2026 at 10:01:57AM +0000, Russell King (Oracle) wrote:
> On Thu, Feb 12, 2026 at 11:13:32AM +0200, Vladimir Oltean wrote:
> > Also thinking out loud, we could do something else - introduce something
> > similar in spirit to CONFIG_DEBUG_TEST_DRIVER_REMOVE, which would be a
> > debug option that sees what power state the PHY is in during the
> > phy_set_mode_ext() call, flips it before calling ->set_mode() (calling
> > either ->power_on() or ->power_off()), and restores it after the call.
> > 
> > Having this option should also give PHY provider developers a quick way
> > of testing both calling orders without modifying the consumers.
> 
> I don't think anyone would enable that option, beause clearly what
> happens is they develop their generic PHY driver, and also develop
> the consumer of that generic PHY driver. Once it works, they say
> "job done" and submit it.
> 
> I was thinking that maybe some automated testing is needed, but
> that runs into other problems:
> 
> 1. any test code doesn't have any way to determine what a PHY
>    driver supports, because phy_validate() is optional. So it has
>    no way to know whether e.g. PHY_MODE_ETHERNET is supported or
>    not. Calling phy_set_mode() isn't sufficient, if ->set_mode()
>    isn't implemented, this is effectively a no-op.
> 
> 2. drivers that just return success for ->set_mode() irrespective
>    of the PHY power state but don't program the hardware would be
>    undetectable.
> 
> I'm also going to point out that phy-core allows ->set_mode() to be
> unimplemented, yet the phy_mode is stored. It looks to me like this is
> intentional part of the API, which means that phy_set_mode*() is not
> expected to always result in the hardware being programmed. That
> brings up the obvious question: if phy_set_mode() is not expected to
> always reprogram the hardware, then what phy API call should follow
> this to ensure the hardware is reprogrammed.
> 
> On the other hand, if the API intention was that ->set_mode() must be
> implemented if phy_set_mode*() is to be accepted, then surely
> phy_set_mode_ext() should be checking that phy->ops->set_mode exists,
> and returning -EOPNOTSUPP if it doesn't.

I'll also point out that other parts of the API don't even give the
driver the opportunity to program hardware. E.g.:

static inline void phy_set_bus_width(struct phy *phy, int bus_width)
{
        phy->attrs.bus_width = bus_width;
}

So, in order for this hardware configuration to take effect, some other
PHY API call is necessary after calling this function.

(While not relevant for ethernet, I think this needs to be considered
in this discussion, since it's all related to how the generic PHY API
should be used.)

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Vladimir Oltean]

On Thu, Feb 12, 2026 at 10:01:57AM +0000, Russell King (Oracle) wrote:
> I'm also going to point out that phy-core allows ->set_mode() to be
> unimplemented, yet the phy_mode is stored. It looks to me like this is
> intentional part of the API, which means that phy_set_mode*() is not
> expected to always result in the hardware being programmed. That
> brings up the obvious question: if phy_set_mode() is not expected to
> always reprogram the hardware, then what phy API call should follow
> this to ensure the hardware is reprogrammed.
> 
> On the other hand, if the API intention was that ->set_mode() must be
> implemented if phy_set_mode*() is to be accepted, then surely
> phy_set_mode_ext() should be checking that phy->ops->set_mode exists,
> and returning -EOPNOTSUPP if it doesn't.

This is a relatively new development.

commit d58c04e305afbaa9dda7969151f06c4efe2c98b0
Author: Dmitry Baryshkov <lumag@kernel.org>
Date:   Sun Feb 9 14:31:45 2025 +0200

    phy: core: don't require set_mode() callback for phy_get_mode() to work

    As reported by Damon Ding, the phy_get_mode() call doesn't work as
    expected unless the PHY driver has a .set_mode() call. This prompts PHY
    drivers to have empty stubs for .set_mode() for the sake of being able
    to get the mode.

    Make .set_mode() callback truly optional and update PHY's mode even if
    it there is none.

    Cc: Damon Ding <damon.ding@rock-chips.com>
    Link: https://lore.kernel.org/r/96f8310f-93f1-4bcb-8637-137e1159ff83@rock-chips.com
    Tested-by: Damon Ding <damon.ding@rock-chips.com>
    Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
    Link: https://lore.kernel.org/r/20250209-phy-fix-set-moe-v2-1-76e248503856@linaro.org
    Signed-off-by: Vinod Koul <vkoul@kernel.org>

If only lore.kernel.org wasn't down, so I could see the back story in
the link...

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Russell King (Oracle)]

On Thu, Feb 12, 2026 at 12:38:03PM +0200, Vladimir Oltean wrote:
> On Thu, Feb 12, 2026 at 10:01:57AM +0000, Russell King (Oracle) wrote:
> > I'm also going to point out that phy-core allows ->set_mode() to be
> > unimplemented, yet the phy_mode is stored. It looks to me like this is
> > intentional part of the API, which means that phy_set_mode*() is not
> > expected to always result in the hardware being programmed. That
> > brings up the obvious question: if phy_set_mode() is not expected to
> > always reprogram the hardware, then what phy API call should follow
> > this to ensure the hardware is reprogrammed.
> > 
> > On the other hand, if the API intention was that ->set_mode() must be
> > implemented if phy_set_mode*() is to be accepted, then surely
> > phy_set_mode_ext() should be checking that phy->ops->set_mode exists,
> > and returning -EOPNOTSUPP if it doesn't.
> 
> This is a relatively new development.
> 
> commit d58c04e305afbaa9dda7969151f06c4efe2c98b0
> Author: Dmitry Baryshkov <lumag@kernel.org>
> Date:   Sun Feb 9 14:31:45 2025 +0200
> 
>     phy: core: don't require set_mode() callback for phy_get_mode() to work
> 
>     As reported by Damon Ding, the phy_get_mode() call doesn't work as
>     expected unless the PHY driver has a .set_mode() call. This prompts PHY
>     drivers to have empty stubs for .set_mode() for the sake of being able
>     to get the mode.
> 
>     Make .set_mode() callback truly optional and update PHY's mode even if
>     it there is none.
> 
>     Cc: Damon Ding <damon.ding@rock-chips.com>
>     Link: https://lore.kernel.org/r/96f8310f-93f1-4bcb-8637-137e1159ff83@rock-chips.com
>     Tested-by: Damon Ding <damon.ding@rock-chips.com>
>     Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>     Link: https://lore.kernel.org/r/20250209-phy-fix-set-moe-v2-1-76e248503856@linaro.org
>     Signed-off-by: Vinod Koul <vkoul@kernel.org>
> 
> If only lore.kernel.org wasn't down, so I could see the back story in
> the link...

Looking at the first link:

> On 2024/12/30 20:45, Dmitry Baryshkov wrote:
> > On Thu, Dec 26, 2024 at 02:33:03PM +0800, Damon Ding wrote:
> > No need for the stub, please drop it. The host controller driver can
> > still call phy_set_mode() / _ext(), the call will return 0.
> 
> Without the &phy_ops.set_mode(), the phy driver can not get phy_mode to 
> distinguish between HDMI and DP mode via the phy_get_mode(), even if the 
> host driver calls phy_set_mode() / _ext(). Additionally, the previous 
> discussion [0] also mentioned future considerations for dynamic 
> switching. Indeed, I should add a related comment before the 'return 0;' 
> to enhance understandability.

The first sentence makes me question the reasoning here - why would
a phy _driver_ call phy _consumer_ functions such as phy_get_mode().
We have drivers that directly access phy->attrs.mode.

It also adds to the question about the intended correct ordering of
PHY consumer calls, because it seems that the intention behind this
is to _not_ implement the ->set_mode() method, but to reconfigure the
PHY in some other generic PHY API call.

By "fixing" phy_set_mode*() in the above commit to allow this, that
action goes against the idea that generic PHY API calls can be made in
any order.

So my conclusion is that there is disagreement between generic PHY
reviewers about how the generic PHY API should be used and implemented,
leading to the mess I've highlighted where consumers need to know the
implementation details of the generic PHY driver to make the calls in
the correct order for that specific driver.

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH net-next] doc: generic phy: update generic PHY documentation

[Russell King (Oracle)]

On Thu, Feb 12, 2026 at 10:36:43AM +0530, Vinod Koul wrote:
> On 11-02-26, 17:48, Vladimir Oltean wrote:
> > On Thu, Feb 05, 2026 at 02:56:15PM +0000, Russell King (Oracle) wrote:
> > > Update the generic PHY documentation as a result of the discussion for
> > > the s32g submission.
> > > 
> > > Link: https://lore.kernel.org/r/aXtvDn_-pCuKPrnf@vaman
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > ---
> > > I didn't get any replies to my follow-up question to Vinod:
> 
> Sorry to have missed that...
> 
> > >   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.
> > 
> > There's certainly nothing *not* permitting that call sequence.
> 
> I agree on that. The documentation can be update to reflect that. Common
> logic would say that we should set things up before powering up, that
> should be the guidance here as well.

This is what my documentation patch does. It adds the possibility of
calling phy_set_mode*() before phy_power_on(), and recommends calling
phy_set_mode*() at that point, rather than the current situation where
the recommendation is after phy_power_on(). It leaves the possibility
of calling phy_set_mode*() afterwards in place.

It isn't clear to me what is required here - it seems that there is
disagreement between Vladimir and yourself, so I'm not planning to
submit an updated patch. Someone who knows what they're doing with
the generic phy stuff needs to propose a patch, and that ain't me.

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy