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

[Hans Verkuil]

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@xxxxxxxxxxxxxxx
> 
> Assisted-by: Codex:GPT-5
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
>  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) {