Re: [PATCH v15 07/19] media: subdev: add v4l2_subdev_set_routing helper()

[Yunke Cao <yunkec@chromium.org>]

Hi Tomi,

On Wed, Oct 12, 2022 at 2:03 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
> Add a helper function to set the subdev routing. The helper can be used
> from subdev driver's set_routing op to store the routing table.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 31 +++++++++++++++++++++++++++
>  include/media/v4l2-subdev.h           | 16 ++++++++++++++
>  2 files changed, 47 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index fff17b8536fc..3ae4f39a50e4 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -12,6 +12,7 @@
>  #include <linux/ioctl.h>
>  #include <linux/mm.h>
>  #include <linux/module.h>
> +#include <linux/overflow.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/version.h>
> @@ -1181,6 +1182,36 @@ int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt);
>
> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
> +                           struct v4l2_subdev_state *state,
> +                           const struct v4l2_subdev_krouting *routing)
> +{
> +       struct v4l2_subdev_krouting *dst = &state->routing;
> +       const struct v4l2_subdev_krouting *src = routing;
> +       struct v4l2_subdev_krouting new_routing = { 0 };
> +       size_t bytes;
> +
> +       if (unlikely(check_mul_overflow(src->num_routes, sizeof(*src->routes),

Do we need to cast (size_t)src->num_routes here?
My compiler is complaining:
./include/linux/overflow.h:85:22: error: comparison of distinct
pointer types lacks a cast [-Werror]
   85 |         (void) (&__a == &__b);                  \

Thanks,
Yunke


> +                                       &bytes)))
> +               return -EOVERFLOW;
> +
> +       lockdep_assert_held(state->lock);
> +
> +       if (src->num_routes > 0) {
> +               new_routing.routes = kmemdup(src->routes, bytes, GFP_KERNEL);
> +               if (!new_routing.routes)
> +                       return -ENOMEM;
> +       }
> +
> +       new_routing.num_routes = src->num_routes;
> +
> +       kfree(dst->routes);
> +       *dst = new_routing;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_set_routing);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 45c41f4d6a2b..7962e6572bda 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1419,6 +1419,22 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd)
>  int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
>                         struct v4l2_subdev_format *format);
>
> +/**
> + * v4l2_subdev_set_routing() - Set given routing to subdev state
> + * @sd: The subdevice
> + * @state: The subdevice state
> + * @routing: Routing that will be copied to subdev state
> + *
> + * This will release old routing table (if any) from the state, allocate
> + * enough space for the given routing, and copy the routing.
> + *
> + * This can be used from the subdev driver's set_routing op, after validating
> + * the routing.
> + */
> +int v4l2_subdev_set_routing(struct v4l2_subdev *sd,
> +                           struct v4l2_subdev_state *state,
> +                           const struct v4l2_subdev_krouting *routing);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> --
> 2.34.1
>
>