Re: [PATCH v5 20/21] drivers: adding type to idpf vc queue switch

[Bruce Richardson <bruce.richardson@intel.com>]

Hi,

When putting commit titles, checkpatch will complain if you use function
names, or macro/variable names. It does this by checking for underscores.
When this gets flagged, please rewrite the commit title to be more generic,
don't just remove the underscores from the function names!  I've fixed this
on the patches already applied to next-net-intel.

Thanks,
/Bruce

On Mon, Jun 24, 2024 at 09:16:43AM +0000, Soumyadeep Hore wrote:
> Adding an argument named type to define queue type
> in idpf_vc_queue_switch(). This solves the issue of
> improper queue type in virtchnl2 message.
> 
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---
>  drivers/common/idpf/idpf_common_virtchnl.c |  8 ++------
>  drivers/common/idpf/idpf_common_virtchnl.h |  2 +-
>  drivers/net/cpfl/cpfl_ethdev.c             | 12 ++++++++----
>  drivers/net/cpfl/cpfl_rxtx.c               | 12 ++++++++----
>  drivers/net/idpf/idpf_rxtx.c               | 12 ++++++++----
>  5 files changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/common/idpf/idpf_common_virtchnl.c b/drivers/common/idpf/idpf_common_virtchnl.c
> index f00202f43c..de511da788 100644
> --- a/drivers/common/idpf/idpf_common_virtchnl.c
> +++ b/drivers/common/idpf/idpf_common_virtchnl.c
> @@ -769,15 +769,11 @@ idpf_vc_ena_dis_one_queue(struct idpf_vport *vport, uint16_t qid,
>  
>  int
>  idpf_vc_queue_switch(struct idpf_vport *vport, uint16_t qid,
> -		     bool rx, bool on)
> +		     bool rx, bool on, uint32_t type)
>  {
> -	uint32_t type;
>  	int err, queue_id;
>  
> -	/* switch txq/rxq */
> -	type = rx ? VIRTCHNL2_QUEUE_TYPE_RX : VIRTCHNL2_QUEUE_TYPE_TX;
> -
> -	if (type == VIRTCHNL2_QUEUE_TYPE_RX)
> +	if (rx)
>  		queue_id = vport->chunks_info.rx_start_qid + qid;
>  	else
>  		queue_id = vport->chunks_info.tx_start_qid + qid;
> diff --git a/drivers/common/idpf/idpf_common_virtchnl.h b/drivers/common/idpf/idpf_common_virtchnl.h
> index 73446ded86..d6555978d5 100644
> --- a/drivers/common/idpf/idpf_common_virtchnl.h
> +++ b/drivers/common/idpf/idpf_common_virtchnl.h
> @@ -31,7 +31,7 @@ int idpf_vc_cmd_execute(struct idpf_adapter *adapter,
>  			struct idpf_cmd_info *args);
>  __rte_internal
>  int idpf_vc_queue_switch(struct idpf_vport *vport, uint16_t qid,
> -			 bool rx, bool on);
> +			 bool rx, bool on, uint32_t type);
>  __rte_internal
>  int idpf_vc_queues_ena_dis(struct idpf_vport *vport, bool enable);
>  __rte_internal
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index e707043bf7..9e2a74371e 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -1907,7 +1907,8 @@ cpfl_stop_cfgqs(struct cpfl_adapter_ext *adapter)
>  	int i, ret;
>  
>  	for (i = 0; i < CPFL_TX_CFGQ_NUM; i++) {
> -		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, false, false);
> +		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, false, false,
> +								VIRTCHNL2_QUEUE_TYPE_CONFIG_TX);
>  		if (ret) {
>  			PMD_DRV_LOG(ERR, "Fail to disable Tx config queue.");
>  			return ret;
> @@ -1915,7 +1916,8 @@ cpfl_stop_cfgqs(struct cpfl_adapter_ext *adapter)
>  	}
>  
>  	for (i = 0; i < CPFL_RX_CFGQ_NUM; i++) {
> -		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, true, false);
> +		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, true, false,
> +								VIRTCHNL2_QUEUE_TYPE_CONFIG_RX);
>  		if (ret) {
>  			PMD_DRV_LOG(ERR, "Fail to disable Rx config queue.");
>  			return ret;
> @@ -1943,7 +1945,8 @@ cpfl_start_cfgqs(struct cpfl_adapter_ext *adapter)
>  	}
>  
>  	for (i = 0; i < CPFL_TX_CFGQ_NUM; i++) {
> -		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, false, true);
> +		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, false, true,
> +								VIRTCHNL2_QUEUE_TYPE_CONFIG_TX);
>  		if (ret) {
>  			PMD_DRV_LOG(ERR, "Fail to enable Tx config queue.");
>  			return ret;
> @@ -1951,7 +1954,8 @@ cpfl_start_cfgqs(struct cpfl_adapter_ext *adapter)
>  	}
>  
>  	for (i = 0; i < CPFL_RX_CFGQ_NUM; i++) {
> -		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, true, true);
> +		ret = idpf_vc_queue_switch(&adapter->ctrl_vport.base, i, true, true,
> +								VIRTCHNL2_QUEUE_TYPE_CONFIG_RX);
>  		if (ret) {
>  			PMD_DRV_LOG(ERR, "Fail to enable Rx config queue.");
>  			return ret;
> diff --git a/drivers/net/cpfl/cpfl_rxtx.c b/drivers/net/cpfl/cpfl_rxtx.c
> index ab8bec4645..47351ca102 100644
> --- a/drivers/net/cpfl/cpfl_rxtx.c
> +++ b/drivers/net/cpfl/cpfl_rxtx.c
> @@ -1200,7 +1200,8 @@ cpfl_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>  	}
>  
>  	/* Ready to switch the queue on */
> -	err = idpf_vc_queue_switch(vport, rx_queue_id, true, true);
> +	err = idpf_vc_queue_switch(vport, rx_queue_id, true, true,
> +							VIRTCHNL2_QUEUE_TYPE_RX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch RX queue %u on",
>  			    rx_queue_id);
> @@ -1252,7 +1253,8 @@ cpfl_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
>  	}
>  
>  	/* Ready to switch the queue on */
> -	err = idpf_vc_queue_switch(vport, tx_queue_id, false, true);
> +	err = idpf_vc_queue_switch(vport, tx_queue_id, false, true,
> +							VIRTCHNL2_QUEUE_TYPE_TX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch TX queue %u on",
>  			    tx_queue_id);
> @@ -1283,7 +1285,8 @@ cpfl_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>  						     rx_queue_id - cpfl_vport->nb_data_txq,
>  						     true, false);
>  	else
> -		err = idpf_vc_queue_switch(vport, rx_queue_id, true, false);
> +		err = idpf_vc_queue_switch(vport, rx_queue_id, true, false,
> +								VIRTCHNL2_QUEUE_TYPE_RX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch RX queue %u off",
>  			    rx_queue_id);
> @@ -1331,7 +1334,8 @@ cpfl_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
>  						     tx_queue_id - cpfl_vport->nb_data_txq,
>  						     false, false);
>  	else
> -		err = idpf_vc_queue_switch(vport, tx_queue_id, false, false);
> +		err = idpf_vc_queue_switch(vport, tx_queue_id, false, false,
> +								VIRTCHNL2_QUEUE_TYPE_TX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch TX queue %u off",
>  			    tx_queue_id);
> diff --git a/drivers/net/idpf/idpf_rxtx.c b/drivers/net/idpf/idpf_rxtx.c
> index 64f2235580..858bbefe3b 100644
> --- a/drivers/net/idpf/idpf_rxtx.c
> +++ b/drivers/net/idpf/idpf_rxtx.c
> @@ -595,7 +595,8 @@ idpf_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>  	}
>  
>  	/* Ready to switch the queue on */
> -	err = idpf_vc_queue_switch(vport, rx_queue_id, true, true);
> +	err = idpf_vc_queue_switch(vport, rx_queue_id, true, true,
> +							VIRTCHNL2_QUEUE_TYPE_RX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch RX queue %u on",
>  			    rx_queue_id);
> @@ -646,7 +647,8 @@ idpf_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
>  	}
>  
>  	/* Ready to switch the queue on */
> -	err = idpf_vc_queue_switch(vport, tx_queue_id, false, true);
> +	err = idpf_vc_queue_switch(vport, tx_queue_id, false, true,
> +							VIRTCHNL2_QUEUE_TYPE_TX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch TX queue %u on",
>  			    tx_queue_id);
> @@ -669,7 +671,8 @@ idpf_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>  	if (rx_queue_id >= dev->data->nb_rx_queues)
>  		return -EINVAL;
>  
> -	err = idpf_vc_queue_switch(vport, rx_queue_id, true, false);
> +	err = idpf_vc_queue_switch(vport, rx_queue_id, true, false,
> +							VIRTCHNL2_QUEUE_TYPE_RX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch RX queue %u off",
>  			    rx_queue_id);
> @@ -701,7 +704,8 @@ idpf_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
>  	if (tx_queue_id >= dev->data->nb_tx_queues)
>  		return -EINVAL;
>  
> -	err = idpf_vc_queue_switch(vport, tx_queue_id, false, false);
> +	err = idpf_vc_queue_switch(vport, tx_queue_id, false, false,
> +							VIRTCHNL2_QUEUE_TYPE_TX);
>  	if (err != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to switch TX queue %u off",
>  			    tx_queue_id);
> -- 
> 2.43.0
>