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

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

[Helen Fornazier]

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

Signed-off-by: Helen Fornazier <helen.fornazier@gmail.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 6359606..5e64342 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -508,7 +508,7 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 	if (source_fmt->format.width != sink_fmt->format.width
 	    || source_fmt->format.height != sink_fmt->format.height
 	    || source_fmt->format.code != sink_fmt->format.code)
-		return -EINVAL;
+		return -EPIPE;
 
 	/* The field order must match, or the sink field order must be NONE
 	 * to support interlaced hardware connected to bridges that support
@@ -516,7 +516,7 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 	 */
 	if (source_fmt->format.field != sink_fmt->format.field &&
 	    sink_fmt->format.field != V4L2_FIELD_NONE)
-		return -EINVAL;
+		return -EPIPE;
 
 	return 0;
 }
-- 
1.9.1

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

[Sakari Ailus]

Hi Helen,

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.
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.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

[Laurent Pinchart]

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

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

[Sakari Ailus]

Hi Laurent,

Laurent Pinchart wrote:
> 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.

Good point. That has escaped me until now.

>> 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.

That was the original intent, I think.

> 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.

That's true, it's been used also for that purpose. At the time this
certainly was not the primary concern. If you can think of a better
error code for the purpose (than EINVAL) I'm certainly fine with using one.

I still think that -EPIPE is worse for telling about incorrect format
configuration than -EINVAL since it's relatively easy to avoid -EINVAL
for the documented reasons.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

[Helen Fornazier]

Hi Sakari and Laurent,

Thanks for reviewing this

On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> Hi Laurent,
>
> Laurent Pinchart wrote:
>> 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.
>
> Good point. That has escaped me until now.
>
>>> 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.
>
> That was the original intent, I think.
>
>> 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.
>
> That's true, it's been used also for that purpose. At the time this
> certainly was not the primary concern. If you can think of a better
> error code for the purpose (than EINVAL) I'm certainly fine with using one.
>
> I still think that -EPIPE is worse for telling about incorrect format
> configuration than -EINVAL since it's relatively easy to avoid -EINVAL
> for the documented reasons.
>
> --
> Kind regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com

I'd like just to point out where in the docs EPIPE for format mismatch
is specified, as it is not described in the streamon page as I thought
it would, but it is in the subdev page in case anyone is looking for
it (as I took some time to find it too):

http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
"Applications are responsible for configuring coherent parameters on
the whole pipeline and making sure that connected pads have compatible
formats. The pipeline is checked for formats mismatch at
VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
configuration is invalid"

So maybe the doc should be improved as you already stated.

-- 
Helen Fornazier

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

[Helen Fornazier]

Hi

On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier
<helen.fornazier@gmail.com> wrote:
>
> Hi Sakari and Laurent,
>
> Thanks for reviewing this
>
> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> > Hi Laurent,
> >
> > Laurent Pinchart wrote:
> >> 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.
> >
> > Good point. That has escaped me until now.
> >
> >>> 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.
> >
> > That was the original intent, I think.
> >
> >> 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.
> >
> > That's true, it's been used also for that purpose. At the time this
> > certainly was not the primary concern. If you can think of a better
> > error code for the purpose (than EINVAL) I'm certainly fine with using one.
> >
> > I still think that -EPIPE is worse for telling about incorrect format
> > configuration than -EINVAL since it's relatively easy to avoid -EINVAL
> > for the documented reasons.
> >
> > --
> > Kind regards,
> >
> > Sakari Ailus
> > sakari.ailus@linux.intel.com
>
> I'd like just to point out where in the docs EPIPE for format mismatch
> is specified, as it is not described in the streamon page as I thought
> it would, but it is in the subdev page in case anyone is looking for
> it (as I took some time to find it too):
>
> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
> "Applications are responsible for configuring coherent parameters on
> the whole pipeline and making sure that connected pads have compatible
> formats. The pipeline is checked for formats mismatch at
> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
> configuration is invalid"
>
> So maybe the doc should be improved as you already stated.
>
> --
> Helen Fornazier


I would like to revive this subject.

Should we change the docs? Change the -EINVAL to -EPIPE, or create
another error code? What are your opinion?

I read in the docs of dev-kmsg that EPIPE is returned when messages
get overwritten, and in other parts of the code EPIPE is returned when
there is an error in the pipeline communication level while trying to
send information through the pipe or a pipe broken error.

But in the error-codes.txt files, the EPIPE error is defined as:
EPIPE "The pipe type specified in the URB doesn't match the endpoint's
actual type"

Then, if EPIPE is used when types don't match between two endpoints,
it seems reasonable to me to use EPIPE when formats don't match
either. Or do "types" in this context have a specific definition? I
don't know much about URB, you may be able to judge this better.

Regards

-- 
Helen Fornazier

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

[Sakari Ailus]

Hi Helen,

