Re: [PATCH v3 1/6] net: phylink: move phylink_pcs_neg_mode() to phylink.c

[Jie Luo]

On 11/15/2023 10:14 PM, Russell King (Oracle) wrote:
> Hi,
>
> You don't need this patch for your series, and you're bypassing my
> ability to decide when this patch should be merged (which is not yet,
> I want things to remain as-is for another cycle.)
>
> In theory, looking at past history, 6.7 will probably be a LTS kernel,
> but until that is known for certain, I don't want to commit to moving
> this function in case LTS gets delayed by a cycle.
>
> Please drop it from your series.
>
> Thanks.

Got it, Russell.
I will drop this patch in the next patch set, Thanks.

> On Wed, Nov 15, 2023 at 10:06:25PM +0800, Luo Jie wrote:
> > From: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> >
> > Russell points out that there is no user of phylink_pcs_neg_mode()
> > outside of phylink.c, nor is there planned to be any, so we can just
> > move it there.
> >
> > Suggested-by: Russell King (Oracle) <linux@xxxxxxxxxxxxxxx>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> > Signed-off-by: Luo Jie <quic_luoj@xxxxxxxxxxx>
> > ---
> >   drivers/net/phy/phylink.c | 65 ++++++++++++++++++++++++++++++++++++++
> >   include/linux/phylink.h   | 66 ---------------------------------------
> >   2 files changed, 65 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 25c19496a336..162f51b0986a 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -162,6 +162,71 @@ static const char *phylink_an_mode_str(unsigned int mode)
> >   	return mode < ARRAY_SIZE(modestr) ? modestr[mode] : "unknown";
> >   }
> >   
> > +/**
> > + * phylink_pcs_neg_mode() - helper to determine PCS inband mode
> > + * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
> > + * @interface: interface mode to be used
> > + * @advertising: adertisement ethtool link mode mask
> > + *
> > + * Determines the negotiation mode to be used by the PCS, and returns
> > + * one of:
> > + *
> > + * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
> > + * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
> > + *   will be used.
> > + * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
> > + *   disabled
> > + * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
> > + *
> > + * Note: this is for cases where the PCS itself is involved in negotiation
> > + * (e.g. Clause 37, SGMII and similar) not Clause 73.
> > + */
> > +static unsigned int phylink_pcs_neg_mode(unsigned int mode, phy_interface_t interface,
> > +					 const unsigned long *advertising)
> > +{
> > +	unsigned int neg_mode;
> > +
> > +	switch (interface) {
> > +	case PHY_INTERFACE_MODE_SGMII:
> > +	case PHY_INTERFACE_MODE_QSGMII:
> > +	case PHY_INTERFACE_MODE_QUSGMII:
> > +	case PHY_INTERFACE_MODE_USXGMII:
> > +		/* These protocols are designed for use with a PHY which
> > +		 * communicates its negotiation result back to the MAC via
> > +		 * inband communication. Note: there exist PHYs that run
> > +		 * with SGMII but do not send the inband data.
> > +		 */
> > +		if (!phylink_autoneg_inband(mode))
> > +			neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> > +		else
> > +			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> > +		break;
> > +
> > +	case PHY_INTERFACE_MODE_1000BASEX:
> > +	case PHY_INTERFACE_MODE_2500BASEX:
> > +		/* 1000base-X is designed for use media-side for Fibre
> > +		 * connections, and thus the Autoneg bit needs to be
> > +		 * taken into account. We also do this for 2500base-X
> > +		 * as well, but drivers may not support this, so may
> > +		 * need to override this.
> > +		 */
> > +		if (!phylink_autoneg_inband(mode))
> > +			neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> > +		else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> > +					   advertising))
> > +			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> > +		else
> > +			neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
> > +		break;
> > +
> > +	default:
> > +		neg_mode = PHYLINK_PCS_NEG_NONE;
> > +		break;
> > +	}
> > +
> > +	return neg_mode;
> > +}
> > +
> >   static unsigned int phylink_interface_signal_rate(phy_interface_t interface)
> >   {
> >   	switch (interface) {
> > diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> > index 875439ab45de..d589f89c612c 100644
> > --- a/include/linux/phylink.h
> > +++ b/include/linux/phylink.h
> > @@ -98,72 +98,6 @@ static inline bool phylink_autoneg_inband(unsigned int mode)
> >   	return mode == MLO_AN_INBAND;
> >   }
> >   
> > -/**
> > - * phylink_pcs_neg_mode() - helper to determine PCS inband mode
> > - * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
> > - * @interface: interface mode to be used
> > - * @advertising: adertisement ethtool link mode mask
> > - *
> > - * Determines the negotiation mode to be used by the PCS, and returns
> > - * one of:
> > - *
> > - * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
> > - * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
> > - *   will be used.
> > - * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
> > - *   disabled
> > - * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
> > - *
> > - * Note: this is for cases where the PCS itself is involved in negotiation
> > - * (e.g. Clause 37, SGMII and similar) not Clause 73.
> > - */
> > -static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
> > -						phy_interface_t interface,
> > -						const unsigned long *advertising)
> > -{
> > -	unsigned int neg_mode;
> > -
> > -	switch (interface) {
> > -	case PHY_INTERFACE_MODE_SGMII:
> > -	case PHY_INTERFACE_MODE_QSGMII:
> > -	case PHY_INTERFACE_MODE_QUSGMII:
> > -	case PHY_INTERFACE_MODE_USXGMII:
> > -		/* These protocols are designed for use with a PHY which
> > -		 * communicates its negotiation result back to the MAC via
> > -		 * inband communication. Note: there exist PHYs that run
> > -		 * with SGMII but do not send the inband data.
> > -		 */
> > -		if (!phylink_autoneg_inband(mode))
> > -			neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> > -		else
> > -			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> > -		break;
> > -
> > -	case PHY_INTERFACE_MODE_1000BASEX:
> > -	case PHY_INTERFACE_MODE_2500BASEX:
> > -		/* 1000base-X is designed for use media-side for Fibre
> > -		 * connections, and thus the Autoneg bit needs to be
> > -		 * taken into account. We also do this for 2500base-X
> > -		 * as well, but drivers may not support this, so may
> > -		 * need to override this.
> > -		 */
> > -		if (!phylink_autoneg_inband(mode))
> > -			neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> > -		else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> > -					   advertising))
> > -			neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> > -		else
> > -			neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
> > -		break;
> > -
> > -	default:
> > -		neg_mode = PHYLINK_PCS_NEG_NONE;
> > -		break;
> > -	}
> > -
> > -	return neg_mode;
> > -}
> > -
> >   /**
> >    * struct phylink_link_state - link state structure
> >    * @advertising: ethtool bitmask containing advertised link modes
> > --
> > 2.42.0