On 12/05/2020 14:28, Russell King - ARM Linux admin wrote:
On Tue, May 12, 2020 at 11:28:40AM +0200, Julien Beraud wrote:
On 11/05/2020 21:06, Florian Fainelli wrote:
On 5/11/2020 11:29 AM, Russell King - ARM Linux admin wrote:
On Mon, May 11, 2020 at 05:45:02PM +0200, Julien Beraud wrote:
Following commit:
commit 52c956003a9d5bcae1f445f9dfd42b624adb6e87
Author: Russell King <[email protected]>
Date: Wed Dec 11 10:56:45 2019 +0000
net: phylink: delay MAC configuration for copper SFP modules
In function phylink_sfp_connect_phy, phylink_sfp_config is called before
phylink_attach_phy.
In the case of a genphy, the "supported" field of the phy_device is filled
by:
phylink_attach_phy->phy_attach_direct->phy_probe->genphy_read_abilities.
It means that:
ret = phylink_sfp_config(pl, mode, phy->supported, phy->advertising);
will have phy->supported with no bits set, and then the first call to
phylink_validate in phylink_sfp_config will return an error:
return phylink_is_empty_linkmode(supported) ? -EINVAL : 0;
this results in putting the sfp driver in "failed" state.
Which PHY is this?
The phy seems to be Marvell 88E1111, so the simple solution is to just add the
driver for this PHY to my config.
That said, if for some reason someone plugs a module for which no phy driver is
found the issue will happen again.
Right, please use the specific PHY driver for modules that contain the
88E1111 to avoid any surprises, otherwise things can end up being
misconfigured - for example, the PHY using 1000base-X and the host
using SGMII, which may work or may lead to problems.
I'll do that, thanks.
Using the generic PHY with a copper SFP module does not sound like a
great idea because without a specialized PHY driver (that is, not the
Generic PHY driver) there is not usually much that can happen.
Thanks for the info. I don't have an advice on whether it is a good idea to use
a copper sfp without a specialized driver,
but before commit 52c956003a9d5bcae1f445f9dfd42b624adb6e87, it used to work and
I could at least get a network connection.
Moreover, this commit didn't explicitely intend to forbid this behavior and the
error is not explicit either.
If phylink+sfp still supports using genphy as a fallback, It may be good to fix
the current behavior.
If not, maybe adding an explicit warning or error would be preferrable.
The commit is designed to increase the range of modules that can be
used, especially when the module supports higher rates than the MAC.
The downside is that we _must_ know the PHYs capabilities before
attaching to it, so that we can choose an appropriate interface to
_attach_ to it with. It's a chicken and egg problem.
For this to work reliably in cases such as those you've identified,
I need phylib to always give me that information earlier than it
currently seems to for the genphy fallback - but the problem is the
genphy fallback only happens after attaching to it. So again,
another chicken and egg problem.
Subsituting the SFP modules capabilities seems like a workaround,
but those are also a guess from poor information in the SFP EEPROM.
It is what we were doing before however...
Thank you very much for the explanations.
I'll try this patch for now.
Regards,
Julien
