diff mbox

[v1,01/19] uvcvideo: Add UVC query tracing.

Message ID 1377829038-4726-2-git-send-email-posciak@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Pawel Osciak Aug. 30, 2013, 2:17 a.m. UTC 
Add a new trace argument enabling UVC query details and contents logging.

Signed-off-by: Pawel Osciak <posciak@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 45 +++++++++++++++++++++++++--------------
 drivers/media/usb/uvc/uvcvideo.h  |  9 ++++++++
 2 files changed, 38 insertions(+), 16 deletions(-)

Comments

Laurent Pinchart Sept. 3, 2013, 8:17 p.m. UTC | #1 
Hi Pawel,

Thank you so much for the patch (set) ! I'll probably need a couple of days to 
review it all, let's start.

On Friday 30 August 2013 11:17:00 Pawel Osciak wrote:
> Add a new trace argument enabling UVC query details and contents logging.
> 
> Signed-off-by: Pawel Osciak <posciak@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 45 ++++++++++++++++++++++--------------
>  drivers/media/usb/uvc/uvcvideo.h  |  9 ++++++++
>  2 files changed, 38 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index 3394c34..695f6d9 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -29,22 +29,6 @@
>  /* ------------------------------------------------------------------------
> * UVC Controls
>   */
> -
> -static int __uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
> -			__u8 intfnum, __u8 cs, void *data, __u16 size,
> -			int timeout)
> -{
> -	__u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
> -	unsigned int pipe;
> -
> -	pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
> -			      : usb_sndctrlpipe(dev->udev, 0);
> -	type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
> -
> -	return usb_control_msg(dev->udev, pipe, query, type, cs << 8,
> -			unit << 8 | intfnum, data, size, timeout);
> -}
> -
>  static const char *uvc_query_name(__u8 query)
>  {
>  	switch (query) {
> @@ -69,6 +53,35 @@ static const char *uvc_query_name(__u8 query)
>  	}
>  }
> 
> +static int __uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
> +			__u8 intfnum, __u8 cs, void *data, __u16 size,
> +			int timeout)
> +{
> +	__u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
> +	unsigned int pipe;
> +	int ret;
> +
> +	pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
> +			      : usb_sndctrlpipe(dev->udev, 0);
> +	type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
> +
> +	uvc_trace(UVC_TRACE_QUERY,
> +			"%s (%d): size=%d, unit=%d, cs=%d, intf=%d\n",

All the fields are unsigned, shouldn't you use %u instead of %d ?

> +			uvc_query_name(query), query, size, unit, cs, intfnum);

That's a pretty strange indentation :-)

> +	uvc_trace(UVC_TRACE_QUERY, "Sent:\n");
> +	uvc_print_hex_dump(UVC_TRACE_QUERY, data, size);
> +
> +	ret = usb_control_msg(dev->udev, pipe, query, type, cs << 8,
> +			unit << 8 | intfnum, data, size, timeout);
> +	if (ret == -EPIPE)
> +		uvc_trace(UVC_TRACE_QUERY, "Got device STALL on query!\n");
> +
> +	uvc_trace(UVC_TRACE_QUERY, "Received:\n");
> +	uvc_print_hex_dump(UVC_TRACE_QUERY, data, size);
> +
> +	return ret;
> +}
> +
>  int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
>  			__u8 intfnum, __u8 cs, void *data, __u16 size)
>  {
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index 9e35982..75e0153 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -574,6 +574,7 @@ struct uvc_driver {
>  #define UVC_TRACE_VIDEO		(1 << 10)
>  #define UVC_TRACE_STATS		(1 << 11)
>  #define UVC_TRACE_CLOCK		(1 << 12)
> +#define UVC_TRACE_QUERY		(1 << 13)
> 
>  #define UVC_WARN_MINMAX		0
>  #define UVC_WARN_PROBE_DEF	1
> @@ -599,6 +600,14 @@ extern unsigned int uvc_timeout_param;
>  #define uvc_printk(level, msg...) \
>  	printk(level "uvcvideo: " msg)
> 
> +#define uvc_print_hex_dump(flag, buf, len) \
> +	do { \
> +		if (uvc_trace_param & flag) { \
> +			print_hex_dump(KERN_DEBUG, "uvcvideo: ", \
> +				DUMP_PREFIX_NONE, 16, 1, buf, len, false); \
> +		} \

No need for braces around the print_hex_dump() call.

> +	} while (0)
> +
>  /*
> --------------------------------------------------------------------------
> * Internal functions.
>   */
diff mbox

Patch

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 3394c34..695f6d9 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -29,22 +29,6 @@ 
 /* ------------------------------------------------------------------------
  * UVC Controls
  */
-
-static int __uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
-			__u8 intfnum, __u8 cs, void *data, __u16 size,
-			int timeout)
-{
-	__u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
-	unsigned int pipe;
-
-	pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
-			      : usb_sndctrlpipe(dev->udev, 0);
-	type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
-
-	return usb_control_msg(dev->udev, pipe, query, type, cs << 8,
-			unit << 8 | intfnum, data, size, timeout);
-}
-
 static const char *uvc_query_name(__u8 query)
 {
 	switch (query) {
@@ -69,6 +53,35 @@  static const char *uvc_query_name(__u8 query)
 	}
 }
 
+static int __uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
+			__u8 intfnum, __u8 cs, void *data, __u16 size,
+			int timeout)
+{
+	__u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
+	unsigned int pipe;
+	int ret;
+
+	pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
+			      : usb_sndctrlpipe(dev->udev, 0);
+	type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
+
+	uvc_trace(UVC_TRACE_QUERY,
+			"%s (%d): size=%d, unit=%d, cs=%d, intf=%d\n",
+			uvc_query_name(query), query, size, unit, cs, intfnum);
+	uvc_trace(UVC_TRACE_QUERY, "Sent:\n");
+	uvc_print_hex_dump(UVC_TRACE_QUERY, data, size);
+
+	ret = usb_control_msg(dev->udev, pipe, query, type, cs << 8,
+			unit << 8 | intfnum, data, size, timeout);
+	if (ret == -EPIPE)
+		uvc_trace(UVC_TRACE_QUERY, "Got device STALL on query!\n");
+
+	uvc_trace(UVC_TRACE_QUERY, "Received:\n");
+	uvc_print_hex_dump(UVC_TRACE_QUERY, data, size);
+
+	return ret;
+}
+
 int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit,
 			__u8 intfnum, __u8 cs, void *data, __u16 size)
 {
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 9e35982..75e0153 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -574,6 +574,7 @@  struct uvc_driver {
 #define UVC_TRACE_VIDEO		(1 << 10)
 #define UVC_TRACE_STATS		(1 << 11)
 #define UVC_TRACE_CLOCK		(1 << 12)
+#define UVC_TRACE_QUERY		(1 << 13)
 
 #define UVC_WARN_MINMAX		0
 #define UVC_WARN_PROBE_DEF	1
@@ -599,6 +600,14 @@  extern unsigned int uvc_timeout_param;
 #define uvc_printk(level, msg...) \
 	printk(level "uvcvideo: " msg)
 
+#define uvc_print_hex_dump(flag, buf, len) \
+	do { \
+		if (uvc_trace_param & flag) { \
+			print_hex_dump(KERN_DEBUG, "uvcvideo: ", \
+				DUMP_PREFIX_NONE, 16, 1, buf, len, false); \
+		} \
+	} while (0)
+
 /* --------------------------------------------------------------------------
  * Internal functions.
  */