Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Kieran Bingham <kieran.bingham@ideasonboard.com>
Cc: Jacopo Mondi <jacopo.mondi@ideasonboard.com>,
	Sakari Ailus <sakari.ailus@iki.fi>,
	Daniel Scally <dan.scally@ideasonboard.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, nayden.kanchev@arm.com,
	robh+dt@kernel.org, mchehab@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	jerome.forissier@linaro.org
Subject: Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver
Date: Wed, 28 Feb 2024 15:38:47 +0200	[thread overview]
Message-ID: <20240228133847.GA9863@pendragon.ideasonboard.com> (raw)
In-Reply-To: <170912697659.1011926.1657561990919797055@ping.linuxembedded.co.uk>

On Wed, Feb 28, 2024 at 01:29:36PM +0000, Kieran Bingham wrote:
> Quoting Sakari Ailus (2024-02-28 13:11:39)
> > On Wed, Feb 28, 2024 at 01:50:14PM +0100, Jacopo Mondi wrote:
> > > > > +const struct mali_c55_fmt *mali_c55_cap_fmt_next(const struct mali_c55_fmt *fmt,
> > > > > +                                          bool allow_raw, bool unique)
> > > > > +{
> > > > > + if (!fmt)
> > > > > +         fmt = &mali_c55_fmts[0];
> > > > > + else
> > > > > +         ++fmt;
> > > >
> > > > fmt++, please.
> > > 
> > > Can I ask why ? (here and in the next occurrences you have reported)
> > 
> > It's much, much more common and using that form makes the code easier to
> > read. The rest of the driver primarily uses variable++, too, AFAIR.
> > 
> > So you should use ++variable only when you need it.
> 
> I don't think this is a hot path, but I'll never forget my C tutor
> telling us how ++i is more efficient than i++ somewhere to do with the
> opcode ordering, and not having to make a copy [*1]
> 
> Though I bet any clever optimising compiler could spot this anyway.
> 
> [*1]. Whatever plausibility there is based on a 20 year old memory and
> should be verified elsewhere.

In C I wouldn't expect any practical difference. C++ is a different
matter, as the prefix and postfix operators can have different
implementations.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel