On Wed, Dec 01, 2021 at 01:38:53AM +0100, Andrew Lunn wrote: > > However, this won't work for PHY devices created _before_ the kernel > > has mounted the rootfs, whether or not they end up being used. So, > > every PHY mentioned in DT will be created before the rootfs is mounted, > > and none of these PHYs will have their modules loaded. > > Hi Russell > > I think what you are saying here is, if the MAC or MDIO bus driver is > built in, the PHY driver also needs to be built in? > > If the MAC or MDIO bus driver is a module, it means the rootfs has > already been mounted in order to get these modules. And so the PHY > driver as a module will also work. Yes, because the module loading is performed by phy_device_create() when it calls phy_request_driver_module(), which will happen when either the MDIO bus is scanned or the DT is parsed for the PHY nodes. > > I believe this is the root cause of Yinbo Zhu's issue. > > You are speculating that in Yinbo Zhu case, the MAC driver is built > in, the PHY is a module. The initial request for the firmware fails. s/firmware/module/ and it could also be the MDIO bus driver that is built in. > Yinbo Zhu would like udev to try again later when the modules are > available. I think so - it's speculation because it seems quite difficult to find out detailed information. > > What we _could_ do is review all device trees and PHY drivers to see > > whether DT modaliases are ever used for module loading. If they aren't, > > then we _could_ make the modalias published by the kernel conditional > > on the type of mdio device - continue with the DT approach for non-PHY > > devices, and switch to the mdio: scheme for PHY devices. I repeat, this > > can only happen if no PHY drivers match using the DT scheme, otherwise > > making this change _will_ cause a regression. > > Take a look at > drivers/net/mdio/of_mdio.c:whitelist_phys[] and the comment above it. > > So there are some DT blobs out there with compatible strings for > PHYs. I've no idea if they actually load that way, or the standard PHY > mechanism is used. Well, this suggests we have no instances - if none of our modules contain a DT table to match a PHY-driver, then we should be pretty safe. $ grep phy_driver drivers/net -rl | xargs grep 'MODULE_ALIAS\|MODULE_DEVICE.*of' drivers/net/phy/xilinx_gmii2rgmii.c:MODULE_DEVICE_TABLE(of, xgmiitorgmii_of_match); drivers/net/mdio/mdio-moxart.c:MODULE_DEVICE_TABLE(of, moxart_mdio_dt_ids); drivers/net/dsa/mt7530.c:MODULE_DEVICE_TABLE(of, mt7530_of_match); All three look to be false hits - none are phy drivers themselves, they just reference "phy_driver". So, I think we can say that we have no instances of PHY driver being matched using DT in net-next in drivers/net. Hopefully, there aren't any PHY drivers elsewhere in the kernel tree. That is not true universally for all MDIO though - as xilinx_gmii2rgmii.c clearly shows. That is a MDIO driver which uses DT the compatible string to do the module load. So, we have proof there that Yinbo Zhu's change will definitely cause a regression which we can not allow. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
