Re: [PATCH v4] net: macb: Fix several edge cases in validate

There were several cases where validate() would return bogus supported
modes with unusual combinations of interfaces and capabilities. For
example, if state->interface was 10GBASER and the macb had HIGH_SPEED
and PCS but not GIGABIT MODE, then 10/100 modes would be set anyway. In
another case, SGMII could be enabled even if the mac was not a GEM
(despite this being checked for later on in mac_config()). These
inconsistencies make it difficult to refactor this function cleanly.

This attempts to address these by reusing the same conditions used to
decide whether to return early when setting mode bits. The logic is
pretty messy, but this preserves the existing logic where possible.

Signed-off-by: Sean Anderson <redacted>
---

Changes in v4:
- Drop cleanup patch

Changes in v3:
- Order bugfix patch first

Changes in v2:
- New

 drivers/net/ethernet/cadence/macb_main.c | 59 +++++++++++++++++-------
 1 file changed, 42 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 309371abfe23..40bd5a069368 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c

@@ -510,11 +510,16 @@ static void macb_validate(struct phylink_config *config,
 			  unsigned long *supported,
 			  struct phylink_link_state *state)
 {
+	bool have_1g = true, have_10g = true;
 	struct net_device *ndev = to_net_dev(config->dev);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };

 	struct macb *bp = netdev_priv(ndev);
 
-	/* We only support MII, RMII, GMII, RGMII & SGMII. */
+	/* There are three major types of interfaces we support:
+	 * - (R)MII supporting 10/100 Mbit/s
+	 * - GMII, RGMII, and SGMII supporting 10/100/1000 Mbit/s
+	 * - 10GBASER supporting 10 Gbit/s only
+	 */
 	if (state->interface != PHY_INTERFACE_MODE_NA &&
 	    state->interface != PHY_INTERFACE_MODE_MII &&
 	    state->interface != PHY_INTERFACE_MODE_RMII &&

@@ -526,27 +531,48 @@ static void macb_validate(struct phylink_config *config,
 		return;
 	}
 
-	if (!macb_is_gem(bp) &&
-	    (state->interface == PHY_INTERFACE_MODE_GMII ||
-	     phy_interface_mode_is_rgmii(state->interface))) {
-		linkmode_zero(supported);
-		return;
+	/* For 1G and up we must have both have a GEM and GIGABIT_MODE */
+	if (!macb_is_gem(bp) ||
+	    (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)) {
+		if (state->interface == PHY_INTERFACE_MODE_GMII ||
+		    phy_interface_mode_is_rgmii(state->interface) ||
+		    state->interface == PHY_INTERFACE_MODE_SGMII ||
+		    state->interface == PHY_INTERFACE_MODE_10GBASER) {
+			linkmode_zero(supported);
+			return;
+		} else if (state->interface == PHY_INTERFACE_MODE_NA) {
+			have_1g = false;
+			have_10g = false;
+		}
 	}