Re: [PATCH v4 16/29] media: v4l2-subdev: Refactor returning routes

Sakari Ailus
2026-04-21T14:50:22Z

Hi Laurent,

On Thu, Apr 16, 2026 at 07:24:15PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Wed, Apr 08, 2026 at 06:39:25PM +0300, Sakari Ailus wrote:
> > Refactor returning the routes by adding a new function that essentially
> > does a memcopy and sets the number of the routes in the routing table.
>
> I'd write "factor out" instead of "refactor", here and in the subject
> line. Then you can add
>
> This avoids code duplication.
>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Michael Riesch <michael.riesch@collabora.com>
> > ---
> > drivers/media/v4l2-core/v4l2-subdev.c | 34 +++++++++++++--------------
> > 1 file changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index 831c69c958b8..f8fde395a53a 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -629,6 +629,19 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh,
> > v4l2_subdev_get_unlocked_active_state(sd);
> > }
> >
> > +static void copy_routes_state_to_routing(struct v4l2_subdev_routing *routing,
> > + const struct v4l2_subdev_state *state)
>
> v4l2_subdev_ prefix.

I'll call it v4l2_subdev_copy_routes().

>
> > +{
> > + struct v4l2_subdev_route *routes =
> > + (struct v4l2_subdev_route *)(uintptr_t)routing->routes;
> > + u32 copy_routes = min(routing->len_routes, state->routing.num_routes);
> > +
> > + for (u32 i = 0; i < copy_routes; i++)
> > + routes[i] = state->routing.routes[i];
>
> Any reason you use a loop instead of memcpy() ? If so, please document
> it in the commit message.

I'll switch to memcpy().

>
> With all that addressed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Thank you.

--
Kind regards,

Sakari Ailus