RE: [PATCH v2 05/15] net/txgbe: add support for PHY configuration via SW-FW mailbox

["Jiawen Wu" <jiawenwu@trustnetic.com>]

> Subject: [PATCH v2 05/15] net/txgbe: add support for PHY configuration via SW-FW mailbox
> 
> Migrate Amber-Lite PHY configuration to firmware due to complexity.
> Driver now sends mailbox commands for link state changes, handled
> by firmware’s intricate PHY setup process

This patch removes what was added by the previous patch.
I don't understand why. Why not simply merge these into the
previous patch. Or just configure nothing there.

And...

> 
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---
>  drivers/net/txgbe/base/txgbe_aml.c   | 52 ++++++++++++++++------------
>  drivers/net/txgbe/base/txgbe_aml40.c | 37 +++++++++++++++++++-
>  drivers/net/txgbe/base/txgbe_hw.c    |  1 +
>  drivers/net/txgbe/base/txgbe_mng.c   | 36 +++++++++++++++++++
>  drivers/net/txgbe/base/txgbe_mng.h   | 17 +++++++++
>  drivers/net/txgbe/base/txgbe_type.h  |  7 ++++
>  6 files changed, 126 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/txgbe/base/txgbe_aml.c b/drivers/net/txgbe/base/txgbe_aml.c
> index 1b5a3783a9..510a539d38 100644
> --- a/drivers/net/txgbe/base/txgbe_aml.c
> +++ b/drivers/net/txgbe/base/txgbe_aml.c
> @@ -131,21 +131,37 @@ u32 txgbe_get_media_type_aml(struct txgbe_hw *hw)
>  	return media_type;
>  }
> 
> +static void txgbe_wait_for_link_up_aml(struct txgbe_hw *hw, u32 speed)
> +{
> +	u32 link_speed = TXGBE_LINK_SPEED_UNKNOWN;
> +	bool link_up = false;
> +	int cnt = 0;
> +	int i;
> +
> +	if (speed == TXGBE_LINK_SPEED_25GB_FULL)
> +		cnt = 4;
> +	else
> +		cnt = 1;
> +
> +	for (i = 0; i < (4 * cnt); i++) {
> +		hw->mac.check_link(hw, &link_speed, &link_up, false);
> +		if (link_up)
> +			break;
> +		msleep(250);
> +	}
> +}
> +
>  s32 txgbe_setup_mac_link_aml(struct txgbe_hw *hw,
>  			       u32 speed,
>  			       bool autoneg_wait_to_complete)
>  {
>  	bool autoneg = false;
>  	s32 status = 0;
> -	s32 ret_status = 0;
>  	u32 link_speed = TXGBE_LINK_SPEED_UNKNOWN;
>  	bool link_up = false;
> -	int i;
>  	u32 link_capabilities = TXGBE_LINK_SPEED_UNKNOWN;
>  	u32 value = 0;
> 
> -	UNREFERENCED_PARAMETER(autoneg_wait_to_complete);
> -
>  	if (hw->phy.sfp_type == txgbe_sfp_type_not_present) {
>  		DEBUGOUT("SFP not detected, skip setup mac link");
>  		return 0;
> @@ -165,30 +181,20 @@ s32 txgbe_setup_mac_link_aml(struct txgbe_hw *hw,
>  	if (value & (TXGBE_SFP1_MOD_ABS_LS | TXGBE_SFP1_RX_LOS_LS))
>  		return status;
> 
> -	for (i = 0; i < 4; i++) {
> -		txgbe_e56_check_phy_link(hw, &link_speed, &link_up);
> -		if (link_up)
> -			break;
> -		msleep(250);
> -	}

txgbe_e56_check_phy_link() is deleted here, but still be invoked in
txgbe_setup_mac_link_multispeed_fiber_aml() ?

> +	status = hw->mac.check_link(hw, &link_speed, &link_up,
> +				    autoneg_wait_to_complete);
> 
> -	if (link_speed == speed && link_up &&
> -	   !(speed == TXGBE_LINK_SPEED_25GB_FULL))
> +	if (link_speed == speed && link_up)
>  		return status;
> 
> -	rte_spinlock_lock(&hw->phy_lock);
> -	ret_status = 0;
> -	rte_spinlock_unlock(&hw->phy_lock);
> +	if (speed & TXGBE_LINK_SPEED_25GB_FULL)
> +		speed = 0x10;
> +	else if (speed & TXGBE_LINK_SPEED_10GB_FULL)
> +		speed = 0x08;
> 
> -	if (ret_status == TXGBE_ERR_PHY_INIT_NOT_DONE)
> -		return status;
> +	status = hw->phy.set_link_hostif(hw, (u8)speed, autoneg, true);
> 
> -	for (i = 0; i < 4; i++) {
> -		txgbe_e56_check_phy_link(hw, &link_speed, &link_up);
> -		if (link_up)
> -			return status;
> -		msleep(250);
> -		}
> +	txgbe_wait_for_link_up_aml(hw, speed);
> 
>  	return status;
>  }
> diff --git a/drivers/net/txgbe/base/txgbe_aml40.c b/drivers/net/txgbe/base/txgbe_aml40.c
> index 2bad990aa8..88408550d2 100644
> --- a/drivers/net/txgbe/base/txgbe_aml40.c
> +++ b/drivers/net/txgbe/base/txgbe_aml40.c
> @@ -106,7 +106,42 @@ s32 txgbe_setup_mac_link_aml40(struct txgbe_hw *hw,
>  			       u32 speed,
>  			       bool autoneg_wait_to_complete)
>  {
> -	return 0;
> +	bool autoneg = false;
> +	s32 status = 0;
> +	u32 link_speed = TXGBE_LINK_SPEED_UNKNOWN;
> +	bool link_up = false;
> +	u32 link_capabilities = TXGBE_LINK_SPEED_UNKNOWN;
> +	u32 value = 0;
> +
> +	if (hw->phy.sfp_type == txgbe_sfp_type_not_present) {
> +		DEBUGOUT("SFP not detected, skip setup mac link");
> +		return 0;
> +	}
> +
> +	/* Check to see if speed passed in is supported. */
> +	status = hw->mac.get_link_capabilities(hw,
> +			&link_capabilities, &autoneg);
> +	if (status)
> +		return status;
> +
> +	speed &= link_capabilities;
> +	if (speed == TXGBE_LINK_SPEED_UNKNOWN)
> +		return TXGBE_ERR_LINK_SETUP;
> +
> +	status = hw->mac.check_link(hw, &link_speed, &link_up,
> +				    autoneg_wait_to_complete);
> +
> +	if (link_speed == speed && link_up)
> +		return status;
> +
> +	if (speed & TXGBE_LINK_SPEED_40GB_FULL)
> +		speed = 0x20;
> +
> +	status = hw->phy.set_link_hostif(hw, (u8)speed, autoneg, true);
> +
> +	txgbe_wait_for_link_up_aml(hw, speed);
> +
> +	return status;
>  }

The commit log does not explain why aml40 should be configured in this patch.