[PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

[PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

[ming_qian]

From: ming_qian <ming_qian@realsil.com.cn>

The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
Change it to 48 for UVC 1.5 device,
and the UVC 1.5 device can be recognized.

More changes to the driver are needed for full UVC 1.5 compatibility.
However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
been reported to work well.

Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
---
 drivers/media/usb/uvc/uvc_video.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index aa0082f..32dfb32 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 	int ret;
 
 	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
+	if (stream->dev->uvc_version >= 0x0150)
+		size = 48;
 	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
 			query == UVC_GET_DEF)
 		return -EIO;
@@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
 	int ret;
 
 	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
+	if (stream->dev->uvc_version >= 0x0150)
+		size = 48;
 	data = kzalloc(size, GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
-- 
2.7.4

Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

[Kai-Heng Feng]

at 10:13, ming_qian@realsil.com.cn wrote:

> From: ming_qian <ming_qian@realsil.com.cn>
>
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
>
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.

I think this should also Cc: stable.

Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

>
> Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c  
> b/drivers/media/usb/uvc/uvc_video.c
> index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming  
> *stream,
>  	int ret;
>
>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>  	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
>  			query == UVC_GET_DEF)
>  		return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming  
> *stream,
>  	int ret;
>
>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>  	data = kzalloc(size, GFP_KERNEL);
>  	if (data == NULL)
>  		return -ENOMEM;
> -- 
> 2.7.4

Re: media: uvcvideo: Support realtek's UVC 1.5 device

[Hans de Goede]

Hi,

On 09-05-18 04:13, ming_qian@realsil.com.cn wrote:
> From: ming_qian <ming_qian@realsil.com.cn>
> 
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
> 
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.
> 
> Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>   drivers/media/usb/uvc/uvc_video.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>   	int ret;
>   
>   	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>   	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
>   			query == UVC_GET_DEF)
>   		return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
>   	int ret;
>   
>   	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>   	data = kzalloc(size, GFP_KERNEL);
>   	if (data == NULL)
>   		return -ENOMEM;
> 

RE: media: uvcvideo: Support realtek's UVC 1.5 device

[Josef Šimánek]

> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
> 
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.
> 
> Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Hello! I have sucessfully tested this patch on Kernel 4.16.1 (Fedora 28) with Dell XPS 9370
using following device (output from lsusb):

Bus 001 Device 002: ID 0bda:58f4 Realtek Semiconductor Corp.

You can also find related dmesg output at https://bugs.launchpad.net/dell-sputnik/+bug/1763748/comments/35

Tested-by: Josef Šimánek <josef.simanek@gmail.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  	int ret;
>  
>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>  	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
>  			query == UVC_GET_DEF)
>  		return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
>  	int ret;
>  
>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>  	data = kzalloc(size, GFP_KERNEL);
>  	if (data == NULL)
>  		return -ENOMEM;

Re: media: uvcvideo: Support realtek's UVC 1.5 device

[Josef Šimánek]

I'm really sorry, but this message was originally meant to be sent
(and was already also sent) to linux-media. Please ignore here.

2018-05-14 19:28 GMT+02:00 Josef Šimánek <josef.simanek@gmail.com>:
>
> > The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> > Change it to 48 for UVC 1.5 device,
> > and the UVC 1.5 device can be recognized.
> >
> > More changes to the driver are needed for full UVC 1.5 compatibility.
> > However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> > been reported to work well.
> >
> > Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> > Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Hello! I have sucessfully tested this patch on Kernel 4.16.1 (Fedora 28) with Dell XPS 9370
> using following device (output from lsusb):
>
> Bus 001 Device 002: ID 0bda:58f4 Realtek Semiconductor Corp.
>
> You can also find related dmesg output at https://bugs.launchpad.net/dell-sputnik/+bug/1763748/comments/35
>
> Tested-by: Josef Šimánek <josef.simanek@gmail.com>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  drivers/media/usb/uvc/uvc_video.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index aa0082f..32dfb32 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> >       int ret;
> >
> >       size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> > +     if (stream->dev->uvc_version >= 0x0150)
> > +             size = 48;
> >       if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> >                       query == UVC_GET_DEF)
> >               return -EIO;
> > @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> >       int ret;
> >
> >       size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> > +     if (stream->dev->uvc_version >= 0x0150)
> > +             size = 48;
> >       data = kzalloc(size, GFP_KERNEL);
> >       if (data == NULL)
> >               return -ENOMEM;

Re: media: uvcvideo: Support realtek's UVC 1.5 device

[Ana Guerrero Lopez]

On Wed, May 09, 2018 at 10:13:08AM +0800, ming_qian@realsil.com.cn wrote:
> From: ming_qian <ming_qian@realsil.com.cn>
> 
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
> 
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.
> 
> Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Josef Šimánek <josef.simanek@gmail.com>

