[dpdk-dev,03/38] net/nfp: use library function for DMA zone reserve

Message ID 1488794430-25179-4-git-send-email-jblunck@infradead.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Jan Blunck March 6, 2017, 9:59 a.m. UTC
  This driver can use the library function rte_eth_dma_zone_reserve()
instead of duplicating the code.

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 drivers/net/nfp/nfp_net.c | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)
  

Comments

Shreyansh Jain March 10, 2017, 7:03 a.m. UTC | #1
Hello Jan,

On Monday 06 March 2017 03:29 PM, Jan Blunck wrote:
> This driver can use the library function rte_eth_dma_zone_reserve()
> instead of duplicating the code.
>
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> ---
>  drivers/net/nfp/nfp_net.c | 30 ++++++------------------------
>  1 file changed, 6 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index d79f262..b9dfe80 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -205,26 +205,6 @@ nn_cfg_writeq(struct nfp_net_hw *hw, int off, uint64_t val)
>  	nn_writeq(rte_cpu_to_le_64(val), hw->ctrl_bar + off);
>  }
>
> -/* Creating memzone for hardware rings. */
> -static const struct rte_memzone *
> -ring_dma_zone_reserve(struct rte_eth_dev *dev, const char *ring_name,
> -		      uint16_t queue_id, uint32_t ring_size, int socket_id)
> -{
> -	char z_name[RTE_MEMZONE_NAMESIZE];
> -	const struct rte_memzone *mz;
> -
> -	snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> -		 dev->driver->pci_drv.driver.name,
> -		 ring_name, dev->data->port_id, queue_id);
> -
> -	mz = rte_memzone_lookup(z_name);
> -	if (mz)
> -		return mz;
> -
> -	return rte_memzone_reserve_aligned(z_name, ring_size, socket_id, 0,
> -					   NFP_MEMZONE_ALIGN);
> -}
> -
>  /*
>   * Atomically reads link status information from global structure rte_eth_dev.
>   *
> @@ -1461,9 +1441,10 @@ nfp_net_rx_queue_setup(struct rte_eth_dev *dev,
>  	 * handle the maximum ring size is allocated in order to allow for
>  	 * resizing in later calls to the queue setup function.
>  	 */
> -	tz = ring_dma_zone_reserve(dev, "rx_ring", queue_idx,
> +	tz = rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
>  				   sizeof(struct nfp_net_rx_desc) *
> -				   NFP_NET_MAX_RX_DESC, socket_id);
> +				   NFP_NET_MAX_RX_DESC, NFP_MEMZONE_ALIGN,
> +				   socket_id);
>
>  	if (tz == NULL) {
>  		RTE_LOG(ERR, PMD, "Error allocatig rx dma\n");
> @@ -1603,9 +1584,10 @@ nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
>  	 * handle the maximum ring size is allocated in order to allow for
>  	 * resizing in later calls to the queue setup function.
>  	 */
> -	tz = ring_dma_zone_reserve(dev, "tx_ring", queue_idx,
> +	tz = rte_eth_dma_zone_reserve(dev, "tx_ring", queue_idx,
>  				   sizeof(struct nfp_net_tx_desc) *
> -				   NFP_NET_MAX_TX_DESC, socket_id);
> +				   NFP_NET_MAX_TX_DESC, NFP_MEMZONE_ALIGN,
> +				   socket_id);
>  	if (tz == NULL) {
>  		RTE_LOG(ERR, PMD, "Error allocating tx dma\n");
>  		nfp_net_tx_queue_release(txq);
>

This change is not part of the eth_driver removal process. Isn't it?

I would suggest this should be a separate series all together. This is
valid for Patch 0004 as well.
  
Jan Blunck March 10, 2017, 7:20 a.m. UTC | #2
On Fri, Mar 10, 2017 at 8:03 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> On Monday 06 March 2017 03:29 PM, Jan Blunck wrote:
>>
>> -/* Creating memzone for hardware rings. */
>> -static const struct rte_memzone *
>> -ring_dma_zone_reserve(struct rte_eth_dev *dev, const char *ring_name,
>> -                     uint16_t queue_id, uint32_t ring_size, int
>> socket_id)
>> -{
>> -       char z_name[RTE_MEMZONE_NAMESIZE];
>> -       const struct rte_memzone *mz;
>> -
>> -       snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
>> -                dev->driver->pci_drv.driver.name,
>> -                ring_name, dev->data->port_id, queue_id);
>> -
>> -       mz = rte_memzone_lookup(z_name);
>> -       if (mz)
>> -               return mz;
>> -
>> -       return rte_memzone_reserve_aligned(z_name, ring_size, socket_id,
>> 0,
>> -                                          NFP_MEMZONE_ALIGN);
>> -}
>> -
>>
>
> This change is not part of the eth_driver removal process. Isn't it?
>
> I would suggest this should be a separate series all together. This is
> valid for Patch 0004 as well.

It is removing a dependency on eth_driver (see
dev->driver->pci_drv.driver.name). Therefore even if I separate it
this series would depend on it.

Thanks for reviewing,
Jan
  

Patch

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index d79f262..b9dfe80 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -205,26 +205,6 @@  nn_cfg_writeq(struct nfp_net_hw *hw, int off, uint64_t val)
 	nn_writeq(rte_cpu_to_le_64(val), hw->ctrl_bar + off);
 }
 
-/* Creating memzone for hardware rings. */
-static const struct rte_memzone *
-ring_dma_zone_reserve(struct rte_eth_dev *dev, const char *ring_name,
-		      uint16_t queue_id, uint32_t ring_size, int socket_id)
-{
-	char z_name[RTE_MEMZONE_NAMESIZE];
-	const struct rte_memzone *mz;
-
-	snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
-		 dev->driver->pci_drv.driver.name,
-		 ring_name, dev->data->port_id, queue_id);
-
-	mz = rte_memzone_lookup(z_name);
-	if (mz)
-		return mz;
-
-	return rte_memzone_reserve_aligned(z_name, ring_size, socket_id, 0,
-					   NFP_MEMZONE_ALIGN);
-}
-
 /*
  * Atomically reads link status information from global structure rte_eth_dev.
  *
@@ -1461,9 +1441,10 @@  nfp_net_rx_queue_setup(struct rte_eth_dev *dev,
 	 * handle the maximum ring size is allocated in order to allow for
 	 * resizing in later calls to the queue setup function.
 	 */
-	tz = ring_dma_zone_reserve(dev, "rx_ring", queue_idx,
+	tz = rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx,
 				   sizeof(struct nfp_net_rx_desc) *
-				   NFP_NET_MAX_RX_DESC, socket_id);
+				   NFP_NET_MAX_RX_DESC, NFP_MEMZONE_ALIGN,
+				   socket_id);
 
 	if (tz == NULL) {
 		RTE_LOG(ERR, PMD, "Error allocatig rx dma\n");
@@ -1603,9 +1584,10 @@  nfp_net_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 	 * handle the maximum ring size is allocated in order to allow for
 	 * resizing in later calls to the queue setup function.
 	 */
-	tz = ring_dma_zone_reserve(dev, "tx_ring", queue_idx,
+	tz = rte_eth_dma_zone_reserve(dev, "tx_ring", queue_idx,
 				   sizeof(struct nfp_net_tx_desc) *
-				   NFP_NET_MAX_TX_DESC, socket_id);
+				   NFP_NET_MAX_TX_DESC, NFP_MEMZONE_ALIGN,
+				   socket_id);
 	if (tz == NULL) {
 		RTE_LOG(ERR, PMD, "Error allocating tx dma\n");
 		nfp_net_tx_queue_release(txq);