| Message ID | 20250226100929.1646454-12-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: phy: Rework linkmodes handling in a dedicated file | expand |
On Wed, Feb 26, 2025 at 11:09:26AM +0100, Maxime Chevallier wrote: > phylink allows MAC drivers to report the capabilities in terms of speed, > duplex and pause support. This is done through a dedicated set of enum > values in the form of the MAC_ capabilities. They are very close to what > the LINK_CAPA_xxx can express, with the difference that LINK_CAPA don't > have any information about Pause/Asym Pause support. > > To prepare converting phylink to using the phy_caps, add the mapping > between MAC capabilities and phy_caps. While doing so, we move the > phylink_caps_params array up a bit to simplify future commits. I still want to know why we need to do this type of thing - unfortunately I don't have time to review all your patches at the moment. I haven't reviewed all your patches since you've started posting them. Sorry.
Hello Russell, On Wed, 26 Feb 2025 14:03:20 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Wed, Feb 26, 2025 at 11:09:26AM +0100, Maxime Chevallier wrote: > > phylink allows MAC drivers to report the capabilities in terms of speed, > > duplex and pause support. This is done through a dedicated set of enum > > values in the form of the MAC_ capabilities. They are very close to what > > the LINK_CAPA_xxx can express, with the difference that LINK_CAPA don't > > have any information about Pause/Asym Pause support. > > > > To prepare converting phylink to using the phy_caps, add the mapping > > between MAC capabilities and phy_caps. While doing so, we move the > > phylink_caps_params array up a bit to simplify future commits. > > I still want to know why we need to do this type of thing - Sorry not to have included more details on the why. The main reason is for the phy_port work. In the previous phy_port series [1] I included an attempt at making a first step forward with PHY-driven SFP, so that phylib itself provides the sfp_upstream_ops and not individual PHY drivers. That's to get a better handling for multi-port PHYs, which is one of the end-goals. [1]: https://lore.kernel.org/netdev/20250213101606.1154014-1-maxime.chevallier@bootlin.com/ As part of that PHY-sfp work, I find it very useful for PHY drivers to be able to tell what phy_interface_t they can expose on their serdes interfaces, and from then build a list of linkmodes to get an idea of what we can support on that port. That's why I'm extracting that out of phylink. I did see that you suggested having phylink involved for PHY sfp maybe, but TBH I don't even know where to start, so I took a safer approach with phylib-driven SFPs. > unfortunately I don't have time to review all your patches at the > moment. I haven't reviewed all your patches since you've started > posting them. Sorry. No worries, I understand that that whole work is quite the mouthful and implies some involved reviews. But even discussing higher level stuff, like we do now, helps me steer that work in the right direction and hopefully make it easier for you and the other PHY maintainers to review that in the future. Maxime
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 63fbf3d8708a..aaf07094b821 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -292,6 +292,31 @@ static int phylink_interface_max_speed(phy_interface_t interface) return SPEED_UNKNOWN; } +static struct { + unsigned long mask; + int speed; + unsigned int duplex; + unsigned int caps_bit; +} phylink_caps_params[] = { + { MAC_400000FD, SPEED_400000, DUPLEX_FULL, BIT(LINK_CAPA_400000FD) }, + { MAC_200000FD, SPEED_200000, DUPLEX_FULL, BIT(LINK_CAPA_200000FD) }, + { MAC_100000FD, SPEED_100000, DUPLEX_FULL, BIT(LINK_CAPA_100000FD) }, + { MAC_56000FD, SPEED_56000, DUPLEX_FULL, BIT(LINK_CAPA_56000FD) }, + { MAC_50000FD, SPEED_50000, DUPLEX_FULL, BIT(LINK_CAPA_50000FD) }, + { MAC_40000FD, SPEED_40000, DUPLEX_FULL, BIT(LINK_CAPA_40000FD) }, + { MAC_25000FD, SPEED_25000, DUPLEX_FULL, BIT(LINK_CAPA_25000FD) }, + { MAC_20000FD, SPEED_20000, DUPLEX_FULL, BIT(LINK_CAPA_20000FD) }, + { MAC_10000FD, SPEED_10000, DUPLEX_FULL, BIT(LINK_CAPA_10000FD) }, + { MAC_5000FD, SPEED_5000, DUPLEX_FULL, BIT(LINK_CAPA_5000FD) }, + { MAC_2500FD, SPEED_2500, DUPLEX_FULL, BIT(LINK_CAPA_2500FD) }, + { MAC_1000FD, SPEED_1000, DUPLEX_FULL, BIT(LINK_CAPA_1000FD) }, + { MAC_1000HD, SPEED_1000, DUPLEX_HALF, BIT(LINK_CAPA_1000HD) }, + { MAC_100FD, SPEED_100, DUPLEX_FULL, BIT(LINK_CAPA_100FD) }, + { MAC_100HD, SPEED_100, DUPLEX_HALF, BIT(LINK_CAPA_100HD) }, + { MAC_10FD, SPEED_10, DUPLEX_FULL, BIT(LINK_CAPA_10FD) }, + { MAC_10HD, SPEED_10, DUPLEX_HALF, BIT(LINK_CAPA_10HD) }, +}; + /** * phylink_caps_to_linkmodes() - Convert capabilities to ethtool link modes * @linkmodes: ethtool linkmode mask (must be already initialised) @@ -445,30 +470,6 @@ static void phylink_caps_to_linkmodes(unsigned long *linkmodes, } } -static struct { - unsigned long mask; - int speed; - unsigned int duplex; -} phylink_caps_params[] = { - { MAC_400000FD, SPEED_400000, DUPLEX_FULL }, - { MAC_200000FD, SPEED_200000, DUPLEX_FULL }, - { MAC_100000FD, SPEED_100000, DUPLEX_FULL }, - { MAC_56000FD, SPEED_56000, DUPLEX_FULL }, - { MAC_50000FD, SPEED_50000, DUPLEX_FULL }, - { MAC_40000FD, SPEED_40000, DUPLEX_FULL }, - { MAC_25000FD, SPEED_25000, DUPLEX_FULL }, - { MAC_20000FD, SPEED_20000, DUPLEX_FULL }, - { MAC_10000FD, SPEED_10000, DUPLEX_FULL }, - { MAC_5000FD, SPEED_5000, DUPLEX_FULL }, - { MAC_2500FD, SPEED_2500, DUPLEX_FULL }, - { MAC_1000FD, SPEED_1000, DUPLEX_FULL }, - { MAC_1000HD, SPEED_1000, DUPLEX_HALF }, - { MAC_100FD, SPEED_100, DUPLEX_FULL }, - { MAC_100HD, SPEED_100, DUPLEX_HALF }, - { MAC_10FD, SPEED_10, DUPLEX_FULL }, - { MAC_10HD, SPEED_10, DUPLEX_HALF }, -}; - /** * phylink_limit_mac_speed - limit the phylink_config to a maximum speed * @config: pointer to a &struct phylink_config
phylink allows MAC drivers to report the capabilities in terms of speed, duplex and pause support. This is done through a dedicated set of enum values in the form of the MAC_ capabilities. They are very close to what the LINK_CAPA_xxx can express, with the difference that LINK_CAPA don't have any information about Pause/Asym Pause support. To prepare converting phylink to using the phy_caps, add the mapping between MAC capabilities and phy_caps. While doing so, we move the phylink_caps_params array up a bit to simplify future commits. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- V1 -> V2: - New patch drivers/net/phy/phylink.c | 49 ++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 24 deletions(-)