Re: [PATCH v2 8/9] media: subdev: Refactor v4l2_subdev_enable/disable_streams()

[Sakari Ailus <sakari.ailus@linux.intel.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