On 03/10/2023 14:08, Sakari Ailus wrote:
> Return the routes set using S_ROUTING back to the user. Also reflect this
> in documentation.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> .../userspace-api/media/v4l/vidioc-subdev-g-routing.rst | 5 +++--
> drivers/media/v4l2-core/v4l2-subdev.c | 5 ++++-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> index 9a9765ddc316..ced53ea5f23c 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> @@ -43,8 +43,9 @@ The routing configuration determines the flows of data inside an entity.
> Drivers report their current routing tables using the
> ``VIDIOC_SUBDEV_G_ROUTING`` ioctl and application may enable or disable routes
> with the ``VIDIOC_SUBDEV_S_ROUTING`` ioctl, by adding or removing routes and
> -setting or clearing flags of the ``flags`` field of a
> -struct :c:type:`v4l2_subdev_route`.
> +setting or clearing flags of the ``flags`` field of a struct
> +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also
> +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user.
>
> All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is
> called. This means that the userspace must reconfigure all streams after calling
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index bd1e8205913c..9a34e13dfd96 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -935,9 +935,12 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
> krouting.len_routes = routing->len_routes;
> krouting.routes = routes;
>
> - return v4l2_subdev_call(sd, pad, set_routing, state,
> + rval = v4l2_subdev_call(sd, pad, set_routing, state,
> routing->which, &krouting);
> + if (rval < 0)
> + return rval;
> }
> + fallthrough;
Admittedly, it is a bit hard to see from just this patch, but I think
the actual code that VIDIOC_SUBDEV_S_ROUTING has to do here is just
a very few lines from VIDIOC_SUBDEV_G_ROUTING. I think it is better
to explicitly have them here, then doing a fallthrough.
Most of what is in VIDIOC_SUBDEV_G_ROUTING are just a bunch of checks
that are already done in VIDIOC_SUBDEV_S_ROUTING.
Regards,
Hans
>
> case VIDIOC_SUBDEV_G_ROUTING: {
> struct v4l2_subdev_routing *routing = arg;
