Skip to contentSkip to conversation

staging: unisys: Convert direct LOGERR uses to pr_err

PATCH In review Joe Perches 2 participants 8 messages Jan 11, 2015 Staging

lore
Share & tools
lore

Joe Perches

author
#0 lore raw

PATCH 1/1 staging: unisys: Convert direct LOGERR uses to pr_err

Use the normal logging facility instead of a new macro.

Miscellanea:

o Realign arguments
o Remove unnecessary alloc error messages as alloc failures already emits a generic OOM message with a dump_stack
o Add missing newlines to these messages to avoid interleaving
o Spelling/typo fixes in those messages

Signed-off-by Joe Perches <[email protected]>
---
 drivers/staging/unisys/include/uisutils.h          |   4 +-
 drivers/staging/unisys/uislib/uislib.c             | 169 ++++++++++-----------
 drivers/staging/unisys/uislib/uisqueue.c           |   4 +-
 drivers/staging/unisys/uislib/uisthread.c          |   4 +-
 drivers/staging/unisys/uislib/uisutils.c           |  34 ++---
 drivers/staging/unisys/virthba/virthba.c           | 119 +++++++--------
 drivers/staging/unisys/virtpci/virtpci.c           |  71 +++++----
 drivers/staging/unisys/visorchipset/file.c         |   2 +-
 .../unisys/visorchipset/visorchipset_main.c        | 159 ++++++++++---------
 9 files changed, 276 insertions(+), 290 deletions(-)
drivers/staging/unisys/include/uisutils.h +2 -2
index 7414220..f937892 100644
@@ -206,10 +206,10 @@ wait_for_valid_guid(uuid_le __iomem *guid)
 			      (void __iomem *)guid, sizeof(uuid_le));
 		if (uuid_le_cmp(tmpguid, NULL_UUID_LE) != 0)
 			break;
-		LOGERR("Waiting for non-0 GUID (why???)...\n");
+		pr_err("Waiting for non-0 GUID (why???)...\n");
 		UIS_THREAD_WAIT_SEC(5);
 	}
-	LOGERR("OK... GUID is non-0 now\n");
+	pr_err("OK... GUID is non-0 now\n");
 }
 
 /* CopyFragsInfoFromSkb returns the number of entries added to frags array
drivers/staging/unisys/uislib/uislib.c +83 -86

500 lines not on this page · raw message

drivers/staging/unisys/uislib/uisqueue.c +2 -2
index f9f8442..b0ade86 100644
@@ -107,11 +107,11 @@ uisqueue_put_cmdrsp_with_lock_client(struct uisqueue_info *queueinfo,
 					issue_irq_if_empty,
 					irq_handle, channel_id)) {
 		if (oktowait != OK_TO_WAIT) {
-			LOGERR("****FAILED visor_signal_insert failed; cannot wait; insert aborted\n");
+			pr_err("****FAILED visor_signal_insert failed; cannot wait; insert aborted\n");
 			return 0;	/* failed to queue */
 		}
 		/* try again */
-		LOGERR("****FAILED visor_signal_insert failed; waiting to try again\n");
+		pr_err("****FAILED visor_signal_insert failed; waiting to try again\n");
 		set_current_state(TASK_INTERRUPTIBLE);
 		schedule_timeout(msecs_to_jiffies(10));
 	}
