Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <7794de4e-b9d6-4b57-aaf1-466a848a311b@quicinc.com>
Date:   Thu, 16 Nov 2023 15:32:38 +0800
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 1/6] net: phylink: move phylink_pcs_neg_mode() to
 phylink.c
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
CC:     <andrew@lunn.ch>, <davem@davemloft.net>, <edumazet@google.com>,
        <kuba@kernel.org>, <pabeni@redhat.com>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <hkallweit1@gmail.com>, <corbet@lwn.net>, <netdev@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-doc@vger.kernel.org>
References: <20231115140630.10858-1-quic_luoj@quicinc.com>
 <20231115140630.10858-2-quic_luoj@quicinc.com>
 <ZVTSVIx+S+W/+X4B@shell.armlinux.org.uk>
Content-Language: en-US
From:   Jie Luo <quic_luoj@quicinc.com>
In-Reply-To: <ZVTSVIx+S+W/+X4B@shell.armlinux.org.uk>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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@nxp.com>
>>
>> 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@armlinux.org.uk>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   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
>>
>>
>