DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/bnxt: code refactoring changes
@ 2021-01-05  4:58 Somnath Kotur
  2021-01-14  2:19 ` [dpdk-dev] [PATCH v2] " Ajit Khaparde
  2021-01-21  7:14 ` [dpdk-dev] [PATCH v3] " Ajit Khaparde
  0 siblings, 2 replies; 4+ messages in thread
From: Somnath Kotur @ 2021-01-05  4:58 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable

Move all the individual driver fields allocation routines to one
routine - bnxt_drv_init(). This houses all such routines where
memory needs to be allocated once during the driver's lifetime
and does not need to be torn down during error recovery.
Rename some function names in accordance with their functionality.
bnxt_init_board() is doing nothing more than mapping the PCI bars,
so rename it as such.
Given that there is a bnxt_shutdown_nic that is called in dev_stop_op,
rename it's counterpart - bnxt_init_chip() that is called in dev_start_op,
to bnxt_start_nic. Also helps avoid confusion with some of the other
bnxt_init_xxx routines.
Rename bnxt_init_fw() to bnxt_get_config() as that is what that routine
is doing mostly functionality wise.

Cc: stable@dpdk.org
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 150 +++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 62 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index a6794a417d..3ecd1999c2 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -675,7 +675,7 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
 	return rc;
 }
 
-static int bnxt_init_chip(struct bnxt *bp)
+static int bnxt_start_nic(struct bnxt *bp)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
@@ -1396,7 +1396,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	bnxt_enable_int(bp);
 
-	rc = bnxt_init_chip(bp);
+	rc = bnxt_start_nic(bp);
 	if (rc)
 		goto error;
 
@@ -1443,6 +1443,27 @@ bnxt_uninit_locks(struct bnxt *bp)
 	}
 }
 
+static void bnxt_drv_uninit(struct bnxt *bp)
+{
+	bnxt_free_switch_domain(bp);
+	bnxt_free_leds_info(bp);
+	bnxt_free_cos_queues(bp);
+	bnxt_free_link_info(bp);
+	bnxt_free_pf_info(bp);
+	bnxt_free_parent_info(bp);
+	bnxt_uninit_locks(bp);
+
+	rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
+	bp->tx_mem_zone = NULL;
+	rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
+	bp->rx_mem_zone = NULL;
+
+	bnxt_hwrm_free_vf_info(bp);
+
+	rte_free(bp->grp_info);
+	bp->grp_info = NULL;
+}
+
 static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
@@ -1467,26 +1488,9 @@ static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 	if (eth_dev->data->dev_started)
 		ret = bnxt_dev_stop(eth_dev);
 
-	bnxt_free_switch_domain(bp);
-
 	bnxt_uninit_resources(bp, false);
 
-	bnxt_free_leds_info(bp);
-	bnxt_free_cos_queues(bp);
-	bnxt_free_link_info(bp);
-	bnxt_free_pf_info(bp);
-	bnxt_free_parent_info(bp);
-	bnxt_uninit_locks(bp);
-
-	rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
-	bp->tx_mem_zone = NULL;
-	rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
-	bp->rx_mem_zone = NULL;
-
-	bnxt_hwrm_free_vf_info(bp);
-
-	rte_free(bp->grp_info);
-	bp->grp_info = NULL;
+	bnxt_drv_uninit(bp);
 
 	return ret;
 }
@@ -4060,7 +4064,7 @@ bool bnxt_stratus_device(struct bnxt *bp)
 	}
 }
 
-static int bnxt_init_board(struct rte_eth_dev *eth_dev)
+static int bnxt_map_pci_bars(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct bnxt *bp = eth_dev->data->dev_private;
@@ -4685,7 +4689,11 @@ static int bnxt_map_hcomm_fw_status_reg(struct bnxt *bp)
 	return 0;
 }
 
-static int bnxt_init_fw(struct bnxt *bp)
+/* This function gets the FW version along with the
+ * capabilities(MAX and current) of the function, vnic,
+ * error recovery, phy and other chip related info
+ */
+static int bnxt_get_config(struct bnxt *bp)
 {
 	uint16_t mtu;
 	int rc = 0;
@@ -4777,7 +4785,7 @@ static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev)
 {
 	int rc = 0;
 
-	rc = bnxt_init_fw(bp);
+	rc = bnxt_get_config(bp);
 	if (rc)
 		return rc;
 
@@ -5224,38 +5232,14 @@ static int bnxt_alloc_switch_domain(struct bnxt *bp)
 	return rc;
 }
 
