| Date | Sat, 16 Jul 2022 22:06:01 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net-next v3 08/47] net: phylink: Support differing link speeds and interface speeds |
| |
> +/** > + * phy_interface_speed() - get the speed of a phy interface > + * @interface: phy interface mode defined by &typedef phy_interface_t > + * @link_speed: the speed of the link > + * > + * Some phy interfaces modes adapt to the speed of the underlying link (such as > + * by duplicating data or changing the clock rate). Others, however, are fixed > + * at a particular rate. Determine the speed of a phy interface mode for a > + * particular link speed. > + * > + * Return: The speed of @interface > + */ > +static int phy_interface_speed(phy_interface_t interface, int link_speed) > +{ > + switch (interface) { > + case PHY_INTERFACE_MODE_100BASEX: > + return SPEED_100; > + > + case PHY_INTERFACE_MODE_TBI: > + case PHY_INTERFACE_MODE_MOCA: > + case PHY_INTERFACE_MODE_RTBI: > + case PHY_INTERFACE_MODE_1000BASEX: > + case PHY_INTERFACE_MODE_1000BASEKX: > + case PHY_INTERFACE_MODE_TRGMII: > + return SPEED_1000; > + > + case PHY_INTERFACE_MODE_2500BASEX: > + return SPEED_2500; > + > + case PHY_INTERFACE_MODE_5GBASER: > + return SPEED_5000; > + > + case PHY_INTERFACE_MODE_XGMII: > + case PHY_INTERFACE_MODE_RXAUI: > + case PHY_INTERFACE_MODE_XAUI: > + case PHY_INTERFACE_MODE_10GBASER: > + case PHY_INTERFACE_MODE_10GKR: > + return SPEED_10000; > + > + case PHY_INTERFACE_MODE_25GBASER: > + return SPEED_25000; > + > + case PHY_INTERFACE_MODE_XLGMII: > + return SPEED_40000; > + > + case PHY_INTERFACE_MODE_USXGMII: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_QSGMII: > + case PHY_INTERFACE_MODE_SGMII: > + case PHY_INTERFACE_MODE_GMII: > + case PHY_INTERFACE_MODE_REVRMII: > + case PHY_INTERFACE_MODE_RMII: > + case PHY_INTERFACE_MODE_SMII: > + case PHY_INTERFACE_MODE_REVMII: > + case PHY_INTERFACE_MODE_MII: > + case PHY_INTERFACE_MODE_INTERNAL: > + return link_speed; > + > + case PHY_INTERFACE_MODE_NA: > + case PHY_INTERFACE_MODE_MAX: > + break; > + } > + > + return SPEED_UNKNOWN;
This seem error prone when new PHY_INTERFACE_MODES are added. I would prefer a WARN_ON_ONCE() in the default: so we get to know about such problems.
I'm also wondering if we need a sanity check here. I've seen quite a few boards a Fast Ethernet MAC, but a 1G PHY because they are cheap. In such cases, the MAC is supposed to call phy_set_max_speed() to indicate it can only do 100Mbs. PHY_INTERFACE_MODE_MII but a link_speed of 1G is clearly wrong. Are there other cases where we could have a link speed faster than what the interface mode allows?
Bike shedding a bit, but would it be better to use host_side_speed and line_side_speed? When you say link_speed, which link are your referring to? Since we are talking about the different sides of the PHY doing different speeds, the naming does need to be clear.
Andrew
|