Re: [Intel-wired-lan] [PATCH iwl-next v2 2/9] ethtool: Add forced speed to supported link modes maps
From: Andrew Lunn <andrew@lunn.ch>
Date: 2023-08-25 13:47:31
Also in:
intel-wired-lan
Let me think how we could do that. Andrew's idea is good. But most high-speed NICs, which have a standalone management firmware for PHY, don't use phylib/phylink. So in order to be able to unify all that, they should have ->supported bitmap somewhere else. Not sure struct net_device is the best place...
I would probably keep it in the driver priv structure, and just pass it as needed. So long as you only need one or two values, i don't see the need for a shared structure.
If I recall Phylink logics correctly (it's been a while since I last time was working with my embedded project), 1) in the NIC (MAC) driver, you initialize ->supported with *speeds* and stuff like duplex, no link modes; 2) Phylink core sets the corresponding link mode bits; 3) phylib core then clears the bits unsupported by the PHY IIRC
No, not really. All i think you need is a low level helper. So don't worry too much about how phylink works, just implement that low level helper passing in values as needed, not phylib or phylink structure. What i don't want is a second infrastructure to be built for those MAC drivers which don't use Linux to control the PHY. Either share a few helpers, or swap to phylink.
The third step in case with those NICs with FW-managed PHYs should be done manually in the MAC driver somewhere. Like "I am qede and I don't support mode XX at 50Gbps, but support the rest, so I clear that one bit".
I don't think that will work. New bits keep getting added, more speeds
added. So 'support the rest' is not well defined. You need an explicit
list of link modes the driver needs. We already have code to convert
an array of link mode bits into an actual mask, e.g:
linkmode_set_bit_array(phy_basic_t1_features_array,
ARRAY_SIZE(phy_basic_t1_features_array),
phy_basic_t1_features);
Andrew