-static int
-bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
+/* Allocate and initialize various fields in bnxt struct that
+ * need to be allocated/destroyed only once in the lifetime of the driver
+ */
+static int bnxt_drv_init(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
-	static int version_printed;
-	struct bnxt *bp;
-	int rc;
-
-	if (version_printed++ == 0)
-		PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
-
-	eth_dev->dev_ops = &bnxt_dev_ops;
-	eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
-	eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
-	eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
-	eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
-	eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
-
-	/*
-	 * For secondary processes, we don't initialise any further
-	 * as primary has already done this work.
-	 */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return 0;
-
-	rte_eth_copy_pci_info(eth_dev, pci_dev);
-	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
-
-	bp = eth_dev->data->dev_private;
-
-	/* Parse dev arguments passed on when starting the DPDK application. */
-	bnxt_parse_dev_args(bp, pci_dev->device.devargs);
+	struct bnxt *bp = eth_dev->data->dev_private;
+	int rc = 0;
 
 	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 
@@ -5287,7 +5271,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 		}
 	}
 
-	rc = bnxt_init_board(eth_dev);
+	rc = bnxt_map_pci_bars(eth_dev);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
 			    "Failed to initialize board rc: %x\n", rc);
@@ -5296,31 +5280,75 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 
 	rc = bnxt_alloc_pf_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_link_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_parent_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_hwrm_resources(bp);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
 			    "Failed to allocate hwrm resource rc: %x\n", rc);
-		goto error_free;
+		return rc;
 	}
 	rc = bnxt_alloc_leds_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_cos_queues(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_init_locks(bp);
+	if (rc)
+		return rc;
+
+	rc = bnxt_alloc_switch_domain(bp);
+	if (rc)
+		return rc;
+
+	return rc;
+}
+
+static int
+bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
+{
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
+	static int version_printed;
+	struct bnxt *bp;
+	int rc;
+
+	if (version_printed++ == 0)
+		PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
+
+	eth_dev->dev_ops = &bnxt_dev_ops;
+	eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
+	eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
+	eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
+	eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
+	eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
+
+	/*
+	 * For secondary processes, we don't initialise any further
+	 * as primary has already done this work.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
+
+	bp = eth_dev->data->dev_private;
+
+	/* Parse dev arguments passed on when starting the DPDK application. */
+	bnxt_parse_dev_args(bp, pci_dev->device.devargs);
+
+	rc = bnxt_drv_init(eth_dev);
 	if (rc)
 		goto error_free;
 
@@ -5332,8 +5360,6 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 	if (rc)
 		goto error_free;
 
-	bnxt_alloc_switch_domain(bp);
-
 	PMD_DRV_LOG(INFO,
 		    DRV_MODULE_NAME "found at mem %" PRIX64 ", node addr %pM\n",
 		    pci_dev->mem_resource[0].phys_addr,
-- 
2.28.0.497.g54e85e7


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev] [PATCH v2] net/bnxt: code refactoring changes
  2021-01-05  4:58 [dpdk-dev] [PATCH] net/bnxt: code refactoring changes Somnath Kotur
@ 2021-01-14  2:19 ` Ajit Khaparde
  2021-01-21  7:14 ` [dpdk-dev] [PATCH v3] " Ajit Khaparde
  1 sibling, 0 replies; 4+ messages in thread
From: Ajit Khaparde @ 2021-01-14  2:19 UTC (permalink / raw)
  To: dev; +Cc: Somnath Kotur, stable

From: Somnath Kotur <somnath.kotur@broadcom.com>

Move all the individual driver fields allocation routines to one
routine - bnxt_drv_init(). This houses all such routines where
memory needs to be allocated once during the driver's lifetime
and does not need to be torn down during error recovery.
Rename some function names in accordance with their functionality.
bnxt_init_board() is doing nothing more than mapping the PCI bars,
so rename it as such.
Given that there is a bnxt_shutdown_nic that is called in dev_stop_op,
rename it's counterpart - bnxt_init_chip() that is called in dev_start_op,
to bnxt_start_nic. Also helps avoid confusion with some of the other
bnxt_init_xxx routines.
Rename bnxt_init_fw() to bnxt_get_config() as that is what that routine
is doing mostly functionality wise.

Cc: stable@dpdk.org
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
--
v1->v2: rebased against latest dpdk-next-net.

---
 drivers/net/bnxt/bnxt_ethdev.c | 150 +++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 62 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 74b0f3d1d..f439aeee4 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -679,7 +679,7 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
 	return rc;
 }
 
