[5/8] drivers/media/video/uvc: Use %pUl to print UUIDs

Message ID ef810b1f134d8b5f07b849b13751445d7d49956b.1254884776.git.joe@perches.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Joe Perches Oct. 7, 2009, 4:45 a.m. UTC
  Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/media/video/uvc/uvc_ctrl.c   |   69 ++++++++++++++++------------------
 drivers/media/video/uvc/uvc_driver.c |    7 +--
 drivers/media/video/uvc/uvcvideo.h   |   10 -----
 3 files changed, 35 insertions(+), 51 deletions(-)
  

Comments

Laurent Pinchart Oct. 11, 2009, 10:34 p.m. UTC | #1
Hi Joe,

On Wednesday 07 October 2009 06:45:38 Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/media/video/uvc/uvc_ctrl.c   |   69 ++++++++++++++++------------------
>  drivers/media/video/uvc/uvc_driver.c |    7 +--
>  drivers/media/video/uvc/uvcvideo.h   |   10 -----
>  3 files changed, 35 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
>  b/drivers/media/video/uvc/uvc_ctrl.c index c3225a5..4d06976 100644
> --- a/drivers/media/video/uvc/uvc_ctrl.c
> +++ b/drivers/media/video/uvc/uvc_ctrl.c
> @@ -1093,8 +1093,8 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> 
>  	if (!found) {
>  		uvc_trace(UVC_TRACE_CONTROL,
> -			"Control " UVC_GUID_FORMAT "/%u not found.\n",
> -			UVC_GUID_ARGS(entity->extension.guidExtensionCode),
> +			"Control %pUl/%u not found.\n",
> +			entity->extension.guidExtensionCode,
>  			xctrl->selector);
>  		return -EINVAL;
>  	}
> @@ -1171,9 +1171,9 @@ int uvc_ctrl_resume_device(struct uvc_device *dev)
>  			    (ctrl->info->flags & UVC_CONTROL_RESTORE) == 0)
>  				continue;
> 
> -			printk(KERN_INFO "restoring control " UVC_GUID_FORMAT
> -				"/%u/%u\n", UVC_GUID_ARGS(ctrl->info->entity),
> -				ctrl->info->index, ctrl->info->selector);
> +			printk(KERN_INFO "restoring control %pUl/%u/%u\n",
> +			       ctrl->info->entity,
> +			       ctrl->info->index, ctrl->info->selector);
>  			ctrl->dirty = 1;
>  		}
> 
> @@ -1228,46 +1228,43 @@ static void uvc_ctrl_add_ctrl(struct uvc_device
>  *dev, dev->intfnum, info->selector, (__u8 *)&size, 2);
>  		if (ret < 0) {
>  			uvc_trace(UVC_TRACE_CONTROL, "GET_LEN failed on "
> -				"control " UVC_GUID_FORMAT "/%u (%d).\n",
> -				UVC_GUID_ARGS(info->entity), info->selector,
> -				ret);
> +				  "control %pUl/%u (%d).\n",
> +				  info->entity, info->selector, ret);
>  			return;
>  		}
> 
>  		if (info->size != le16_to_cpu(size)) {
> -			uvc_trace(UVC_TRACE_CONTROL, "Control " UVC_GUID_FORMAT
> -				"/%u size doesn't match user supplied "
> -				"value.\n", UVC_GUID_ARGS(info->entity),
> -				info->selector);
> +			uvc_trace(UVC_TRACE_CONTROL,
> +				  "Control %pUl/%u size doesn't match user supplied value.\n",
> +				  info->entity, info->selector);
>  			return;
>  		}
> 
>  		ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id,
>  			dev->intfnum, info->selector, &inf, 1);
>  		if (ret < 0) {
> -			uvc_trace(UVC_TRACE_CONTROL, "GET_INFO failed on "
> -				"control " UVC_GUID_FORMAT "/%u (%d).\n",
> -				UVC_GUID_ARGS(info->entity), info->selector,
> -				ret);
> +			uvc_trace(UVC_TRACE_CONTROL,
> +				  "GET_INFO failed on control %pUl/%u (%d).\n",
> +				  info->entity, info->selector, ret);
>  			return;
>  		}
> 
>  		flags = info->flags;
>  		if (((flags & UVC_CONTROL_GET_CUR) && !(inf & (1 << 0))) ||
>  		    ((flags & UVC_CONTROL_SET_CUR) && !(inf & (1 << 1)))) {
> -			uvc_trace(UVC_TRACE_CONTROL, "Control "
> -				UVC_GUID_FORMAT "/%u flags don't match "
> -				"supported operations.\n",
> -				UVC_GUID_ARGS(info->entity), info->selector);
> +			uvc_trace(UVC_TRACE_CONTROL,
> +				  "Control %pUl/%u flags don't match supported operations.\n",
> +				  info->entity, info->selector);
>  			return;
>  		}
>  	}
> 
>  	ctrl->info = info;
>  	ctrl->data = kmalloc(ctrl->info->size * UVC_CTRL_NDATA, GFP_KERNEL);
> -	uvc_trace(UVC_TRACE_CONTROL, "Added control " UVC_GUID_FORMAT "/%u "
> -		"to device %s entity %u\n", UVC_GUID_ARGS(ctrl->info->entity),
> -		ctrl->info->selector, dev->udev->devpath, entity->id);
> +	uvc_trace(UVC_TRACE_CONTROL,
> +		  "Added control %pUl/%u to device %s entity %u\n",
> +		  ctrl->info->entity, ctrl->info->selector,
> +		  dev->udev->devpath, entity->id);
>  }
> 
>  /*
> @@ -1293,17 +1290,16 @@ int uvc_ctrl_add_info(struct uvc_control_info
>  *info) continue;
> 
>  		if (ctrl->selector == info->selector) {
> -			uvc_trace(UVC_TRACE_CONTROL, "Control "
> -				UVC_GUID_FORMAT "/%u is already defined.\n",
> -				UVC_GUID_ARGS(info->entity), info->selector);
> +			uvc_trace(UVC_TRACE_CONTROL,
> +				  "Control %pUl/%u is already defined.\n",
> +				  info->entity, info->selector);
>  			ret = -EEXIST;
>  			goto end;
>  		}
>  		if (ctrl->index == info->index) {
> -			uvc_trace(UVC_TRACE_CONTROL, "Control "
> -				UVC_GUID_FORMAT "/%u would overwrite index "
> -				"%d.\n", UVC_GUID_ARGS(info->entity),
> -				info->selector, info->index);
> +			uvc_trace(UVC_TRACE_CONTROL,
> +				  "Control %pUl/%u would overwrite index %d.\n",
> +				  info->entity, info->selector, info->index);
>  			ret = -EEXIST;
>  			goto end;
>  		}
> @@ -1344,10 +1340,9 @@ int uvc_ctrl_add_mapping(struct uvc_control_mapping
>  *mapping) continue;
> 
>  		if (info->size * 8 < mapping->size + mapping->offset) {
> -			uvc_trace(UVC_TRACE_CONTROL, "Mapping '%s' would "
> -				"overflow control " UVC_GUID_FORMAT "/%u\n",
> -				mapping->name, UVC_GUID_ARGS(info->entity),
> -				info->selector);
> +			uvc_trace(UVC_TRACE_CONTROL,
> +				  "Mapping '%s' would overflow control %pUl/%u\n",
> +				  mapping->name, info->entity, info->selector);
>  			ret = -EOVERFLOW;
>  			goto end;
>  		}
> @@ -1366,9 +1361,9 @@ int uvc_ctrl_add_mapping(struct uvc_control_mapping
>  *mapping)
> 
>  		mapping->ctrl = info;
>  		list_add_tail(&mapping->list, &info->mappings);
> -		uvc_trace(UVC_TRACE_CONTROL, "Adding mapping %s to control "
> -			UVC_GUID_FORMAT "/%u.\n", mapping->name,
> -			UVC_GUID_ARGS(info->entity), info->selector);
> +		uvc_trace(UVC_TRACE_CONTROL,
> +			  "Adding mapping %s to control %pUl/%u.\n",
> +			  mapping->name, info->entity, info->selector);
> 
>  		ret = 0;
>  		break;
> diff --git a/drivers/media/video/uvc/uvc_driver.c
>  b/drivers/media/video/uvc/uvc_driver.c index 8756be5..411dc63 100644
> --- a/drivers/media/video/uvc/uvc_driver.c
> +++ b/drivers/media/video/uvc/uvc_driver.c
> @@ -328,11 +328,10 @@ static int uvc_parse_format(struct uvc_device *dev,
>  				sizeof format->name);
>  			format->fcc = fmtdesc->fcc;
>  		} else {
> -			uvc_printk(KERN_INFO, "Unknown video format "
> -				UVC_GUID_FORMAT "\n",
> -				UVC_GUID_ARGS(&buffer[5]));
> +			uvc_printk(KERN_INFO, "Unknown video format %pUl\n",
> +				   &buffer[5]);
>  			snprintf(format->name, sizeof format->name,
> -				UVC_GUID_FORMAT, UVC_GUID_ARGS(&buffer[5]));
> +				 "%pUl", &Buffer[5]);

Buffer should still be written buffer.

As this will go through the linuxtv v4l-dvb tree, I'll have to add backward
compatibility code (that will not make it to mainline). If that's ok with you
it will be easier for me to test and apply that part of the patch through my
tree once the vsprintf extension gets in.
  
Mauro Carvalho Chehab Oct. 31, 2009, 9:07 a.m. UTC | #2
Hi Laurent,

Em Mon, 12 Oct 2009 00:34:58 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> As this will go through the linuxtv v4l-dvb tree, I'll have to add backward
> compatibility code (that will not make it to mainline). If that's ok with you
> it will be easier for me to test and apply that part of the patch through my
> tree once the vsprintf extension gets in. 

I'm assuming that those printk patches from Joe to uvc will go via your tree,
so please submit a pull request when they'll be ready for upstream.




Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Laurent Pinchart Oct. 31, 2009, 7:10 p.m. UTC | #3
On Saturday 31 October 2009 10:07:01 Mauro Carvalho Chehab wrote:
> Hi Laurent,
> 
> Em Mon, 12 Oct 2009 00:34:58 +0200
> 
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:
> > As this will go through the linuxtv v4l-dvb tree, I'll have to add
> > backward compatibility code (that will not make it to mainline). If
> > that's ok with you it will be easier for me to test and apply that part
> > of the patch through my tree once the vsprintf extension gets in.
> 
> I'm assuming that those printk patches from Joe to uvc will go via your
>  tree, so please submit a pull request when they'll be ready for upstream.

I'll submit the pull request as soon as the printk core patch hits upstream.

The change will break the driver when used with older versions (it will 
compile, load and run, but will print broken messages), and compat.h 
compatibility magic will not be possible. As the messages are purely 
informational, I'm pondering not even keeping #ifdef compatibility. Any 
thought on that ?
  
Joe Perches Oct. 31, 2009, 7:27 p.m. UTC | #4
On Sat, 2009-10-31 at 20:10 +0100, Laurent Pinchart wrote:
> On Saturday 31 October 2009 10:07:01 Mauro Carvalho Chehab wrote:
> > I'm assuming that those printk patches from Joe to uvc will go via your
> >  tree, so please submit a pull request when they'll be ready for upstream.
> I'll submit the pull request as soon as the printk core patch hits upstream.

I believe Andrew Morton has picked up the patches for
his mm-commits set.  If you do nothing, these should
show up in Linus' tree after awhile.

lib-vsprintfc-add-%pu-to-print-uuid-guids.patch
fs-xfs-xfs_log_recoverc-use-%pu-to-print-uuids.patch
randomc-use-%pu-to-print-uuids.patch
drivers-firmware-dmi_scanc-use-%pub-to-print-uuids.patch
drivers-md-mdc-use-%pu-to-print-uuids.patch
fs-gfs2-sysc-use-%pub-to-print-uuids.patch
fs-ubifs-use-%pub-to-print-uuids.patch
efih-use-%pul-to-print-uuids.patch
drivers-media-video-uvc-use-%pul-to-print-uuids.patch
lib-unified-uuid-guid-definition.patch
gfs2-use-unified-uuid-guid-code.patch


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Laurent Pinchart Nov. 4, 2009, 2:42 p.m. UTC | #5
Hi Joe,

On Saturday 31 October 2009 20:27:38 Joe Perches wrote:
> On Sat, 2009-10-31 at 20:10 +0100, Laurent Pinchart wrote:
> > On Saturday 31 October 2009 10:07:01 Mauro Carvalho Chehab wrote:
> > > I'm assuming that those printk patches from Joe to uvc will go via your
> > >  tree, so please submit a pull request when they'll be ready for
> > > upstream.
> >
> > I'll submit the pull request as soon as the printk core patch hits
> > upstream.
> 
> I believe Andrew Morton has picked up the patches for
> his mm-commits set.  If you do nothing, these should
> show up in Linus' tree after awhile.

Thanks for the notice.

Andrew, could you please drop drivers-media-video-uvc-use-%pul-to-print-
uuids.patch ? I will push it through the v4l-dvb tree as I need to add 
backward compatibility support.
  
Joe Perches Nov. 4, 2009, 4:42 p.m. UTC | #6
On Wed, 2009-11-04 at 15:42 +0100, Laurent Pinchart wrote:
> Andrew, could you please drop drivers-media-video-uvc-use-%pul-to-print-
> uuids.patch ? I will push it through the v4l-dvb tree as I need to add 
> backward compatibility support.

One thing you might evaluate is how useful it is to continue
to support backward compatibility.

I think drivers/media is the only drivers directory except
staging to continue to use KERNEL_VERSION checks.

cheers, Joe

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index c3225a5..4d06976 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -1093,8 +1093,8 @@  int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
 
 	if (!found) {
 		uvc_trace(UVC_TRACE_CONTROL,
-			"Control " UVC_GUID_FORMAT "/%u not found.\n",
-			UVC_GUID_ARGS(entity->extension.guidExtensionCode),
+			"Control %pUl/%u not found.\n",
+			entity->extension.guidExtensionCode,
 			xctrl->selector);
 		return -EINVAL;
 	}
@@ -1171,9 +1171,9 @@  int uvc_ctrl_resume_device(struct uvc_device *dev)
 			    (ctrl->info->flags & UVC_CONTROL_RESTORE) == 0)
 				continue;
 
-			printk(KERN_INFO "restoring control " UVC_GUID_FORMAT
-				"/%u/%u\n", UVC_GUID_ARGS(ctrl->info->entity),
-				ctrl->info->index, ctrl->info->selector);
+			printk(KERN_INFO "restoring control %pUl/%u/%u\n",
+			       ctrl->info->entity,
+			       ctrl->info->index, ctrl->info->selector);
 			ctrl->dirty = 1;
 		}
 
@@ -1228,46 +1228,43 @@  static void uvc_ctrl_add_ctrl(struct uvc_device *dev,
 			dev->intfnum, info->selector, (__u8 *)&size, 2);
 		if (ret < 0) {
 			uvc_trace(UVC_TRACE_CONTROL, "GET_LEN failed on "
-				"control " UVC_GUID_FORMAT "/%u (%d).\n",
-				UVC_GUID_ARGS(info->entity), info->selector,
-				ret);
+				  "control %pUl/%u (%d).\n",
+				  info->entity, info->selector, ret);
 			return;
 		}
 
 		if (info->size != le16_to_cpu(size)) {
-			uvc_trace(UVC_TRACE_CONTROL, "Control " UVC_GUID_FORMAT
-				"/%u size doesn't match user supplied "
-				"value.\n", UVC_GUID_ARGS(info->entity),
-				info->selector);
+			uvc_trace(UVC_TRACE_CONTROL,
+				  "Control %pUl/%u size doesn't match user supplied value.\n",
+				  info->entity, info->selector);
 			return;
 		}
 
 		ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id,
 			dev->intfnum, info->selector, &inf, 1);
 		if (ret < 0) {
-			uvc_trace(UVC_TRACE_CONTROL, "GET_INFO failed on "
-				"control " UVC_GUID_FORMAT "/%u (%d).\n",
-				UVC_GUID_ARGS(info->entity), info->selector,
-				ret);
+			uvc_trace(UVC_TRACE_CONTROL,
+				  "GET_INFO failed on control %pUl/%u (%d).\n",
+				  info->entity, info->selector, ret);
 			return;
 		}
 
 		flags = info->flags;
 		if (((flags & UVC_CONTROL_GET_CUR) && !(inf & (1 << 0))) ||
 		    ((flags & UVC_CONTROL_SET_CUR) && !(inf & (1 << 1)))) {
-			uvc_trace(UVC_TRACE_CONTROL, "Control "
-				UVC_GUID_FORMAT "/%u flags don't match "
-				"supported operations.\n",
-				UVC_GUID_ARGS(info->entity), info->selector);
+			uvc_trace(UVC_TRACE_CONTROL,
+				  "Control %pUl/%u flags don't match supported operations.\n",
+				  info->entity, info->selector);
 			return;
 		}
 	}
 
 	ctrl->info = info;
 	ctrl->data = kmalloc(ctrl->info->size * UVC_CTRL_NDATA, GFP_KERNEL);
-	uvc_trace(UVC_TRACE_CONTROL, "Added control " UVC_GUID_FORMAT "/%u "
-		"to device %s entity %u\n", UVC_GUID_ARGS(ctrl->info->entity),
-		ctrl->info->selector, dev->udev->devpath, entity->id);
+	uvc_trace(UVC_TRACE_CONTROL,
+		  "Added control %pUl/%u to device %s entity %u\n",
+		  ctrl->info->entity, ctrl->info->selector,
+		  dev->udev->devpath, entity->id);
 }
 
 /*
@@ -1293,17 +1290,16 @@  int uvc_ctrl_add_info(struct uvc_control_info *info)
 			continue;
 
 		if (ctrl->selector == info->selector) {
-			uvc_trace(UVC_TRACE_CONTROL, "Control "
-				UVC_GUID_FORMAT "/%u is already defined.\n",
-				UVC_GUID_ARGS(info->entity), info->selector);
+			uvc_trace(UVC_TRACE_CONTROL,
+				  "Control %pUl/%u is already defined.\n",
+				  info->entity, info->selector);
 			ret = -EEXIST;
 			goto end;
 		}
 		if (ctrl->index == info->index) {
-			uvc_trace(UVC_TRACE_CONTROL, "Control "
-				UVC_GUID_FORMAT "/%u would overwrite index "
-				"%d.\n", UVC_GUID_ARGS(info->entity),
-				info->selector, info->index);
+			uvc_trace(UVC_TRACE_CONTROL,
+				  "Control %pUl/%u would overwrite index %d.\n",
+				  info->entity, info->selector, info->index);
 			ret = -EEXIST;
 			goto end;
 		}
@@ -1344,10 +1340,9 @@  int uvc_ctrl_add_mapping(struct uvc_control_mapping *mapping)
 			continue;
 
 		if (info->size * 8 < mapping->size + mapping->offset) {
-			uvc_trace(UVC_TRACE_CONTROL, "Mapping '%s' would "
-				"overflow control " UVC_GUID_FORMAT "/%u\n",
-				mapping->name, UVC_GUID_ARGS(info->entity),
-				info->selector);
+			uvc_trace(UVC_TRACE_CONTROL,
+				  "Mapping '%s' would overflow control %pUl/%u\n",
+				  mapping->name, info->entity, info->selector);
 			ret = -EOVERFLOW;
 			goto end;
 		}
@@ -1366,9 +1361,9 @@  int uvc_ctrl_add_mapping(struct uvc_control_mapping *mapping)
 
 		mapping->ctrl = info;
 		list_add_tail(&mapping->list, &info->mappings);
-		uvc_trace(UVC_TRACE_CONTROL, "Adding mapping %s to control "
-			UVC_GUID_FORMAT "/%u.\n", mapping->name,
-			UVC_GUID_ARGS(info->entity), info->selector);
+		uvc_trace(UVC_TRACE_CONTROL,
+			  "Adding mapping %s to control %pUl/%u.\n",
+			  mapping->name, info->entity, info->selector);
 
 		ret = 0;
 		break;
diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c
index 8756be5..411dc63 100644
--- a/drivers/media/video/uvc/uvc_driver.c
+++ b/drivers/media/video/uvc/uvc_driver.c
@@ -328,11 +328,10 @@  static int uvc_parse_format(struct uvc_device *dev,
 				sizeof format->name);
 			format->fcc = fmtdesc->fcc;
 		} else {
-			uvc_printk(KERN_INFO, "Unknown video format "
-				UVC_GUID_FORMAT "\n",
-				UVC_GUID_ARGS(&buffer[5]));
+			uvc_printk(KERN_INFO, "Unknown video format %pUl\n",
+				   &buffer[5]);
 			snprintf(format->name, sizeof format->name,
-				UVC_GUID_FORMAT, UVC_GUID_ARGS(&buffer[5]));
+				 "%pUl", &Buffer[5]);
 			format->fcc = 0;
 		}
 
diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h
index e7958aa..9f4a437 100644
--- a/drivers/media/video/uvc/uvcvideo.h
+++ b/drivers/media/video/uvc/uvcvideo.h
@@ -555,16 +555,6 @@  extern unsigned int uvc_trace_param;
 #define uvc_printk(level, msg...) \
 	printk(level "uvcvideo: " msg)
 
-#define UVC_GUID_FORMAT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-" \
-			"%02x%02x%02x%02x%02x%02x"
-#define UVC_GUID_ARGS(guid) \
-	(guid)[3],  (guid)[2],  (guid)[1],  (guid)[0], \
-	(guid)[5],  (guid)[4], \
-	(guid)[7],  (guid)[6], \
-	(guid)[8],  (guid)[9], \
-	(guid)[10], (guid)[11], (guid)[12], \
-	(guid)[13], (guid)[14], (guid)[15]
-
 /* --------------------------------------------------------------------------
  * Internal functions.
  */