Helen Fornazier wrote:
> Hi,
> 
> On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier <helen.fornazier@gmail.com>
> wrote:
> 
>> Hi Sakari and Laurent,
>>
>> Thanks for reviewing this
>>
>> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus
>> <sakari.ailus@linux.intel.com> wrote:
>>> Hi Laurent,
>>>
>>> Laurent Pinchart wrote:
>>>> 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.
>>>
>>> Good point. That has escaped me until now.
>>>
>>>>> 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.
>>>
>>> That was the original intent, I think.
>>>
>>>> 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.
>>>
>>> That's true, it's been used also for that purpose. At the time this
>>> certainly was not the primary concern. If you can think of a better
>>> error code for the purpose (than EINVAL) I'm certainly fine with using
>> one.
>>>
>>> I still think that -EPIPE is worse for telling about incorrect format
>>> configuration than -EINVAL since it's relatively easy to avoid -EINVAL
>>> for the documented reasons.
>>>
>>> --
>>> Kind regards,
>>>
>>> Sakari Ailus
>>> sakari.ailus@linux.intel.com
>>
>> I'd like just to point out where in the docs EPIPE for format mismatch
>> is specified, as it is not described in the streamon page as I thought
>> it would, but it is in the subdev page in case anyone is looking for
>> it (as I took some time to find it too):
>>
>> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
>> "Applications are responsible for configuring coherent parameters on
>> the whole pipeline and making sure that connected pads have compatible
>> formats. The pipeline is checked for formats mismatch at
>> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
>> configuration is invalid"
>>
>> So maybe the doc should be improved as you already stated.
>>
>> --
>> Helen Fornazier
>>
> 
> I would like to revive this subject.
> 
> Should we change the docs? Change the -EINVAL to -EPIPE, or create another
> error code? What are your opinion?
> 
> I read in the docs of dev-kmsg that EPIPE is returned when messages get
> overwritten, and in other parts of the code EPIPE is returned when there is
> an error in the pipeline communication level while trying to send
> information through the pipe or a pipe broken error.
> 
> But in the error-codes.txt files, the EPIPE error is defined as:
> *EPIPE "The pipe type specified in the URB doesn't match the endpoint's
> actual type"*

This exact definition sound USB specific to me.

> Then, if EPIPE is used when types don't match between two endpoints, it
> seems reasonable to me to use EPIPE when formats don't match either. Or do
> "types" in this context have a specific definition? I don't know much about
> URB, you may be able to judge this better.

A short recap of the current situation as far as I understand it:

- MC link validation failure yields EPIPE to the user space,

- V4L2 sub-device format validation failure generally results in EINVAL,
except that

- omap3isp CCDC driver returns EPIPE instead and

- EINVAL is used for many other purposes.

The issues are inconsistency between omap3isp CCDC and other drivers in
informing the user the sub-device format configuration is wrong. Also
V4L2 sub-device format validation error cannot be told apart from other
errors. These problems should be fixed, so that all three sources of
errors yield a different error code (MC link validation, V4L2 format
configuration and other plain V4L2 related errors).

V4L2 will continue using EINVAL, that's for sure.

Another error code I could think of is EMLINK ("Too many links"), which
is not a perfect match, but could be used. This is a better match for a
link validation failure; V4L2 sub-device link validation failure would
then use EPIPE (as omap3isp CCDC driver already does).

Another option could be that V4L2 format validation failure would use
ENOEXEC ("Exec format error") instead, and EPIPE would be left to link
validation failures.

Better suggestions are welcome of course. I think I'm leaning towards
the first option, but from backwards compatibility point of view the
latter is better. The MC is no longer experimental so the latter might
be the only option.

My view is that this boils down to picking the most suitable error
codes. Then fixing the documentation is easy.

I wonder what Laurent and Hans think.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

[Laurent Pinchart]

Hello,

On Monday 13 July 2015 11:03:43 Sakari Ailus wrote:
> Helen Fornazier wrote:
> > On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier wrote:
> >> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus wrote:
> >>> Laurent Pinchart wrote:
> >>>> 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.
> >>> 
> >>> Good point. That has escaped me until now.
> >>> 
> >>>>> 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.
> >>> 
> >>> That was the original intent, I think.
> >>> 
> >>>> 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.
> >>> 
> >>> That's true, it's been used also for that purpose. At the time this
> >>> certainly was not the primary concern. If you can think of a better
> >>> error code for the purpose (than EINVAL) I'm certainly fine with using
> >>> one.
> >>>
> >>> I still think that -EPIPE is worse for telling about incorrect format
> >>> configuration than -EINVAL since it's relatively easy to avoid -EINVAL
> >>> for the documented reasons.
> >>> 
> >> 
> >> I'd like just to point out where in the docs EPIPE for format mismatch
> >> is specified, as it is not described in the streamon page as I thought
> >> it would, but it is in the subdev page in case anyone is looking for
> >> it (as I took some time to find it too):
> >> 
> >> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
> >> "Applications are responsible for configuring coherent parameters on
> >> the whole pipeline and making sure that connected pads have compatible
> >> formats. The pipeline is checked for formats mismatch at
> >> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
> >> configuration is invalid"
> >> 
> >> So maybe the doc should be improved as you already stated.
> > 
> > I would like to revive this subject.
> > 
> > Should we change the docs? Change the -EINVAL to -EPIPE, or create another
> > error code? What are your opinion?
> > 
> > I read in the docs of dev-kmsg that EPIPE is returned when messages get
> > overwritten, and in other parts of the code EPIPE is returned when there
> > is an error in the pipeline communication level while trying to send
> > information through the pipe or a pipe broken error.
> > 
> > But in the error-codes.txt files, the EPIPE error is defined as:
> > *EPIPE "The pipe type specified in the URB doesn't match the endpoint's
> > actual type"*

Just a bit of background information first. The Linux kernel uses error codes 
standardized by POSIX. A limited number of additional error codes have been 
added over time, but the usual approach when an error that doesn't match POSIX 
semantics is to reuse an existing error code whose name matches the error 
relatively well. EPIPE in USB is such an example, POSIX error codes have no 
knowledge of USB. We have similarly selected EPIPE for the media controller 
API as it seemed a good match to report errors related to the pipeline.

> This exact definition sound USB specific to me.

Yes, error-codes.txt is in Documentation/usb/, so that's expected :-)

