linux-kernel - Re: [PATCH v2 0/3] media: add v4l2_subdev_state_xlate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250220143902.GA17500@pendragon.ideasonboard.com>
Date: Thu, 20 Feb 2025 16:39:02 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: Cosmin Tanislav <demonsingur@...il.com>,
	Jacopo Mondi <jacopo.mondi@...asonboard.com>,
	Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Julien Massot <julien.massot@...labora.com>,
	Bingbu Cao <bingbu.cao@...el.com>,
	Tianshu Qiu <tian.shu.qiu@...el.com>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Hans Verkuil <hverkuil@...all.nl>,
	Umang Jain <umang.jain@...asonboard.com>,
	Paweł Anikiel <panikiel@...gle.com>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 0/3] media: add v4l2_subdev_state_xlate_streams()

On Thu, Feb 20, 2025 at 02:16:51PM +0000, Sakari Ailus wrote:
> On Thu, Feb 20, 2025 at 03:01:41PM +0200, Cosmin Tanislav wrote:
> > On 2/20/25 1:38 PM, Jacopo Mondi wrote:
> > > On Thu, Feb 20, 2025 at 11:20:32AM +0200, Cosmin Tanislav wrote:
> > > > Currently, the v4l2_subdev_state_xlate_streams() function is used
> > > > to translate streams from one pad to another.
> > > > This function takes the entire subdev state as argument, but only makes
> > > > use of the routing.
> > > 
> > > Correct, but is this a problem ?
> > > 
> > 
> > No, it's not a problem.
> 
> I think I have a slight preference to keep the pattern of referring to the
> state as other functions do.
> 
> I wonder what Laurent and Hans think, too.

I agree, I think the state should be passed everywhere. This lowers the
risk of subsystem-wide refactoring if a function that receives a pointer
to part of a state (such as v4l2_subdev_routing_xlate_streams()) is
later found to need more information from the state.

The situation would be different if the states were not monolithic, for
instance if the routing table could be locked separatly from other parts
of the state, but that's not the case and I don't foresee moving to
finer-grained objects.

> > > Is this the first step for a larger rework or is this a drive-by
> > > beautification ?
> > 
> > Mostly a drive-by beautification to avoid passing the whole state around
> > where we only need the routing. I'm planning to submit drivers for more
> > GMSL2/3 chips and we're using this just to not pass the whole state
> > around. I think I can just use v4l2_subdev_state_xlate_streams(),
> > but I had these patches in my tree and it would have been good to get
> > them upstream, in preparations for submitting the GMSL2/3 drivers.
> > 
> > > I'm asking because (and I know it's hard to strike a balance) this
> > > kind of changes tend to make back-porting a more painful, and if
> > > only justified by "it looks better" I would be a bit hesitant in
> > > taking them.
> > > 
> > > > Introduce a v4l2_subdev_routing_xlate_streams() function which can be
> > > > used without the entire subdev state, to avoid passing the entire state
> > > > around when not needed.
> > > > 
> > > > Convert all usages of v4l2_subdev_state_xlate_streams() to
> > > > v4l2_subdev_routing_xlate_streams().
> > > > 
> > > > Remove v4l2_subdev_state_xlate_streams().
> > > > 
> > > > V2:
> > > >    * Fix description of parameters
> > > > 
> > > > Cosmin Tanislav (3):
> > > >    media: v4l: subdev: add v4l2_subdev_routing_xlate_streams()
> > > >    media: use v4l2_subdev_routing_xlate_streams()
> > > >    media: v4l: subdev: remove v4l2_subdev_state_xlate_streams()
> > > > 
> > > >   drivers/media/i2c/ds90ub913.c                 | 14 ++++++-----
> > > >   drivers/media/i2c/ds90ub953.c                 | 14 ++++++-----
> > > >   drivers/media/i2c/max96714.c                  | 16 ++++++-------
> > > >   drivers/media/i2c/max96717.c                  | 23 ++++++++++---------
> > > >   drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 14 ++++++-----
> > > >   .../platform/nxp/imx8-isi/imx8-isi-crossbar.c |  2 +-
> > > >   drivers/media/v4l2-core/v4l2-subdev.c         |  7 +++---
> > > >   include/media/v4l2-subdev.h                   | 10 ++++----
> > > >   8 files changed, 53 insertions(+), 47 deletions(-)

-- 
Regards,

Laurent Pinchart