It works perfectly here on 4.16.5 with a Dell XPS 9370 in Debian.

Tested-by: Ana Guerrero Lopez <ana.guerrero@collabora.com>

Cheers,
Ana

Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

[Laurent Pinchart]

Hello,

Thank you for the patch.

On Wednesday, 9 May 2018 05:13:08 EEST ming_qian@realsil.com.cn wrote:
> From: ming_qian <ming_qian@realsil.com.cn>
> 
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> Change it to 48 for UVC 1.5 device,
> and the UVC 1.5 device can be recognized.
> 
> More changes to the driver are needed for full UVC 1.5 compatibility.
> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> been reported to work well.

This patch is however not specific to Realtek devices, so I think we should
make the subject line more generic. It's fine mentioning in the commit message
itself that the Realtek RTS5847/RTS5852 cameras have been successfully tested.

> Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index aa0082f..32dfb32 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> *stream, int ret;
> 
>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>  	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
>  			query == UVC_GET_DEF)
>  		return -EIO;
> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming
> *stream, int ret;
> 
>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> +	if (stream->dev->uvc_version >= 0x0150)
> +		size = 48;
>  	data = kzalloc(size, GFP_KERNEL);
>  	if (data == NULL)
>  		return -ENOMEM;

Instead of duplicating the computation in both functions, I think we should
move the code to a helper function.

Furthermore there are equality checks further down both functions that compare
the size to 34, they should be updated to also support UVC 1.5.

I propose the following updated patch. If you're fine with it there's no need
to resubmit, I'll queue it for v4.19.

I have dropped the Reviewed-by and Tested-by tags as the patch has changed.

commit a9c002732695eab2096580a0d1a1687bc2f95928
Author: ming_qian <ming_qian@realsil.com.cn>
Date:   Wed May 9 10:13:08 2018 +0800

    media: uvcvideo: Support UVC 1.5 video probe & commit controls
    
    The length of UVC 1.5 video control is 48, and it is 34 for UVC 1.1.
    Change it to 48 for UVC 1.5 device, and the UVC 1.5 device can be
    recognized.
    
    More changes to the driver are needed for full UVC 1.5 compatibility.
    However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have been
    reported to work well.
    
    Cc: stable@vger.kernel.org
    Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
    [Factor out code to helper function, update size checks]
    Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index eb9e04a59427..285b0e813b9d 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -207,14 +207,27 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream,
 	}
 }
 
+static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
+{
+	/*
+	 * Return the size of the video probe and commit controls, which depends
+	 * on the protocol version.
+	 */
+	if (stream->dev->uvc_version < 0x0110)
+		return 26;
+	else if (stream->dev->uvc_version < 0x0150)
+		return 34;
+	else
+		return 48;
+}
+
 static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 	struct uvc_streaming_control *ctrl, int probe, u8 query)
 {
+	u16 size = uvc_video_ctrl_size(stream);
 	u8 *data;
-	u16 size;
 	int ret;
 
-	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
 	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
 			query == UVC_GET_DEF)
 		return -EIO;
@@ -271,7 +284,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 	ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
 	ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);
 
