media: microchip: isc: Initialize DMA lock before requesting IRQ

[PATCH] media: microchip: isc: Initialize DMA lock before requesting IRQ

Posted by Runyu Xiao 3 weeks, 6 days ago

The ISC interrupt can run before asynchronous subdevice completion, but the DMA queue lock was initialized from isc_async_complete().  Initialize it in each SoC probe before requesting the auto-enabled IRQ and remove the late initialization from the asynchronous completion path.

Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")

Cc: stable@vger.kernel.org

Assisted-by: Codex:GPT-5
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/media/platform/microchip/microchip-isc-base.c    | 1 -
 drivers/media/platform/microchip/microchip-sama5d2-isc.c | 2 ++
 drivers/media/platform/microchip/microchip-sama7g5-isc.c | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index a7cdc743f..a2534d8eb 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -1792,7 +1792,6 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
 
 	/* Init video dma queues */
 	INIT_LIST_HEAD(&isc->dma_queue);
-	spin_lock_init(&isc->dma_queue_lock);
 	spin_lock_init(&isc->awb_lock);
 
 	ret = isc_set_default_fmt(isc);
diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
index 66d3d7891..f0f7ca47f 100644
--- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
@@ -432,6 +432,8 @@ static int microchip_isc_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
+	spin_lock_init(&isc->dma_queue_lock);
+
 	ret = devm_request_irq(dev, irq, microchip_isc_interrupt, 0,
 			       "microchip-sama5d2-isc", isc);
 	if (ret < 0) {
diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index 7383341ec..b1aa3fe55 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -421,6 +421,8 @@ static int microchip_xisc_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
+	spin_lock_init(&isc->dma_queue_lock);
+
 	ret = devm_request_irq(dev, irq, microchip_isc_interrupt, 0,
 			       "microchip-sama7g5-xisc", isc);
 	if (ret < 0) {
-- 
2.34.1

Re: [PATCH] media: microchip: isc: Initialize DMA lock before requesting IRQ

Posted by Hans Verkuil 2 weeks, 2 days ago

On 30/08/2026 04:32, Runyu Xiao wrote:
> The ISC interrupt can run before asynchronous subdevice completion, but the DMA queue lock was initialized from isc_async_complete().  Initialize it in each SoC probe before requesting the auto-enabled IRQ and remove the late initialization from the asynchronous completion path.

Line is much too long.

> 
> Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")
> 

Spurious empty lines.

> Cc: stable@vger.kernel.org
> 
> Assisted-by: Codex:GPT-5
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>  drivers/media/platform/microchip/microchip-isc-base.c    | 1 -
>  drivers/media/platform/microchip/microchip-sama5d2-isc.c | 2 ++
>  drivers/media/platform/microchip/microchip-sama7g5-isc.c | 2 ++
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
> index a7cdc743f..a2534d8eb 100644
> --- a/drivers/media/platform/microchip/microchip-isc-base.c
> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
> @@ -1792,7 +1792,6 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
>  
>  	/* Init video dma queues */
>  	INIT_LIST_HEAD(&isc->dma_queue);
> -	spin_lock_init(&isc->dma_queue_lock);
>  	spin_lock_init(&isc->awb_lock);
>  
>  	ret = isc_set_default_fmt(isc);
> diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
> index 66d3d7891..f0f7ca47f 100644
> --- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
> +++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
> @@ -432,6 +432,8 @@ static int microchip_isc_probe(struct platform_device *pdev)
>  	if (irq < 0)
>  		return irq;
>  
> +	spin_lock_init(&isc->dma_queue_lock);

It's dubious to just init dma_queue_lock here but not INIT_LIST_HEAD(&isc->dma_queue);

I think Eugen needs to look at this as well to see if this is actually a valid
issue at all.

In any case, this patch doesn't look right to me.

Regards,

	Hans

> +
>  	ret = devm_request_irq(dev, irq, microchip_isc_interrupt, 0,
>  			       "microchip-sama5d2-isc", isc);
>  	if (ret < 0) {
> diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> index 7383341ec..b1aa3fe55 100644
> --- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> +++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> @@ -421,6 +421,8 @@ static int microchip_xisc_probe(struct platform_device *pdev)
>  	if (irq < 0)
>  		return irq;
>  
> +	spin_lock_init(&isc->dma_queue_lock);
> +
>  	ret = devm_request_irq(dev, irq, microchip_isc_interrupt, 0,
>  			       "microchip-sama7g5-xisc", isc);
>  	if (ret < 0) {

Re: [PATCH] media: microchip: isc: Initialize DMA lock before requesting IRQ

Posted by Runyu Xiao 1 week ago

Hi Hans and Eugen,

Thanks for taking a look at this.

On Sun, 30 Aug 2026, I wrote:
> The ISC interrupt can run before asynchronous subdevice completion,
> but the DMA queue lock was initialized from isc_async_complete().
> Initialize it in each SoC probe before requesting the auto-enabled IRQ.

On Thu, 10 Sep 2026, Hans Verkuil wrote:
> It's dubious to just init dma_queue_lock here but not
> INIT_LIST_HEAD(&isc->dma_queue);
>
> I think Eugen needs to look at this as well to see if this is actually
> a valid issue at all.
>
> In any case, this patch doesn't look right to me.

You're right that moving dma_queue_lock alone is not enough. The interrupt
handler also accesses dma_queue, comp, and awb_work, which are initialized
later from isc_async_complete(). I also need to confirm whether an interrupt
can actually be pending at this point, and whether the required clocks and
hardware state are ready during probe.

I have not established that yet, so I'll drop this version for now rather
than send a v2 that only moves another initialization. Eugen, could you
please confirm the expected reset and interrupt state during probe? If there
is a real probe-time path, I'll revisit this with the complete ordering.

Thanks,
Runyu

Re: [PATCH] media: microchip: isc: Initialize DMA lock before requesting IRQ

Posted by Eugen Hristev 3 days, 1 hour ago

On 9/19/26 04:10, Runyu Xiao wrote:
> Hi Hans and Eugen,
> 
> Thanks for taking a look at this.
> 
> On Sun, 30 Aug 2026, I wrote:
>> The ISC interrupt can run before asynchronous subdevice completion,
>> but the DMA queue lock was initialized from isc_async_complete().
>> Initialize it in each SoC probe before requesting the auto-enabled IRQ.
> 
> On Thu, 10 Sep 2026, Hans Verkuil wrote:
>> It's dubious to just init dma_queue_lock here but not
>> INIT_LIST_HEAD(&isc->dma_queue);
>>
>> I think Eugen needs to look at this as well to see if this is actually
>> a valid issue at all.
>>
>> In any case, this patch doesn't look right to me.
> 
> You're right that moving dma_queue_lock alone is not enough. The interrupt
> handler also accesses dma_queue, comp, and awb_work, which are initialized
> later from isc_async_complete(). I also need to confirm whether an interrupt
> can actually be pending at this point, and whether the required clocks and
> hardware state are ready during probe.
> 
> I have not established that yet, so I'll drop this version for now rather
> than send a v2 that only moves another initialization. Eugen, could you
> please confirm the expected reset and interrupt state during probe? If there
> is a real probe-time path, I'll revisit this with the complete ordering.

Added Balakrishnan , he can test and review your patch.

Eugen>
> Thanks,
> Runyu