Hi Russell,

On Thu, Nov 29, 2018 at 10:00:43PM +0000, Russell King - ARM Linux wrote:

On Thu, Nov 29, 2018 at 11:31:23AM -0800, Florian Fainelli wrote:

quoted

On 11/29/2018 4:49 AM, Baruch Siach wrote:

quoted

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.chevallier@bootlin.com>
Cc: Antoine Tenart <redacted>
Signed-off-by: Baruch Siach <baruch@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?

Two calls are made - the first with PHY_INTERFACE_MODE_NA to
determine what the advertising link mode may be, and then again
once the interface mode has been selected from the advertising mask.

Why?

Consider a 4.3Mbps fiberchannel SFP plugged into a 1G-only MAC.
If we did it as a single pass, we would end up passing an
interface mode of 2500BASEX first time around which is illogical.

So you consider this to be the right fix, right?

I should have added:

Fixes: d97c9f4ab000b ("net: mvpp2: 1000baseX support")

Antoine, is this fix OK with you?

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -