Re: [PATCH] media: rzg2l-cru: serialize state transitions with qlock

Jacopo Mondi
2026-04-22T06:46:47Z

Hello Shuhao Fu

On Tue, Apr 21, 2026 at 02:03:07PM +0800, Shuhao Fu wrote:
> struct rzg2l_cru_dev.state is documented as protected by qlock, and the
> IRQ path already reads and updates it under that lock. However,
> rzg2l_cru_stop_streaming() writes STOPPING and
> rzg2l_cru_start_streaming_vq() writes STARTING without taking qlock.
>
> That lets process-context stream control race with rzg2l_cru_irq().
> If the IRQ handler misses a concurrent STOPPING update, it can continue
> normal frame completion and slot refill after streamoff has begun. A
> similar race around STARTING can make the IRQ path observe the wrong
> phase during startup synchronization.
>
> Fix both state transitions by serializing the writes with qlock, while
> still keeping rzg2l_cru_set_stream() outside the locked region.

Thanks, but I would rather complete my series that removes 'state'
completely:
https://patchwork.linuxtv.org/project/linux-media/list/?series=23674

Thanks
j

>
> Fixes: 07fc05bd0a79 ("media: platform: Add Renesas RZ/G2L CRU driver")
> Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
> ---
> drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 162e2ace693184..434754fd155a8e 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -560,7 +560,11 @@ pipe_line_stop:
>
> static void rzg2l_cru_stop_streaming(struct rzg2l_cru_dev *cru)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&cru->qlock, flags);
> cru->state = RZG2L_CRU_DMA_STOPPING;
> + spin_unlock_irqrestore(&cru->qlock, flags);
>
> rzg2l_cru_set_stream(cru, 0);
> }
> @@ -749,6 +753,7 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
> static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count)
> {
> struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
> + unsigned long flags;
> int ret;
>
> ret = pm_runtime_resume_and_get(cru->dev);
> @@ -791,7 +796,9 @@ static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count
> goto out;
> }
>
> + spin_lock_irqsave(&cru->qlock, flags);
> cru->state = RZG2L_CRU_DMA_STARTING;
> + spin_unlock_irqrestore(&cru->qlock, flags);
> dev_dbg(cru->dev, "Starting to capture\n");
> return 0;
>
> --
> 2.25.1
>