> > Then, if EPIPE is used when types don't match between two endpoints, it
> > seems reasonable to me to use EPIPE when formats don't match either. Or do
> > "types" in this context have a specific definition? I don't know much
> > about URB, you may be able to judge this better.
> 
> A short recap of the current situation as far as I understand it:
> 
> - MC link validation failure yields EPIPE to the user space,
> 
> - V4L2 sub-device format validation failure generally results in EINVAL,
> except that
> 
> - omap3isp CCDC driver returns EPIPE instead and
> 
> - EINVAL is used for many other purposes.
> 
> The issues are inconsistency between omap3isp CCDC and other drivers in
> informing the user the sub-device format configuration is wrong. Also
> V4L2 sub-device format validation error cannot be told apart from other
> errors. These problems should be fixed, so that all three sources of
> errors yield a different error code (MC link validation, V4L2 format
> configuration and other plain V4L2 related errors).
> 
> V4L2 will continue using EINVAL, that's for sure.
> 
> Another error code I could think of is EMLINK ("Too many links"), which

ENOLINK might be better.

> is not a perfect match, but could be used. This is a better match for a
> link validation failure; V4L2 sub-device link validation failure would
> then use EPIPE (as omap3isp CCDC driver already does).
> 
> Another option could be that V4L2 format validation failure would use
> ENOEXEC ("Exec format error") instead, and EPIPE would be left to link
> validation failures.

Granted, ENOEXEC mentions the word "format" in its documentation, but it's a 
bit far-fetched :-)

> Better suggestions are welcome of course. I think I'm leaning towards
> the first option, but from backwards compatibility point of view the
> latter is better. The MC is no longer experimental so the latter might
> be the only option.
> 
> My view is that this boils down to picking the most suitable error
> codes. Then fixing the documentation is easy.
> 
> I wonder what Laurent and Hans think.

Using three different error codes as you mention above has my preference, but 
we need to care about backward compatibility. No solution will be perfect 
though, as the OMAP3 ISP returns different error codes for the same error 
depending on which entity is concerned, so unifying the error codes will 
result in user-visible changes.

Replacing EPIPE with EMLINK or ENOLINK is tempting but might be too risky in 
terms of backward compatibility. If we can't do that, I'd prefer using EPIPE 
to indicate broken pipelines due to both link setup issues and format 
validation failures. This would at least match the documentation.

Hans, any opinion ?

-- 
Regards,

Laurent Pinchart

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

[Hans Verkuil]

On 07/13/15 10:03, Sakari Ailus wrote:
> Hi Helen,
> 
> Helen Fornazier wrote:
>> Hi,
>>
>> On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier <helen.fornazier@gmail.com>
>> wrote:
>>
>>> Hi Sakari and Laurent,
>>>
>>> Thanks for reviewing this
>>>
>>> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus
>>> <sakari.ailus@linux.intel.com> wrote:
>>>> Hi Laurent,
>>>>
>>>> Laurent Pinchart wrote:
>>>>> 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.
>>>>
>>>> Good point. That has escaped me until now.
>>>>
>>>>>> 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.
>>>>
>>>> That was the original intent, I think.
>>>>
>>>>> 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.
>>>>
>>>> That's true, it's been used also for that purpose. At the time this
>>>> certainly was not the primary concern. If you can think of a better
>>>> error code for the purpose (than EINVAL) I'm certainly fine with using
>>> one.
>>>>
>>>> I still think that -EPIPE is worse for telling about incorrect format
>>>> configuration than -EINVAL since it's relatively easy to avoid -EINVAL
>>>> for the documented reasons.
>>>>
>>>> --
>>>> Kind regards,
>>>>
>>>> Sakari Ailus
>>>> sakari.ailus@linux.intel.com
>>>
>>> I'd like just to point out where in the docs EPIPE for format mismatch
>>> is specified, as it is not described in the streamon page as I thought
>>> it would, but it is in the subdev page in case anyone is looking for
>>> it (as I took some time to find it too):
>>>
>>> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
>>> "Applications are responsible for configuring coherent parameters on
>>> the whole pipeline and making sure that connected pads have compatible
>>> formats. The pipeline is checked for formats mismatch at
>>> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
>>> configuration is invalid"
>>>
>>> So maybe the doc should be improved as you already stated.
>>>
>>> --
>>> Helen Fornazier
>>>
>>
>> I would like to revive this subject.
>>
>> Should we change the docs? Change the -EINVAL to -EPIPE, or create another
>> error code? What are your opinion?
>>
>> I read in the docs of dev-kmsg that EPIPE is returned when messages get
>> overwritten, and in other parts of the code EPIPE is returned when there is
>> an error in the pipeline communication level while trying to send
>> information through the pipe or a pipe broken error.
>>
>> But in the error-codes.txt files, the EPIPE error is defined as:
>> *EPIPE "The pipe type specified in the URB doesn't match the endpoint's
>> actual type"*
> 
> This exact definition sound USB specific to me.
> 
>> Then, if EPIPE is used when types don't match between two endpoints, it
>> seems reasonable to me to use EPIPE when formats don't match either. Or do
>> "types" in this context have a specific definition? I don't know much about
>> URB, you may be able to judge this better.
> 
> A short recap of the current situation as far as I understand it:
> 
> - MC link validation failure yields EPIPE to the user space,
> 
> - V4L2 sub-device format validation failure generally results in EINVAL,
> except that

I think that returning EINVAL here is wrong. EINVAL is returned when you
set e.g. a format and the format is invalid. In this case the format for
each subdev is perfectly fine, it's just that the sink and source formats
do not match.

Rather than returning EINVAL I think ENOLINK would work well here as you
can't setup a link between two entities. And EPIPE can still be used
for other higher-level pipeline errors.