-static int bnxt_init_chip(struct bnxt *bp)
+static int bnxt_start_nic(struct bnxt *bp)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
@@ -1417,7 +1417,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	bnxt_enable_int(bp);
 
-	rc = bnxt_init_chip(bp);
+	rc = bnxt_start_nic(bp);
 	if (rc)
 		goto error;
 
@@ -1464,6 +1464,27 @@ bnxt_uninit_locks(struct bnxt *bp)
 	}
 }
 
+static void bnxt_drv_uninit(struct bnxt *bp)
+{
+	bnxt_free_switch_domain(bp);
+	bnxt_free_leds_info(bp);
+	bnxt_free_cos_queues(bp);
+	bnxt_free_link_info(bp);
+	bnxt_free_pf_info(bp);
+	bnxt_free_parent_info(bp);
+	bnxt_uninit_locks(bp);
+
+	rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
+	bp->tx_mem_zone = NULL;
+	rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
+	bp->rx_mem_zone = NULL;
+
+	bnxt_hwrm_free_vf_info(bp);
+
+	rte_free(bp->grp_info);
+	bp->grp_info = NULL;
+}
+
 static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
@@ -1488,26 +1509,9 @@ static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 	if (eth_dev->data->dev_started)
 		ret = bnxt_dev_stop(eth_dev);
 
-	bnxt_free_switch_domain(bp);
-
 	bnxt_uninit_resources(bp, false);
 
-	bnxt_free_leds_info(bp);
-	bnxt_free_cos_queues(bp);
-	bnxt_free_link_info(bp);
-	bnxt_free_pf_info(bp);
-	bnxt_free_parent_info(bp);
-	bnxt_uninit_locks(bp);
-
-	rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
-	bp->tx_mem_zone = NULL;
-	rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
-	bp->rx_mem_zone = NULL;
-
-	bnxt_hwrm_free_vf_info(bp);
-
-	rte_free(bp->grp_info);
-	bp->grp_info = NULL;
+	bnxt_drv_uninit(bp);
 
 	return ret;
 }
@@ -4086,7 +4090,7 @@ bool bnxt_stratus_device(struct bnxt *bp)
 	}
 }
 
-static int bnxt_init_board(struct rte_eth_dev *eth_dev)
+static int bnxt_map_pci_bars(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct bnxt *bp = eth_dev->data->dev_private;
@@ -4723,7 +4727,11 @@ static int bnxt_map_hcomm_fw_status_reg(struct bnxt *bp)
 	return 0;
 }
 
-static int bnxt_init_fw(struct bnxt *bp)
+/* This function gets the FW version along with the
+ * capabilities(MAX and current) of the function, vnic,
+ * error recovery, phy and other chip related info
+ */
+static int bnxt_get_config(struct bnxt *bp)
 {
 	uint16_t mtu;
 	int rc = 0;
@@ -4819,7 +4827,7 @@ static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev)
 {
 	int rc = 0;
 
-	rc = bnxt_init_fw(bp);
+	rc = bnxt_get_config(bp);
 	if (rc)
 		return rc;
 
@@ -5266,38 +5274,14 @@ static int bnxt_alloc_switch_domain(struct bnxt *bp)
 	return rc;
 }
 
