diff mbox series

[21/26] media: ipu3-cio2: Don't use devm_request_irq()

Message ID 20230201214535.347075-22-sakari.ailus@linux.intel.com
State New
Headers show
Series Media device lifetime management | expand

Commit Message

Sakari Ailus Feb. 1, 2023, 9:45 p.m. UTC 
Use request_irq() instead of devm_request_irq(), as a handler set using
devm_request_irq() may still be called once the driver's remove() callback
has been called.

Also register the IRQ earlier on.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Sakari Ailus April 12, 2023, 4:45 p.m. UTC | #1 
Hi Hans,

On Fri, Mar 03, 2023 at 12:58:52PM +0200, Sakari Ailus wrote:
> > devm_request_irq() is used a lot in the kernel, so if this is a
> > common issue, then just fixing it in two drivers isn't going to make
> > much of a difference.
> 
> I came to think of this after sending the patch as well. It's memory that
> is the problem, any hardware access needs to end before remove is called.
> 
> I'll drop the devm removal.

Actually, the reason why devm_request_irq() isn't a great idea is that if
an interrupt arrives between device's remove function and actually
releasing that IRQ, bad things will (again) happen. So it's only safe to
use devm_request_irq() if you can guarantee you're actually not going to
get an interrupt then. You generally shouldn't be getting one, but that's
not the same thing than being sure.
diff mbox series

Patch

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
index d1bfcfba112f..9fdfb2a794db 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
@@ -1773,6 +1773,12 @@  static int cio2_pci_probe(struct pci_dev *pci_dev,
 	if (r)
 		return r;
 
+	r = request_irq(pci_dev->irq, cio2_irq, IRQF_SHARED, CIO2_NAME, cio2);
+	if (r) {
+		dev_err(dev, "failed to request IRQ (%d)\n", r);
+		goto fail_mutex_destroy;
+	}
+
 	mutex_init(&cio2->lock);
 
 	cio2->media_dev.dev = dev;
@@ -1783,7 +1789,7 @@  static int cio2_pci_probe(struct pci_dev *pci_dev,
 	media_device_init(&cio2->media_dev);
 	r = media_device_register(&cio2->media_dev);
 	if (r < 0)
-		goto fail_mutex_destroy;
+		goto fail_free_irq;
 
 	cio2->v4l2_dev.mdev = &cio2->media_dev;
 	r = v4l2_device_register(dev, &cio2->v4l2_dev);
@@ -1803,13 +1809,6 @@  static int cio2_pci_probe(struct pci_dev *pci_dev,
 	if (r)
 		goto fail_clean_notifier;
 
-	r = devm_request_irq(dev, pci_dev->irq, cio2_irq, IRQF_SHARED,
-			     CIO2_NAME, cio2);
-	if (r) {
-		dev_err(dev, "failed to request IRQ (%d)\n", r);
-		goto fail_clean_notifier;
-	}
-
 	pm_runtime_put_noidle(dev);
 	pm_runtime_allow(dev);
 
@@ -1824,6 +1823,8 @@  static int cio2_pci_probe(struct pci_dev *pci_dev,
 fail_media_device_unregister:
 	media_device_unregister(&cio2->media_dev);
 	media_device_cleanup(&cio2->media_dev);
+fail_free_irq:
+	free_irq(pci_dev->irq, cio2);
 fail_mutex_destroy:
 	mutex_destroy(&cio2->lock);
 	cio2_fbpt_exit_dummy(cio2);
@@ -1837,6 +1838,7 @@  static void cio2_pci_remove(struct pci_dev *pci_dev)
 
 	media_device_unregister(&cio2->media_dev);
 	v4l2_device_unregister(&cio2->v4l2_dev);
+	free_irq(pci_dev->irq, cio2);
 	v4l2_async_nf_unregister(&cio2->notifier);
 	v4l2_async_nf_cleanup(&cio2->notifier);
 	cio2_queues_exit(cio2);