> - omap3isp CCDC driver returns EPIPE instead and

That seems definitely the wrong thing to do.

> - EINVAL is used for many other purposes.
> 
> The issues are inconsistency between omap3isp CCDC and other drivers in
> informing the user the sub-device format configuration is wrong. Also
> V4L2 sub-device format validation error cannot be told apart from other
> errors. These problems should be fixed, so that all three sources of
> errors yield a different error code (MC link validation, V4L2 format
> configuration and other plain V4L2 related errors).
> 
> V4L2 will continue using EINVAL, that's for sure.
> 
> Another error code I could think of is EMLINK ("Too many links"), which
> is not a perfect match, but could be used. This is a better match for a
> link validation failure; V4L2 sub-device link validation failure would
> then use EPIPE (as omap3isp CCDC driver already does).
> 
> Another option could be that V4L2 format validation failure would use
> ENOEXEC ("Exec format error") instead, and EPIPE would be left to link
> validation failures.
> 
> Better suggestions are welcome of course. I think I'm leaning towards
> the first option, but from backwards compatibility point of view the
> latter is better. The MC is no longer experimental so the latter might
> be the only option.
> 
> My view is that this boils down to picking the most suitable error
> codes. Then fixing the documentation is easy.
> 
> I wonder what Laurent and Hans think.
> 

Regards,

	Hans

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

[Laurent Pinchart]

Hi Hans,

On Tuesday 14 July 2015 16:19:50 Hans Verkuil wrote:
> On 07/13/15 10:03, Sakari Ailus wrote:
> > Helen Fornazier wrote:
> >> On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier wrote:
> >>> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus wrote:
> >>>> Laurent Pinchart wrote:
> >>>>> 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.
> >>>> 
> >>>> Good point. That has escaped me until now.
> >>>> 
> >>>>>> 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.
> >>>> 
> >>>> That was the original intent, I think.
> >>>> 
> >>>>> 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.
> >>>> 
> >>>> That's true, it's been used also for that purpose. At the time this
> >>>> certainly was not the primary concern. If you can think of a better
> >>>> error code for the purpose (than EINVAL) I'm certainly fine with using
> >>>> one. I still think that -EPIPE is worse for telling about incorrect
> >>>> format configuration than -EINVAL since it's relatively easy to avoid -
> >>>> EINVAL for the documented reasons.
> >>> 
> >>> I'd like just to point out where in the docs EPIPE for format mismatch
> >>> is specified, as it is not described in the streamon page as I thought
> >>> it would, but it is in the subdev page in case anyone is looking for
> >>> it (as I took some time to find it too):
> >>> 
> >>> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
> >>> "Applications are responsible for configuring coherent parameters on
> >>> the whole pipeline and making sure that connected pads have compatible
> >>> formats. The pipeline is checked for formats mismatch at
> >>> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
> >>> configuration is invalid"
> >>> 
> >>> So maybe the doc should be improved as you already stated.
> >> 
> >> I would like to revive this subject.
> >> 
> >> Should we change the docs? Change the -EINVAL to -EPIPE, or create
> >> another error code? What are your opinion?
> >> 
> >> I read in the docs of dev-kmsg that EPIPE is returned when messages get
> >> overwritten, and in other parts of the code EPIPE is returned when there
> >> is an error in the pipeline communication level while trying to send
> >> information through the pipe or a pipe broken error.
> >> 
> >> But in the error-codes.txt files, the EPIPE error is defined as:
> >> *EPIPE "The pipe type specified in the URB doesn't match the endpoint's
> >> actual type"*
> > 
> > This exact definition sound USB specific to me.
> > 
> >> Then, if EPIPE is used when types don't match between two endpoints, it
> >> seems reasonable to me to use EPIPE when formats don't match either. Or
> >> do "types" in this context have a specific definition? I don't know much
> >> about URB, you may be able to judge this better.
> > 
> > A short recap of the current situation as far as I understand it:
> > 
> > - MC link validation failure yields EPIPE to the user space,
> > 
> > - V4L2 sub-device format validation failure generally results in EINVAL,
> > except that
> 
> I think that returning EINVAL here is wrong. EINVAL is returned when you
> set e.g. a format and the format is invalid. In this case the format for
> each subdev is perfectly fine, it's just that the sink and source formats
> do not match.
> 
> Rather than returning EINVAL I think ENOLINK would work well here as you
> can't setup a link between two entities. And EPIPE can still be used
> for other higher-level pipeline errors.
> 
> > - omap3isp CCDC driver returns EPIPE instead and
> 
> That seems definitely the wrong thing to do.

The VIDIOC_STREAMON documentation states that

"EPIPE

The driver implements pad-level format configuration and the pipeline 
configuration is invalid."

According to the documentation, EINVAL is clearly wrong, and EPIPE should be 
used. I'm open to the idea of using different error codes to indicate severed 
link errors and links with an invalid configuration, but how about backward 
compatibility ?

> > - EINVAL is used for many other purposes.
> > 
> > The issues are inconsistency between omap3isp CCDC and other drivers in
> > informing the user the sub-device format configuration is wrong. Also
> > V4L2 sub-device format validation error cannot be told apart from other
> > errors. These problems should be fixed, so that all three sources of
> > errors yield a different error code (MC link validation, V4L2 format
> > configuration and other plain V4L2 related errors).
> > 
> > V4L2 will continue using EINVAL, that's for sure.
> > 
> > Another error code I could think of is EMLINK ("Too many links"), which
> > is not a perfect match, but could be used. This is a better match for a
> > link validation failure; V4L2 sub-device link validation failure would
> > then use EPIPE (as omap3isp CCDC driver already does).
> > 
> > Another option could be that V4L2 format validation failure would use
> > ENOEXEC ("Exec format error") instead, and EPIPE would be left to link
> > validation failures.
> > 
> > Better suggestions are welcome of course. I think I'm leaning towards
> > the first option, but from backwards compatibility point of view the
> > latter is better. The MC is no longer experimental so the latter might
> > be the only option.
> > 
> > My view is that this boils down to picking the most suitable error
> > codes. Then fixing the documentation is easy.
> > 
> > I wonder what Laurent and Hans think.