-static int
-bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
+/* Allocate and initialize various fields in bnxt struct that
+ * need to be allocated/destroyed only once in the lifetime of the driver
+ */
+static int bnxt_drv_init(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
-	static int version_printed;
-	struct bnxt *bp;
-	int rc;
-
-	if (version_printed++ == 0)
-		PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
-
-	eth_dev->dev_ops = &bnxt_dev_ops;
-	eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
-	eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
-	eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
-	eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
-	eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
-
-	/*
-	 * For secondary processes, we don't initialise any further
-	 * as primary has already done this work.
-	 */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return 0;
-
-	rte_eth_copy_pci_info(eth_dev, pci_dev);
-	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
-
-	bp = eth_dev->data->dev_private;
-
-	/* Parse dev arguments passed on when starting the DPDK application. */
-	bnxt_parse_dev_args(bp, pci_dev->device.devargs);
+	struct bnxt *bp = eth_dev->data->dev_private;
+	int rc = 0;
 
 	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 
@@ -5329,7 +5313,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 		}
 	}
 
-	rc = bnxt_init_board(eth_dev);
+	rc = bnxt_map_pci_bars(eth_dev);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
 			    "Failed to initialize board rc: %x\n", rc);
@@ -5338,31 +5322,75 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 
 	rc = bnxt_alloc_pf_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_link_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_parent_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_hwrm_resources(bp);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
 			    "Failed to allocate hwrm resource rc: %x\n", rc);
