[PATCH 1/1] media: uvcvideo: Add quirk to support light switch on Dino-Lite cameras

2017-11-30 Thread Alexandre Macabies
The Dino-Lite cameras are equipped with LED lights that can be switched
on and off by setting a proprietary control. For this control, the
camera reports a length of 1 byte, but actually the value set by the
original Windows driver is 3 byte long. This makes it impossible to
toggle the camera lights from uvcvideo, as the length from GET_LEN is
trusted as being the right one.

This is to make GET_LEN indicate a length of 3 instead of 1 for this
specific device.

Signed-off-by: Alexandre Macabies 
---
 drivers/media/usb/uvc/uvc_driver.c |  9 +
 drivers/media/usb/uvc/uvc_video.c  | 10 ++
 drivers/media/usb/uvc/uvcvideo.h   |  1 +
 3 files changed, 20 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 28b91b7d7..17689a242 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2734,6 +2734,15 @@ static const struct usb_device_id uvc_ids[] = {
  .bInterfaceSubClass   = 1,
  .bInterfaceProtocol   = 0,
  .driver_info  = UVC_QUIRK_FORCE_Y8 },
+   /* Dino-Lite Premier AM4111T */
+   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
+   | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0xa168,
+ .idProduct= 0x0870,
+ .bInterfaceClass  = USB_CLASS_VIDEO,
+ .bInterfaceSubClass   = 1,
+ .bInterfaceProtocol   = 0,
+ .driver_info  = UVC_QUIRK_LIGHT_CTRL_LEN },
/* Generic USB Video Class */
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) },
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) },
diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index fb86d6af3..702bf03c7 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -83,6 +83,16 @@ int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 
unit,
return -EIO;
}
 
+   /* The Dino-Lite Premier camera lies about a specific query length:
+* control 3 unit 4 (LED light on/off) expects a 3 byte payload but
+* the camera reports only 1 byte when queried for GET_LEN.
+*/
+   if ((dev->quirks & UVC_QUIRK_LIGHT_CTRL_LEN) && query == UVC_GET_LEN
+   && cs == 3 && unit == 4 && size == 2) {
+   put_unaligned_le16(3, data);
+   return 0;
+   }
+
return 0;
 }
 
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 05398784d..bb6a74920 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -186,6 +186,7 @@
 #define UVC_QUIRK_RESTRICT_FRAME_RATE  0x0200
 #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT0x0400
 #define UVC_QUIRK_FORCE_Y8 0x0800
+#define UVC_QUIRK_LIGHT_CTRL_LEN   0x1000
 
 /* Format flags */
 #define UVC_FMT_FLAG_COMPRESSED0x0001
-- 
2.15.1



[PATCH 0/1] Add quirk to support light switch on some cameras

2017-11-30 Thread Alexandre Macabies
The Dino-Lite cameras are equipped with LED lights that can be switched
on and off by setting a proprietary control. For this control, the
camera reports a length of 1 byte, but actually the value set by the
original Windows driver is 3 byte long. This makes it impossible to
toggle the camera lights from uvcvideo, as the length from GET_LEN is
trusted as being the right one.

This patch makes GET_LEN indicate a length of 3 instead of 1 for this
specific device and control cs/unit.

This patch is a follow-up of a previous patch [1] sent in June, that
went unnoticed but addresses the same issue. It uses a table of local
fixups instead of a module quirk. If you prefer this approach, please
tell me, so I can rework it a bit and submit it in place of this one.

[1] https://patchwork.kernel.org/patch/9764937/

Alexandre Macabies (1):
  media: uvcvideo: Add quirk to support light switch on Dino-Lite
cameras

 drivers/media/usb/uvc/uvc_driver.c |  9 +
 drivers/media/usb/uvc/uvc_video.c  | 10 ++
 drivers/media/usb/uvc/uvcvideo.h   |  1 +
 3 files changed, 20 insertions(+)

-- 
2.15.1



Re: [PATCH] uvcvideo: Hardcoded CTRL_QUERY GET_LEN for a lying device

2017-06-14 Thread Alexandre Macabies
On 06/04/2017 03:41 PM, Alexandre Macabies wrote:
> Hello,

I forgot to Cc: the full list of maintainers for this patch. This follow-up
includes them. Sorry for the noise! My original email & patch is quoted below.

Best,
Alexandre

