Re: [dpdk-dev] [PATCH v2 3/9] bus/fslmc: keep Tx queues information for DPCI devices too

[Shreyansh Jain <shreyansh.jain@nxp.com>]

Hi Nipun,

Apologies for delay in review. 2 quick comments inline.

> -----Original Message-----
> From: Nipun Gupta
> Sent: Saturday, April 7, 2018 8:04 PM
> To: thomas@monjalon.net; Hemant Agrawal <hemant.agrawal@nxp.com>;
> Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: dev@dpdk.org; Nipun Gupta <nipun.gupta@nxp.com>
> Subject: [PATCH v2 3/9] bus/fslmc: keep Tx queues information for DPCI
> devices too
> 
> The DPCI devices have oth Tx and Rx queues. Event devices use
> DPCI Rx queues only, but CMDIF (AIOP) uses both Tx and Rx queues.
> This patch enables Tx queues configuration too.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
>  drivers/bus/fslmc/portal/dpaa2_hw_dpci.c | 57
> +++++++++++++++++++++++++-------
>  drivers/bus/fslmc/portal/dpaa2_hw_pvt.h  |  3 +-
>  drivers/event/dpaa2/dpaa2_eventdev.c     | 10 +++---
>  3 files changed, 52 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpci.c
> b/drivers/bus/fslmc/portal/dpaa2_hw_dpci.c
> index aee870a..3bf7e7f 100644
> --- a/drivers/bus/fslmc/portal/dpaa2_hw_dpci.c
> +++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpci.c
> @@ -39,6 +39,7 @@
>  	struct dpci_attr attr;
>  	struct dpci_rx_queue_cfg rx_queue_cfg;
>  	struct dpci_rx_queue_attr rx_attr;
> +	struct dpci_tx_queue_attr tx_attr;
>  	int ret, i;
> 
>  	/* Allocate DPAA2 dpci handle */
> @@ -67,17 +68,38 @@
>  		return -1;
>  	}
> 
> -	/* Set up the Rx Queue */
> -	memset(&rx_queue_cfg, 0, sizeof(struct dpci_rx_queue_cfg));
> -	ret = dpci_set_rx_queue(&dpci_node->dpci,
> -				CMD_PRI_LOW,
> -				dpci_node->token,
> -				0, &rx_queue_cfg);
> -	if (ret) {
> -		DPAA2_BUS_ERR("Setting Rx queue failed with err code: %d",
> -			      ret);
> -		rte_free(dpci_node);
> -		return -1;
> +	for (i = 0; i < DPAA2_DPCI_MAX_QUEUES; i++) {
> +		struct dpaa2_queue *rxq;
> +
> +		memset(&rx_queue_cfg, 0, sizeof(struct dpci_rx_queue_cfg));
> +		ret = dpci_set_rx_queue(&dpci_node->dpci,
> +					CMD_PRI_LOW,
> +					dpci_node->token,
> +					i, &rx_queue_cfg);
> +		if (ret) {
> +			DPAA2_BUS_ERR("Setting Rx queue failed with err code:
> %d",
> +				      ret);
> +			rte_free(dpci_node);
> +			return -1;
> +		}
> +
> +		/* Allocate DQ storage for the DPCI Rx queues */
> +		rxq = &(dpci_node->rx_queue[i]);
> +		rxq->q_storage = rte_malloc("dq_storage",
> +					sizeof(struct queue_storage_info_t),
> +					RTE_CACHE_LINE_SIZE);
> +		if (!rxq->q_storage) {
> +			DPAA2_BUS_ERR("q_storage allocation failed\n");
> +			rte_free(dpci_node);
> +			return -ENOMEM;
> +		}
> +
> +		memset(rxq->q_storage, 0, sizeof(struct
> queue_storage_info_t));
> +		if (dpaa2_alloc_dq_storage(rxq->q_storage)) {
> +			DPAA2_BUS_ERR("dpaa2_alloc_dq_storage failed\n");
> +			rte_free(dpci_node);

If the q_storage->dq_storage allocation has failed, q_storage too needs to be free'd.

> +			return -ENOMEM;
> +		}
>  	}
> 
>  	/* Enable the device */
> @@ -101,8 +123,19 @@
>  			rte_free(dpci_node);
>  			return -1;
>  		}
> +		dpci_node->rx_queue[i].fqid = rx_attr.fqid;
> 
> -		dpci_node->queue[i].fqid = rx_attr.fqid;
> +		ret = dpci_get_tx_queue(&dpci_node->dpci,
> +					CMD_PRI_LOW,
> +					dpci_node->token, i,
> +					&tx_attr);
> +		if (ret != 0) {
> +			DPAA2_BUS_ERR("Reading device failed with err code:"
> +				      " %d",ret);
> +			rte_free(dpci_node);

Maybe in this case as well where both, q_storage and dq_storage, need to be released.

> +			return -1;
> +		}
> +		dpci_node->tx_queue[i].fqid = tx_attr.fqid;
>  	}
> 
>  	dpci_node->dpci_id = dpci_id;

[...]