| Message ID | 1327361932-28774-1-git-send-email-jitendra.kalsaria@qlogic.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi, On Mon, Jan 23, 2012 at 6:38 PM, Jitendra Kalsaria <jitendra.kalsaria@qlogic.com> wrote: > -static const u32 default_msg = > - NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK | > -/* NETIF_MSG_TIMER | */ > - NETIF_MSG_IFDOWN | > - NETIF_MSG_IFUP | > - NETIF_MSG_RX_ERR | > - NETIF_MSG_TX_ERR | > -/* NETIF_MSG_TX_QUEUED | */ > -/* NETIF_MSG_INTR | NETIF_MSG_TX_DONE | NETIF_MSG_RX_STATUS | */ > -/* NETIF_MSG_PKTDATA | */ > - NETIF_MSG_HW | NETIF_MSG_WOL | 0; > - > -static int debug = -1; /* defaults above */ > +#define DEFAULT_DEBUG_MSG_LEVEL 3 > +static int debug = DEFAULT_DEBUG_MSG_LEVEL; > module_param(debug, int, 0664); > MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); > > @@ -4614,7 +4603,7 @@ static int __devinit ql_init_device(struct pci_dev *pdev, > err = -EIO; > goto err_out2; > } > - qdev->msg_enable = netif_msg_init(debug, default_msg); > + qdev->msg_enable = netif_msg_init(debug, DEFAULT_DEBUG_MSG_LEVEL); Do you want to initialize qdev->msg_enable with 0x11 ? (I assume this based on above line where you initialized debug with 3 (decimal value)) If so, the above line will initialize with 0x111 As per netif_msg_init function (1 << 3 ) -1 Poiniting out because it can be a mistake in hurry. If not, and you think what you have done is what you wanted to do then sorry for the noise But would like to read more clear commit log message.
Viral, Thanks and yes I want to initialize with 0x111. Sorry about my typo error in commit log, I should have said debug level shift to 3. Jiten -----Original Message----- From: Viral Mehta [mailto:viral.vkm@gmail.com] Sent: Tuesday, January 24, 2012 8:52 AM To: Jitendra Kalsaria Cc: David Miller; netdev; Ron Mercer; Dept-NX Linux NIC Driver Subject: Re: [net PATCH 1/1] qlge:Changing netdev msg level to default debug Hi, On Mon, Jan 23, 2012 at 6:38 PM, Jitendra Kalsaria <jitendra.kalsaria@qlogic.com> wrote: > -static const u32 default_msg = > - NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK | > -/* NETIF_MSG_TIMER | */ > - NETIF_MSG_IFDOWN | > - NETIF_MSG_IFUP | > - NETIF_MSG_RX_ERR | > - NETIF_MSG_TX_ERR | > -/* NETIF_MSG_TX_QUEUED | */ > -/* NETIF_MSG_INTR | NETIF_MSG_TX_DONE | NETIF_MSG_RX_STATUS | */ > -/* NETIF_MSG_PKTDATA | */ > - NETIF_MSG_HW | NETIF_MSG_WOL | 0; > - > -static int debug = -1; /* defaults above */ > +#define DEFAULT_DEBUG_MSG_LEVEL 3 > +static int debug = DEFAULT_DEBUG_MSG_LEVEL; > module_param(debug, int, 0664); > MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); > > @@ -4614,7 +4603,7 @@ static int __devinit ql_init_device(struct pci_dev *pdev, > err = -EIO; > goto err_out2; > } > - qdev->msg_enable = netif_msg_init(debug, default_msg); > + qdev->msg_enable = netif_msg_init(debug, DEFAULT_DEBUG_MSG_LEVEL); Do you want to initialize qdev->msg_enable with 0x11 ? (I assume this based on above line where you initialized debug with 3 (decimal value)) If so, the above line will initialize with 0x111 As per netif_msg_init function (1 << 3 ) -1 Poiniting out because it can be a mistake in hurry. If not, and you think what you have done is what you wanted to do then sorry for the noise But would like to read more clear commit log message.
From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com> Date: Mon, 23 Jan 2012 15:38:52 -0800 > Driver was passing -1 so that custom default get used but which > makes it even more sane than 15 which was excessively verbose > printing an unusual amount of messages. > > Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com> Please don't move this code away from the canonical way in which default message levels are handled. The canonical way is to specify a module param that starts with the value "-1" which when passed as the first argument to netif_msg_init() means "use the default mask" which is the second argument. Moving to a default of "3" makes no sense at all, express the default in the second argument of netif_msg_init(), not the first. I'm not applying this patch, use other drivers such as tg3.c as a guide. -- 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
David, Ok, will do as suggested. Jiten -----Original Message----- From: David Miller [mailto:davem@davemloft.net] Sent: Tuesday, January 24, 2012 12:39 PM To: Jitendra Kalsaria Cc: netdev; Ron Mercer; Dept-NX Linux NIC Driver Subject: Re: [net PATCH 1/1] qlge:Changing netdev msg level to default debug From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com> Date: Mon, 23 Jan 2012 15:38:52 -0800 > Driver was passing -1 so that custom default get used but which > makes it even more sane than 15 which was excessively verbose > printing an unusual amount of messages. > > Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com> Please don't move this code away from the canonical way in which default message levels are handled. The canonical way is to specify a module param that starts with the value "-1" which when passed as the first argument to netif_msg_init() means "use the default mask" which is the second argument. Moving to a default of "3" makes no sense at all, express the default in the second argument of netif_msg_init(), not the first. I'm not applying this patch, use other drivers such as tg3.c as a guide. -- 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
> > Moving to a default of "3" makes no sense at all, express the default > in the second argument of netif_msg_init(), not the first. Yes, and i think that was the source of confusion for me. > > I'm not applying this patch, use other drivers such as tg3.c as a > guide.
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c index b548987..6ad006a 100644 --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c @@ -52,19 +52,8 @@ MODULE_DESCRIPTION(DRV_STRING " "); MODULE_LICENSE("GPL"); MODULE_VERSION(DRV_VERSION); -static const u32 default_msg = - NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK | -/* NETIF_MSG_TIMER | */ - NETIF_MSG_IFDOWN | - NETIF_MSG_IFUP | - NETIF_MSG_RX_ERR | - NETIF_MSG_TX_ERR | -/* NETIF_MSG_TX_QUEUED | */ -/* NETIF_MSG_INTR | NETIF_MSG_TX_DONE | NETIF_MSG_RX_STATUS | */ -/* NETIF_MSG_PKTDATA | */ - NETIF_MSG_HW | NETIF_MSG_WOL | 0; - -static int debug = -1; /* defaults above */ +#define DEFAULT_DEBUG_MSG_LEVEL 3 +static int debug = DEFAULT_DEBUG_MSG_LEVEL; module_param(debug, int, 0664); MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); @@ -4614,7 +4603,7 @@ static int __devinit ql_init_device(struct pci_dev *pdev, err = -EIO; goto err_out2; } - qdev->msg_enable = netif_msg_init(debug, default_msg); + qdev->msg_enable = netif_msg_init(debug, DEFAULT_DEBUG_MSG_LEVEL); spin_lock_init(&qdev->hw_lock); spin_lock_init(&qdev->stats_lock);
Driver was passing -1 so that custom default get used but which makes it even more sane than 15 which was excessively verbose printing an unusual amount of messages. Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com> --- drivers/net/ethernet/qlogic/qlge/qlge_main.c | 17 +++-------------- 1 files changed, 3 insertions(+), 14 deletions(-)