Re: [PATCH 1/2] staging: kpc2000: improve label names in kp2000_pcie_probe
Thanks!
Reviewed-by: Dan Carpenter
Not related to your patch (IOW ignore if you want to) the error handling
is slightly more complicated than required:
drivers/staging/kpc2000/kpc2000/core.c
356 * Step 4: Setup the Register BAR
357 */
358 reg_bar_phys_addr = pci_resource_start(pcard->pdev, REG_BAR);
359 reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
360
361 pcard->regs_bar_base = ioremap_nocache(reg_bar_phys_addr,
PAGE_SIZE);
362 if (!pcard->regs_bar_base) {
363 dev_err(>pdev->dev,
364 "probe: REG_BAR could not remap memory to
virtual space\n");
365 err = -ENODEV;
366 goto err_disable_device;
367 }
368 dev_dbg(>pdev->dev,
369 "probe: REG_BAR virt hardware address start [%p]\n",
370 pcard->regs_bar_base);
371
372 err = pci_request_region(pcard->pdev, REG_BAR,
KP_DRIVER_NAME_KP2000);
373 if (err) {
374 iounmap(pcard->regs_bar_base);
We could move this to the bottom of the function. We would need to
re-order it slightly to free things in the mirror of how they are
allocated. (Always just free the most recent allocation).
375 dev_err(>pdev->dev,
376 "probe: failed to acquire PCI region (%d)\n",
377 err);
378 err = -ENODEV;
379 goto err_disable_device;
380 }
381
382 pcard->regs_base_resource.start = reg_bar_phys_addr;
383 pcard->regs_base_resource.end = reg_bar_phys_addr +
384reg_bar_phys_len - 1;
385 pcard->regs_base_resource.flags = IORESOURCE_MEM;
386
387 /*
388 * Step 5: Setup the DMA BAR
389 */
390 dma_bar_phys_addr = pci_resource_start(pcard->pdev, DMA_BAR);
391 dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
392
393 pcard->dma_bar_base = ioremap_nocache(dma_bar_phys_addr,
394dma_bar_phys_len);
395 if (!pcard->dma_bar_base) {
396 dev_err(>pdev->dev,
397 "probe: DMA_BAR could not remap memory to
virtual space\n");
398 err = -ENODEV;
399 goto err_unmap_regs;
400 }
401 dev_dbg(>pdev->dev,
402 "probe: DMA_BAR virt hardware address start [%p]\n",
403 pcard->dma_bar_base);
404
405 pcard->dma_common_regs = pcard->dma_bar_base +
KPC_DMA_COMMON_OFFSET;
406
407 err = pci_request_region(pcard->pdev, DMA_BAR, "kp2000_pcie");
408 if (err) {
409 iounmap(pcard->dma_bar_base);
Same.
410 dev_err(>pdev->dev,
411 "probe: failed to acquire PCI region (%d)\n",
err);
412 err = -ENODEV;
413 goto err_unmap_regs;
414 }
415
416 pcard->dma_base_resource.start = dma_bar_phys_addr;
[ snip ]
509 dev_dbg(>pdev->dev, "%s() complete!\n", __func__);
510 mutex_unlock(>sem);
511 return 0;
512
513 err_remove_sysfs:
514 sysfs_remove_files(>dev.kobj, kp_attr_list);
515 err_free_irq:
516 free_irq(pcard->pdev->irq, pcard);
517 err_disable_msi:
518 pci_disable_msi(pcard->pdev);
519 err_unmap_dma:
520 iounmap(pcard->dma_bar_base);
521 pci_release_region(pdev, DMA_BAR);
522 pcard->dma_bar_base = NULL;
523 err_unmap_regs:
524 iounmap(pcard->regs_bar_base);
525 pci_release_region(pdev, REG_BAR);
526 pcard->regs_bar_base = NULL;
err_release_dma:
pci_release_region(pdev, DMA_BAR);
err_unmap_dma:
iounmap(pcard->dma_bar_base);
err_release_reg:
pci_release_region(pdev, REG_BAR);
err_unmap_reg:
iounmap(pcard->regs_bar_base);
I moved swapped the pci_release_region() and the iounmap() order. There
is no need to set "pcard->regs_bar_base = NULL;" so just remove that.
527 err_disable_device:
528 pci_disable_device(pcard->pdev);
529 err_remove_ida:
530 mutex_unlock(>sem);
531 ida_simple_remove(_num_ida, pcard->card_num);
532 err_free_pcard:
533 kfree(pcard);
534 return err;
535 }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
[PATCH 1/2] staging: kpc2000: improve label names in kp2000_pcie_probe
Use self-explanatory label names instead of the generic numbered ones,
to make it easier to follow and understand the code.
Signed-off-by: Simon Sandström
---
drivers/staging/kpc2000/kpc2000/core.c | 42 --
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000/core.c
b/drivers/staging/kpc2000/kpc2000/core.c
index 9b9b29ac90c5..ee6b9be7127d 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -327,7 +327,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
if (err < 0) {
dev_err(>dev, "probe: failed to get card number (%d)\n",
err);
- goto out2;
+ goto err_free_pcard;
}
pcard->card_num = err;
scnprintf(pcard->name, 16, "kpcard%u", pcard->card_num);
@@ -346,7 +346,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(>pdev->dev,
"probe: failed to enable PCIE2000 PCIe device (%d)\n",
err);
- goto out3;
+ goto err_remove_ida;
}
/*
@@ -360,7 +360,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(>pdev->dev,
"probe: REG_BAR could not remap memory to virtual
space\n");
err = -ENODEV;
- goto out4;
+ goto err_disable_device;
}
dev_dbg(>pdev->dev,
"probe: REG_BAR virt hardware address start [%p]\n",
@@ -373,7 +373,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
"probe: failed to acquire PCI region (%d)\n",
err);
err = -ENODEV;
- goto out4;
+ goto err_disable_device;
}
pcard->regs_base_resource.start = reg_bar_phys_addr;
@@ -393,7 +393,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(>pdev->dev,
"probe: DMA_BAR could not remap memory to virtual
space\n");
err = -ENODEV;
- goto out5;
+ goto err_unmap_regs;
}
dev_dbg(>pdev->dev,
"probe: DMA_BAR virt hardware address start [%p]\n",
@@ -407,7 +407,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(>pdev->dev,
"probe: failed to acquire PCI region (%d)\n", err);
err = -ENODEV;
- goto out5;
+ goto err_unmap_regs;
}
pcard->dma_base_resource.start = dma_bar_phys_addr;
@@ -421,7 +421,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
pcard->sysinfo_regs_base = pcard->regs_bar_base;
err = read_system_regs(pcard);
if (err)
- goto out6;
+ goto err_unmap_dma;
// Disable all "user" interrupts because they're not used yet.
writeq(0x,
@@ -461,7 +461,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
if (err) {
dev_err(>pdev->dev,
"CANNOT use DMA mask %0llx\n", DMA_BIT_MASK(64));
- goto out7;
+ goto err_unmap_dma;
}
dev_dbg(>pdev->dev,
"Using DMA mask %0llx\n", dma_get_mask(PCARD_TO_DEV(pcard)));
@@ -471,14 +471,14 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
*/
err = pci_enable_msi(pcard->pdev);
if (err < 0)
- goto out8a;
+ goto err_unmap_dma;
rv = request_irq(pcard->pdev->irq, kp2000_irq_handler, IRQF_SHARED,
pcard->name, pcard);
if (rv) {
dev_err(>pdev->dev,
"%s: failed to request_irq: %d\n", __func__, rv);
- goto out8b;
+ goto err_disable_msi;
}
/*
@@ -487,7 +487,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
err = sysfs_create_files(>dev.kobj, kp_attr_list);
if (err) {
dev_err(>dev, "Failed to add sysfs files: %d\n", err);
- goto out9;
+ goto err_free_irq;
}
/*
@@ -495,7 +495,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
*/
err = kp2000_probe_cores(pcard);
if (err)
- goto out10;
+ goto err_remove_sysfs;
/*
* Step 11: Enable IRQs in HW
@@ -506,28 +506,26 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
mutex_unlock(>sem);
return 0;
-out10:
+err_remove_sysfs:
sysfs_remove_files(>dev.kobj, kp_attr_list);
-out9:
+err_free_irq:
free_irq(pcard->pdev->irq, pcard);
-out8b:
+err_disable_msi:
pci_disable_msi(pcard->pdev);
-out8a:
-out7:
-out6:
+err_unmap_dma:
iounmap(pcard->dma_bar_base);
pci_release_region(pdev, DMA_BAR);
pcard->dma_bar_base =
