public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
To: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Cc: "Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>,
	"Biju Das" <biju.das.jz@bp.renesas.com>,
	"Hans Verkuil" <hverkuil+cisco@kernel.org>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Daniel Scally" <dan.scally@ideasonboard.com>,
	"Barnabás Pőcze" <pobrn@protonmail.com>,
	"Lad Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Jacopo Mondi" <jacopo.mondi+renesas@ideasonboard.com>
Subject: Re: [PATCH 04/14] media: rzg2l-cru: Use proper guard() in irq handler
Date: Mon, 30 Mar 2026 13:15:03 +0200	[thread overview]
Message-ID: <acpbN_fbNxR3rvIs@tom-desktop> (raw)
In-Reply-To: <20260327-b4-cru-rework-v1-4-3b7d0430f538@ideasonboard.com>

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@ideasonboard.com>
> 
> 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@bp.renesas.com>
Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Kind Regards,
Tommaso

> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>  .../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
> 

  parent reply	other threads:[~2026-03-30 11:15 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-27 17:10 [PATCH 00/14] media: rzg2l-cru: Rework slot programming for V2H/G3E Jacopo Mondi
2026-03-27 17:10 ` [PATCH 01/14] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used Jacopo Mondi
2026-03-27 17:10 ` [PATCH 02/14] media: rzg2l-cru: Use only frame end interrupts Jacopo Mondi
2026-03-27 17:10 ` [PATCH 03/14] media: rzg2l-cru: Modernize spin_lock usage with cleanup.h Jacopo Mondi
2026-03-30  7:30   ` Dan Scally
2026-03-30 11:12   ` Tommaso Merciai
2026-03-30 11:41     ` Biju Das
2026-03-27 17:10 ` [PATCH 04/14] media: rzg2l-cru: Use proper guard() in irq handler Jacopo Mondi
2026-03-30  7:32   ` Dan Scally
2026-03-30 11:15   ` Tommaso Merciai [this message]
2026-03-27 17:10 ` [PATCH 05/14] media: rzg2l-cru: Remove locking from start/stop routines Jacopo Mondi
2026-03-27 17:10 ` [PATCH 06/14] media: rzg2l-cru: Do not use irqsave when not needed Jacopo Mondi
2026-03-30  7:50   ` Dan Scally
2026-03-27 17:10 ` [PATCH 07/14] media: rzg2l-cru: Remove wrong locking comment Jacopo Mondi
2026-03-30  7:38   ` Dan Scally
2026-03-27 17:10 ` [PATCH 08/14] media: rz2gl-cru: Introduce a spinlock for hw operations Jacopo Mondi
2026-03-30 10:46   ` Dan Scally
2026-03-27 17:10 ` [PATCH 09/14] media: rzg2l-cru: Split hw locking from buffers Jacopo Mondi
2026-03-30 11:19   ` Dan Scally
2026-03-27 17:10 ` [PATCH 10/14] media: rzg2l-cru: Manually track active slot number Jacopo Mondi
2026-03-27 17:10 ` [PATCH 11/14] media: rz2gl-cru: Return pending buffers in order Jacopo Mondi
2026-03-27 17:10 ` [PATCH 12/14] media: rzg2l-cru: Rework rzg2l_cru_fill_hw_slot() Jacopo Mondi
2026-03-27 17:10 ` [PATCH 13/14] media: rzg2l-cru: Remove the 'state' variable Jacopo Mondi
2026-03-30 11:55   ` Dan Scally
2026-03-27 17:10 ` [PATCH 14/14] media: rzg2l-cru: Simplify irq return value handling Jacopo Mondi
2026-03-30 11:55   ` Dan Scally
2026-03-27 17:25 ` [PATCH 00/14] media: rzg2l-cru: Rework slot programming for V2H/G3E Lad, Prabhakar
2026-03-28 11:55   ` Jacopo Mondi
2026-03-28 12:56     ` Lad, Prabhakar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=acpbN_fbNxR3rvIs@tom-desktop \
    --to=tommaso.merciai.xr@bp.renesas.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=dan.scally@ideasonboard.com \
    --cc=hverkuil+cisco@kernel.org \
    --cc=jacopo.mondi+renesas@ideasonboard.com \
    --cc=jacopo.mondi@ideasonboard.com \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=pobrn@protonmail.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox