Re: [PATCH 05/14] media: rzg2l-cru: Remove locking from start/stop routines

[Dan Scally <dan.scally@ideasonboard.com>]

Hi Jacopo

On 27/03/2026 17:10, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> 
> The start/stop streaming routines do not need to lock the whole function
> body against possible concurrent accesses to the CRU buffers or hardware
> registers.
> 
> The stop function starts by disabling interrupts, and only this portion
> needs to be protected not to race against a possible IRQ.
> 
> Once interrupts are disabled, nothing in the video device driver can race
> and once the peripheral has been disabled we can release all pending
> buffers.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---

Looks ok to me:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 18 +++++-------------
>   1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index b041c72837c6..43b1d35fb963 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -341,23 +341,19 @@ bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
>   void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
>   {
>   	unsigned int retries = 0;
> -	unsigned long flags;
>   	u32 icnms;
>   
> -	spin_lock_irqsave(&cru->qlock, flags);
> -
> -	/* Disable and clear the interrupt */
> -	cru->info->disable_interrupts(cru);
> +	scoped_guard(spinlock_irq, &cru->qlock) {
> +		/* Disable and clear the interrupt */
> +		cru->info->disable_interrupts(cru);
> +	}
>   
>   	/* Stop the operation of image conversion */
>   	rzg2l_cru_write(cru, ICnEN, 0);
>   
>   	/* Wait for streaming to stop */
> -	while ((rzg2l_cru_read(cru, ICnMS) & ICnMS_IA) && retries++ < RZG2L_RETRIES) {
> -		spin_unlock_irqrestore(&cru->qlock, flags);
> +	while ((rzg2l_cru_read(cru, ICnMS) & ICnMS_IA) && retries++ < RZG2L_RETRIES)
>   		msleep(RZG2L_TIMEOUT_MS);
> -		spin_lock_irqsave(&cru->qlock, flags);
> -	}
>   
>   	icnms = rzg2l_cru_read(cru, ICnMS) & ICnMS_IA;
>   	if (icnms)
> @@ -401,8 +397,6 @@ void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
>   
>   	/* Resets the image processing module */
>   	rzg2l_cru_write(cru, CRUnRST, 0);
> -
> -	spin_unlock_irqrestore(&cru->qlock, flags);
>   }
>   
>   static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
> @@ -470,8 +464,6 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
>   	csi_vc = ret;
>   	cru->svc_channel = csi_vc;
>   
> -	guard(spinlock_irqsave)(&cru->qlock);
> -
>   	/* Select a video input */
>   	rzg2l_cru_write(cru, CRUnCTRL, CRUnCTRL_VINSEL(0));
>   
>