Hi Florian, Florian Fainelli writes: > On 11/29/2018 4:49 AM, Baruch Siach wrote: >> The mvpp2_phylink_validate() relies on the interface field of >> phylink_link_state to determine valid link modes. However, when called >> from phylink_sfp_module_insert() this field in not initialized. The >> default switch case then excludes 10G link modes. This allows 10G SFP >> modules that are detected correctly to be configured at max rate of >> 2.5G. >> >> Catch the uninitialized PHY mode case, and allow 10G rates. >> >> Cc: Maxime Chevallier <maxime.chevall...@bootlin.com> >> Cc: Antoine Tenart <antoine.ten...@bootlin.com> >> Signed-off-by: Baruch Siach <bar...@tkos.co.il> >> --- >> Is that the right fix? > > It would be a bit surprising that this is the right fix, you would > expect validate to be called once everything has been parsed > successfully from the SFP, is not that the case here? If not, can you > find out what happens?
This is the code from phylink_sfp_module_insert() at drivers/net/phy/phylink.c: __ETHTOOL_DECLARE_LINK_MODE_MASK(support) = { 0, }; struct phylink_link_state config; ... sfp_parse_support(pl->sfp_bus, id, support); port = sfp_parse_port(pl->sfp_bus, id, support); memset(&config, 0, sizeof(config)); linkmode_copy(config.advertising, support); config.interface = PHY_INTERFACE_MODE_NA; /* ... more 'config' fields initialization ... */ ret = phylink_validate(pl, support, &config); The 'interface' field is not detected at this stage. I think it is not meant to represent the detected information, but the actual configuration. baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -