Linux-PHY Archive on lore.kernel.org

* Re: [PATCH net-next] doc: generic phy: update generic PHY documentation
From: Russell King (Oracle) @ 2026-02-12 10:05 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Vinod Koul, Neil Armstrong, Jonathan Corbet, linux-doc, linux-phy
In-Reply-To: <aY2lFTIALH7qEJmM@shell.armlinux.org.uk>

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

^ permalink raw reply