Hi Hans, On Thu, Oct 05, 2023 at 01:05:17PM +0200, Hans Verkuil wrote: > 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. Ack, I'll change this for v7. -- Regards, Sakari Ailus
