Message ID | 1357899831-4168-10-git-send-email-jeffrey.t.kirsher@intel.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, 2013-01-11 at 02:23 -0800, Jeff Kirsher wrote: > WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... > then pr_info(... to printk(KERN_INFO ... [] > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c [] > @@ -4300,9 +4300,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, Don't these changes break a user-space tool? If not, please remove the comment above the pr_info()s and remove the unnecessary e1000e: prefixes. > @@ -4521,8 +4520,8 @@ static void e1000_watchdog_task(struct work_struct *work) [] > /* Link status message must follow this format */ > - printk(KERN_INFO "e1000e: %s NIC Link is Down\n", > - adapter->netdev->name); > + pr_info("e1000e: %s NIC Link is Down\n", > + adapter->netdev->name); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Joe Perches [mailto:joe@perches.com] > Sent: Friday, January 11, 2013 9:04 AM > To: Kirsher, Jeffrey T > Cc: davem@davemloft.net; Allan, Bruce W; netdev@vger.kernel.org; > gospo@redhat.com; sassmann@redhat.com > Subject: Re: [net-next 09/15] e1000e: resolve checkpatch PREFER_PR_LEVEL > warning > > On Fri, 2013-01-11 at 02:23 -0800, Jeff Kirsher wrote: > > WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... > > then pr_info(... to printk(KERN_INFO ... > [] > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c > b/drivers/net/ethernet/intel/e1000e/netdev.c > [] > > @@ -4300,9 +4300,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, > > Don't these changes break a user-space tool? > > If not, please remove the comment above the pr_info()s > and remove the unnecessary e1000e: prefixes. > > > @@ -4521,8 +4520,8 @@ static void e1000_watchdog_task(struct > work_struct *work) > [] > > /* Link status message must follow this format */ > > - printk(KERN_INFO "e1000e: %s NIC Link is Down\n", > > - adapter->netdev->name); > > + pr_info("e1000e: %s NIC Link is Down\n", > > + adapter->netdev->name); > Hmm, I'm not sure how that got past our testing even after I asked to check for redundant instances of "e1000e". This patch can be dropped and I will submit an updated one but it will still have the "must follow this format for user tools" comment since the user tools key off of "e1000e: <iface> NIC Link is ..." text. Thanks, Bruce. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 7ad8ef6..e922bba 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -4300,9 +4300,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" : @@ -4521,8 +4520,8 @@ static void e1000_watchdog_task(struct work_struct *work) adapter->link_speed = 0; adapter->link_duplex = 0; /* Link status message must follow this format */ - printk(KERN_INFO "e1000e: %s NIC Link is Down\n", - adapter->netdev->name); + pr_info("e1000e: %s NIC Link is Down\n", + adapter->netdev->name); netif_carrier_off(netdev); if (!test_bit(__E1000_DOWN, &adapter->state)) mod_timer(&adapter->phy_info_timer,