-- 
Regards,

Laurent Pinchart

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

[Helen Fornazier]

Hello

On Tue, Jul 14, 2015 at 11:32 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Hans,
>
> On Tuesday 14 July 2015 16:19:50 Hans Verkuil wrote:
>> On 07/13/15 10:03, Sakari Ailus wrote:
>> > Helen Fornazier wrote:
>> >> On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier wrote:
>> >>> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus wrote:
>> >>>> Laurent Pinchart wrote:
>> >>>>> 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.
>> >>>>
>> >>>> Good point. That has escaped me until now.
>> >>>>
>> >>>>>> 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.
>> >>>>
>> >>>> That was the original intent, I think.
>> >>>>
>> >>>>> 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.
>> >>>>
>> >>>> That's true, it's been used also for that purpose. At the time this
>> >>>> certainly was not the primary concern. If you can think of a better
>> >>>> error code for the purpose (than EINVAL) I'm certainly fine with using
>> >>>> one. I still think that -EPIPE is worse for telling about incorrect
>> >>>> format configuration than -EINVAL since it's relatively easy to avoid -
>> >>>> EINVAL for the documented reasons.
>> >>>
>> >>> I'd like just to point out where in the docs EPIPE for format mismatch
>> >>> is specified, as it is not described in the streamon page as I thought
>> >>> it would, but it is in the subdev page in case anyone is looking for
>> >>> it (as I took some time to find it too):
>> >>>
>> >>> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
>> >>> "Applications are responsible for configuring coherent parameters on
>> >>> the whole pipeline and making sure that connected pads have compatible
>> >>> formats. The pipeline is checked for formats mismatch at
>> >>> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
>> >>> configuration is invalid"
>> >>>
>> >>> So maybe the doc should be improved as you already stated.
>> >>
>> >> I would like to revive this subject.
>> >>
>> >> Should we change the docs? Change the -EINVAL to -EPIPE, or create
>> >> another error code? What are your opinion?
>> >>
>> >> I read in the docs of dev-kmsg that EPIPE is returned when messages get
>> >> overwritten, and in other parts of the code EPIPE is returned when there
>> >> is an error in the pipeline communication level while trying to send
>> >> information through the pipe or a pipe broken error.
>> >>
>> >> But in the error-codes.txt files, the EPIPE error is defined as:
>> >> *EPIPE "The pipe type specified in the URB doesn't match the endpoint's
>> >> actual type"*
>> >
>> > This exact definition sound USB specific to me.
>> >
>> >> Then, if EPIPE is used when types don't match between two endpoints, it
>> >> seems reasonable to me to use EPIPE when formats don't match either. Or
>> >> do "types" in this context have a specific definition? I don't know much
>> >> about URB, you may be able to judge this better.
>> >
>> > A short recap of the current situation as far as I understand it:
>> >
>> > - MC link validation failure yields EPIPE to the user space,
>> >
>> > - V4L2 sub-device format validation failure generally results in EINVAL,
>> > except that
>>
>> I think that returning EINVAL here is wrong. EINVAL is returned when you
>> set e.g. a format and the format is invalid. In this case the format for
>> each subdev is perfectly fine, it's just that the sink and source formats
>> do not match.
>>
>> Rather than returning EINVAL I think ENOLINK would work well here as you
>> can't setup a link between two entities. And EPIPE can still be used
>> for other higher-level pipeline errors.
>>
>> > - omap3isp CCDC driver returns EPIPE instead and
>>
>> That seems definitely the wrong thing to do.
>
> The VIDIOC_STREAMON documentation states that
>
> "EPIPE
>
> The driver implements pad-level format configuration and the pipeline
> configuration is invalid."
>
> According to the documentation, EINVAL is clearly wrong, and EPIPE should be
> used. I'm open to the idea of using different error codes to indicate severed
> link errors and links with an invalid configuration, but how about backward
> compatibility ?
>

I don't think backward compatibility is a big issue, as it seems the
code wasn't returning what the docs says and nobody complained about
it so far.

I see 4 options then:

1) Keep -EINVAL and update the docs
2) Change the code to -EPIPE (there is no need in updating the docs here)
3) Change the code to -ENOLINK and update the docs
4) Create another error (-EFMT for example), change the code to it and
update the docs

which one do you prefer?

>> > - EINVAL is used for many other purposes.
>> >
>> > The issues are inconsistency between omap3isp CCDC and other drivers in
>> > informing the user the sub-device format configuration is wrong. Also
>> > V4L2 sub-device format validation error cannot be told apart from other
>> > errors. These problems should be fixed, so that all three sources of
>> > errors yield a different error code (MC link validation, V4L2 format
>> > configuration and other plain V4L2 related errors).
>> >
>> > V4L2 will continue using EINVAL, that's for sure.
>> >
>> > Another error code I could think of is EMLINK ("Too many links"), which
>> > is not a perfect match, but could be used. This is a better match for a
>> > link validation failure; V4L2 sub-device link validation failure would
>> > then use EPIPE (as omap3isp CCDC driver already does).
>> >
>> > Another option could be that V4L2 format validation failure would use
>> > ENOEXEC ("Exec format error") instead, and EPIPE would be left to link
>> > validation failures.
>> >
>> > Better suggestions are welcome of course. I think I'm leaning towards
>> > the first option, but from backwards compatibility point of view the
>> > latter is better. The MC is no longer experimental so the latter might
>> > be the only option.
>> >
>> > My view is that this boils down to picking the most suitable error
>> > codes. Then fixing the documentation is easy.
>> >
>> > I wonder what Laurent and Hans think.
>
> --
> Regards,
>
> Laurent Pinchart
>



