| Message ID | 20211011165517.2857893-2-sean.anderson@seco.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,1/2] net: macb: Clean up macb_validate | expand |
On Mon, 11 Oct 2021 12:55:17 -0400 Sean Anderson wrote: > This aligns mac_validate with mac_config. In mac_config, SGMII is only > enabled if macb_is_gem. Validate should care if the mac is a gem as > well. This also simplifies the logic now that all gigabit modes depend > on the mac being a GEM. > > Fixes: 7897b071ac3b ("net: macb: convert to phylink") > Signed-off-by: Sean Anderson <sean.anderson@seco.com> Please make sure you CC the author of the original patch, they tend to be a good person to review the change. Or at the very least validate the Fixes tag. Adding Antoine. > drivers/net/ethernet/cadence/macb_main.c | 22 +++++++++------------- > 1 file changed, 9 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index a9105ec1b885..ae8c969a609c 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -534,22 +534,18 @@ static void macb_validate(struct phylink_config *config, > case PHY_INTERFACE_MODE_RGMII_ID: > case PHY_INTERFACE_MODE_RGMII_RXID: > case PHY_INTERFACE_MODE_RGMII_TXID: > - if (!macb_is_gem(bp)) { > - if (one) > - goto none; > - else > - goto mii; > - } > - 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); > + if (macb_is_gem(bp)) { > + 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); > + } > + } else if (one) { > + goto none; > } > fallthrough; > - mii: > case PHY_INTERFACE_MODE_MII: > case PHY_INTERFACE_MODE_RMII: > phylink_set(mask, 10baseT_Half);
Hello Sean, Quoting Jakub Kicinski (2021-10-12 02:17:59) > On Mon, 11 Oct 2021 12:55:17 -0400 Sean Anderson wrote: > > This aligns mac_validate with mac_config. In mac_config, SGMII is only > > enabled if macb_is_gem. Validate should care if the mac is a gem as > > well. This also simplifies the logic now that all gigabit modes depend > > on the mac being a GEM. This looks correct. > > Fixes: 7897b071ac3b ("net: macb: convert to phylink") If this is a fix, the patch has to be first in the series as it is depending on the first one which is not a fix. Thanks, Antoine
On 10/12/21 4:27 AM, Antoine Tenart wrote: > Hello Sean, > > Quoting Jakub Kicinski (2021-10-12 02:17:59) >> On Mon, 11 Oct 2021 12:55:17 -0400 Sean Anderson wrote: >> > This aligns mac_validate with mac_config. In mac_config, SGMII is only >> > enabled if macb_is_gem. Validate should care if the mac is a gem as >> > well. This also simplifies the logic now that all gigabit modes depend >> > on the mac being a GEM. > > This looks correct. > >> > Fixes: 7897b071ac3b ("net: macb: convert to phylink") > > If this is a fix, the patch has to be first in the series as it is > depending on the first one which is not a fix. Ok, I can put this first then. --Sean
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index a9105ec1b885..ae8c969a609c 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -534,22 +534,18 @@ static void macb_validate(struct phylink_config *config, case PHY_INTERFACE_MODE_RGMII_ID: case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: - if (!macb_is_gem(bp)) { - if (one) - goto none; - else - goto mii; - } - 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); + if (macb_is_gem(bp)) { + 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); + } + } else if (one) { + goto none; } fallthrough; - mii: case PHY_INTERFACE_MODE_MII: case PHY_INTERFACE_MODE_RMII: phylink_set(mask, 10baseT_Half);
This aligns mac_validate with mac_config. In mac_config, SGMII is only enabled if macb_is_gem. Validate should care if the mac is a gem as well. This also simplifies the logic now that all gigabit modes depend on the mac being a GEM. Fixes: 7897b071ac3b ("net: macb: convert to phylink") Signed-off-by: Sean Anderson <sean.anderson@seco.com> --- Changes in v2: - New drivers/net/ethernet/cadence/macb_main.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)