Re: [PATCH v11 04/66] media: Documentation: Document -ENXIO for VIDIOC_SUBDEV_S_ROUTING

[Sakari Ailus <sakari.ailus@linux.intel.com>]

On Wed, Sep 03, 2025 at 03:47:55PM +0200, Laurent Pinchart wrote:
> On Tue, Sep 02, 2025 at 01:00:20PM +0300, Sakari Ailus wrote:
> > On Mon, Sep 01, 2025 at 01:33:56PM +0200, Jacopo Mondi wrote:
> > > On Mon, Aug 25, 2025 at 12:50:05PM +0300, Sakari Ailus wrote:
> > > > Document that -ENXIO is returned when the user tries to set a routing
> > > > configuration not supported by the hardware (or rather the driver). The
> > > > documentation details the exact cases of this, besides -EINVAL that is
> > > > already documented for VIDIOC_SUBDEV_S_ROUTING.
> > > >
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > >  .../media/v4l/vidioc-subdev-g-routing.rst             | 11 ++++++++++-
> > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > >
> > > > 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 1cf795480602..15f448664e6b 100644
> > > > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> > > > @@ -155,9 +155,18 @@ appropriately. The generic error codes are described at the
> > > >  :ref:`Generic Error Codes <gen-errors>` chapter.
> > > >
> > > >  EINVAL
> > > > +
> > > 
> > > Is this intended ?
> > 
> > Oops.
> > 
> > > >     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
> > > > -   pad), or the ``which`` field has an unsupported value.
> > > > +   pad), the ``which`` field has an unsupported value, or, for
> > > > +   ``VIDIOC_SUBDEV_S_ROUTING``, the num_routes field value is larger than that
> > > > +   of the len_routes field.
> > > 
> > > Doesn't this contradicts the above
> > > 
> > > The kernel can return a ``num_routes`` value larger than ``len_routes`` from
> > > both ioctls. This indicates thare are more routes in the routing table than fits
> > > the ``routes`` array. In this case, the ``routes`` array is filled by the kernel
> > > with the first ``len_routes`` entries of the subdevice routing table. This is
> > > not considered to be an error, and the ioctl call succeeds. If the applications
> > > wants to retrieve the missing routes, it can issue a new
> > > ``VIDIOC_SUBDEV_G_ROUTING`` call with a large enough ``routes`` array.
> > > 
> > > Looking at the handler for VIDIOC_SUBDEV_S_ROUTING in v4l2-subdev.c I
> > > think this should specify that is invalid to have userspace set a num_routes
> > > value larger than len_routes.
> > > 
> > > I would simply add this to the above hunk
> > > 
> > > -   pad), or the ``which`` field has an unsupported value.
> > > +   pad), the ``which`` field has an unsupported value, or, for
> > > +   ``VIDIOC_SUBDEV_S_ROUTING``, the num_routes field provided by the
> > > +   application is larger than the len_routes field value.
> > 
> > s/provided/set/ ?
> > 
> > Looks good to me.
> > 
> > > > +
> > > > +ENXIO
> > > > +   No such link can be created or such link state change can be made. Either the
> > > > +   sink or source (pad, stream) pair or the combination of the sink and source
> > > > +   is not supported by the hardware, or no multiple routes from or to the same
> > > > +   (pad, stream) pair are supported.
> > > 
> > > If I'm not mistaken this only applies to VIDIOC_SUBDEV_S_ROUTING, so I
> > > would mention that. Also ENXIO is not returned by the core but by
> > > drivers, so I would not limit the possible cases where ENXIO is
> > > returned by making examples.
> > > 
> > > What about a simpler:
> > > 
> > > +ENXIO
> > > +   The application requested routes cannot be created or the state of
> > > +   the specified routes cannot be modified. Only returned for
> > > +   ``VIDIOC_SUBDEV_S_ROUTING``.
> 
> This sounds fairly vague. I'd prefer keeping the description of the
> error conditions, but turning them from an exhaustive list into
> examples. How about the following ?
> 
> ENXIO
>    The routes specified by the application can't be applied because they violate
>    the driver's validity requirements. For instance, the application may be
>    trying to disable an immutable route, create a route that is not supported by
>    the device, or that conflicts with other routes. Only returned for
>    ``VIDIOC_SUBDEV_S_ROUTING``.
> 
> I'm also not too sure about ENXIO ("No such device or address"). There's
> no other error code that immediately strikes me as a good match though.
> If I were to propose a hack, I'd say EBADR could be used to mean "bad
> route" instead of "bad request" :-)

:-) What I think really counts is that it's different from EINVAL, albeit
I'm open to better suggestions. We could use e.g. EBADSLT, at least it'd
be easy to spot. Same goes for EBADR probably.

-- 
Sakari Ailus