Subject: Re: [PATCH v2 net-next 2/2] net: phy: air_en8811h: Add the Airoha EN8811H PHY driver

From: "Russell King (Oracle)" <[email-masked]>
Date: Tue, 5 Mar 2024 14:24:25 +0000

On Tue, Mar 05, 2024 at 03:06:45PM +0100, Andrew Lunn wrote:
> > The only way I can see around this problem would be to look up the
> > PHY in order to get a pointer to the struct phy_device in the network
> > device's probe function, and attach the PHY there _before_ you register
> > the network device. You can then return EPROBE_DEFER and, because you
> > are returning it in a .probe function, the probe will be retried once
> > other probes in the system (such as your PHY driver) have finished.
> > This also means that userspace doesn't see the appearance of the
> > non-functional network device until it's ready, and thus can use
> > normal hotplug mechanisms to notice the network device.
> 
> What i'm thinking is we add another op to phy_driver dedicated to
> firmware download. We let probe run as is, so the PHY is registered
> and available. But if the firmware op is set, we start a thread and
> call the op in it. Once the op exits, we signal a completion event.
> phy_attach_direct() would then wait on the completion.

That's really not good, because phy_attach_direct() can be called
from .ndo_open, which will result in the rtnl lock being held while
we wait - so this is not much better than having the firmware load
in .config_init.

If we drop the lock, then we need to audit what the effect of that
would be - for example, if the nic is being opened, it may mean
that another attempt to open the nic could be started. Or it may
mean that an attempt to configure the nic down could be started.
Then the original open proceeds and state is now messed up.

I do get the feeling that trying to work around "I don't want the
firmware in the initramfs" is creating more problems and pain than
it's worth.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!