public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-phy@lists.infradead.org
Subject: Re: [PATCH net-next] doc: generic phy: update generic PHY documentation
Date: Thu, 12 Feb 2026 11:55:39 +0000	[thread overview]
Message-ID: <aY2_u88tkEb5geAg@shell.armlinux.org.uk> (raw)
In-Reply-To: <20260212103803.xut4sjbypgb26mo4@skbuf>

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!

  reply	other threads:[~2026-02-12 11:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-05 14:56 [PATCH net-next] doc: generic phy: update generic PHY documentation Russell King (Oracle)
2026-02-11 15:48 ` Vladimir Oltean
2026-02-11 18:15   ` Russell King (Oracle)
2026-02-11 19:30     ` Vladimir Oltean
2026-02-11 19:45       ` Vladimir Oltean
2026-02-11 20:31       ` Russell King (Oracle)
2026-02-12  5:14         ` Vinod Koul
2026-02-12  9:13           ` Vladimir Oltean
2026-02-12 10:01             ` Russell King (Oracle)
2026-02-12 10:05               ` Russell King (Oracle)
2026-02-12 10:38               ` Vladimir Oltean
2026-02-12 11:55                 ` Russell King (Oracle) [this message]
2026-02-12  5:06   ` Vinod Koul
2026-02-18 13:15     ` Russell King (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aY2_u88tkEb5geAg@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox