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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel