cron job: media_tree daily build: OK

2018-08-20 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Aug 21 05:00:14 CEST 2018
media-tree git hash:da2048b7348a0be92f706ac019e022139e29495e
media_build git hash:   baf45935ffad914f33faf751ad9f4d0dd276c021
v4l-utils git hash: 015ca7524748fa7cef296102c68b631b078b63c6
edid-decode git hash:   b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4
gcc version:i686-linux-gcc (GCC) 8.1.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.17.0-1-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-2.6.36.4-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-i686: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-i686: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-i686: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.101-i686: OK
linux-3.0.101-x86_64: OK
linux-3.1.10-i686: OK
linux-3.1.10-x86_64: OK
linux-3.2.102-i686: OK
linux-3.2.102-x86_64: OK
linux-3.3.8-i686: OK
linux-3.3.8-x86_64: OK
linux-3.4.113-i686: OK
linux-3.4.113-x86_64: OK
linux-3.5.7-i686: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-i686: OK
linux-3.6.11-x86_64: OK
linux-3.7.10-i686: OK
linux-3.7.10-x86_64: OK
linux-3.8.13-i686: OK
linux-3.8.13-x86_64: OK
linux-3.9.11-i686: OK
linux-3.9.11-x86_64: OK
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.57-i686: OK
linux-3.16.57-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.115-i686: OK
linux-3.18.115-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.18-i686: OK
linux-4.18-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


Re: [PATCH v2 1/2] uvcvideo: rename UVC_QUIRK_INFO to UVC_INFO_QUIRK

2018-08-20 Thread Laurent Pinchart
Hi Guennadi,

Thank you for the patch.

On Monday, 20 August 2018 09:45:16 EEST Guennadi Liakhovetski wrote:
> Hi Laurent,
> 
> As far as I understand we've missed the 4.19 cycle. Can we move on with
> this please?

I've taken this patch in my tree and will try to handle the rest this week.

-- 
Regards,

Laurent Pinchart





Re: [PATCH v2 1/2] uvcvideo: rename UVC_QUIRK_INFO to UVC_INFO_QUIRK

2018-08-20 Thread Laurent Pinchart
Hi Guennadi,

On Monday, 20 August 2018 15:02:53 EEST Laurent Pinchart wrote:
> On Friday, 3 August 2018 14:36:56 EEST Guennadi Liakhovetski wrote:
> > This macro defines "information about quirks," not "quirks for
> > information."

To address Nicolas' objection, would you mind saying "device information 
containing quirks" instead of "information about quirks" ? It you're fine with 
that I'll fix it in my tree, there's no need to resubmit.

> > Signed-off-by: Guennadi Liakhovetski 
> 
> Reviewed-by: Laurent Pinchart 
> 
> and taken in my tree.
> 
> > ---
> > 
> >  drivers/media/usb/uvc/uvc_driver.c | 18 +-
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c
> > b/drivers/media/usb/uvc/uvc_driver.c index d46dc43..699984b 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2344,7 +2344,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param *kp) .quirks = UVC_QUIRK_FORCE_Y8,
> > 
> >  };
> > 
> > -#define UVC_QUIRK_INFO(q) (kernel_ulong_t)&(struct
> > uvc_device_info){.quirks = q} +#define UVC_INFO_QUIRK(q)
> > (kernel_ulong_t)&(struct
> > uvc_device_info){.quirks = q}
> > 
> >  /*
> >  
> >   * The Logitech cameras listed below have their interface class set to
> > 
> > @@ -2453,7 +2453,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param *kp) .bInterfaceClass   = USB_CLASS_VIDEO,
> > 
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  = 
> > UVC_QUIRK_INFO(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) 
},
> > + .driver_info  = 
> > UVC_INFO_QUIRK(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) 
},
> > 
> > /* Chicony CNF7129 (Asus EEE 100HE) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > | USB_DEVICE_ID_MATCH_INT_INFO,
> > 
> > @@ -2462,7 +2462,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param *kp) .bInterfaceClass   = USB_CLASS_VIDEO,
> > 
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_RESTRICT_FRAME_RATE) 
> > },
> > + .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) 
> > },
> > 
> > /* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > | USB_DEVICE_ID_MATCH_INT_INFO,
> > 
> > @@ -2525,7 +2525,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param *kp) .bInterfaceClass   = USB_CLASS_VIDEO,
> > 
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> > + .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> > 
> > | UVC_QUIRK_BUILTIN_ISIGHT) },
> > 
> > /* Apple Built-In iSight via iBridge */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > @@ -2607,7 +2607,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param *kp) .bInterfaceClass   = USB_CLASS_VIDEO,
> > 
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> > + .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> > 
> > | UVC_QUIRK_PROBE_DEF) },
> > 
> > /* IMC Networks (Medion Akoya) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > @@ -2707,7 +2707,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param *kp) .bInterfaceClass   = USB_CLASS_VIDEO,
> > 
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> > + .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> > 
> > | UVC_QUIRK_PROBE_EXTRAFIELDS) },
> > 
> > /* Aveo Technology USB 2.0 Camera (Tasco USB Microscope) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > @@ -2725,7 +2725,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param *kp) .bInterfaceClass   = USB_CLASS_VIDEO,
> > 
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_EXTRAFIELDS) },
> > + .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_EXTRAFIELDS) },
> > 
> > /* Manta MM-353 Plako */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > | USB_DEVICE_ID_MATCH_INT_INFO,
> > 
> > @@ -2771,7 +2771,7 @@ static int uvc_clock_param_set(const char *val,
> > const
> > struct kernel_param 

Re: [PATCH v2 1/2] uvcvideo: rename UVC_QUIRK_INFO to UVC_INFO_QUIRK

2018-08-20 Thread Laurent Pinchart
Hi Guennadi,

Thank you for the patch.

On Friday, 3 August 2018 14:36:56 EEST Guennadi Liakhovetski wrote:
> This macro defines "information about quirks," not "quirks for
> information."
> 
> Signed-off-by: Guennadi Liakhovetski 

Reviewed-by: Laurent Pinchart 

and taken in my tree.

> ---
>  drivers/media/usb/uvc/uvc_driver.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index d46dc43..699984b 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2344,7 +2344,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .quirks = UVC_QUIRK_FORCE_Y8,
>  };
> 
> -#define UVC_QUIRK_INFO(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks
> = q} +#define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct
> uvc_device_info){.quirks = q}
> 
>  /*
>   * The Logitech cameras listed below have their interface class set to
> @@ -2453,7 +2453,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .bInterfaceClass = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = 
> UVC_QUIRK_INFO(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
> +   .driver_info  = 
> UVC_INFO_QUIRK(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
>   /* Chicony CNF7129 (Asus EEE 100HE) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> 
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> 
> @@ -2462,7 +2462,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .bInterfaceClass = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_RESTRICT_FRAME_RATE) 
> },
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) 
> },
>   /* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> 
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> 
> @@ -2525,7 +2525,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .bInterfaceClass = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> 
>   | UVC_QUIRK_BUILTIN_ISIGHT) },
> 
>   /* Apple Built-In iSight via iBridge */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> @@ -2607,7 +2607,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .bInterfaceClass = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> 
>   | UVC_QUIRK_PROBE_DEF) },
> 
>   /* IMC Networks (Medion Akoya) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> @@ -2707,7 +2707,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .bInterfaceClass = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> 
>   | UVC_QUIRK_PROBE_EXTRAFIELDS) },
> 
>   /* Aveo Technology USB 2.0 Camera (Tasco USB Microscope) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> @@ -2725,7 +2725,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .bInterfaceClass = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_EXTRAFIELDS) },
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_EXTRAFIELDS) },
>   /* Manta MM-353 Plako */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> 
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> 
> @@ -2771,7 +2771,7 @@ static int uvc_clock_param_set(const char *val, const
> struct kernel_param *kp) .bInterfaceClass = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_STATUS_INTERVAL) },
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_STATUS_INTERVAL) },
>   /* MSI StarCam 370i */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> 
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> 
> @@ -2798,7 +2798,7 @@ static int uvc_clock_param_set(const char *val, const
> struct 

Re: [PATCH v2 1/2] uvcvideo: rename UVC_QUIRK_INFO to UVC_INFO_QUIRK

2018-08-20 Thread Laurent Pinchart
Hi Nicolas,

On Saturday, 4 August 2018 18:58:18 EEST Nicolas Dufresne wrote:
> Le vendredi 03 août 2018 à 13:36 +0200, Guennadi Liakhovetski a écrit :
> > This macro defines "information about quirks," not "quirks for
> > information."
> 
> Does not sound better to me. It's "Quirk's information", vs
> "information about quirks". I prefer the first one. In term of C
> namespace the orignal is also better. So the name space is UVC_QUIRK,
> and the detail is INFO.
> 
> If we where to apply your logic, you'd rename driver_info, into
> info_driver, because it's information about the driver.

The macro initializes an info structure with the .quirks field. We'll need a 
similar macro that will set the .meta_format field, and I proposed naming it 
UVC_INFO_META, hence the rename of this macro.

The alternative would be to call the two macros UVC_QUIRK_INFO and 
UVC_META_INFO, but I don't think that would be a good idea. All the macros 
that initialize an info structure should start with the same UVC_INFO_ prefix.

> > Signed-off-by: Guennadi Liakhovetski  > 
> > ---
> > 
> >  drivers/media/usb/uvc/uvc_driver.c | 18 +-
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c
> > b/drivers/media/usb/uvc/uvc_driver.c
> > index d46dc43..699984b 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2344,7 +2344,7 @@ static int uvc_clock_param_set(const char *val,
> > const struct kernel_param *kp)
> > 
> > .quirks = UVC_QUIRK_FORCE_Y8,
> >  
> >  };
> > 
> > -#define UVC_QUIRK_INFO(q) (kernel_ulong_t)&(struct
> > uvc_device_info){.quirks = q}
> > +#define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct
> > uvc_device_info){.quirks = q}
> > 
> >  /*
> >  
> >   * The Logitech cameras listed below have their interface class set
> > 
> > to
> > @@ -2453,7 +2453,7 @@ static int uvc_clock_param_set(const char *val,
> > const struct kernel_param *kp)
> > 
> >   .bInterfaceClass  = USB_CLASS_VIDEO,
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  =
> > UVC_QUIRK_INFO(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
> > + .driver_info  =
> > UVC_INFO_QUIRK(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
> > 
> > /* Chicony CNF7129 (Asus EEE 100HE) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > | USB_DEVICE_ID_MATCH_INT_INFO,
> > 
> > @@ -2462,7 +2462,7 @@ static int uvc_clock_param_set(const char *val,
> > const struct kernel_param *kp)
> > 
> >   .bInterfaceClass  = USB_CLASS_VIDEO,
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  =
> > UVC_QUIRK_INFO(UVC_QUIRK_RESTRICT_FRAME_RATE) },
> > + .driver_info  =
> > UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) },
> > 
> > /* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > | USB_DEVICE_ID_MATCH_INT_INFO,
> > 
> > @@ -2525,7 +2525,7 @@ static int uvc_clock_param_set(const char *val,
> > const struct kernel_param *kp)
> > 
> >   .bInterfaceClass  = USB_CLASS_VIDEO,
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  =
> > UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> > + .driver_info  =
> > UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> > 
> > | UVC_QUIRK_BUILTIN_ISIGHT) },
> > 
> > /* Apple Built-In iSight via iBridge */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > @@ -2607,7 +2607,7 @@ static int uvc_clock_param_set(const char *val,
> > const struct kernel_param *kp)
> > 
> >   .bInterfaceClass  = USB_CLASS_VIDEO,
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  =
> > UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> > + .driver_info  =
> > UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> > 
> > | UVC_QUIRK_PROBE_DEF) },
> > 
> > /* IMC Networks (Medion Akoya) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > @@ -2707,7 +2707,7 @@ static int uvc_clock_param_set(const char *val,
> > const struct kernel_param *kp)
> > 
> >   .bInterfaceClass  = USB_CLASS_VIDEO,
> >   .bInterfaceSubClass   = 1,
> >   .bInterfaceProtocol   = 0,
> > 
> > - .driver_info  =
> > UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> > + .driver_info  =
> > UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
> > 
> > | UVC_QUIRK_PROBE_EXTRAFIELDS)
> > 
> > },
> > 
> > /* Aveo Technology USB 2.0 Camera (Tasco USB Microscope) */
> > { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> > 
> > @@ -2725,7 +2725,7 @@ static int uvc_clock_param_set(const char *val,
> > const 

Re: [RFC] Request API questions

2018-08-20 Thread Sakari Ailus
Hi Hans,

On Thu, Aug 16, 2018 at 12:25:25PM +0200, Hans Verkuil wrote:
> Laurent raised a few API issues/questions in his review of the documentation.
> 
> I've consolidated those in this RFC. I would like to know what others think
> and if I should make changes.
> 
> 1) Should you be allowed to set controls directly if they are also used in
>requests? Right now this is allowed, although we warn in the spec that
>this can lead to undefined behavior.
> 
>In my experience being able to do this is very useful while testing,
>and restricting this is not all that easy to implement. I also think it is
>not our job. It is not as if something will break when you do this.

It should be easy for drivers to disable controls that need to go through
requests whenever there are requests queued. There will be lots of corner
cases when you poking requests that are already validated with stuff that
was not considered when the request was validated.

Controls such as exposure time for cameras would be generally fine whereas
those such as rotation would be not.

That said, I'd guess we don't run into these issues with stateless codecs.

We still should be careful with this: allowing setting controls (or other
bits) that can also be a part of a request can prove troublesome for some
individual controls such as rotation. If we do allow setting them now and
disallow that later, that may break applications however dangerous setting
those controls may be.

> 
>If there really is a good reason why you can't mix this for a specific
>control, then the driver can check this and return -EBUSY.
> 
> 2) If request_fd in QBUF or the control ioctls is not a request fd, then we
>now return ENOENT. Laurent suggests using EBADR ('Invalid request 
> descriptor')
>instead. This seems like a good idea to me. Should I change this?

I agree, this sounds like a good idea. The next question is though: should
this apply to requests in general, independently of the IOCTL? It would be
logical.

> 
> 3) Calling VIDIOC_G_EXT_CTRLS for a request that has not been queued yet will
>return either the value of the control you set earlier in the request, or
>the current HW control value if it was never set in the request.
> 
>I believe it makes sense to return what was set in the request previously
>(if you set it, you should be able to get it), but it is an idea to return
>ENOENT when calling this for controls that are NOT in the request.
> 
>I'm inclined to implement that. Opinions?

Is there any particular reason for that kind of a change? In general, the
device state is changed by the requests and what is not part of a request
should remain intact.

What would be the behaviour if the requests would result in changing a
value of the control that the user would read back?

> 
> 4) When queueing a buffer to a request with VIDIOC_QBUF you set 
> V4L2_BUF_FLAG_REQUEST_FD
>to indicate a valid request_fd. For other queue ioctls that take a struct 
> v4l2_buffer
>this flag and the request_fd field are just ignored. Should we return 
> EINVAL
>instead if the flag is set for those ioctls?

Good question. If there is no need to use the flag for other IOCTLs now,
will there be such a need in the future? Can we guarantee there will not
be? I think it'd be safer to return an error if there's no need to use
these IOCTLs with requests now.

> 
>The argument for just ignoring it is that older kernels that do not know 
> about
>this flag will ignore it as well. There is no check against unknown flags.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


KINDLY REPLY stemlightresour...@gmail.com URGENTLY

2018-08-20 Thread STEMLIGHTRESOURCES
KINDLY REPLY stemlightresour...@gmail.com URGENTLY


Re: [RFC] Request API questions

2018-08-20 Thread Tomasz Figa
Hi Hans,

On Fri, Aug 17, 2018 at 7:09 PM Hans Verkuil  wrote:
>
> On 17/08/18 12:02, Tomasz Figa wrote:
> > On Thu, Aug 16, 2018 at 8:15 PM Mauro Carvalho Chehab
> >  wrote:
> >>
> >> Em Thu, 16 Aug 2018 12:25:25 +0200
> >> Hans Verkuil  escreveu:
> >>
[snip]
> >>> 3) Calling VIDIOC_G_EXT_CTRLS for a request that has not been queued yet 
> >>> will
> >>>return either the value of the control you set earlier in the request, 
> >>> or
> >>>the current HW control value if it was never set in the request.
> >>>
> >>>I believe it makes sense to return what was set in the request 
> >>> previously
> >>>(if you set it, you should be able to get it), but it is an idea to 
> >>> return
> >>>ENOENT when calling this for controls that are NOT in the request.
> >>>
> >>>I'm inclined to implement that. Opinions?
> >>
> >> Return the request "cached" value, IMO, doesn't make sense. If the
> >> application needs such cache, it can implement itself.
> >
> > Can we think about any specific use cases for a user space that first
> > sets a control value to a request and then needs to ask the kernel to
> > get the value back? After all, it was the user space which set the
> > value, so I'm not sure if there is any need for the kernel to be an
> > intermediary here.
> >
> >>
> >> Return an error code if the request has not yet completed makes
> >> sense. Not sure what would be the best error code here... if the
> >> request is queued already (but not processed), EBUSY seems to be the
> >> better choice, but, if it was not queued yet, I'm not sure. I guess
> >> ENOENT would work.
> >
> > IMHO, as far as we assign unique error codes for different conditions
> > and document them well, we should be okay with any not absurdly
> > mismatched code. After all, most of those codes are defined for file
> > system operations and don't really map directly to anything else.
> >
> > FYI, VIDIOC_G_(EXT_)CTRL returns EINVAL if an unsupported control is
> > queried, so if we decided to keep the "cache" functionality after all,
> > perhaps we should stay consistent with it?
> > Reference: 
> > https://www.kernel.org/doc/html/latest/media/uapi/v4l/vidioc-g-ext-ctrls.html#return-value
> >
> > My suggestion would be:
> >  - EINVAL: the control was not in the request, (if we keep the cache
> > functionality)
> >  - EPERM: the value is not ready, (we selected this code for Decoder
> > Interface to mean that CAPTURE format is not ready, which is similar;
> > perhaps that could be consistent?)
> >
> > Note that EINVAL would only apply to writable controls, while EPERM
> > only to volatile controls, since the latter can only change due to
> > request completion (non-volatile controls can only change as an effect
> > of user space action).
> >
>
> I'm inclined to just always return EPERM when calling G_EXT_CTRLS for
> a request. We can always relax this in the future.
>
> So when a request is not yet queued G_EXT_CTRLS returns EPERM, when
> queued but not completed it returns EBUSY and once completed it will
> work as it does today.

