On Wed, Dec 01, 2021 at 01:38:53AM +0100, Andrew Lunn wrote:

quoted

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.

quoted

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.

quoted

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!