Netdev Archive on lore.kernel.org help / color / mirror / Atom feed
From: Vladimir Oltean <firstname.lastname@example.org> To: Greg Kroah-Hartman <email@example.com> Cc: "Vladimir Oltean" <firstname.lastname@example.org>, email@example.com, "Rafael J. Wysocki" <firstname.lastname@example.org>, "Andrew Lunn" <email@example.com>, "Heiner Kallweit" <firstname.lastname@example.org>, "Russell King" <email@example.com>, "David S. Miller" <firstname.lastname@example.org>, "Jakub Kicinski" <email@example.com>, "Vivien Didelot" <firstname.lastname@example.org>, "Florian Fainelli" <email@example.com>, firstname.lastname@example.org, "Linus Walleij" <email@example.com>, "Alvin Šipraga" <firstname.lastname@example.org>, "ACPI Devel Maling List" <email@example.com>, kernel-team <firstname.lastname@example.org>, "Len Brown" <email@example.com> Subject: Re: [RFC PATCH net-next 1/3] net: phy: don't bind genphy in phy_attach_direct if the specific driver defers probe Date: Thu, 2 Sep 2021 14:17:02 +0300 [thread overview] Message-ID: <20210902111702.4n6suxfbze46wcgb@skbuf> (raw) In-Reply-To: <YTCpbkDMUfBtJM1V@kroah.com> On Thu, Sep 02, 2021 at 12:37:34PM +0200, Greg Kroah-Hartman wrote: > On Thu, Sep 02, 2021 at 01:11:50PM +0300, Vladimir Oltean wrote: > > On Thu, Sep 02, 2021 at 07:43:10AM +0200, Greg Kroah-Hartman wrote: > > > Wait, no, this should not be a "special" thing, and why would the list > > > of deferred probe show this? > > > > Why as in why would it work/do what I want, or as in why would you want to do that? > > Both! :) So first: why would it work. You seem to have a misconception that I am "messing with the probe function list". I am not, I am just exporting the information whether the device had a driver which returned -EPROBE_DEFER during probe, or not. For that I am looking at the presence of this device on the deferred_probe_pending_list. driver_probe_device -> if (ret == -EPROBE_DEFER || ret == EPROBE_DEFER) driver_deferred_probe_add(dev); -> list_add_tail(&dev->p->deferred_probe, &deferred_probe_pending_list); driver_bound -> driver_deferred_probe_del -> list_del_init(&dev->p->deferred_probe); So the presence of "dev" inside deferred_probe_pending_list means precisely that a driver is pending to be bound. Second: why would I want to do that. In the case of PHY devices, the driver binding process starts here: phy_device_register -> device_add It begins synchronously, but may not finish due to probe deferral. So after device_add finishes, phydev->drv might be NULL due to 2 reasons: 1. -EPROBE_DEFER triggered by "somebody", either by the PHY driver probe function itself, or by third parties (like device_links_check_suppliers happening to notice that before even calling the driver's probe fn). Anyway, the distinction between these 2 is pretty much irrelevant. 2. There genuinely was no driver loaded in the system for this PHY. Note that the way things are written, the Generic PHY driver will not match on any device in phy_bus_match(). It is bound manually, separately. The PHY library is absolutely happy to work with a headless chicken, a phydev with a NULL phydev->drv. Just search for "if (!phydev->drv)" inside drivers/net/phy/phy.c and drivers/net/phy/phy_device.c. However, the phydev walking with a NULL drv can only last for so long. An Ethernet port will soon need that PHY device, and will attach to it. There are many code paths, all ending in phy_attach_direct. However, when an Ethernet port decides to attach to a PHY device is completely asynchronous to the lifetime of the PHY device itself. This moment is where a driver is really needed, and if none is present, the generic one is force-bound. My patch only distinguishes between case 1 and 2 for which phydev->drv might be NULL. It avoids force-binding the generic PHY when a specific PHY driver was found, but did not finish binding due to probe deferral. > > > If a bus wants to have this type of "generic vs. specific" logic, then > > > it needs to handle it in the bus logic itself as that does NOT fit into > > > the normal driver model at all. Don't try to get a "hint" of this by > > > messing with the probe function list. > > > > Where and how? Do you have an example? > > No I do not, sorry, most busses do not do this for obvious ordering / > loading / we are not that crazy reasons. > > What is causing this all to suddenly break? The devlink stuff? There was a report related to fw_devlink indeed, however strictly speaking, I wouldn't say it is the cause of all this. It is pretty uncommon for a PHY device to defer probing I think, hence the bad assumptions made around it.
next prev parent reply other threads:[~2021-09-02 11:17 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 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 [this message] 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
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=20210902111702.4n6suxfbze46wcgb@skbuf \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /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: linkBe 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; as well as URLs for NNTP newsgroup(s).