RE: [PATCH v2] IGC: Remove I225_I_PHY_ID checking

["Mah, Yock Gen" <yock.gen.mah@intel.com>]

-----Original Message-----
From: Kevin Traynor <ktraynor@redhat.com> 
Sent: Tuesday, 11 October, 2022 7:05 PM
To: IOTG DPDK Ref App <iotg.dpdk.ref.app@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] IGC: Remove I225_I_PHY_ID checking

On 31/08/2022 23:51, iotg.dpdk.ref.app@intel.com wrote:
> From: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>
> 
> i225 devices have only one PHY vendor. There is unnecessary to check 
> _I_PHY_ID during the link establishment and auto-negotiation process, 
> the checking also caused devices like i225-IT failed. This patch is to 
> remove the mentioned unnecessary checking.
> 
> Cc: stable@dpdk.org
> Signed-off-by: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>
> ---
>   drivers/net/igc/base/igc_i225.c | 15 ++-------------
>   drivers/net/igc/base/igc_phy.c  |  6 ++----
>   2 files changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/igc/base/igc_i225.c 
> b/drivers/net/igc/base/igc_i225.c index 5f3d535490..af26602afb 100644
> --- a/drivers/net/igc/base/igc_i225.c
> +++ b/drivers/net/igc/base/igc_i225.c
> @@ -173,19 +173,8 @@ static s32 igc_init_phy_params_i225(struct igc_hw *hw)
>   	phy->ops.write_reg = igc_write_phy_reg_gpy;
>   
>   	ret_val = igc_get_phy_id(hw);
> -	/* Verify phy id and set remaining function pointers */
> -	switch (phy->id) {
> -	case I225_I_PHY_ID:
> -	case I226_LM_PHY_ID:
> -		phy->type		= igc_phy_i225;
> -		phy->ops.set_d0_lplu_state = igc_set_d0_lplu_state_i225;
> -		phy->ops.set_d3_lplu_state = igc_set_d3_lplu_state_i225;

> - The commit log says it is removing a check on the ID, but it does not say why these function pointers are being removed.

> - Why are they removed, were they not needed?


i225 devices have only one PHY vendor. There is no point checking _I_PHY_ID during the link establishment and auto-negotiation process.

> -		/* TODO - complete with GPY PHY information */
> -		break;
> -	default:
> -		ret_val = -IGC_ERR_PHY;
> -		goto out;
> -	}
> +        phy->type = igc_phy_i225;
> +
>   
>   out:
>   	return ret_val;
> diff --git a/drivers/net/igc/base/igc_phy.c 
> b/drivers/net/igc/base/igc_phy.c index 43bbe69bca..2906bae21a 100644
> --- a/drivers/net/igc/base/igc_phy.c
> +++ b/drivers/net/igc/base/igc_phy.c
> @@ -1474,8 +1474,7 @@ s32 igc_phy_setup_autoneg(struct igc_hw *hw)
>   			return ret_val;
>   	}
>   
> -	if ((phy->autoneg_mask & ADVERTISE_2500_FULL) &&
> -	    hw->phy.id == I225_I_PHY_ID) {
> +	if (phy->autoneg_mask & ADVERTISE_2500_FULL) {
>   	/* Read the MULTI GBT AN Control Register - reg 7.32 */
>   		ret_val = phy->ops.read_reg(hw, (STANDARD_AN_REG_MASK <<
>   					    MMD_DEVADDR_SHIFT) |
> @@ -1615,8 +1614,7 @@ s32 igc_phy_setup_autoneg(struct igc_hw *hw)
>   		ret_val = phy->ops.write_reg(hw, PHY_1000T_CTRL,
>   					     mii_1000t_ctrl_reg);
>   
> -	if ((phy->autoneg_mask & ADVERTISE_2500_FULL) &&
> -	    hw->phy.id == I225_I_PHY_ID)
> +	if (phy->autoneg_mask & ADVERTISE_2500_FULL)
>   		ret_val = phy->ops.write_reg(hw,
>   					     (STANDARD_AN_REG_MASK <<
>   					     MMD_DEVADDR_SHIFT) |