> This thread comes after two others[1][2] about a similar issue.
> 
> I own a USB video microscope[3] from Dino-Lite. Even if the constructor does
> not advertise it as being supported on Linux, it is mostly a "good citizen"
> camera: it registers as a standard USB video device and as such, it is 
> properly
> recognized by uvcvideo.
> 
> This device is equipped with an integrated illuminator/lamp -- a set of LEDs.
> After some research (using a USB sniffer) I managed to identify the
> non-standard XU control used to switch this lamp on and off: one shall send
> either 80 01 f0 (off) or 80 01 f1 (on) to XU control unit 4 selector 3.
> 
> So at first I tried to send a raw ctrl_set using:
> 
> $ uvcdynctrl -S 4:3 8001f0
> [...]
> query control size of : 1
> [...]
> ERROR: Unable to set the control value: Invalid argument. (Code: 3)
> 
> Indeed, the device reports this XU as being only 1 in length, but the payload
> has to be 3 bytes. So I assume there is a bug (or deliberate inaccuracy) in 
> the
> GET_LEN reply from the device firmware. To overcome this issue, I compiled
> a patched version of uvcvideo in which uvc_query_ctrl[4] returns an hardcoded
> size of 3 for this specific device & UX control. I was finally able to switch
> the lamp on and off:
> 
> $ uvcdynctrl -S 4:3 8001f0
> [39252.854261] uvcvideo: Fixing USB a168:0870 UX control 4/3 len: 1 -> 3
> [...]
> query control size of : 3
> [...]
> set value of  : (LE)0x8001f0  (BE)0xf00180
> [lamp goes off]
> 
> You can find the patch below. I abstracted it in the spirit of
> uvc_ctrl_fixup_xu_info[5] so we can add more entries to the table in the
> future. What do you think, would it be relevant to merge? AFAICT there is no
> API in uvcvideo or v4l for controlling this kind of illuminator/lamp features,
> so giving userland the ability to control the devices via XU by lying seems to
> be the only solution.
> 
> Best,
> 
> Alexandre
> 
> [1] "Dino-Lite uvc support", 2008, 
> https://sourceforge.net/p/linux-uvc/mailman/message/29831153/
> [2] "switching light on device Dino-Lite Premier", 2013, 
> https://sourceforge.net/p/linux-uvc/mailman/message/31219122/
> [3] https://www.dinolite.us/products/digital-microscopes/usb/basic/am4111t
> [4] 
> http://elixir.free-electrons.com/linux/v4.11/source/drivers/media/usb/uvc/uvc_video.c#L72
> [5] 
> http://elixir.free-electrons.com/linux/v4.11/source/drivers/media/usb/uvc/uvc_ctrl.c#L1593
> 
> Signed-off-by: Alexandre Macabies 
> 
> ---
>  drivers/media/usb/uvc/uvc_video.c | 37 +
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c 
> b/drivers/media/usb/uvc/uvc_video.c
> index 07a6c833ef7b..839dc02b4f33 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -69,6 +69,40 @@ static const char *uvc_query_name(__u8 query)
>   }
>  }
>  
> +static void uvc_fixup_query_ctrl_len(const struct uvc_device *dev, __u8 unit,
> + __u8 cs, void *data)
> +{
> + struct uvc_ctrl_fixup {
> + struct usb_device_id id;
> + u8 unit;
> + u8 selector;
> + u16 len;
> + };
> +
> + static const struct uvc_ctrl_fixup fixups[] = {
> + // Dino-Lite Premier (AM4111T)
> + { { USB_DEVICE(0xa168, 0x0870) }, 4, 3, 3 },
> + };
> +
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(fixups); ++i) {
> + if (!usb_match_one_id(dev->intf, &fixups[i].id))
> + continue;
> +
> + if (!(fixups[i].unit == unit && fixups[i].selector == cs))
> + continue;
> +
> + uvc_trace(UVC_TRACE_CONTROL,
> +   "Fixing USB %04x:%04x %u/%u GET_LEN: %u -> %u",
> +   fixups[i].id.idVendor, fixups[i].id.idProduct,
> +   unit, cs,
> +   le16_to_cpup((__le16 *)data), fixups[i].len);
> + *((__le16 *)data) = cpu_to_le16(fixups[i].len);
> + break;
> + }
> +}
> +
>  int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
>   __u8 intfnum, __u8 cs, void *data, __u16 size)
>  {
> @@ -83,6 +117,9 @@ int uvc_query_ctrl(struct uvc_device *dev, __u8 query, 
> __u8 unit,
>   return -EIO;
>   }
>  
> + if (query == UVC_GET_LEN && size == 2)
> + uvc_fixup_query_ctrl_len(dev, unit, cs, data);
> +
>   return 0;
>  }
>  
> 
> -- 
> 2.13.0
> 




