linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH net-next 0/3] Make the PHY library stop being so greedy when binding the generic PHY driver
@ 2021-09-01 22:50 Vladimir Oltean
  2021-09-01 22:50 ` [RFC PATCH net-next 1/3] net: phy: don't bind genphy in phy_attach_direct if the specific driver defers probe Vladimir Oltean
                   ` (4 more replies)
  0 siblings, 5 replies; 52+ messages in thread
From: Vladimir Oltean @ 2021-09-01 22:50 UTC (permalink / raw)
  To: netdev
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Andrew Lunn,
	Heiner Kallweit, Russell King, David S. Miller, Jakub Kicinski,
	Vivien Didelot, Florian Fainelli, Vladimir Oltean, linux-kernel,
	Linus Walleij, Alvin Šipraga, ACPI Devel Maling List,
	kernel-team, Len Brown

This is a continuation of the discussion on patch "[v1,1/2] driver core:
fw_devlink: Add support for FWNODE_FLAG_BROKEN_PARENT" from here:
https://patchwork.kernel.org/project/netdevbpf/patch/20210826074526.825517-2-saravanak@google.com/

Summary: in a complex combination of device dependencies which is not
really relevant to what is being proposed here, DSA ends up calling
phylink_of_phy_connect during a period in which the PHY driver goes
through a series of probe deferral events.

The central point of that discussion is that DSA seems "broken" for
expecting the PHY driver to probe immediately on PHYs belonging to the
internal MDIO buses of switches. A few suggestions were made about what
to do, but some were not satisfactory and some did not solve the problem.

In fact, fw_devlink, the mechanism that causes the PHY driver to defer
probing in this particular case, has some significant "issues" too, but
its "issues" are only in quotes "because at worst it'd allow a few
unnecessary deferred probes":
https://patchwork.kernel.org/project/netdevbpf/patch/20210826074526.825517-2-saravanak@google.com/#24418895

So if that's the criterion by which an issue is an issue, maybe we
should take a step back and look at the bigger picture.

There is nothing about the idea that a PHY might defer probing, or about
the changes proposed here, that has anything with DSA. Furthermore, the
changes done by this series solve the problem in the same way: "they
allow a few unnecessary deferred probes" <- in this case they provoke
this to the caller of phy_attach_direct.

