diff mbox series

[06/15] media: atomisp: Call pcim_enable_device() and pcim_iomap_regions() later

Message ID 20231231103057.35837-7-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series media: atomisp: NULL pointer deref + missing firmware fixes | expand

Commit Message

Hans de Goede Dec. 31, 2023, 10:30 a.m. UTC 
ATM the atomisp firmware is not available in linux-firmware,
so most users will not have it installed.

Move pcim_enable_device() and pcim_iomap_regions() down in
atomisp_pci_probe() so that they are never called when the firmware
is not there.

This is a preparation patch for making the atomisp driver handle
missing firmware better.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 45 +++++++++----------
 1 file changed, 20 insertions(+), 25 deletions(-)

Comments

Andy Shevchenko Jan. 2, 2024, 12:22 a.m. UTC | #1 
On Sun, Dec 31, 2023 at 12:31 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> ATM the atomisp firmware is not available in linux-firmware,
> so most users will not have it installed.
>
> Move pcim_enable_device() and pcim_iomap_regions() down in
> atomisp_pci_probe() so that they are never called when the firmware
> is not there.
>
> This is a preparation patch for making the atomisp driver handle
> missing firmware better.

...

> -atomisp_dev_alloc_fail:
> -       pcim_iounmap_regions(pdev, BIT(ATOM_ISP_PCI_BAR));
> -
> -ioremap_fail:
>         return err;

Before this patch, can you drop these two labels as one is not doing
anything useful and the other diminishes the idea behind "m" (for
managed)?
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 176b39906d10..0eea20704e66 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1209,33 +1209,16 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	if (!pdata)
 		dev_warn(&pdev->dev, "no platform data available\n");
 
-	err = pcim_enable_device(pdev);
-	if (err) {
-		dev_err(&pdev->dev, "Failed to enable CI ISP device (%d)\n", err);
-		return err;
-	}
-
 	start = pci_resource_start(pdev, ATOM_ISP_PCI_BAR);
 	dev_dbg(&pdev->dev, "start: 0x%x\n", start);
 
-	err = pcim_iomap_regions(pdev, BIT(ATOM_ISP_PCI_BAR), pci_name(pdev));
-	if (err) {
-		dev_err(&pdev->dev, "Failed to I/O memory remapping (%d)\n", err);
-		goto ioremap_fail;
-	}
-
 	isp = devm_kzalloc(&pdev->dev, sizeof(*isp), GFP_KERNEL);
-	if (!isp) {
-		err = -ENOMEM;
-		goto atomisp_dev_alloc_fail;
-	}
+	if (!isp)
+		return -ENOMEM;
 
 	isp->dev = &pdev->dev;
-	isp->base = pcim_iomap_table(pdev)[ATOM_ISP_PCI_BAR];
 	isp->saved_regs.ispmmadr = start;
 
-	dev_dbg(&pdev->dev, "atomisp mmio base: %p\n", isp->base);
-
 	mutex_init(&isp->mutex);
 	spin_lock_init(&isp->lock);
 
@@ -1337,8 +1320,7 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 		break;
 	default:
 		dev_err(&pdev->dev, "un-supported IUNIT device\n");
-		err = -ENODEV;
-		goto atomisp_dev_alloc_fail;
+		return -ENODEV;
 	}
 
 	if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) {
@@ -1364,6 +1346,20 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 		goto fw_validation_fail;
 	}
 
+	err = pcim_enable_device(pdev);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to enable ISP PCI device (%d)\n", err);
+		goto pci_enable_fail;
+	}
+
+	err = pcim_iomap_regions(pdev, BIT(ATOM_ISP_PCI_BAR), pci_name(pdev));
+	if (err) {
+		dev_err(&pdev->dev, "Failed to I/O memory remapping (%d)\n", err);
+		goto ioremap_fail;
+	}
+
+	isp->base = pcim_iomap_table(pdev)[ATOM_ISP_PCI_BAR];
+
 	pci_set_master(pdev);
 
 	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
@@ -1495,6 +1491,9 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	atomisp_msi_irq_uninit(isp);
 	pci_free_irq_vectors(pdev);
 enable_msi_fail:
+	pcim_iounmap_regions(pdev, BIT(ATOM_ISP_PCI_BAR));
+ioremap_fail:
+pci_enable_fail:
 fw_validation_fail:
 	release_firmware(isp->firmware);
 load_fw_fail:
@@ -1519,10 +1518,6 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power(isp, false))
 		dev_err(&pdev->dev, "Failed to switch off ISP\n");
 
-atomisp_dev_alloc_fail:
-	pcim_iounmap_regions(pdev, BIT(ATOM_ISP_PCI_BAR));
-
-ioremap_fail:
 	return err;
 }