[PATCH] uvcvideo: Hardcoded CTRL_QUERY GET_LEN for a lying device

2017-06-04 Thread Alexandre Macabies
Hello,

I sent this patch on the linux-uvc ML but it seems effectively dead nowadays.
This thread comes after two others[1][2] about a similar issue.

I own a USB video microscope[3] from Dino-Lite. Even if the constructor does
not advertise it as being supported on Linux, it is mostly a "good citizen"
camera: it registers as a standard USB video device and as such, it is properly
recognized by uvcvideo.

This device is equipped with an integrated illuminator/lamp -- a set of LEDs.
After some research (using a USB sniffer) I managed to identify the
non-standard XU control used to switch this lamp on and off: one shall send
either 80 01 f0 (off) or 80 01 f1 (on) to XU control unit 4 selector 3.

So at first I tried to send a raw ctrl_set using:

$ uvcdynctrl -S 4:3 8001f0
[...]
query control size of : 1
[...]
ERROR: Unable to set the control value: Invalid argument. (Code: 3)

Indeed, the device reports this XU as being only 1 in length, but the payload
has to be 3 bytes. So I assume there is a bug (or deliberate inaccuracy) in the
GET_LEN reply from the device firmware. To overcome this issue, I compiled
a patched version of uvcvideo in which uvc_query_ctrl[4] returns an hardcoded
size of 3 for this specific device & UX control. I was finally able to switch
the lamp on and off:

$ uvcdynctrl -S 4:3 8001f0
[39252.854261] uvcvideo: Fixing USB a168:0870 UX control 4/3 len: 1 -> 3
[...]
query control size of : 3
[...]
set value of  : (LE)0x8001f0  (BE)0xf00180
[lamp goes off]

You can find the patch below. I abstracted it in the spirit of
uvc_ctrl_fixup_xu_info[5] so we can add more entries to the table in the
future. What do you think, would it be relevant to merge? AFAICT there is no
API in uvcvideo or v4l for controlling this kind of illuminator/lamp features,
so giving userland the ability to control the devices via XU by lying seems to
be the only solution.

Best,

Alexandre

[1] "Dino-Lite uvc support", 2008, 
https://sourceforge.net/p/linux-uvc/mailman/message/29831153/
[2] "switching light on device Dino-Lite Premier", 2013, 
https://sourceforge.net/p/linux-uvc/mailman/message/31219122/
[3] https://www.dinolite.us/products/digital-microscopes/usb/basic/am4111t
[4] 
http://elixir.free-electrons.com/linux/v4.11/source/drivers/media/usb/uvc/uvc_video.c#L72
[5] 
http://elixir.free-electrons.com/linux/v4.11/source/drivers/media/usb/uvc/uvc_ctrl.c#L1593

Signed-off-by: Alexandre Macabies 

---
 drivers/media/usb/uvc/uvc_video.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index 07a6c833ef7b..839dc02b4f33 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -69,6 +69,40 @@ static const char *uvc_query_name(__u8 query)
}
 }
 
+static void uvc_fixup_query_ctrl_len(const struct uvc_device *dev, __u8 unit,
+   __u8 cs, void *data)
+{
+   struct uvc_ctrl_fixup {
+   struct usb_device_id id;
+   u8 unit;
+   u8 selector;
+   u16 len;
+   };
+
+   static const struct uvc_ctrl_fixup fixups[] = {
+   // Dino-Lite Premier (AM4111T)
+   { { USB_DEVICE(0xa168, 0x0870) }, 4, 3, 3 },
+   };
+
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(fixups); ++i) {
+   if (!usb_match_one_id(dev->intf, &fixups[i].id))
+   continue;
+
+   if (!(fixups[i].unit == unit && fixups[i].selector == cs))
+   continue;
+
+   uvc_trace(UVC_TRACE_CONTROL,
+ "Fixing USB %04x:%04x %u/%u GET_LEN: %u -> %u",
+ fixups[i].id.idVendor, fixups[i].id.idProduct,
+ unit, cs,
+ le16_to_cpup((__le16 *)data), fixups[i].len);
+   *((__le16 *)data) = cpu_to_le16(fixups[i].len);
+   break;
+   }
+}
+
 int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
__u8 intfnum, __u8 cs, void *data, __u16 size)
 {
@@ -83,6 +117,9 @@ int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 
unit,
return -EIO;
}
 
+   if (query == UVC_GET_LEN && size == 2)
+   uvc_fixup_query_ctrl_len(dev, unit, cs, data);
+
return 0;
 }
 
-- 
2.13.0