If we look at commit 16983507742c ("net: phy: probe PHY drivers
synchronously"), we see that the PHY library expectation is for the PHY
device to have a PHY driver bound to it as soon as device_add() finishes.

Well, as it turns out, in case the PHY device has any supplier which is
not ready, this is not possible, but that patch still seems to ensure
that the process of binding a driver to the device has at least started.
That process will continue for a while, and will race with
phy_attach_direct calls, so we need to make the latter observe the fact
that a driver is struggling to probe, and wait for it a bit more.

What I've not tested is loading the PHY module at runtime, and seeing
how phy_attach_direct behaves then. I expect that this change set will
not alter the behavior in that case: the genphy will still bind to a
device with no driver, and phy_attach_direct will not return -EPROBE_DEFER
in that case.

I might not be very versed in the device core/internals, but the patches
make sense to me, and worked as intended from the first try on my system
(Turris MOX with mv88e6xxx), which was modified to make the same "sins"
as those called out in the thread above:

- use PHY interrupts provided by the switch itself as an interrupt-controller
- call of_mdiobus_register from setup() and not from probe(), so as to
  not circumvent fw_devlink's limitations, and still get to hit the PHY
  probe deferral conditions.

So feedback and testing on other platforms is very appreciated.

Vladimir Oltean (3):
  net: phy: don't bind genphy in phy_attach_direct if the specific
    driver defers probe
  net: dsa: destroy the phylink instance on any error in
    dsa_slave_phy_setup
  net: dsa: allow the phy_connect() call to return -EPROBE_DEFER

 drivers/base/dd.c            | 21 +++++++++++++++++++--
 drivers/net/phy/phy_device.c |  8 ++++++++
 include/linux/device.h       |  1 +
 net/dsa/dsa2.c               |  2 ++
 net/dsa/slave.c              | 12 +++++-------
 5 files changed, 35 insertions(+), 9 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2021-09-05  0:44 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 22:50 [RFC PATCH net-next 0/3] Make the PHY library stop being so greedy when binding the generic PHY driver Vladimir Oltean
2021-09-01 22:50 ` [RFC PATCH net-next 1/3] net: phy: don't bind genphy in phy_attach_direct if the specific driver defers probe Vladimir Oltean
2021-09-02  5:43   ` Greg Kroah-Hartman
2021-09-02 10:11     ` Vladimir Oltean
2021-09-02 10:37       ` Greg Kroah-Hartman
2021-09-02 11:17         ` Vladimir Oltean
2021-09-02 14:37     ` Rafael J. Wysocki
2021-09-02 18:50   ` Russell King (Oracle)
2021-09-02 19:23     ` Vladimir Oltean
2021-09-02 19:51     ` Andrew Lunn
2021-09-02 20:33       ` Florian Fainelli
2021-09-02 21:33         ` Russell King (Oracle)
2021-09-02 21:39           ` Vladimir Oltean
2021-09-02 22:24             ` Russell King (Oracle)
2021-09-02 22:45               ` Vladimir Oltean
2021-09-02 23:02                 ` Andrew Lunn
2021-09-02 23:26                   ` Vladimir Oltean
2021-09-03  0:04                     ` Russell King (Oracle)
2021-09-03 20:48                       ` Vladimir Oltean
2021-09-03 22:06                         ` Russell King (Oracle)
2021-09-04 21:59                           ` Vladimir Oltean
2021-09-04 23:25                             ` Russell King (Oracle)
2021-09-05  0:41                               ` Vladimir Oltean
2021-09-03  9:27               ` Ioana Ciornei
2021-09-01 22:50 ` [RFC PATCH net-next 2/3] net: dsa: destroy the phylink instance on any error in dsa_slave_phy_setup Vladimir Oltean
2021-09-02 12:25   ` Russell King (Oracle)
2021-09-02 23:21   ` Florian Fainelli
2021-09-01 22:50 ` [RFC PATCH net-next 3/3] net: dsa: allow the phy_connect() call to return -EPROBE_DEFER Vladimir Oltean
2021-09-02 12:19 ` [RFC PATCH net-next 0/3] Make the PHY library stop being so greedy when binding the generic PHY driver Russell King (Oracle)
2021-09-02 12:35   ` Vladimir Oltean
2021-09-02 12:59     ` Vladimir Oltean
2021-09-02 13:26     ` Russell King (Oracle)
2021-09-02 15:23       ` Vladimir Oltean
2021-09-02 16:31         ` Russell King (Oracle)
2021-09-02 17:10           ` Vladimir Oltean
2021-09-02 17:50             ` Russell King (Oracle)
2021-09-02 19:05               ` Vladimir Oltean
2021-09-02 20:03                 ` Russell King (Oracle)
2021-09-02 20:21                   ` Vladimir Oltean
2021-09-02 20:29                     ` Russell King (Oracle)
2021-09-03 16:22                       ` Vladimir Oltean
2021-09-03 17:21                         ` Andrew Lunn
2021-09-03 18:58                           ` Russell King (Oracle)
2021-09-03 19:56                             ` Andrew Lunn
2021-09-03 20:08                               ` Russell King (Oracle)
2021-09-03 18:54                         ` Russell King (Oracle)
2021-09-03 20:11                           ` Vladimir Oltean
2021-09-02 20:07     ` Andrew Lunn
2021-09-02 20:32       ` Vladimir Oltean
2021-09-02 21:39         ` Russell King (Oracle)
2021-09-02 22:05 ` Vladimir Oltean
2021-09-02 23:29   ` Saravana Kannan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).