-	if (size == 34) {
+	if (size >= 34) {
 		ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
 		ctrl->bmFramingInfo = data[30];
 		ctrl->bPreferedVersion = data[31];
@@ -300,11 +313,10 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 static int uvc_set_video_ctrl(struct uvc_streaming *stream,
 	struct uvc_streaming_control *ctrl, int probe)
 {
+	u16 size = uvc_video_ctrl_size(stream);
 	u8 *data;
-	u16 size;
 	int ret;
 
-	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
 	data = kzalloc(size, GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
@@ -321,7 +333,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
 	put_unaligned_le32(ctrl->dwMaxVideoFrameSize, &data[18]);
 	put_unaligned_le32(ctrl->dwMaxPayloadTransferSize, &data[22]);
 
-	if (size == 34) {
+	if (size >= 34) {
 		put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
 		data[30] = ctrl->bmFramingInfo;
 		data[31] = ctrl->bPreferedVersion;

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

[Kai Heng Feng]

> On May 23, 2018, at 4:32 AM, Laurent Pinchart  
> <laurent.pinchart@ideasonboard.com> wrote:
>
> Hello,
>
> Thank you for the patch.
>
> On Wednesday, 9 May 2018 05:13:08 EEST ming_qian@realsil.com.cn wrote:
>> From: ming_qian <ming_qian@realsil.com.cn>
>>
>> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
>> Change it to 48 for UVC 1.5 device,
>> and the UVC 1.5 device can be recognized.
>>
>> More changes to the driver are needed for full UVC 1.5 compatibility.
>> However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
>> been reported to work well.
>
> This patch is however not specific to Realtek devices, so I think we should
> make the subject line more generic. It's fine mentioning in the commit  
> message
> itself that the Realtek RTS5847/RTS5852 cameras have been successfully  
> tested.
>
>> Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
>> ---
>>  drivers/media/usb/uvc/uvc_video.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_video.c
>> b/drivers/media/usb/uvc/uvc_video.c index aa0082f..32dfb32 100644
>> --- a/drivers/media/usb/uvc/uvc_video.c
>> +++ b/drivers/media/usb/uvc/uvc_video.c
>> @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming
>> *stream, int ret;
>>
>>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
>> +	if (stream->dev->uvc_version >= 0x0150)
>> +		size = 48;
>>  	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
>>  			query == UVC_GET_DEF)
>>  		return -EIO;
>> @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming
>> *stream, int ret;
>>
>>  	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
>> +	if (stream->dev->uvc_version >= 0x0150)
>> +		size = 48;
>>  	data = kzalloc(size, GFP_KERNEL);
>>  	if (data == NULL)
>>  		return -ENOMEM;
>
> Instead of duplicating the computation in both functions, I think we should
> move the code to a helper function.
>
> Furthermore there are equality checks further down both functions that  
> compare
> the size to 34, they should be updated to also support UVC 1.5.
>
> I propose the following updated patch. If you're fine with it there's no  
> need
> to resubmit, I'll queue it for v4.19.
>
> I have dropped the Reviewed-by and Tested-by tags as the patch has changed.
>
> commit a9c002732695eab2096580a0d1a1687bc2f95928
> Author: ming_qian <ming_qian@realsil.com.cn>
> Date:   Wed May 9 10:13:08 2018 +0800
>
>     media: uvcvideo: Support UVC 1.5 video probe & commit controls
>
>     The length of UVC 1.5 video control is 48, and it is 34 for UVC 1.1.
>     Change it to 48 for UVC 1.5 device, and the UVC 1.5 device can be
>     recognized.
>
>     More changes to the driver are needed for full UVC 1.5 compatibility.
>     However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have been
>     reported to work well.
>
>     Cc: stable@vger.kernel.org
>     Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
>     [Factor out code to helper function, update size checks]
>     Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I tested this new patch and it works well.

Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

>
> diff --git a/drivers/media/usb/uvc/uvc_video.c  
> b/drivers/media/usb/uvc/uvc_video.c
> index eb9e04a59427..285b0e813b9d 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -207,14 +207,27 @@ static void uvc_fixup_video_ctrl(struct  
> uvc_streaming *stream,
>  	}
>  }
>
> +static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
> +{
> +	/*
> +	 * Return the size of the video probe and commit controls, which depends
> +	 * on the protocol version.
> +	 */
> +	if (stream->dev->uvc_version < 0x0110)
> +		return 26;
> +	else if (stream->dev->uvc_version < 0x0150)
> +		return 34;
> +	else
> +		return 48;
> +}
> +
>  static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  	struct uvc_streaming_control *ctrl, int probe, u8 query)
>  {
> +	u16 size = uvc_video_ctrl_size(stream);
>  	u8 *data;
> -	u16 size;
>  	int ret;
>
> -	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
>  	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
>  			query == UVC_GET_DEF)
>  		return -EIO;
> @@ -271,7 +284,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming  
> *stream,
>  	ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
>  	ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);
>
> -	if (size == 34) {
> +	if (size >= 34) {
>  		ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
>  		ctrl->bmFramingInfo = data[30];
>  		ctrl->bPreferedVersion = data[31];
> @@ -300,11 +313,10 @@ static int uvc_get_video_ctrl(struct uvc_streaming  
> *stream,
>  static int uvc_set_video_ctrl(struct uvc_streaming *stream,
>  	struct uvc_streaming_control *ctrl, int probe)
>  {
> +	u16 size = uvc_video_ctrl_size(stream);
>  	u8 *data;
> -	u16 size;
>  	int ret;
>
> -	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
>  	data = kzalloc(size, GFP_KERNEL);
>  	if (data == NULL)
>  		return -ENOMEM;
> @@ -321,7 +333,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming  
> *stream,
>  	put_unaligned_le32(ctrl->dwMaxVideoFrameSize, &data[18]);
>  	put_unaligned_le32(ctrl->dwMaxPayloadTransferSize, &data[22]);
>
> -	if (size == 34) {
> +	if (size >= 34) {
>  		put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
>  		data[30] = ctrl->bmFramingInfo;
>  		data[31] = ctrl->bPreferedVersion;
>
> -- 
> Regards,
>
> Laurent Pinchart

Re: [PATCH] media: uvcvideo: Support realtek's UVC 1.5 device

[Ana Guerrero Lopez]

> > commit a9c002732695eab2096580a0d1a1687bc2f95928
> > Author: ming_qian <ming_qian@realsil.com.cn>
> > Date:   Wed May 9 10:13:08 2018 +0800
> > 
> >     media: uvcvideo: Support UVC 1.5 video probe & commit controls
> > 
> >     The length of UVC 1.5 video control is 48, and it is 34 for UVC 1.1.
> >     Change it to 48 for UVC 1.5 device, and the UVC 1.5 device can be
> >     recognized.
> > 
> >     More changes to the driver are needed for full UVC 1.5 compatibility.
> >     However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have been
> >     reported to work well.
> > 
> >     Cc: stable@vger.kernel.org
> >     Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> >     [Factor out code to helper function, update size checks]
> >     Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I tested this new patch and it works well.
> 
> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

I tested it as well and it works well.

Tested-by: Ana Guerrero Lopez <ana.guerrero@collabora.com>


> > 
> > diff --git a/drivers/media/usb/uvc/uvc_video.c
> > b/drivers/media/usb/uvc/uvc_video.c
> > index eb9e04a59427..285b0e813b9d 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -207,14 +207,27 @@ static void uvc_fixup_video_ctrl(struct
> > uvc_streaming *stream,
> >  	}
> >  }
> > 
> > +static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
> > +{
> > +	/*
> > +	 * Return the size of the video probe and commit controls, which depends
> > +	 * on the protocol version.
> > +	 */
> > +	if (stream->dev->uvc_version < 0x0110)
> > +		return 26;
> > +	else if (stream->dev->uvc_version < 0x0150)
> > +		return 34;
> > +	else
> > +		return 48;
> > +}
> > +
> >  static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> >  	struct uvc_streaming_control *ctrl, int probe, u8 query)
> >  {
> > +	u16 size = uvc_video_ctrl_size(stream);
> >  	u8 *data;
> > -	u16 size;
> >  	int ret;
> > 
> > -	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> >  	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> >  			query == UVC_GET_DEF)
> >  		return -EIO;
> > @@ -271,7 +284,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> > *stream,
> >  	ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
> >  	ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);
> > 
> > -	if (size == 34) {
> > +	if (size >= 34) {
> >  		ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
> >  		ctrl->bmFramingInfo = data[30];
> >  		ctrl->bPreferedVersion = data[31];
> > @@ -300,11 +313,10 @@ static int uvc_get_video_ctrl(struct uvc_streaming
> > *stream,
> >  static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> >  	struct uvc_streaming_control *ctrl, int probe)
> >  {
> > +	u16 size = uvc_video_ctrl_size(stream);
> >  	u8 *data;
> > -	u16 size;
> >  	int ret;
> > 
> > -	size = stream->dev->uvc_version >= 0x0110 ? 34 : 26;
> >  	data = kzalloc(size, GFP_KERNEL);
> >  	if (data == NULL)
> >  		return -ENOMEM;
> > @@ -321,7 +333,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming
> > *stream,
> >  	put_unaligned_le32(ctrl->dwMaxVideoFrameSize, &data[18]);
> >  	put_unaligned_le32(ctrl->dwMaxPayloadTransferSize, &data[22]);
> > 
> > -	if (size == 34) {
> > +	if (size >= 34) {
> >  		put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
> >  		data[30] = ctrl->bmFramingInfo;
> >  		data[31] = ctrl->bPreferedVersion;
> > 
> > -- 
> > Regards,
> > 
> > Laurent Pinchart

Re: media: uvcvideo: Support realtek's UVC 1.5 device

[Darren Hart]

On Fri, May 18, 2018 at 04:04:35PM +0200, Ana Guerrero Lopez wrote:
> On Wed, May 09, 2018 at 10:13:08AM +0800, ming_qian@realsil.com.cn wrote:
> > From: ming_qian <ming_qian@realsil.com.cn>
> > 
> > The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1.
> > Change it to 48 for UVC 1.5 device,
> > and the UVC 1.5 device can be recognized.
> > 
> > More changes to the driver are needed for full UVC 1.5 compatibility.
> > However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have
> > been reported to work well.
> > 
> > Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
> > Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > Tested-by: Josef Šimánek <josef.simanek@gmail.com>
> 
> It works perfectly here on 4.16.5 with a Dell XPS 9370 in Debian.
> 
> Tested-by: Ana Guerrero Lopez <ana.guerrero@collabora.com>

I worked with Eilís Ní Fhlannagáin on social media who has also confirmed this to solve
her camera issue with her Dell XPS 13 (I didn't get the specific model).

Took a looking at linux-next today, I didn't see it. Anything else needed to get
this queued up?  We'd love to get this in the 4.18 merge window, and stable if
possible.

-- 
Darren Hart
VMware Open Source Technology Center