On 01/02/2023 22:45, Sakari Ailus wrote:
> 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.
Why register it earlier? You do not explain the reason.
Also, does this patch (and also 18/26) belong in this patch series?
It seems more like a normal bug fix and not related to life-time management.
And isn't it the responsibility of the driver to ensure that the irqs are
masked in the remove() callback to prevent the irq from being called?
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.
Regards,
Hans
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> 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);