-- 
Helen Fornazier

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

[Hans Verkuil]

On 07/14/2015 04:32 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Tuesday 14 July 2015 16:19:50 Hans Verkuil wrote:
>> On 07/13/15 10:03, Sakari Ailus wrote:
>>> Helen Fornazier wrote:
>>>> On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier wrote:
>>>>> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus wrote:
>>>>>> Laurent Pinchart wrote:
>>>>>>> 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.
>>>>>>
>>>>>> Good point. That has escaped me until now.
>>>>>>
>>>>>>>> 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.
>>>>>>
>>>>>> That was the original intent, I think.
>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> That's true, it's been used also for that purpose. At the time this
>>>>>> certainly was not the primary concern. If you can think of a better
>>>>>> error code for the purpose (than EINVAL) I'm certainly fine with using
>>>>>> one. I still think that -EPIPE is worse for telling about incorrect
>>>>>> format configuration than -EINVAL since it's relatively easy to avoid -
>>>>>> EINVAL for the documented reasons.
>>>>>
>>>>> I'd like just to point out where in the docs EPIPE for format mismatch
>>>>> is specified, as it is not described in the streamon page as I thought
>>>>> it would, but it is in the subdev page in case anyone is looking for
>>>>> it (as I took some time to find it too):
>>>>>
>>>>> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
>>>>> "Applications are responsible for configuring coherent parameters on
>>>>> the whole pipeline and making sure that connected pads have compatible
>>>>> formats. The pipeline is checked for formats mismatch at
>>>>> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
>>>>> configuration is invalid"
>>>>>
>>>>> So maybe the doc should be improved as you already stated.
>>>>
>>>> I would like to revive this subject.
>>>>
>>>> Should we change the docs? Change the -EINVAL to -EPIPE, or create
>>>> another error code? What are your opinion?
>>>>
>>>> I read in the docs of dev-kmsg that EPIPE is returned when messages get
>>>> overwritten, and in other parts of the code EPIPE is returned when there
>>>> is an error in the pipeline communication level while trying to send
>>>> information through the pipe or a pipe broken error.
>>>>
>>>> But in the error-codes.txt files, the EPIPE error is defined as:
>>>> *EPIPE "The pipe type specified in the URB doesn't match the endpoint's
>>>> actual type"*
>>>
>>> This exact definition sound USB specific to me.
>>>
>>>> Then, if EPIPE is used when types don't match between two endpoints, it
>>>> seems reasonable to me to use EPIPE when formats don't match either. Or
>>>> do "types" in this context have a specific definition? I don't know much
>>>> about URB, you may be able to judge this better.
>>>
>>> A short recap of the current situation as far as I understand it:
>>>
>>> - MC link validation failure yields EPIPE to the user space,
>>>
>>> - V4L2 sub-device format validation failure generally results in EINVAL,
>>> except that
>>
>> I think that returning EINVAL here is wrong. EINVAL is returned when you
>> set e.g. a format and the format is invalid. In this case the format for
>> each subdev is perfectly fine, it's just that the sink and source formats
>> do not match.
>>
>> Rather than returning EINVAL I think ENOLINK would work well here as you
>> can't setup a link between two entities. And EPIPE can still be used
>> for other higher-level pipeline errors.
>>
>>> - omap3isp CCDC driver returns EPIPE instead and
>>
>> That seems definitely the wrong thing to do.

I think I was ambiguous here. What is wrong here is that it replaces the
actual error code with EPIPE instead of passing it along to userspace.

> 
> The VIDIOC_STREAMON documentation states that
> 
> "EPIPE
> 
> The driver implements pad-level format configuration and the pipeline 
> configuration is invalid."
> 
> According to the documentation, EINVAL is clearly wrong, and EPIPE should be 
> used. I'm open to the idea of using different error codes to indicate severed 
> link errors and links with an invalid configuration, but how about backward 
> compatibility ?

Applications should *always* check for any error. An application that only
checks for a specific error and assumes that any other error is the same as
'Success' is obviously broken. I have no problem with adding a separate error
for link validation errors (keeping EPIPE for format validation errors).

My preferences for such a link validation error are (in descending order):

- ENOLINK
- EMLINK

Personally I feel that if you can't validate the video pipeline, then you
can't setup the video data links, i.e. ENOLINK.

EMLINK is when you have too many links which feels wrong to me, although
Sakari prefers it. Could this actually be a separate error? I.e. if you
make too many links active? Perhaps we should allow both errors?

Regards,

	Hans

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

[Sakari Ailus]

Hi Hans,

