Re: [PATCH net-next v6 4/7] stmmac: intel: configure SerDes according to the interface mode

[Choong Yong Liang <yong.liang.choong@linux.intel.com>]

On 5/2/2025 2:13 am, Simon Horman wrote:
>> +static int intel_config_serdes(struct net_device *ndev,
>> +			       void *intel_data,
>> +			       phy_interface_t interface)
>> +{
>> +	struct intel_priv_data *intel_priv = intel_data;
>> +	struct stmmac_priv *priv = netdev_priv(ndev);
>> +	int ret = 0;
>> +
>> +	if (!intel_tsn_lane_is_available(ndev, intel_priv)) {
>> +		netdev_info(priv->dev,
>> +			    "No TSN lane available to set the registers.\n");
>> +		goto pmc_read_error;
>> +	}
>> +
>> +	if (intel_priv->pid_modphy == PID_MODPHY1) {
>> +		if (interface == PHY_INTERFACE_MODE_2500BASEX) {
>> +			ret = intel_set_reg_access(pid_modphy1_2p5g_regs,
>> +						   ARRAY_SIZE(pid_modphy1_2p5g_regs));
>> +		} else {
>> +			ret = intel_set_reg_access(pid_modphy1_1g_regs,
>> +						   ARRAY_SIZE(pid_modphy1_1g_regs));
>> +		}
>> +	} else {
>> +		if (interface == PHY_INTERFACE_MODE_2500BASEX) {
>> +			ret = intel_set_reg_access(pid_modphy3_2p5g_regs,
>> +						   ARRAY_SIZE(pid_modphy3_2p5g_regs));
>> +		} else {
>> +			ret = intel_set_reg_access(pid_modphy3_1g_regs,
>> +						   ARRAY_SIZE(pid_modphy3_1g_regs));
>> +		}
>> +	}
>> +
>> +	priv->plat->phy_interface = interface;
>> +
>> +	if (ret < 0)
>> +		goto pmc_read_error;
> 
> Perhaps this is an artifact of earlier refactoring,
> but the condition above seems to be without meaning
> as in either case the code goes directly to pmc_read_error.
> 
>> +
>> +pmc_read_error:
>> +	intel_serdes_powerdown(ndev, intel_priv);
>> +	intel_serdes_powerup(ndev, intel_priv);
>> +
>> +	return ret;
>> +}
>> +
>>   static void common_default_data(struct plat_stmmacenet_data *plat)
>>   {
>>   	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
> 
> ...
> 

Hi Simon,

You are right.
I will perform the cleanup on the code and submit the next version.

Thank you for your feedback.