Thanks!
Reviewed-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
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(&pcard->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(&pcard->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(&pcard->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 +
384 reg_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,
394 dma_bar_phys_len);
395 if (!pcard->dma_bar_base) {
396 dev_err(&pcard->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(&pcard->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(&pcard->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(&pcard->pdev->dev, "%s() complete!\n", __func__);
510 mutex_unlock(&pcard->sem);
511 return 0;
512
513 err_remove_sysfs:
514 sysfs_remove_files(&pdev->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(&pcard->sem);
531 ida_simple_remove(&card_num_ida, pcard->card_num);
532 err_free_pcard:
533 kfree(pcard);
534 return err;
535 }
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