Not sure I'm following. What do we differentiate between with EPERM
and EBUSY? In both cases the value is not available yet and for codecs
we decided to have that signaled by EPERM.

On top of that, I still think we should be able to tell the case of
querying for a control that can never show up in the request, even
after it completes, specifically for any non-volatile control. That
could be done by returning a different code and -EINVAL would match
the control API behavior without requests.

The general logic would be like the pseudo code below:

G_EXT_CTRLS()
{
if ( control is not volatile )
return -EINVAL;

if ( request is not complete )
return -EPERM;

return control from the request;
}

Best regards,
Tomasz


Re: [PATCH v2 1/2] uvcvideo: rename UVC_QUIRK_INFO to UVC_INFO_QUIRK

2018-08-20 Thread Guennadi Liakhovetski
Hi Laurent,

As far as I understand we've missed the 4.19 cycle. Can we move on with 
this please?

Thanks
Guennadi

On Fri, 3 Aug 2018, Guennadi Liakhovetski wrote:

> This macro defines "information about quirks," not "quirks for
> information."
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c 
> b/drivers/media/usb/uvc/uvc_driver.c
> index d46dc43..699984b 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2344,7 +2344,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
>   .quirks = UVC_QUIRK_FORCE_Y8,
>  };
>  
> -#define UVC_QUIRK_INFO(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks 
> = q}
> +#define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks 
> = q}
>  
>  /*
>   * The Logitech cameras listed below have their interface class set to
> @@ -2453,7 +2453,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
> .bInterfaceClass  = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = 
> UVC_QUIRK_INFO(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
> +   .driver_info  = 
> UVC_INFO_QUIRK(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
>   /* Chicony CNF7129 (Asus EEE 100HE) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> @@ -2462,7 +2462,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
> .bInterfaceClass  = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_RESTRICT_FRAME_RATE) 
> },
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) 
> },
>   /* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> @@ -2525,7 +2525,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
> .bInterfaceClass  = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
>   | UVC_QUIRK_BUILTIN_ISIGHT) },
>   /* Apple Built-In iSight via iBridge */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> @@ -2607,7 +2607,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
> .bInterfaceClass  = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
>   | UVC_QUIRK_PROBE_DEF) },
>   /* IMC Networks (Medion Akoya) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> @@ -2707,7 +2707,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
> .bInterfaceClass  = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
>   | UVC_QUIRK_PROBE_EXTRAFIELDS) },
>   /* Aveo Technology USB 2.0 Camera (Tasco USB Microscope) */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
> @@ -2725,7 +2725,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
> .bInterfaceClass  = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_EXTRAFIELDS) },
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_EXTRAFIELDS) },
>   /* Manta MM-353 Plako */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> @@ -2771,7 +2771,7 @@ static int uvc_clock_param_set(const char *val, const 
> struct kernel_param *kp)
> .bInterfaceClass  = USB_CLASS_VIDEO,
> .bInterfaceSubClass   = 1,
> .bInterfaceProtocol   = 0,
> -   .driver_info  = UVC_QUIRK_INFO(UVC_QUIRK_STATUS_INTERVAL) },
> +   .driver_info  = UVC_INFO_QUIRK(UVC_QUIRK_STATUS_INTERVAL) },
>   /* MSI StarCam 370i */
>   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
>   | USB_DEVICE_ID_MATCH_INT_INFO,
> @@ -2798,7 +2798,7 @@