Re: [PATCH] [media] v4l2-subdev: return -EPIPE instead of -EINVAL in link validate default

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

On Monday 29 June 2015 10:23:34 Sakari Ailus wrote:
> Helen Fornazier wrote:
> > According to the V4L2 API, the VIDIOC_STREAMON ioctl should return EPIPE
> > when the pipeline configuration is invalid.
> > 
> > As the .vidioc_streamon in the v4l2_ioctl_ops usually forwards the error
> > caused by the v4l2_subdev_link_validate_default (if it is in use), it
> > should return -EPIPE if it detects a format mismatch in the pipeline
> > configuration
> 
> Only link configuration errors have yielded -EPIPE so far, sub-device
> format configuration error has returned -INVAL instead as you noticed.

It should also be noted that while v4l2_subdev_link_validate() will return -
EINVAL in case of error, the only driver that performs custom link validation 
(omap3isp/ispccdc.c) will return -EPIPE.

> There are not many sources of -EINVAL while enabling streaming and all
> others are directly caused by the application; I lean towards thinking
> the code is good as it was. The documentation could be improved though.
> It may not be clear which error codes could be caused by different
> conditions.
> 
> The debug level messages from media module
> (drivers/media/media-entity.c) do provide more information if needed,
> albeit this certainly is not an application interface.
> 
> I wonder what others think.

There's a discrepancy between the implementation and the documentation, so at 
least one of them need to be fixed. -EPIPE would be coherent with the 
documentation and seems appropriately named, but another error code would 
allow userspace to tell link configuration and format configuration problems 
apart.

Do you think -EINVAL is the most appropriate error code for format 
configuration ? It's already used to indicate that the stream type is invalid 
or that not enough buffers have been allocated, and is also used by drivers 
directly for various purposes.

-- 
Regards,

Laurent Pinchart