Re: [PATCH v3] media: saa7164: add missing ioremap error handling

[Markus Elfring]

> Add checks for ioremap return values in saa7164_dev_setup(). If
> ioremap for BAR0 or BAR2 fails, release the already allocated PCI
> memory regions, remove the device from the global list, decrement
> the device count, and return -ENODEV.

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.0-rc3#n659


> This prevents potential null pointer dereferences and ensures proper
> cleanup on memory mapping failures.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.0-rc3#n145


…
> +++ b/drivers/media/pci/saa7164/saa7164-core.c
> @@ -998,9 +998,21 @@ static int saa7164_dev_setup(struct saa7164_dev *dev)
>  	/* PCI/e allocations */
>  	dev->lmmio = ioremap(pci_resource_start(dev->pci, 0),
>  			     pci_resource_len(dev->pci, 0));
> +	if (!dev->lmmio) {
> +		dev_err(&dev->pci->dev,
> +				"failed to remap MMIO memory @ 0x%llx\n",
> +			(u64)pci_resource_start(dev->pci, 0));
> +		goto err_ioremap;
> +	}
>  
>  	dev->lmmio2 = ioremap(pci_resource_start(dev->pci, 2),
>  			     pci_resource_len(dev->pci, 2));
…

Would you like to avoid duplicate source code here?

…
> @@ -1019,6 +1031,23 @@ static int saa7164_dev_setup(struct saa7164_dev *dev)
>  	saa7164_pci_quirks(dev);
>  
>  	return 0;
…
> +	/* Remove from device list and decrement count */
> +	mutex_lock(&devlist);
> +	list_del(&dev->devlist);
> +	mutex_unlock(&devlist);
> +	saa7164_devcount--;
…

Will development interests grow to apply a call like “scoped_guard(mutex, &devlist)”?
https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/mutex.h#L253

Regards,
Markus