On Mon, Aug 10, 2015 at 04:07:30PM +0200, Hans Verkuil wrote:
> On 07/14/2015 04:32 PM, Laurent Pinchart wrote:
> > Hi Hans,
> > 
> > On Tuesday 14 July 2015 16:19:50 Hans Verkuil wrote:
> >> On 07/13/15 10:03, Sakari Ailus wrote:
> >>> Helen Fornazier wrote:
> >>>> On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier wrote:
> >>>>> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus wrote:
> >>>>>> Laurent Pinchart wrote:
> >>>>>>> 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.
> >>>>>>
> >>>>>> Good point. That has escaped me until now.
> >>>>>>
> >>>>>>>> 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.
> >>>>>>
> >>>>>> That was the original intent, I think.
> >>>>>>
> >>>>>>> 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.
> >>>>>>
> >>>>>> That's true, it's been used also for that purpose. At the time this
> >>>>>> certainly was not the primary concern. If you can think of a better
> >>>>>> error code for the purpose (than EINVAL) I'm certainly fine with using
> >>>>>> one. I still think that -EPIPE is worse for telling about incorrect
> >>>>>> format configuration than -EINVAL since it's relatively easy to avoid -
> >>>>>> EINVAL for the documented reasons.
> >>>>>
> >>>>> I'd like just to point out where in the docs EPIPE for format mismatch
> >>>>> is specified, as it is not described in the streamon page as I thought
> >>>>> it would, but it is in the subdev page in case anyone is looking for
> >>>>> it (as I took some time to find it too):
> >>>>>
> >>>>> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
> >>>>> "Applications are responsible for configuring coherent parameters on
> >>>>> the whole pipeline and making sure that connected pads have compatible
> >>>>> formats. The pipeline is checked for formats mismatch at
> >>>>> VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
> >>>>> configuration is invalid"
> >>>>>
> >>>>> So maybe the doc should be improved as you already stated.
> >>>>
> >>>> I would like to revive this subject.
> >>>>
> >>>> Should we change the docs? Change the -EINVAL to -EPIPE, or create
> >>>> another error code? What are your opinion?
> >>>>
> >>>> I read in the docs of dev-kmsg that EPIPE is returned when messages get
> >>>> overwritten, and in other parts of the code EPIPE is returned when there
> >>>> is an error in the pipeline communication level while trying to send
> >>>> information through the pipe or a pipe broken error.
> >>>>
> >>>> But in the error-codes.txt files, the EPIPE error is defined as:
> >>>> *EPIPE "The pipe type specified in the URB doesn't match the endpoint's
> >>>> actual type"*
> >>>
> >>> This exact definition sound USB specific to me.
> >>>
> >>>> Then, if EPIPE is used when types don't match between two endpoints, it
> >>>> seems reasonable to me to use EPIPE when formats don't match either. Or
> >>>> do "types" in this context have a specific definition? I don't know much
> >>>> about URB, you may be able to judge this better.
> >>>
> >>> A short recap of the current situation as far as I understand it:
> >>>
> >>> - MC link validation failure yields EPIPE to the user space,
> >>>
> >>> - V4L2 sub-device format validation failure generally results in EINVAL,
> >>> except that
> >>
> >> I think that returning EINVAL here is wrong. EINVAL is returned when you
> >> set e.g. a format and the format is invalid. In this case the format for
> >> each subdev is perfectly fine, it's just that the sink and source formats
> >> do not match.
> >>
> >> Rather than returning EINVAL I think ENOLINK would work well here as you
> >> can't setup a link between two entities. And EPIPE can still be used
> >> for other higher-level pipeline errors.
> >>
> >>> - omap3isp CCDC driver returns EPIPE instead and
> >>
> >> That seems definitely the wrong thing to do.
> 
> I think I was ambiguous here. What is wrong here is that it replaces the
> actual error code with EPIPE instead of passing it along to userspace.

I'm not sure if I follow you. ccdc_link_validate() will not obtain the error
code from anywhere else; instead, it returns -EPIPE if the media bus formats
at both ends of the link do not match.

> 
> > 
> > The VIDIOC_STREAMON documentation states that
> > 
> > "EPIPE
> > 
> > The driver implements pad-level format configuration and the pipeline 
> > configuration is invalid."
> > 
> > According to the documentation, EINVAL is clearly wrong, and EPIPE should be 
> > used. I'm open to the idea of using different error codes to indicate severed 
> > link errors and links with an invalid configuration, but how about backward 
> > compatibility ?
> 
> Applications should *always* check for any error. An application that only
> checks for a specific error and assumes that any other error is the same as
> 'Success' is obviously broken. I have no problem with adding a separate error
> for link validation errors (keeping EPIPE for format validation errors).
> 
> My preferences for such a link validation error are (in descending order):
> 
> - ENOLINK
> - EMLINK
> 
> Personally I feel that if you can't validate the video pipeline, then you
> can't setup the video data links, i.e. ENOLINK.
> 
> EMLINK is when you have too many links which feels wrong to me, although
> Sakari prefers it. Could this actually be a separate error? I.e. if you
> make too many links active? Perhaps we should allow both errors?

I originally proposed EMLINK since it was POSIX while ENOLINK at some point
apparently was not, but man 3 errno now tells both are. I think we agree,
and my understanding is Laurent is fine with this as well. To summarise:

- change v4l2_subdev_link_validate_default() return -EPIPE instead of
  -EINVAL on error,

- change media_entity_pipeline_start() to return -ENOLINK instead of -EPIPE
  if link configuration error is encountered and

- change the documentation accordingly.

Please ack.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

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

[Laurent Pinchart]

Hi Sakari,

