| Message ID | 20230717170001.30539-3-shannon.nelson@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ionic: add FLR support | expand |
On Mon, Jul 17, 2023 at 09:59:59AM -0700, Shannon Nelson wrote: > Pull out some chunks of code from ionic_probe() that will > be common in rebuild paths. > > Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> ... > +static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > +{ > + struct device *dev = &pdev->dev; > + struct ionic *ionic; > + int num_vfs; > + int err; > + > + ionic = ionic_devlink_alloc(dev); > + if (!ionic) > + return -ENOMEM; > + > + ionic->pdev = pdev; > + ionic->dev = dev; > + pci_set_drvdata(pdev, ionic); > + mutex_init(&ionic->dev_cmd_lock); > + > + /* Query system for DMA addressing limitation for the device. */ > + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN)); > + if (err) { > + dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting. err=%d\n", > + err); > + goto err_out; > + } > + > + err = ionic_setup_one(ionic); > + if (err) > + goto err_out; > + > /* Allocate and init the LIF */ > err = ionic_lif_size(ionic); > if (err) { > dev_err(dev, "Cannot size LIF: %d, aborting\n", err); > - goto err_out_port_reset; > + goto err_out_pci; Hi Shannon, Prior to this patch, if this error occurred then the following cleanup would occur. ionic_port_reset(ionic); ionic_reset(ionic); ionic_dev_teardown(ionic); ionic_clear_pci(ionic); ionic_debugfs_del_dev(ionic); mutex_destroy(&ionic->dev_cmd_lock); ionic_devlink_free(ionic); With this patch I am assuming that the same setup has occurred at this point (maybe I am mistaken). But with the following cleanup on error. ionic_clear_pci(ionic); mutex_destroy(&ionic->dev_cmd_lock); ionic_devlink_free(ionic); I feel that I'm reading this wrong. > } > > err = ionic_lif_alloc(ionic); > @@ -354,17 +375,9 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > ionic->lif = NULL; > err_out_free_irqs: > ionic_bus_free_irq_vectors(ionic); > -err_out_port_reset: > - ionic_port_reset(ionic); > -err_out_reset: > - ionic_reset(ionic); > -err_out_teardown: > - ionic_dev_teardown(ionic); > -err_out_clear_pci: > +err_out_pci: > ionic_clear_pci(ionic); > -err_out_debugfs_del_dev: > - ionic_debugfs_del_dev(ionic); > -err_out_clear_drvdata: > +err_out: > mutex_destroy(&ionic->dev_cmd_lock); > ionic_devlink_free(ionic); > > -- > 2.17.1 > >
On 7/19/23 10:48 AM, Simon Horman wrote: > > On Mon, Jul 17, 2023 at 09:59:59AM -0700, Shannon Nelson wrote: >> Pull out some chunks of code from ionic_probe() that will >> be common in rebuild paths. >> >> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> > > ... > >> +static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >> +{ >> + struct device *dev = &pdev->dev; >> + struct ionic *ionic; >> + int num_vfs; >> + int err; >> + >> + ionic = ionic_devlink_alloc(dev); >> + if (!ionic) >> + return -ENOMEM; >> + >> + ionic->pdev = pdev; >> + ionic->dev = dev; >> + pci_set_drvdata(pdev, ionic); >> + mutex_init(&ionic->dev_cmd_lock); >> + >> + /* Query system for DMA addressing limitation for the device. */ >> + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN)); >> + if (err) { >> + dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting. err=%d\n", >> + err); >> + goto err_out; >> + } >> + >> + err = ionic_setup_one(ionic); >> + if (err) >> + goto err_out; >> + >> /* Allocate and init the LIF */ >> err = ionic_lif_size(ionic); >> if (err) { >> dev_err(dev, "Cannot size LIF: %d, aborting\n", err); >> - goto err_out_port_reset; >> + goto err_out_pci; > > Hi Shannon, > > Prior to this patch, if this error occurred then the following cleanup > would occur. > > ionic_port_reset(ionic); > ionic_reset(ionic); > ionic_dev_teardown(ionic); > ionic_clear_pci(ionic); > ionic_debugfs_del_dev(ionic); > mutex_destroy(&ionic->dev_cmd_lock); > ionic_devlink_free(ionic); > > With this patch I am assuming that the same setup has occurred at > this point (maybe I am mistaken). But with the following cleanup on error. > > ionic_clear_pci(ionic); > mutex_destroy(&ionic->dev_cmd_lock); > ionic_devlink_free(ionic); > > I feel that I'm reading this wrong. You read that right. The port_reset and reset are superflous so are dropped here. However, that dev_teardown() does need to happen to be sure we don't leak a couple CMB related things. I'll add that. Thanks, sln > >> } >> >> err = ionic_lif_alloc(ionic); >> @@ -354,17 +375,9 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >> ionic->lif = NULL; >> err_out_free_irqs: >> ionic_bus_free_irq_vectors(ionic); >> -err_out_port_reset: >> - ionic_port_reset(ionic); >> -err_out_reset: >> - ionic_reset(ionic); >> -err_out_teardown: >> - ionic_dev_teardown(ionic); >> -err_out_clear_pci: >> +err_out_pci: >> ionic_clear_pci(ionic); >> -err_out_debugfs_del_dev: >> - ionic_debugfs_del_dev(ionic); >> -err_out_clear_drvdata: >> +err_out: >> mutex_destroy(&ionic->dev_cmd_lock); >> ionic_devlink_free(ionic); >> >> -- >> 2.17.1 >> >>
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c index 2bc3cab3967d..b141a29177df 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c @@ -220,30 +220,12 @@ static void ionic_clear_pci(struct ionic *ionic) pci_disable_device(ionic->pdev); } -static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) +static int ionic_setup_one(struct ionic *ionic) { - struct device *dev = &pdev->dev; - struct ionic *ionic; - int num_vfs; + struct pci_dev *pdev = ionic->pdev; + struct device *dev = ionic->dev; int err; - ionic = ionic_devlink_alloc(dev); - if (!ionic) - return -ENOMEM; - - ionic->pdev = pdev; - ionic->dev = dev; - pci_set_drvdata(pdev, ionic); - mutex_init(&ionic->dev_cmd_lock); - - /* Query system for DMA addressing limitation for the device. */ - err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN)); - if (err) { - dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting. err=%d\n", - err); - goto err_out_clear_drvdata; - } - ionic_debugfs_add_dev(ionic); /* Setup PCI device */ @@ -258,7 +240,6 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) dev_err(dev, "Cannot request PCI regions: %d, aborting\n", err); goto err_out_clear_pci; } - pcie_print_link_status(pdev); err = ionic_map_bars(ionic); @@ -286,24 +267,64 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) goto err_out_teardown; } - /* Configure the ports */ + /* Configure the port */ err = ionic_port_identify(ionic); if (err) { dev_err(dev, "Cannot identify port: %d, aborting\n", err); - goto err_out_reset; + goto err_out_teardown; } err = ionic_port_init(ionic); if (err) { dev_err(dev, "Cannot init port: %d, aborting\n", err); - goto err_out_reset; + goto err_out_teardown; } + return 0; + +err_out_teardown: + ionic_dev_teardown(ionic); +err_out_clear_pci: + ionic_clear_pci(ionic); +err_out_debugfs_del_dev: + ionic_debugfs_del_dev(ionic); + + return err; +} + +static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) +{ + struct device *dev = &pdev->dev; + struct ionic *ionic; + int num_vfs; + int err; + + ionic = ionic_devlink_alloc(dev); + if (!ionic) + return -ENOMEM; + + ionic->pdev = pdev; + ionic->dev = dev; + pci_set_drvdata(pdev, ionic); + mutex_init(&ionic->dev_cmd_lock); + + /* Query system for DMA addressing limitation for the device. */ + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN)); + if (err) { + dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting. err=%d\n", + err); + goto err_out; + } + + err = ionic_setup_one(ionic); + if (err) + goto err_out; + /* Allocate and init the LIF */ err = ionic_lif_size(ionic); if (err) { dev_err(dev, "Cannot size LIF: %d, aborting\n", err); - goto err_out_port_reset; + goto err_out_pci; } err = ionic_lif_alloc(ionic); @@ -354,17 +375,9 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) ionic->lif = NULL; err_out_free_irqs: ionic_bus_free_irq_vectors(ionic); -err_out_port_reset: - ionic_port_reset(ionic); -err_out_reset: - ionic_reset(ionic); -err_out_teardown: - ionic_dev_teardown(ionic); -err_out_clear_pci: +err_out_pci: ionic_clear_pci(ionic); -err_out_debugfs_del_dev: - ionic_debugfs_del_dev(ionic); -err_out_clear_drvdata: +err_out: mutex_destroy(&ionic->dev_cmd_lock); ionic_devlink_free(ionic);
Pull out some chunks of code from ionic_probe() that will be common in rebuild paths. Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> --- .../ethernet/pensando/ionic/ionic_bus_pci.c | 85 +++++++++++-------- 1 file changed, 49 insertions(+), 36 deletions(-)