diff mbox series

[05/15] media: atomisp: Drop is_valid_device() function

Message ID 20231231103057.35837-6-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 
Now that a single build supports both the ISP 2400 and the ISP 2401
this function is no longer necessary. The main probe() already
contains a similar switch (id->device & ATOMISP_PCI_DEVICE_SOC_MASK)
checking for a known device_id.

Move the revision check into the main probe() and drop
the is_valid_device() function.

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

Comments

Andy Shevchenko Jan. 2, 2024, 12:19 a.m. UTC | #1 
On Sun, Dec 31, 2023 at 12:31 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Now that a single build supports both the ISP 2400 and the ISP 2401
> this function is no longer necessary. The main probe() already
> contains a similar switch (id->device & ATOMISP_PCI_DEVICE_SOC_MASK)
> checking for a known device_id.
>
> Move the revision check into the main probe() and drop
> the is_valid_device() function.

...

> +       if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) {
> +               dev_err(&pdev->dev, "revision %d is not unsupported\n", pdev->revision);

While at it, can you fix the logic error in the message ("is not un"
-- double negation)?

> +               return -ENODEV;
> +       }
Hans de Goede Feb. 19, 2024, 1:25 p.m. UTC | #2 
Hi,

On 1/2/24 01:19, Andy Shevchenko wrote:
> On Sun, Dec 31, 2023 at 12:31 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Now that a single build supports both the ISP 2400 and the ISP 2401
>> this function is no longer necessary. The main probe() already
>> contains a similar switch (id->device & ATOMISP_PCI_DEVICE_SOC_MASK)
>> checking for a known device_id.
>>
>> Move the revision check into the main probe() and drop
>> the is_valid_device() function.
> 
> ...
> 
>> +       if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) {
>> +               dev_err(&pdev->dev, "revision %d is not unsupported\n", pdev->revision);
> 
> While at it, can you fix the logic error in the message ("is not un"
> -- double negation)?

Good point, I've added a patch for this to my upcoming
atomisp pull-req for 6.9

Regards,

Hans
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 206fdaee5952..176b39906d10 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1192,48 +1192,6 @@  atomisp_load_firmware(struct atomisp_device *isp)
 	return fw;
 }
 
-/*
- * Check for flags the driver was compiled with against the PCI
- * device. Always returns true on other than ISP 2400.
- */
-static bool is_valid_device(struct pci_dev *pdev, const struct pci_device_id *id)
-{
-	const char *name;
-	const char *product;
-
-	product = dmi_get_system_info(DMI_PRODUCT_NAME);
-
-	switch (id->device & ATOMISP_PCI_DEVICE_SOC_MASK) {
-	case ATOMISP_PCI_DEVICE_SOC_MRFLD:
-		name = "Merrifield";
-		break;
-	case ATOMISP_PCI_DEVICE_SOC_BYT:
-		name = "Baytrail";
-		break;
-	case ATOMISP_PCI_DEVICE_SOC_ANN:
-		name = "Anniedale";
-		break;
-	case ATOMISP_PCI_DEVICE_SOC_CHT:
-		name = "Cherrytrail";
-		break;
-	default:
-		dev_err(&pdev->dev, "%s: unknown device ID %x04:%x04\n",
-			product, id->vendor, id->device);
-		return false;
-	}
-
-	if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) {
-		dev_err(&pdev->dev, "%s revision %d is not unsupported\n",
-			name, pdev->revision);
-		return false;
-	}
-
-	dev_info(&pdev->dev, "Detected %s version %d (ISP240%c) on %s\n",
-		 name, pdev->revision, IS_ISP2401 ? '1' : '0', product);
-
-	return true;
-}
-
 #define ATOM_ISP_PCI_BAR	0
 
 static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
@@ -1244,9 +1202,6 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	int err, val;
 	u32 irq;
 
-	if (!is_valid_device(pdev, id))
-		return -ENODEV;
-
 	/* Pointer to struct device. */
 	atomisp_dev = &pdev->dev;
 
@@ -1386,6 +1341,11 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 		goto atomisp_dev_alloc_fail;
 	}
 
+	if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) {
+		dev_err(&pdev->dev, "revision %d is not unsupported\n", pdev->revision);
+		return -ENODEV;
+	}
+
 	dev_info(&pdev->dev, "ISP HPLL frequency base = %d MHz\n", isp->hpll_freq);
 
 	isp->max_isr_latency = ATOMISP_MAX_ISR_LATENCY;