[Intel-wired-lan] [PATCH net-next v10 3/5] ice: Enable switching default tx scheduler topology

[Tony Nguyen]

On 3/14/2023 3:25 AM, Michal Wilczynski wrote:
> Introduce support for tx scheduler topology change, based on user
> selection, from default 9-layer to 5-layer. In order for switch to be
> successful there is a new NVM(version 3.20 or older) and DDP package(OS
> Package 1.3.29 or older).
> 
> Enable 5-layer topology switch in init path of the driver. To accomplish
> that upload of the DDP package needs to be delayed, until change in Tx
> topology is finished. To trigger the Tx change user selection should be
> changed in NVM using devlink. Then the platform should be rebooted.
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski at intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_common.c |   2 +
>   drivers/net/ethernet/intel/ice/ice_ddp.c    |   3 +-
>   drivers/net/ethernet/intel/ice/ice_main.c   | 104 ++++++++++++++++----
>   3 files changed, 89 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 477a6b15100f..37f27b147122 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -1696,6 +1696,8 @@ ice_aq_send_cmd(struct ice_hw *hw, struct ice_aq_desc *desc, void *buf,
>   	case ice_aqc_opc_set_port_params:
>   	case ice_aqc_opc_get_vlan_mode_parameters:
>   	case ice_aqc_opc_set_vlan_mode_parameters:
> +	case ice_aqc_opc_set_tx_topo:
> +	case ice_aqc_opc_get_tx_topo:

Seems like this should be in patch 1?

>   	case ice_aqc_opc_add_recipe:
>   	case ice_aqc_opc_recipe_to_profile:
>   	case ice_aqc_opc_get_recipe:

[...]

> +/**
> + * ice_init_ddp_config - DDP related configuration
> + * @hw: pointer to the hardware structure
> + * @pf: pointer to pf structure
> + *
> + * This function loads DDP file from the disk, then initializes Tx
> + * topology. At the end DDP package is loaded on the card.
> + */
> +static int ice_init_ddp_config(struct ice_hw *hw, struct ice_pf *pf)
> +{
> +	struct device *dev = ice_pf_to_dev(pf);
> +	const struct firmware *firmware = NULL;
> +	int err;
> +
> +	err = ice_request_fw(pf, &firmware);
> +	if (err)
> +		return err;
> +
> +	err = ice_init_tx_topology(hw, firmware);
>   	if (err) {
> -		dev_err(dev, "The DDP package file was not found or could not be read. Entering Safe Mode\n");
> -		return;
> +		dev_err(dev, "ice_init_hw during change of tx topology failed: %d\n",
> +			err);
> +		release_firmware(firmware);
> +		return err;
>   	}
>   
> -	/* request for firmware was successful. Download to device */
> +	/* Download firmware to device */
>   	ice_load_pkg(firmware, pf);
>   	release_firmware(firmware);
> +
> +	return err;
>   }
>   
>   /**
> @@ -4726,9 +4792,9 @@ static int ice_init_dev(struct ice_pf *pf)
>   
>   	ice_init_feature_support(pf);
>   
> -	ice_request_fw(pf);
> +	ice_init_ddp_config(hw, pf);

This function returns a value, but it's not being used/checked. Should 
it be void or be used for the check below? I think former sounds better 
as checking safe mode, to set safe mode caps sounds safer :)

>   
> -	/* if ice_request_fw fails, ICE_FLAG_ADV_FEATURES bit won't be
> +	/* if ice_init_ddp_config fails, ICE_FLAG_ADV_FEATURES bit won't be
>   	 * set in pf->state, which will cause ice_is_safe_mode to return
>   	 * true
>   	 */