Re: [PATCH 04/14] media: rzg2l-cru: Use proper guard() in irq handler

[Tommaso Merciai]

Hi Jacopo,
Thanks for your patch.

On Fri, Mar 27, 2026 at 06:10:09PM +0100, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo.mondi+renesas@xxxxxxxxxxxxxxxx>
> 
> The irq handler uses a scoped_guard() that covers the whole function
> body.
> 
> Replace it with a more appropriate guard() and reduce the indentation.
> 

LGTM. Tested on RZ/G3E evk + ov5645 image sensor.

Tested-by: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>

Kind Regards,
Tommaso

> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@xxxxxxxxxxxxxxxx>
> ---
>  .../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 104 ++++++++++-----------
>  1 file changed, 52 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 2d7ac9f37291..b041c72837c6 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -671,70 +671,70 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
>  	u32 irq_status;
>  	int slot;
>  
> -	scoped_guard(spinlock, &cru->qlock) {
> -		irq_status = rzg2l_cru_read(cru, CRUnINTS2);
> -		if (!irq_status)
> -			return IRQ_NONE;
> +	guard(spinlock)(&cru->qlock);
>  
> -		dev_dbg(cru->dev, "CRUnINTS2 0x%x\n", irq_status);
> +	irq_status = rzg2l_cru_read(cru, CRUnINTS2);
> +	if (!irq_status)
> +		return IRQ_NONE;
>  
> -		rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
> +	dev_dbg(cru->dev, "CRUnINTS2 0x%x\n", irq_status);
>  
> -		/* Nothing to do if capture status is 'RZG2L_CRU_DMA_STOPPED' */
> -		if (cru->state == RZG2L_CRU_DMA_STOPPED) {
> -			dev_dbg(cru->dev, "IRQ while state stopped\n");
> -			return IRQ_HANDLED;
> -		}
> +	rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
>  
> -		if (cru->state == RZG2L_CRU_DMA_STOPPING) {
> -			if (irq_status & CRUnINTS2_FExS(0) ||
> -			    irq_status & CRUnINTS2_FExS(1) ||
> -			    irq_status & CRUnINTS2_FExS(2) ||
> -			    irq_status & CRUnINTS2_FExS(3))
> -				dev_dbg(cru->dev, "IRQ while state stopping\n");
> -			return IRQ_HANDLED;
> -		}
> +	/* Nothing to do if capture status is 'RZG2L_CRU_DMA_STOPPED' */
> +	if (cru->state == RZG2L_CRU_DMA_STOPPED) {
> +		dev_dbg(cru->dev, "IRQ while state stopped\n");
> +		return IRQ_HANDLED;
> +	}
>  
> -		slot = rzg3e_cru_get_current_slot(cru);
> -		if (slot < 0)
> -			return IRQ_HANDLED;
> +	if (cru->state == RZG2L_CRU_DMA_STOPPING) {
> +		if (irq_status & CRUnINTS2_FExS(0) ||
> +		    irq_status & CRUnINTS2_FExS(1) ||
> +		    irq_status & CRUnINTS2_FExS(2) ||
> +		    irq_status & CRUnINTS2_FExS(3))
> +			dev_dbg(cru->dev, "IRQ while state stopping\n");
> +		return IRQ_HANDLED;
> +	}
>  
> -		dev_dbg(cru->dev, "Current written slot: %d\n", slot);
> -		cru->buf_addr[slot] = 0;
> -
> -		/*
> -		 * To hand buffers back in a known order to userspace start
> -		 * to capture first from slot 0.
> -		 */
> -		if (cru->state == RZG2L_CRU_DMA_STARTING) {
> -			if (slot != 0) {
> -				dev_dbg(cru->dev, "Starting sync slot: %d\n", slot);
> -				return IRQ_HANDLED;
> -			}
> -			dev_dbg(cru->dev, "Capture start synced!\n");
> -			cru->state = RZG2L_CRU_DMA_RUNNING;
> -		}
> +	slot = rzg3e_cru_get_current_slot(cru);
> +	if (slot < 0)
> +		return IRQ_HANDLED;
>  
> -		/* Capture frame */
> -		if (cru->queue_buf[slot]) {
> -			struct vb2_v4l2_buffer *buf = cru->queue_buf[slot];
> -
> -			buf->field = cru->format.field;
> -			buf->sequence = cru->sequence;
> -			buf->vb2_buf.timestamp = ktime_get_ns();
> -			vb2_buffer_done(&buf->vb2_buf, VB2_BUF_STATE_DONE);
> -			cru->queue_buf[slot] = NULL;
> -		} else {
> -			/* Scratch buffer was used, dropping frame. */
> -			dev_dbg(cru->dev, "Dropping frame %u\n", cru->sequence);
> +	dev_dbg(cru->dev, "Current written slot: %d\n", slot);
> +	cru->buf_addr[slot] = 0;
> +
> +	/*
> +	 * To hand buffers back in a known order to userspace start
> +	 * to capture first from slot 0.
> +	 */
> +	if (cru->state == RZG2L_CRU_DMA_STARTING) {
> +		if (slot != 0) {
> +			dev_dbg(cru->dev, "Starting sync slot: %d\n", slot);
> +			return IRQ_HANDLED;
>  		}
> +		dev_dbg(cru->dev, "Capture start synced!\n");
> +		cru->state = RZG2L_CRU_DMA_RUNNING;
> +	}
>  
> -		cru->sequence++;
> +	/* Capture frame */
> +	if (cru->queue_buf[slot]) {
> +		struct vb2_v4l2_buffer *buf = cru->queue_buf[slot];
>  
> -		/* Prepare for next frame */
> -		rzg2l_cru_fill_hw_slot(cru, slot);
> +		buf->field = cru->format.field;
> +		buf->sequence = cru->sequence;
> +		buf->vb2_buf.timestamp = ktime_get_ns();
> +		vb2_buffer_done(&buf->vb2_buf, VB2_BUF_STATE_DONE);
> +		cru->queue_buf[slot] = NULL;
> +	} else {
> +		/* Scratch buffer was used, dropping frame. */
> +		dev_dbg(cru->dev, "Dropping frame %u\n", cru->sequence);
>  	}
>  
> +	cru->sequence++;
> +
> +	/* Prepare for next frame */
> +	rzg2l_cru_fill_hw_slot(cru, slot);
> +
>  	return IRQ_HANDLED;
>  }
>  
> 
> -- 
> 2.53.0
>