-		goto error_free;
+		return rc;
 	}
 	rc = bnxt_alloc_leds_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_cos_queues(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_init_locks(bp);
+	if (rc)
+		return rc;
+
+	rc = bnxt_alloc_switch_domain(bp);
+	if (rc)
+		return rc;
+
+	return rc;
+}
+
+static int
+bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
+{
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
+	static int version_printed;
+	struct bnxt *bp;
+	int rc;
+
+	if (version_printed++ == 0)
+		PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
+
+	eth_dev->dev_ops = &bnxt_dev_ops;
+	eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
+	eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
+	eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
+	eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
+	eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
+
+	/*
+	 * For secondary processes, we don't initialise any further
+	 * as primary has already done this work.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
+
+	bp = eth_dev->data->dev_private;
+
+	/* Parse dev arguments passed on when starting the DPDK application. */
+	bnxt_parse_dev_args(bp, pci_dev->device.devargs);
+
+	rc = bnxt_drv_init(eth_dev);
 	if (rc)
 		goto error_free;
 
@@ -5374,8 +5402,6 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 	if (rc)
 		goto error_free;
 
-	bnxt_alloc_switch_domain(bp);
-
 	PMD_DRV_LOG(INFO,
 		    DRV_MODULE_NAME "found at mem %" PRIX64 ", node addr %pM\n",
 		    pci_dev->mem_resource[0].phys_addr,
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev] [PATCH v3] net/bnxt: code refactoring changes
  2021-01-05  4:58 [dpdk-dev] [PATCH] net/bnxt: code refactoring changes Somnath Kotur
  2021-01-14  2:19 ` [dpdk-dev] [PATCH v2] " Ajit Khaparde
@ 2021-01-21  7:14 ` Ajit Khaparde
  2021-01-21 16:02   ` Ajit Khaparde
  1 sibling, 1 reply; 4+ messages in thread
From: Ajit Khaparde @ 2021-01-21  7:14 UTC (permalink / raw)
  To: dev; +Cc: Somnath Kotur, stable

From: Somnath Kotur <somnath.kotur@broadcom.com>

Move all the individual driver fields allocation routines to one
routine - bnxt_drv_init(). This houses all such routines where
memory needs to be allocated once during the driver's lifetime
and does not need to be torn down during error recovery.
Rename some function names in accordance with their functionality.
bnxt_init_board() is doing nothing more than mapping the PCI bars,
so rename it as such.
Given that there is a bnxt_shutdown_nic that is called in dev_stop_op,
rename it's counterpart - bnxt_init_chip() that is called in dev_start_op,
to bnxt_start_nic. Also helps avoid confusion with some of the other
bnxt_init_xxx routines.
Rename bnxt_init_fw() to bnxt_get_config() as that is what that routine
is doing mostly functionality wise.

Cc: stable@dpdk.org
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
v1->v2: rebased against latest dpdk-next-net.
v2->v3: rebased against latest dpdk-next-net.
---
 drivers/net/bnxt/bnxt_ethdev.c | 150 +++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 62 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 74b0f3d1d..f439aeee4 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -679,7 +679,7 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
 	return rc;
 }
 
-static int bnxt_init_chip(struct bnxt *bp)
+static int bnxt_start_nic(struct bnxt *bp)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
@@ -1417,7 +1417,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	bnxt_enable_int(bp);
 
-	rc = bnxt_init_chip(bp);
+	rc = bnxt_start_nic(bp);
 	if (rc)
 		goto error;
 
@@ -1464,6 +1464,27 @@ bnxt_uninit_locks(struct bnxt *bp)
 	}
 }
 
+static void bnxt_drv_uninit(struct bnxt *bp)
+{
+	bnxt_free_switch_domain(bp);
+	bnxt_free_leds_info(bp);
+	bnxt_free_cos_queues(bp);
+	bnxt_free_link_info(bp);
+	bnxt_free_pf_info(bp);
+	bnxt_free_parent_info(bp);
+	bnxt_uninit_locks(bp);
+
+	rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
+	bp->tx_mem_zone = NULL;
+	rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
+	bp->rx_mem_zone = NULL;
+
+	bnxt_hwrm_free_vf_info(bp);
+
+	rte_free(bp->grp_info);
+	bp->grp_info = NULL;
+}
+
 static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
@@ -1488,26 +1509,9 @@ static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 	if (eth_dev->data->dev_started)
 		ret = bnxt_dev_stop(eth_dev);
 
-	bnxt_free_switch_domain(bp);
-
 	bnxt_uninit_resources(bp, false);
 
-	bnxt_free_leds_info(bp);
-	bnxt_free_cos_queues(bp);
-	bnxt_free_link_info(bp);
-	bnxt_free_pf_info(bp);
-	bnxt_free_parent_info(bp);
-	bnxt_uninit_locks(bp);
-
-	rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
-	bp->tx_mem_zone = NULL;
-	rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
-	bp->rx_mem_zone = NULL;
-
-	bnxt_hwrm_free_vf_info(bp);
-
-	rte_free(bp->grp_info);
-	bp->grp_info = NULL;
+	bnxt_drv_uninit(bp);
 
 	return ret;
 }
@@ -4086,7 +4090,7 @@ bool bnxt_stratus_device(struct bnxt *bp)
 	}
 }
 
-static int bnxt_init_board(struct rte_eth_dev *eth_dev)
+static int bnxt_map_pci_bars(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct bnxt *bp = eth_dev->data->dev_private;
@@ -4723,7 +4727,11 @@ static int bnxt_map_hcomm_fw_status_reg(struct bnxt *bp)
 	return 0;
 }
 
-static int bnxt_init_fw(struct bnxt *bp)
+/* This function gets the FW version along with the
+ * capabilities(MAX and current) of the function, vnic,
+ * error recovery, phy and other chip related info
+ */
+static int bnxt_get_config(struct bnxt *bp)
 {
 	uint16_t mtu;
 	int rc = 0;
@@ -4819,7 +4827,7 @@ static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev)
 {
 	int rc = 0;
 
-	rc = bnxt_init_fw(bp);
+	rc = bnxt_get_config(bp);
 	if (rc)
 		return rc;
 
@@ -5266,38 +5274,14 @@ static int bnxt_alloc_switch_domain(struct bnxt *bp)
 	return rc;
 }
 
-static int
-bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
+/* Allocate and initialize various fields in bnxt struct that
+ * need to be allocated/destroyed only once in the lifetime of the driver
+ */
+static int bnxt_drv_init(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
-	static int version_printed;
-	struct bnxt *bp;
-	int rc;
-
-	if (version_printed++ == 0)
-		PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
-
-	eth_dev->dev_ops = &bnxt_dev_ops;
-	eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
-	eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
-	eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
-	eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
-	eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
-
-	/*
-	 * For secondary processes, we don't initialise any further
-	 * as primary has already done this work.
-	 */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return 0;
-
-	rte_eth_copy_pci_info(eth_dev, pci_dev);
-	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
-
-	bp = eth_dev->data->dev_private;
-
-	/* Parse dev arguments passed on when starting the DPDK application. */
-	bnxt_parse_dev_args(bp, pci_dev->device.devargs);
+	struct bnxt *bp = eth_dev->data->dev_private;
+	int rc = 0;
 
 	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 
@@ -5329,7 +5313,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 		}
 	}
 
-	rc = bnxt_init_board(eth_dev);
+	rc = bnxt_map_pci_bars(eth_dev);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
 			    "Failed to initialize board rc: %x\n", rc);
@@ -5338,31 +5322,75 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 
 	rc = bnxt_alloc_pf_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_link_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_parent_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_hwrm_resources(bp);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
 			    "Failed to allocate hwrm resource rc: %x\n", rc);
-		goto error_free;
+		return rc;
 	}
 	rc = bnxt_alloc_leds_info(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_alloc_cos_queues(bp);
 	if (rc)
-		goto error_free;
+		return rc;
 
 	rc = bnxt_init_locks(bp);
+	if (rc)
+		return rc;
+
+	rc = bnxt_alloc_switch_domain(bp);
+	if (rc)
+		return rc;
+
+	return rc;
+}
+
+static int
+bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
+{
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
+	static int version_printed;
+	struct bnxt *bp;
+	int rc;
+
+	if (version_printed++ == 0)
+		PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
+
+	eth_dev->dev_ops = &bnxt_dev_ops;
+	eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
+	eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
+	eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
+	eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
+	eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
+
+	/*
+	 * For secondary processes, we don't initialise any further
+	 * as primary has already done this work.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
+	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
+
+	bp = eth_dev->data->dev_private;
+
+	/* Parse dev arguments passed on when starting the DPDK application. */
+	bnxt_parse_dev_args(bp, pci_dev->device.devargs);
+
+	rc = bnxt_drv_init(eth_dev);
 	if (rc)
 		goto error_free;
 
@@ -5374,8 +5402,6 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
 	if (rc)
 		goto error_free;
 
-	bnxt_alloc_switch_domain(bp);
-
 	PMD_DRV_LOG(INFO,
 		    DRV_MODULE_NAME "found at mem %" PRIX64 ", node addr %pM\n",
 		    pci_dev->mem_resource[0].phys_addr,
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/bnxt: code refactoring changes
  2021-01-21  7:14 ` [dpdk-dev] [PATCH v3] " Ajit Khaparde
@ 2021-01-21 16:02   ` Ajit Khaparde
  0 siblings, 0 replies; 4+ messages in thread
From: Ajit Khaparde @ 2021-01-21 16:02 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: dpdk-dev, Somnath Kotur, dpdk stable

[-- Attachment #1: Type: text/plain, Size: 9810 bytes --]

On Wed, Jan 20, 2021 at 11:15 PM Ajit Khaparde <ajitkhaparde@gmail.com> wrote:
>
> From: Somnath Kotur <somnath.kotur@broadcom.com>
>
> Move all the individual driver fields allocation routines to one
> routine - bnxt_drv_init(). This houses all such routines where
> memory needs to be allocated once during the driver's lifetime
> and does not need to be torn down during error recovery.
> Rename some function names in accordance with their functionality.
> bnxt_init_board() is doing nothing more than mapping the PCI bars,
> so rename it as such.
> Given that there is a bnxt_shutdown_nic that is called in dev_stop_op,
> rename it's counterpart - bnxt_init_chip() that is called in dev_start_op,
> to bnxt_start_nic. Also helps avoid confusion with some of the other
> bnxt_init_xxx routines.
> Rename bnxt_init_fw() to bnxt_get_config() as that is what that routine
> is doing mostly functionality wise.
>
> Cc: stable@dpdk.org
> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Patch applied to dpdk-next-net-brcm.

> ---
> v1->v2: rebased against latest dpdk-next-net.
> v2->v3: rebased against latest dpdk-next-net.
> ---
>  drivers/net/bnxt/bnxt_ethdev.c | 150 +++++++++++++++++++--------------
>  1 file changed, 88 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 74b0f3d1d..f439aeee4 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -679,7 +679,7 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
>         return rc;
>  }
>
> -static int bnxt_init_chip(struct bnxt *bp)
> +static int bnxt_start_nic(struct bnxt *bp)
>  {
>         struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
>         struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
> @@ -1417,7 +1417,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
>
>         bnxt_enable_int(bp);
>
> -       rc = bnxt_init_chip(bp);
> +       rc = bnxt_start_nic(bp);
>         if (rc)
>                 goto error;
>
> @@ -1464,6 +1464,27 @@ bnxt_uninit_locks(struct bnxt *bp)
>         }
>  }
>
> +static void bnxt_drv_uninit(struct bnxt *bp)
> +{
> +       bnxt_free_switch_domain(bp);
> +       bnxt_free_leds_info(bp);
> +       bnxt_free_cos_queues(bp);
> +       bnxt_free_link_info(bp);
> +       bnxt_free_pf_info(bp);
> +       bnxt_free_parent_info(bp);
> +       bnxt_uninit_locks(bp);
> +
> +       rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
> +       bp->tx_mem_zone = NULL;
> +       rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
> +       bp->rx_mem_zone = NULL;
> +
> +       bnxt_hwrm_free_vf_info(bp);
> +
> +       rte_free(bp->grp_info);
> +       bp->grp_info = NULL;
> +}
> +
>  static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
>  {
>         struct bnxt *bp = eth_dev->data->dev_private;
> @@ -1488,26 +1509,9 @@ static int bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
>         if (eth_dev->data->dev_started)
>                 ret = bnxt_dev_stop(eth_dev);
>
> -       bnxt_free_switch_domain(bp);
> -
>         bnxt_uninit_resources(bp, false);
>
> -       bnxt_free_leds_info(bp);
> -       bnxt_free_cos_queues(bp);
> -       bnxt_free_link_info(bp);
> -       bnxt_free_pf_info(bp);
> -       bnxt_free_parent_info(bp);
> -       bnxt_uninit_locks(bp);
> -
> -       rte_memzone_free((const struct rte_memzone *)bp->tx_mem_zone);
> -       bp->tx_mem_zone = NULL;
> -       rte_memzone_free((const struct rte_memzone *)bp->rx_mem_zone);
> -       bp->rx_mem_zone = NULL;
> -
> -       bnxt_hwrm_free_vf_info(bp);
> -
> -       rte_free(bp->grp_info);
> -       bp->grp_info = NULL;
> +       bnxt_drv_uninit(bp);
>
>         return ret;
>  }
> @@ -4086,7 +4090,7 @@ bool bnxt_stratus_device(struct bnxt *bp)
>         }
>  }
>
> -static int bnxt_init_board(struct rte_eth_dev *eth_dev)
> +static int bnxt_map_pci_bars(struct rte_eth_dev *eth_dev)
>  {
>         struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
>         struct bnxt *bp = eth_dev->data->dev_private;
> @@ -4723,7 +4727,11 @@ static int bnxt_map_hcomm_fw_status_reg(struct bnxt *bp)
>         return 0;
>  }
>
> -static int bnxt_init_fw(struct bnxt *bp)
> +/* This function gets the FW version along with the
> + * capabilities(MAX and current) of the function, vnic,
> + * error recovery, phy and other chip related info
> + */
> +static int bnxt_get_config(struct bnxt *bp)
>  {
>         uint16_t mtu;
>         int rc = 0;
> @@ -4819,7 +4827,7 @@ static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev)
>  {
>         int rc = 0;
>
> -       rc = bnxt_init_fw(bp);
> +       rc = bnxt_get_config(bp);
>         if (rc)
>                 return rc;
>
> @@ -5266,38 +5274,14 @@ static int bnxt_alloc_switch_domain(struct bnxt *bp)
>         return rc;
>  }
>
> -static int
> -bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
> +/* Allocate and initialize various fields in bnxt struct that
> + * need to be allocated/destroyed only once in the lifetime of the driver
> + */
> +static int bnxt_drv_init(struct rte_eth_dev *eth_dev)
>  {
>         struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> -       static int version_printed;
> -       struct bnxt *bp;
> -       int rc;
> -
> -       if (version_printed++ == 0)
> -               PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
> -
> -       eth_dev->dev_ops = &bnxt_dev_ops;
> -       eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
> -       eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
> -       eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
> -       eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
> -       eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
> -
> -       /*
> -        * For secondary processes, we don't initialise any further
> -        * as primary has already done this work.
> -        */
> -       if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> -               return 0;
> -
> -       rte_eth_copy_pci_info(eth_dev, pci_dev);
> -       eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> -
> -       bp = eth_dev->data->dev_private;
> -
> -       /* Parse dev arguments passed on when starting the DPDK application. */
> -       bnxt_parse_dev_args(bp, pci_dev->device.devargs);
> +       struct bnxt *bp = eth_dev->data->dev_private;
> +       int rc = 0;
>
>         bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
>
> @@ -5329,7 +5313,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
>                 }
>         }
>
> -       rc = bnxt_init_board(eth_dev);
> +       rc = bnxt_map_pci_bars(eth_dev);
>         if (rc) {
>                 PMD_DRV_LOG(ERR,
>                             "Failed to initialize board rc: %x\n", rc);
> @@ -5338,31 +5322,75 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
>
>         rc = bnxt_alloc_pf_info(bp);
>         if (rc)
> -               goto error_free;
> +               return rc;
>
>         rc = bnxt_alloc_link_info(bp);
>         if (rc)
> -               goto error_free;
> +               return rc;
>
>         rc = bnxt_alloc_parent_info(bp);
>         if (rc)
> -               goto error_free;
> +               return rc;
>
>         rc = bnxt_alloc_hwrm_resources(bp);
>         if (rc) {
>                 PMD_DRV_LOG(ERR,
>                             "Failed to allocate hwrm resource rc: %x\n", rc);
> -               goto error_free;
> +               return rc;
>         }
>         rc = bnxt_alloc_leds_info(bp);
>         if (rc)
> -               goto error_free;
> +               return rc;
>
>         rc = bnxt_alloc_cos_queues(bp);
>         if (rc)
> -               goto error_free;
> +               return rc;
>
>         rc = bnxt_init_locks(bp);
> +       if (rc)
> +               return rc;
> +
> +       rc = bnxt_alloc_switch_domain(bp);
> +       if (rc)
> +               return rc;
> +
> +       return rc;
> +}
> +
> +static int
> +bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
> +{
> +       struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> +       static int version_printed;
> +       struct bnxt *bp;
> +       int rc;
> +
> +       if (version_printed++ == 0)
> +               PMD_DRV_LOG(INFO, "%s\n", bnxt_version);
> +
> +       eth_dev->dev_ops = &bnxt_dev_ops;
> +       eth_dev->rx_queue_count = bnxt_rx_queue_count_op;
> +       eth_dev->rx_descriptor_status = bnxt_rx_descriptor_status_op;
> +       eth_dev->tx_descriptor_status = bnxt_tx_descriptor_status_op;
> +       eth_dev->rx_pkt_burst = &bnxt_recv_pkts;
> +       eth_dev->tx_pkt_burst = &bnxt_xmit_pkts;
> +
> +       /*
> +        * For secondary processes, we don't initialise any further
> +        * as primary has already done this work.
> +        */
> +       if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +               return 0;
> +
> +       rte_eth_copy_pci_info(eth_dev, pci_dev);
> +       eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
> +
> +       bp = eth_dev->data->dev_private;
> +
> +       /* Parse dev arguments passed on when starting the DPDK application. */
> +       bnxt_parse_dev_args(bp, pci_dev->device.devargs);
> +
> +       rc = bnxt_drv_init(eth_dev);
>         if (rc)
>                 goto error_free;
>
> @@ -5374,8 +5402,6 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void *params __rte_unused)
>         if (rc)
>                 goto error_free;
>
> -       bnxt_alloc_switch_domain(bp);
> -
>         PMD_DRV_LOG(INFO,
>                     DRV_MODULE_NAME "found at mem %" PRIX64 ", node addr %pM\n",
>                     pci_dev->mem_resource[0].phys_addr,
> --
> 2.21.1 (Apple Git-122.3)
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-21 16:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05  4:58 [dpdk-dev] [PATCH] net/bnxt: code refactoring changes Somnath Kotur
2021-01-14  2:19 ` [dpdk-dev] [PATCH v2] " Ajit Khaparde
2021-01-21  7:14 ` [dpdk-dev] [PATCH v3] " Ajit Khaparde
2021-01-21 16:02   ` Ajit Khaparde

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git