[RFC net-next PATCH 08/16] net: macb: Clean up macb_validate
From: Sean Anderson <hidden>
Date: 2021-10-04 19:16:34
Also in:
lkml
Subsystem:
atmel macb ethernet driver, networking drivers, the rest · Maintainers:
Théo Lebrun, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
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 <redacted> --- 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