Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Sean Anderson <sean.anderson@seco.com>
To:     netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Sean Anderson <sean.anderson@seco.com>,
        Claudiu Beznea <claudiu.beznea@microchip.com>,
        Nicolas Ferre <nicolas.ferre@microchip.com>
Subject: [RFC net-next PATCH 08/16] net: macb: Clean up macb_validate
Date:   Mon,  4 Oct 2021 15:15:19 -0400
Message-Id: <20211004191527.1610759-9-sean.anderson@seco.com>
In-Reply-To: <20211004191527.1610759-1-sean.anderson@seco.com>
References: <20211004191527.1610759-1-sean.anderson@seco.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Ebg7k1uEQi/JojvSj7NfJf7YXJfEld9A1zaZiBQ9cwTL3HyejQwfOAnV29Bv?=
 =?us-ascii?Q?qWLs80ADiAnpeEa04bV6Q4GlTe2N8uJV2/0S/Blbd0kGt+alDycLkiP8aH0V?=
 =?us-ascii?Q?qkAyu3yfu1op3ziS6PC9ApVmfmP5/6aJ+vMS6wT9rAVpMfyWUTZFBqub+Ad7?=
 =?us-ascii?Q?DPEwUx3Itmh+wG6CSAnA6MT/XdUKpsJyv/QHRKGWtoe2ksOSWc9MsSefVJu3?=
 =?us-ascii?Q?aHfC5282vQzznbnx47kHRul6vtfJzz4iHYBqEdfTFqdCCRHJsnxY2y9qwlX0?=
 =?us-ascii?Q?zA0qxqbdV76DUrI5HlRUcdY3OqvN0hqRtXbJjEgDmjA0xQ85Gwh+UmnD2aAD?=
 =?us-ascii?Q?DnyG89vQ8XIPeK4YlvaB73aG+xTiZquDBd2zy999LHoTWCe/h9fsyh3M8Tr8?=
 =?us-ascii?Q?oPPRJg9db54vmtqkAPD4+L3fXm2dkU34JatgjFereVA2N0fNsMW78C3LtbjY?=
 =?us-ascii?Q?gyovHcxOF0/tIaVYL/Fv6H16qkcywdWuHArM68Qniye6vbftAmCyjWCspSQs?=
 =?us-ascii?Q?ZfC2QHc5vN0F7LU8WMSSkB94IUJkLMtaHrmmi+JhRJzY8g+NHqAtEyomCWo6?=
 =?us-ascii?Q?mgjddZE379soi+3yKnvDcDe0ithoxgP5l0WdjYzJoAh19IVikYjETAvstXzT?=
 =?us-ascii?Q?jMtK79Gbg4N7cc4zEXIOBlA45ojQ7CJRaIx+CPC0hzCCjb7LnrSApBF49PAT?=
 =?us-ascii?Q?gWrE9ff0hir2so71AMXF/O2wipgfX435cHlvy18iOGeBVL0cAThTqxFbF0zP?=
 =?us-ascii?Q?LGdNiWr5xJ31Fwajb6IbbfEfQ4EWUc/tZ0Qaa2zYlOUrEl7PEp9/Cjn/Zj6/?=
 =?us-ascii?Q?jySJ0tA5Cu/SmTkdaa3A2f96ef7cG3Kh4DGvqyklUHf/1vfJtV7Vw3PenxBf?=
 =?us-ascii?Q?hO8fvg8+iQvUIsftEGF3Qq/HtxaxO8AGl+UzQX2oVrYI1TN0Qsn9AKfKGqZa?=
 =?us-ascii?Q?eCrqeyYDRqr9ECo22ZKQrjhY4Fpe6UbFkKHFd/fn2F0/rMdSGdmNKVBEA34j?=
 =?us-ascii?Q?Cc+jATth9am4nVkoNRhnu4o4hLGM7SP08fFIdMBvQvJTtwummssy3EO0/caF?=
 =?us-ascii?Q?CdRkqY6dBbcngOYrYsAFl/T5sxxu+LSkLOyk+JXk5tvkpimHUPCTxPQRaMkU?=
 =?us-ascii?Q?t/RTNhmbwpm/yh7DYL0wVFBUEF74a5/JAmaX6qVloOc1oWtNL8q4GAuXRo8Z?=
 =?us-ascii?Q?dcio9oaBFyaPsHRlpcE7rsPISHthVOFX0aavvfg+qglzKL3jk1YBex7OXmn/?=
 =?us-ascii?Q?/hGA3+jrD084rqAiNo61u2eSldqB8y77dpUOX+UHvyzC7DHy3XEojRt+hhGg?=
 =?us-ascii?Q?1z32mr+nRvPYh87pJ2s2qu/l?=
X-OriginatorOrg: seco.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e02bd6a3-185a-4542-9656-08d9876b65ff
X-MS-Exchange-CrossTenant-AuthSource: DB7PR03MB4523.eurprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Oct 2021 19:15:59.3168
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EJO9pfe0W0l+OFOxXDeHwU9dRkQKCCKx3ABvgttz1pgRCk3j9TwyLg3udIMX8hCOT0zR+/P4wK6x6lESbKmqmw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR03MB7434
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

