Message ID | 20210121071458.32732-1-ajit.khaparde@broadcom.com (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Ajit Khaparde |
Headers | show |
Series | [v3] net/bnxt: code refactoring changes | expand |
Context | Check | Description |
---|---|---|
ci/iol-testing | warning | Testing issues |
ci/iol-mellanox-Functional | success | Functional Testing PASS |
ci/iol-intel-Performance | success | Performance Testing PASS |
ci/iol-intel-Functional | success | Functional Testing PASS |
ci/iol-broadcom-Functional | success | Functional Testing PASS |
ci/intel-Testing | success | Testing PASS |
ci/iol-broadcom-Performance | success | Performance Testing PASS |
ci/Intel-compilation | success | Compilation OK |
ci/checkpatch | success | coding style OK |
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) >
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,