Received-SPF: pass (google.com: domain of linux-kernel+bounces-138385-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Wed, 10 Apr 2024 11:17:20 +0000
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Umang Jain <umang.jain@ideasonboard.com>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 8/9] media: subdev: Refactor
 v4l2_subdev_enable/disable_streams()
Message-ID: <ZhZ1QNAaRtplcero@kekkonen.localdomain>
References: <20240405-enable-streams-impro-v2-0-22bca967665d@ideasonboard.com>
 <20240405-enable-streams-impro-v2-8-22bca967665d@ideasonboard.com>
 <ZhZmNC4hQv1leL-y@kekkonen.localdomain>
 <ec113029-bf85-44c7-9e56-d242e5eecba0@ideasonboard.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ec113029-bf85-44c7-9e56-d242e5eecba0@ideasonboard.com>

On Wed, Apr 10, 2024 at 01:35:41PM +0300, Tomi Valkeinen wrote:
> On 10/04/2024 13:13, Sakari Ailus wrote:
> > Moi,
> > 
> > Thank you for working on this.
> > 
> > On Fri, Apr 05, 2024 at 12:14:26PM +0300, Tomi Valkeinen wrote:
> > > Add two internal helper functions, v4l2_subdev_collect_streams() and
> > > v4l2_subdev_set_streams_enabled(), which allows us to refactor
> > > v4l2_subdev_enable/disable_streams() functions.
> > > 
> > > This (I think) makes the code a bit easier to read, and lets us more
> > > easily add new functionality in the helper functions in the following
> > > patch.
> > > 
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > > ---
> > >   drivers/media/v4l2-core/v4l2-subdev.c | 111 +++++++++++++++++++---------------
> > >   1 file changed, 62 insertions(+), 49 deletions(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > > index 015f2fb423c9..6c3c9069f1e2 100644
> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > > @@ -2099,6 +2099,44 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
> > >   }
> > >   EXPORT_SYMBOL_GPL(v4l2_subdev_routing_validate);
> > > +static void v4l2_subdev_collect_streams(struct v4l2_subdev *sd,
> > > +					struct v4l2_subdev_state *state,
> > > +					u32 pad, u64 streams_mask,
> > > +					u64 *found_streams,
> > > +					u64 *enabled_streams)
> > > +{
> > > +	*found_streams = 0;
> > > +	*enabled_streams = 0;
> > > +
> > > +	for (unsigned int i = 0; i < state->stream_configs.num_configs; ++i) {
> > > +		const struct v4l2_subdev_stream_config *cfg;
> > > +
> > > +		cfg = &state->stream_configs.configs[i];
> > 
> > You can do the assignment in declaration.
> 
> I can, but you want the lines to be split at 80, so that'll end up being in
> two lines, which, I think, looks messier than the one above.
> 
> Usually I think doing initialization when declaring the variable is best
> done if it fits into one line.

I don't consider a line break being an issue here (or almost anywhere else,
except possibly for arrays of definitions).

-- 
Sakari Ailus