On Thursday 13 August 2015 15:23:26 Sakari Ailus wrote:
> On Mon, Aug 10, 2015 at 04:07:30PM +0200, Hans Verkuil wrote:
> > On 07/14/2015 04:32 PM, Laurent Pinchart wrote:
> >> On Tuesday 14 July 2015 16:19:50 Hans Verkuil wrote:
> >>> On 07/13/15 10:03, Sakari Ailus wrote:
> >>>> Helen Fornazier wrote:
> >>>>> On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier wrote:
> >>>>>> On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus wrote:
> >>>>>>> Laurent Pinchart wrote:
> >>>>>>>> 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.
> >>>>>>> 
> >>>>>>> Good point. That has escaped me until now.
> >>>>>>> 
> >>>>>>>>> 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.
> >>>>>>> 
> >>>>>>> That was the original intent, I think.
> >>>>>>> 
> >>>>>>>> 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.
> >>>>>>> 
> >>>>>>> That's true, it's been used also for that purpose. At the time this
> >>>>>>> certainly was not the primary concern. If you can think of a better
> >>>>>>> error code for the purpose (than EINVAL) I'm certainly fine with
> >>>>>>> using one. I still think that -EPIPE is worse for telling about
> >>>>>>> incorrect format configuration than -EINVAL since it's relatively
> >>>>>>> easy to avoid -EINVAL for the documented reasons.
> >>>>>> 
> >>>>>> I'd like just to point out where in the docs EPIPE for format
> >>>>>> mismatch is specified, as it is not described in the streamon page
> >>>>>> as I thought it would, but it is in the subdev page in case anyone
> >>>>>> is looking for it (as I took some time to find it too):
> >>>>>> 
> >>>>>> http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
> >>>>>> "Applications are responsible for configuring coherent parameters on
> >>>>>> the whole pipeline and making sure that connected pads have
> >>>>>> compatible formats. The pipeline is checked for formats mismatch at
> >>>>>> VIDIOC_STREAMON time, and an EPIPE error code is then returned if
> >>>>>> the configuration is invalid"
> >>>>>> 
> >>>>>> So maybe the doc should be improved as you already stated.
> >>>>> 
> >>>>> I would like to revive this subject.
> >>>>> 
> >>>>> Should we change the docs? Change the -EINVAL to -EPIPE, or create
> >>>>> another error code? What are your opinion?
> >>>>> 
> >>>>> I read in the docs of dev-kmsg that EPIPE is returned when messages
> >>>>> get overwritten, and in other parts of the code EPIPE is returned
> >>>>> when there is an error in the pipeline communication level while
> >>>>> trying to send information through the pipe or a pipe broken error.
> >>>>> 
> >>>>> But in the error-codes.txt files, the EPIPE error is defined as:
> >>>>> *EPIPE "The pipe type specified in the URB doesn't match the
> >>>>> endpoint's actual type"*
> >>>> 
> >>>> This exact definition sound USB specific to me.
> >>>> 
> >>>>> Then, if EPIPE is used when types don't match between two endpoints,
> >>>>> it seems reasonable to me to use EPIPE when formats don't match
> >>>>> either. Or do "types" in this context have a specific definition? I
> >>>>> don't know much about URB, you may be able to judge this better.
> >>>> 
> >>>> A short recap of the current situation as far as I understand it:
> >>>> 
> >>>> - MC link validation failure yields EPIPE to the user space,
> >>>> 
> >>>> - V4L2 sub-device format validation failure generally results in
> >>>> EINVAL, except that
> >>> 
> >>> I think that returning EINVAL here is wrong. EINVAL is returned when
> >>> you set e.g. a format and the format is invalid. In this case the
> >>> format for each subdev is perfectly fine, it's just that the sink and
> >>> source formats do not match.
> >>> 
> >>> Rather than returning EINVAL I think ENOLINK would work well here as
> >>> you can't setup a link between two entities. And EPIPE can still be
> >>> used for other higher-level pipeline errors.
> >>> 
> >>>> - omap3isp CCDC driver returns EPIPE instead and
> >>> 
> >>> That seems definitely the wrong thing to do.
> > 
> > I think I was ambiguous here. What is wrong here is that it replaces the
> > actual error code with EPIPE instead of passing it along to userspace.
> 
> I'm not sure if I follow you. ccdc_link_validate() will not obtain the error
> code from anywhere else; instead, it returns -EPIPE if the media bus
> formats at both ends of the link do not match.
> 
> >> The VIDIOC_STREAMON documentation states that
> >> 
> >> "EPIPE
> >> 
> >> The driver implements pad-level format configuration and the pipeline
> >> configuration is invalid."
> >> 
> >> According to the documentation, EINVAL is clearly wrong, and EPIPE
> >> should be used. I'm open to the idea of using different error codes to
> >> indicate severed link errors and links with an invalid configuration,
> >> but how about backward compatibility ?
> > 
> > Applications should *always* check for any error. An application that only
> > checks for a specific error and assumes that any other error is the same
> > as 'Success' is obviously broken. I have no problem with adding a separate
> > error for link validation errors (keeping EPIPE for format validation
> > errors).
> > 
> > My preferences for such a link validation error are (in descending order):
> > 
> > - ENOLINK
> > - EMLINK
> > 
> > Personally I feel that if you can't validate the video pipeline, then you
> > can't setup the video data links, i.e. ENOLINK.
> > 
> > EMLINK is when you have too many links which feels wrong to me, although
> > Sakari prefers it. Could this actually be a separate error? I.e. if you
> > make too many links active? Perhaps we should allow both errors?
> 
> I originally proposed EMLINK since it was POSIX while ENOLINK at some point
> apparently was not, but man 3 errno now tells both are. I think we agree,
> and my understanding is Laurent is fine with this as well. To summarise:
> 
> - change v4l2_subdev_link_validate_default() return -EPIPE instead of
>   -EINVAL on error,
> 
> - change media_entity_pipeline_start() to return -ENOLINK instead of -EPIPE
>   if link configuration error is encountered and
> 
> - change the documentation accordingly.
> 
> Please ack.

That's fine with me.

-- 
Regards,

Laurent Pinchart