On Tue, Apr 23, 2024 at 10:49:11AM +0000, Sakari Ailus wrote:
> On Sat, Apr 20, 2024 at 01:55:52AM +0300, Laurent Pinchart wrote:
> > On Tue, Apr 16, 2024 at 10:32:49PM +0300, 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>
> > > ---
> > > .../media/v4l/vidioc-subdev-g-routing.rst | 7 ++-----
> > > drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++++++-
> > > 2 files changed, 13 insertions(+), 6 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 27eb4c82a0e1..88df7bf80b00 100644
> > > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> > > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> > > @@ -44,7 +44,8 @@ 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`.
> > > +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also
> > > +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user.
> >
> > :c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``,
> > ``VIDIOC_SUBDEV_S_ROUTING`` also returns the routes back to the user.
>
> The intention here is to say that S_ROUTING returns the routes back to the
> user, just as G_ROUTING does. Moving "also" after S_ROUTING makes the
> meaning of the sentence unclear.
Keeping it before makes it sound non-native English to me :-) But I'm
not a native speaker.
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> Thanks!
>
> > >
> > > All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called.
> > > This means that the userspace must reconfigure all stream formats and selections
> > > @@ -157,10 +158,6 @@ On success 0 is returned, on error -1 and the ``errno`` variable is set
> > > appropriately. The generic error codes are described at the
> > > :ref:`Generic Error Codes <gen-errors>` chapter.
> > >
> > > -ENOSPC
> > > - The application provided ``num_routes`` is not big enough to contain
> > > - all the available routes the subdevice exposes.
> > > -
> > > EINVAL
> > > The sink or source pad identifiers reference a non-existing pad or reference
> > > pads of different types (ie. the sink_pad identifiers refers to a source
> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > > index 904378007a79..36b2f78cd551 100644
> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > > @@ -959,8 +959,18 @@ 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;
> > > +
> > > + memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes,
> > > + state->routing.routes,
> > > + min(state->routing.num_routes, routing->len_routes) *
> > > + sizeof(*state->routing.routes));
> > > + routing->num_routes = state->routing.num_routes;
> > > +
> > > + return 0;
> > > }
> > >
> > > case VIDIOC_SUBDEV_G_ROUTING: {
--
Regards,
Laurent Pinchart