drivers/staging/unisys/uislib/uisthread.c +2 -2
index c0fc812..fb7211e 100644
@@ -70,13 +70,13 @@ uisthread_stop(struct uisthread_info *thrinfo)
 	thrinfo->should_stop = 1;
 	ret = KILL(thrinfo->id, SIGHUP, 1);
 	if (ret) {
-		LOGERR("unable to signal thread %d\n", ret);
+		pr_err("unable to signal thread %d\n", ret);
 	} else {
 		/* give up if the thread has NOT died in 1 minute */
 		if (wait_for_completion_timeout(&thrinfo->has_stopped, 60 * HZ))
 			stopped = 1;
 		else
-			LOGERR("timed out trying to signal thread\n");
+			pr_err("timed out trying to signal thread\n");
 	}
 	if (stopped) {
 		LOGINF("uisthread_stop stopped id:%d\n", thrinfo->id);
drivers/staging/unisys/uislib/uisutils.c +17 -17
index 4a5b867..bd13669 100644
@@ -61,7 +61,7 @@ uisutil_add_proc_line_ex(int *total, char **buffer, int *buffer_remaining,
 		*buffer += *buffer_remaining;
 		*total += *buffer_remaining;
 		*buffer_remaining = 0;
-		LOGERR("bytes remaining is too small!\n");
+		pr_err("bytes remaining is too small!\n");
 		return -1;
 	}
 	*buffer_remaining -= len;

@@ -91,7 +91,7 @@ uisctrl_register_req_handler(int type, void *fptr,
 		break;
 
 	default:
-		LOGERR("invalid type %d.\n", type);
+		pr_err("invalid type %d\n", type);
 		return 0;
 	}
 	if (chipset_driver_info)

@@ -119,17 +119,17 @@ uisctrl_register_req_handler_ex(uuid_le switch_uuid,
 	LOGINF("type=%pUL, controlfunc=0x%p.\n",
 	       &switch_uuid, controlfunc);
 	if (!controlfunc) {
-		LOGERR("%pUL: controlfunc must be supplied\n", &switch_uuid);
+		pr_err("%pUL: controlfunc must be supplied\n", &switch_uuid);
 		goto Away;
 	}
 	if (!server_channel_ok) {
-		LOGERR("%pUL: Server_Channel_Ok must be supplied\n",
-				&switch_uuid);
+		pr_err("%pUL: Server_Channel_Ok must be supplied\n",
+		       &switch_uuid);
 		goto Away;
 	}
 	if (!server_channel_init) {
-		LOGERR("%pUL: Server_Channel_Init must be supplied\n",
-				&switch_uuid);
+		pr_err("%pUL: Server_Channel_Init must be supplied\n",
+		       &switch_uuid);
 		goto Away;
 	}
 	pReqHandlerInfo = req_handler_add(switch_uuid,

@@ -138,7 +138,7 @@ uisctrl_register_req_handler_ex(uuid_le switch_uuid,
 					min_channel_bytes,
 					server_channel_ok, server_channel_init);
 	if (!pReqHandlerInfo) {
-		LOGERR("failed to add %pUL to server list\n", &switch_uuid);
+		pr_err("failed to add %pUL to server list\n", &switch_uuid);
 		goto Away;
 	}
 

@@ -150,7 +150,7 @@ Away:
 			bus_device_info_init(chipset_driver_info, "chipset",
 					   "uislib", VERSION, NULL);
 	} else
-		LOGERR("failed to register type %pUL.\n", &switch_uuid);
+		pr_err("failed to register type %pUL\n", &switch_uuid);
 
 	return rc;
 }

@@ -163,15 +163,15 @@ uisctrl_unregister_req_handler_ex(uuid_le switch_uuid)
 
 	LOGINF("type=%pUL.\n", &switch_uuid);
 	if (req_handler_del(switch_uuid) < 0) {
-		LOGERR("failed to remove %pUL from server list\n",
-				&switch_uuid);
+		pr_err("failed to remove %pUL from server list\n",
+		       &switch_uuid);
 		goto Away;
 	}
 	atomic_dec(&uisutils_registered_services);
 	rc = 1;			/* success */
 Away:
 	if (!rc)
-		LOGERR("failed to unregister type %pUL.\n", &switch_uuid);
+		pr_err("failed to unregister type %pUL\n", &switch_uuid);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(uisctrl_unregister_req_handler_ex);

@@ -207,7 +207,7 @@ uisutil_copy_fragsinfo_from_skb(unsigned char *calling_ctx, void *skb_in,
 
 	while (firstfraglen) {
 		if (count == frags_max) {
-			LOGERR("%s frags array too small: max:%d count:%d\n",
+			pr_err("%s frags array too small: max:%d count:%d\n",
 			       calling_ctx, frags_max, count);
 			return -1;	/* failure */
 		}

@@ -230,8 +230,8 @@ uisutil_copy_fragsinfo_from_skb(unsigned char *calling_ctx, void *skb_in,
 		goto dolist;
 
 	if ((count + numfrags) > frags_max) {
-		LOGERR("**** FAILED %s frags array too small: max:%d count+nr_frags:%d\n",
-		     calling_ctx, frags_max, count + numfrags);
+		pr_err("**** FAILED %s frags array too small: max:%d count+nr_frags:%d\n",
+		       calling_ctx, frags_max, count + numfrags);
 		return -1;	/* failure */
 	}
 

@@ -244,7 +244,7 @@ uisutil_copy_fragsinfo_from_skb(unsigned char *calling_ctx, void *skb_in,
 					size, count, frags_max,
 					frags);
 		if (count == 0) {
-			LOGERR("**** FAILED to add physinfo entries\n");
+			pr_err("**** FAILED to add physinfo entries\n");
 			return -1;	/* failure */
 		}
 	}

@@ -262,7 +262,7 @@ dolist: if (skb_shinfo(skb)->frag_list) {
 				frags_max - count,
 				&frags[count]);
 			if (c == -1) {
-				LOGERR("**** FAILED recursive call failed\n");
+				pr_err("**** FAILED recursive call failed\n");
 				return -1;
 			}
 			count += c;
drivers/staging/unisys/virthba/virthba.c +55 -64

342 lines not on this page · raw message

drivers/staging/unisys/virtpci/virtpci.c +35 -36

274 lines not on this page · raw message

drivers/staging/unisys/visorchipset/file.c +1 -1
index 373fa36..d5dea8d 100644
@@ -217,7 +217,7 @@ visorchipset_ioctl(struct inode *inode, struct file *file,
 		rc = issue_vmcall_update_physical_time(adjustment);
 		break;
 	default:
-		LOGERR("visorchipset_ioctl received invalid command");
+		pr_err("visorchipset_ioctl received invalid command\n");
 		rc = -EFAULT;
 		break;
 	}
drivers/staging/unisys/visorchipset/visorchipset_main.c +79 -80

512 lines not on this page · raw message

maintainer
#1 lore raw
Quoted 28 lines · Joe Perches

Use the normal logging facility instead of a new macro.

Miscellanea:

o Realign arguments
o Remove unnecessary alloc error messages as alloc failures already emits a generic OOM message with a dump_stack
o Add missing newlines to these messages to avoid interleaving
o Spelling/typo fixes in those messages

Signed-off-by Joe Perches <[email protected]>
---
 drivers/staging/unisys/include/uisutils.h          |   4 +-
 drivers/staging/unisys/uislib/uislib.c             | 169 ++++++++++-----------
 drivers/staging/unisys/uislib/uisqueue.c           |   4 +-
 drivers/staging/unisys/uislib/uisthread.c          |   4 +-
 drivers/staging/unisys/uislib/uisutils.c           |  34 ++---
 drivers/staging/unisys/virthba/virthba.c           | 119 +++++++--------
 drivers/staging/unisys/virtpci/virtpci.c           |  71 +++++----
 drivers/staging/unisys/visorchipset/file.c         |   2 +-
 .../unisys/visorchipset/visorchipset_main.c        | 159 ++++++++++---------
 9 files changed, 276 insertions(+), 290 deletions(-)
drivers/staging/unisys/include/uisutils.h +0 -0
index 7414220..f937892 100644

drivers/staging/unisys/include/uisutils.h wait_for_valid_guid(uuid_le __iomem *guid) · line 209

Quoted 5 lines
drivers/staging/unisys/include/uisutils.h +1 -1
@@ -206,10 +206,10 @@ wait_for_valid_guid(uuid_le __iomem *guid)
 			      (void __iomem *)guid, sizeof(uuid_le));
 		if (uuid_le_cmp(tmpguid, NULL_UUID_LE) != 0)
 			break;
-		LOGERR("Waiting for non-0 GUID (why???)...\n");
+		pr_err("Waiting for non-0 GUID (why???)...\n");

I rejected the other patch that did much this same thing, almost all of these should be converted to use dev_* calls instead of pr_* calls, as this is a driver, and drivers should always have access to a struct device. If not, they need to be fixed.

thanks,

greg k-h

author
#2 lore raw
Quoted 10 lines · Greg Kroah-Hartman
Quoted 9 lines · Joe Perches

Use the normal logging facility instead of a new macro.

Miscellanea:

o Realign arguments
o Remove unnecessary alloc error messages as alloc failures already emits a generic OOM message with a dump_stack
o Add missing newlines to these messages to avoid interleaving
o Spelling/typo fixes in those messages

[]

Quoted 4 lines

I rejected the other patch that did much this same thing, almost all of these should be converted to use dev_* calls instead of pr_* calls, as this is a driver, and drivers should always have access to a struct device. If not, they need to be fixed.

Baby steps.

This one prevents interleaving
and fixes typos.

maintainer
#3 lore raw
Quoted 17 lines · Joe Perches
Quoted 10 lines · Greg Kroah-Hartman
Quoted 9 lines · Joe Perches

Use the normal logging facility instead of a new macro.

Miscellanea:

o Realign arguments
o Remove unnecessary alloc error messages as alloc failures already emits a generic OOM message with a dump_stack
o Add missing newlines to these messages to avoid interleaving
o Spelling/typo fixes in those messages

[]

Quoted 4 lines

I rejected the other patch that did much this same thing, almost all of these should be converted to use dev_* calls instead of pr_* calls, as this is a driver, and drivers should always have access to a struct device. If not, they need to be fixed.

Quoted 4 lines

Baby steps.

This one prevents interleaving
and fixes typos.

Yes, but converting it to dev_* at the same time saves a step, and gets it done properly. Leaving messages at pr_* levels takes away the checkpatch warning, which makes the code "seem" to be acceptable.

sorry,

greg k-h

author
#4 lore raw
Quoted 18 lines · Greg Kroah-Hartman
Quoted 17 lines · Joe Perches
Quoted 10 lines · Greg Kroah-Hartman
Quoted 9 lines · Joe Perches

Use the normal logging facility instead of a new macro.

Miscellanea:

o Realign arguments
o Remove unnecessary alloc error messages as alloc failures already emits a generic OOM message with a dump_stack
o Add missing newlines to these messages to avoid interleaving
o Spelling/typo fixes in those messages

[]

Quoted 4 lines

I rejected the other patch that did much this same thing, almost all of these should be converted to use dev_* calls instead of pr_* calls, as this is a driver, and drivers should always have access to a struct device. If not, they need to be fixed.

Quoted 8 lines
Quoted 4 lines

Baby steps.

This one prevents interleaving
and fixes typos.

Yes, but converting it to dev_* at the same time saves a step, and gets it done properly. Leaving messages at pr_* levels takes away the checkpatch warning, which makes the code "seem" to be acceptable.

I think you've not looked at the code very much.

Many of these things can not be converted to dev_<level> as there is not a single device there.

Many of these are error messages referring to busses that support many devices.

Converting them to dev_<level> piecemeal or wholesale isn't possible.

maintainer
#5 lore raw
Quoted 24 lines · Joe Perches
Quoted 23 lines · Greg Kroah-Hartman
Quoted 21 lines · Joe Perches
Quoted 10 lines · Greg Kroah-Hartman
Quoted 9 lines · Joe Perches

Use the normal logging facility instead of a new macro.

Miscellanea:

o Realign arguments
o Remove unnecessary alloc error messages as alloc failures already emits a generic OOM message with a dump_stack
o Add missing newlines to these messages to avoid interleaving
o Spelling/typo fixes in those messages

[]

Quoted 4 lines

I rejected the other patch that did much this same thing, almost all of these should be converted to use dev_* calls instead of pr_* calls, as this is a driver, and drivers should always have access to a struct device. If not, they need to be fixed.

Baby steps.

This one prevents interleaving
and fixes typos.

Quoted 8 lines
Quoted 3 lines

Yes, but converting it to dev_* at the same time saves a step, and gets it done properly. Leaving messages at pr_* levels takes away the checkpatch warning, which makes the code "seem" to be acceptable.

I think you've not looked at the code very much.

Many of these things can not be converted to dev_<level> as there is not a single device there.

Then that needs to be fixed.

Quoted 2 lines

Many of these are error messages referring to busses that support many devices.

The device that caused the error should emit the message, or the host controller for that bus. "general" messages shouldn't ever be happening as they are useless. If they are, something is wrong with the code.

Quoted 2 lines

Converting them to dev_<level> piecemeal or wholesale isn't possible.

I beg to differ, no driver should be using pr_* except in init/exit functions when there is no hardware yet found. Everywhere else a struct device is there to be used. If the code isn't passing it around, that is usually an indication that something is really wrong and should be fixed.

thanks,

greg k-h

maintainer
#6 lore raw
In reply to Joe Perches
Quoted 21 lines · Joe Perches
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Joe Perches
Quoted 10 lines · Greg Kroah-Hartman
Quoted 9 lines · Joe Perches

Use the normal logging facility instead of a new macro.

Miscellanea:

o Realign arguments
o Remove unnecessary alloc error messages as alloc failures already emits a generic OOM message with a dump_stack
o Add missing newlines to these messages to avoid interleaving
o Spelling/typo fixes in those messages

[]

Quoted 4 lines

I rejected the other patch that did much this same thing, almost all of these should be converted to use dev_* calls instead of pr_* calls, as this is a driver, and drivers should always have access to a struct device. If not, they need to be fixed.

Baby steps.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

This one prevents interleaving
and fixes typos.

Yes, but converting it to dev_* at the same time saves a step, and gets it done properly. Leaving messages at pr_* levels takes away the checkpatch warning, which makes the code "seem" to be acceptable.

I think you've not looked at the code very much.

Oh I have :(

Quoted 2 lines

Many of these things can not be converted to dev_<level> as there is not a single device there.

And that needs to be fixed.

Quoted 2 lines

Many of these are error messages referring to busses that support many devices.

Those busses have controllers that are devices.

Quoted 2 lines

Converting them to dev_<level> piecemeal or wholesale isn't possible.

It will need to be done to get the code merged out of staging, so it better happen...

thanks,

greg k-h

author
#7 lore raw
Quoted 2 lines · Greg Kroah-Hartman

It will need to be done to get the code merged out of staging, so it better happen...

Maybe the unisys folk will do something in the
order you want. Good luck.