As the number of interfaces grows, the number of if statements grows
ever more unweildy. Clean everything up a bit by using a switch
statement. No functional change intended.

While we're on the subject, could someone clarify the relationship
between the various speed capabilities? What's the difference between
MACB_CAPS_GIGABIT_MODE_AVAILABLE, MACB_CAPS_HIGH_SPEED, MACB_CAPS_PCS,
and macb_is_gem()? Would there ever be a GEM without GIGABIT_MODE?
HIGH_SPEED without PCS? Why doesn't SGMII care if we're a gem (I think
this one is a bug, because it cares later on)?

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 drivers/net/ethernet/cadence/macb_main.c | 99 +++++++++++-------------
 1 file changed, 45 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e2730b3e1a57..18afa544b623 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -510,32 +510,55 @@ static void macb_validate(struct phylink_config *config,
 			  unsigned long *supported,
 			  struct phylink_link_state *state)
 {
+	bool one = state->interface == PHY_INTERFACE_MODE_NA;
 	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. */
-	if (state->interface != PHY_INTERFACE_MODE_NA &&
-	    state->interface != PHY_INTERFACE_MODE_MII &&
-	    state->interface != PHY_INTERFACE_MODE_RMII &&
-	    state->interface != PHY_INTERFACE_MODE_GMII &&
-	    state->interface != PHY_INTERFACE_MODE_SGMII &&
-	    state->interface != PHY_INTERFACE_MODE_10GBASER &&
-	    !phy_interface_mode_is_rgmii(state->interface)) {
-		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
-		return;
-	}
-
-	if (!macb_is_gem(bp) &&
-	    (state->interface == PHY_INTERFACE_MODE_GMII ||
-	     phy_interface_mode_is_rgmii(state->interface))) {
-		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
-		return;
-	}
-
-	if (state->interface == PHY_INTERFACE_MODE_10GBASER &&
-	    !(bp->caps & MACB_CAPS_HIGH_SPEED &&
-	      bp->caps & MACB_CAPS_PCS)) {
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_NA:
+	case PHY_INTERFACE_MODE_10GBASER:
+		if (bp->caps & MACB_CAPS_HIGH_SPEED &&
+		    bp->caps & MACB_CAPS_PCS &&
+		    bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
+			phylink_set(mask, 10000baseCR_Full);
+			phylink_set(mask, 10000baseER_Full);
+			phylink_set(mask, 10000baseKR_Full);
+			phylink_set(mask, 10000baseLR_Full);
+			phylink_set(mask, 10000baseLRM_Full);
+			phylink_set(mask, 10000baseSR_Full);
+			phylink_set(mask, 10000baseT_Full);
+		} else if (one) {
+			goto none;
+		}
+		if (one)
+			break;
+		fallthrough;
+	case PHY_INTERFACE_MODE_GMII:
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		if (!macb_is_gem(bp) && one)
+			goto none;
+		fallthrough;
+	case PHY_INTERFACE_MODE_SGMII:
+		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
+			phylink_set(mask, 1000baseT_Full);
+			phylink_set(mask, 1000baseX_Full);
+			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF))
+				phylink_set(mask, 1000baseT_Half);
+		}
+		fallthrough;
+	case PHY_INTERFACE_MODE_MII:
+	case PHY_INTERFACE_MODE_RMII:
+		phylink_set(mask, 10baseT_Half);
+		phylink_set(mask, 10baseT_Full);
+		phylink_set(mask, 100baseT_Half);
+		phylink_set(mask, 100baseT_Full);
+		break;
+	none:
+	default:
 		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
 		return;
 	}
@@ -543,38 +566,6 @@ static void macb_validate(struct phylink_config *config,
 	phylink_set_port_modes(mask);
 	phylink_set(mask, Autoneg);
 	phylink_set(mask, Asym_Pause);
-
-	if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
-	    (state->interface == PHY_INTERFACE_MODE_NA ||
-	     state->interface == PHY_INTERFACE_MODE_10GBASER)) {
-		phylink_set(mask, 10000baseCR_Full);
-		phylink_set(mask, 10000baseER_Full);
-		phylink_set(mask, 10000baseKR_Full);
-		phylink_set(mask, 10000baseLR_Full);
-		phylink_set(mask, 10000baseLRM_Full);
-		phylink_set(mask, 10000baseSR_Full);
-		phylink_set(mask, 10000baseT_Full);
-		if (state->interface != PHY_INTERFACE_MODE_NA)
-			goto out;
-	}
-
-	phylink_set(mask, 10baseT_Half);
-	phylink_set(mask, 10baseT_Full);
-	phylink_set(mask, 100baseT_Half);
-	phylink_set(mask, 100baseT_Full);
-
-	if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
-	    (state->interface == PHY_INTERFACE_MODE_NA ||
-	     state->interface == PHY_INTERFACE_MODE_GMII ||
-	     state->interface == PHY_INTERFACE_MODE_SGMII ||
-	     phy_interface_mode_is_rgmii(state->interface))) {
-		phylink_set(mask, 1000baseT_Full);
-		phylink_set(mask, 1000baseX_Full);
-
-		if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF))
-			phylink_set(mask, 1000baseT_Half);
-	}
-out:
 	bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_and(state->advertising, state->advertising, mask,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
-- 
2.25.1