Re: [net-next 04/13] e1000e: cleanup checkpatch PREFER_PR_LEVEL warning

On Fri, 2012-08-24 at 11:02 -0700, Joe Perches wrote:

On Thu, 2012-08-23 at 07:01 -0700, Joe Perches wrote:

quoted

On Thu, 2012-08-23 at 02:56 -0700, Jeff Kirsher wrote:

quoted

From: Bruce Allan <redacted>

checkpatch warning: Prefer pr_info(... to printk(KERN_INFO, ...

[]

quoted

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c

[]

quoted

@@ -4330,9 +4330,8 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
 	u32 ctrl = er32(CTRL);
 
 	/* Link status message must follow this format for user tools */
-	printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
-		adapter->netdev->name,
-		adapter->link_speed,
+	pr_info("e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
+		adapter->netdev->name, adapter->link_speed,
 		adapter->link_duplex == FULL_DUPLEX ? "Full" : "Half",
 		(ctrl & E1000_CTRL_TFCE) && (ctrl & E1000_CTRL_RFCE) ? "Rx/Tx" :
 		(ctrl & E1000_CTRL_RFCE) ? "Rx" :

I think these conversions are not a good idea.

When you have a specific message format that must be
followed, use printk.

pr_<level> may at some point in the near future use
#define pr_fmt(fmt) KBUiLD_MODNAME ": " fmt
as a global default equivalent.

Hey Jeff.

The comment above this change (and the other) reads

  	/* Link status message must follow this format for user tools */

This file already uses #define pr_fmt(fmt) KBUILD_MODNAME...
With this patch, the output form changes to use 2 prefixes.

Is that really desired?  Probably not.

If the comments are old and don't apply any more, they
should be removed.

Bruce really should answer this since this is his patch and there was a
reason why he made the change.  My guess was the current output was
providing incorrect or mis-leading information.