From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1EA5D170A26; Sat, 14 Jun 2025 21:23:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749936220; cv=none; b=r6qkCKkFxksLm25dDeGHUXoXzMdv8JWq+PDxSm0pWcgPB8HQUBpbM5u0OLXFqJjay7U23TCbH1EjVHq5Oq0eLOd04+DQelUAG770rUll8SrgpJLqp/uDwt9MG+08PCYQ/Esi+c0F46A2KnVyITG8WNO5V0bqC//NW3uPPoaAIRA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749936220; c=relaxed/simple; bh=+buu05lf5puXVyjRW6oMuYv3VcTEXEdgXl3ir+gLyOo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hJjemLf1U/R3fjqxBttqibdhFinP6/ZZRbAHndzOaIcTdWU+bmiWOAz1C6OuMfB6Gsz3mJCfsK/Dx+6LVZntzVnneLpbNAW06f/wyHYy0DxyPIMYCxvujvgM3dKyA/7FhK+78JqXzAnattj55vVcou4jvuoezNzmTcTapTMSMLg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=UiAXShK2; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="UiAXShK2" Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 968C65B3; Sat, 14 Jun 2025 23:23:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1749936198; bh=+buu05lf5puXVyjRW6oMuYv3VcTEXEdgXl3ir+gLyOo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UiAXShK2K0kgB0Dbtl6oxci/XAI632Mh3JzI0WXAEfZ0gYXT9Chu+r1Wt5fs3qLH4 w2ARnbvLoWF20AwvhqLq+MqHiDHtWENVzVf6hVah7Bl9jSNw0xPqa2Bj34Uv4Mfjdy kDpLPudNUdZbZyRIvyRJE9VprYPvlTSoizN1POs8= Date: Sun, 15 Jun 2025 00:23:14 +0300 From: Laurent Pinchart To: Niklas =?utf-8?Q?S=C3=B6derlund?= Cc: Mauro Carvalho Chehab , Geert Uytterhoeven , Sakari Ailus , linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] media: rcar-vin: Fold interrupt helpers into only callers Message-ID: <20250614212314.GL10542@pendragon.ideasonboard.com> References: <20250614141545.2860860-1-niklas.soderlund+renesas@ragnatech.se> <20250614141545.2860860-2-niklas.soderlund+renesas@ragnatech.se> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20250614141545.2860860-2-niklas.soderlund+renesas@ragnatech.se> Hi Niklas, Thank you for the patch. On Sat, Jun 14, 2025 at 04:15:43PM +0200, Niklas Söderlund wrote: > The call sites using the interrupt helper functions have all been > reworked to only one for each. Fold echo of them into the only call > sites left. > > While at it rename the variable holding the current interrupt status to > make the code easier to read. > > There is no functional change. > > Signed-off-by: Niklas Söderlund > --- > .../platform/renesas/rcar-vin/rcar-dma.c | 27 +++++-------------- > 1 file changed, 6 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > index 5c08ee2c9807..585b8b3dcfd8 100644 > --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > @@ -912,21 +912,6 @@ static int rvin_setup(struct rvin_dev *vin) > return 0; > } > > -static void rvin_disable_interrupts(struct rvin_dev *vin) > -{ > - rvin_write(vin, 0, VNIE_REG); > -} > - > -static u32 rvin_get_interrupt_status(struct rvin_dev *vin) > -{ > - return rvin_read(vin, VNINTS_REG); > -} > - > -static void rvin_ack_interrupt(struct rvin_dev *vin) > -{ > - rvin_write(vin, rvin_read(vin, VNINTS_REG), VNINTS_REG); > -} > - > static bool rvin_capture_active(struct rvin_dev *vin) > { > return rvin_read(vin, VNMS_REG) & VNMS_CA; > @@ -1049,22 +1034,22 @@ static void rvin_capture_stop(struct rvin_dev *vin) > static irqreturn_t rvin_irq(int irq, void *data) > { > struct rvin_dev *vin = data; > - u32 int_status, vnms; > + u32 status, vnms; > int slot; > unsigned int handled = 0; > unsigned long flags; > > spin_lock_irqsave(&vin->qlock, flags); > > - int_status = rvin_get_interrupt_status(vin); > - if (!int_status) > + status = rvin_read(vin, VNINTS_REG); > + if (!status) > goto done; > > - rvin_ack_interrupt(vin); > + rvin_write(vin, status, VNINTS_REG); Actually there is a functional change here. Before this change, if an interrupt occured between reading VNINTS_REG in rvin_get_interrupt_status() and reading it again in rvin_ack_interrupt(), it would get lost. This patch fixes a possible bug. With an updated commit message, Reviewed-by: Laurent Pinchart > handled = 1; > > /* Nothing to do if nothing was captured. */ > - if (!(int_status & VNINTS_FIS)) > + if (!(status & VNINTS_FIS)) > goto done; > > /* Nothing to do if not running. */ > @@ -1417,7 +1402,7 @@ void rvin_stop_streaming(struct rvin_dev *vin) > rvin_set_stream(vin, 0); > > /* disable interrupts */ > - rvin_disable_interrupts(vin); > + rvin_write(vin, 0, VNIE_REG); > > /* Return unprocessed buffers from hardware. */ > for (unsigned int i = 0; i < HW_BUFFER_NUM; i++) { -- Regards, Laurent Pinchart