[PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[Sonny Rao]

Driver appears to be mistaking the permission field with default value
in the case of debug and qlge_irq_type.

Driver is also passing debug as a bitmask into netif_msg_init()
which wants a number of bits.  Ron Mercer suggests we should
change this to pass in -1 so the defaults get used instead,
which makes the default much less verbose.

Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Sonny Rao <sonnyrao@linux.vnet.ibm.com>
---

v2: Changed debug to be -1 and updated changelog

 drivers/net/qlge/qlge_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index d9a7626..66878bb 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -62,15 +62,15 @@ static const u32 default_msg =
 /* NETIF_MSG_PKTDATA | */
     NETIF_MSG_HW | NETIF_MSG_WOL | 0;
 
-static int debug = 0x00007fff;	/* defaults above */
-module_param(debug, int, 0);
+static int debug = -1;	/* defaults above */
+module_param(debug, int, 0664);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
 #define MSIX_IRQ 0
 #define MSI_IRQ 1
 #define LEG_IRQ 2
 static int qlge_irq_type = MSIX_IRQ;
-module_param(qlge_irq_type, int, MSIX_IRQ);
+module_param(qlge_irq_type, int, 0664);
 MODULE_PARM_DESC(qlge_irq_type, "0 = MSI-X, 1 = MSI, 2 = Legacy.");
 
 static int qlge_mpi_coredump;

Re: [PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[Ron Mercer]

On Thu, Nov 18, 2010 at 01:50:02PM -0800, Sonny Rao wrote:
> Driver appears to be mistaking the permission field with default value
> in the case of debug and qlge_irq_type.
> 
> Driver is also passing debug as a bitmask into netif_msg_init()
> which wants a number of bits.  Ron Mercer suggests we should
> change this to pass in -1 so the defaults get used instead,
> which makes the default much less verbose.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> Signed-off-by: Sonny Rao <sonnyrao@linux.vnet.ibm.com>

Milton/Sonny,
Did you happen to test this to see if the log output was the same? 

Re: [PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[Sonny Rao]

On Thu, Nov 18, 2010 at 02:17:14PM -0800, Ron Mercer wrote:
> On Thu, Nov 18, 2010 at 01:50:02PM -0800, Sonny Rao wrote:
> > Driver appears to be mistaking the permission field with default value
> > in the case of debug and qlge_irq_type.
> > 
> > Driver is also passing debug as a bitmask into netif_msg_init()
> > which wants a number of bits.  Ron Mercer suggests we should
> > change this to pass in -1 so the defaults get used instead,
> > which makes the default much less verbose.
> > 
> > Signed-off-by: Milton Miller <miltonm@bga.com>
> > Signed-off-by: Sonny Rao <sonnyrao@linux.vnet.ibm.com>
> 
> Milton/Sonny,
> Did you happen to test this to see if the log output was the same? 

Well, I haven't tried the -1 but I suspect it is much more sane than
the 15 I had before because that was excessively verbose and I always
ended up turning it down.  I'll try it out and see.

-- 
Sonny Rao, LTC OzLabs, BML team

Re: [PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[Sonny Rao]

On Thu, Nov 18, 2010 at 02:17:14PM -0800, Ron Mercer wrote:
> On Thu, Nov 18, 2010 at 01:50:02PM -0800, Sonny Rao wrote:
> > Driver appears to be mistaking the permission field with default value
> > in the case of debug and qlge_irq_type.
> > 
> > Driver is also passing debug as a bitmask into netif_msg_init()
> > which wants a number of bits.  Ron Mercer suggests we should
> > change this to pass in -1 so the defaults get used instead,
> > which makes the default much less verbose.
> > 
> > Signed-off-by: Milton Miller <miltonm@bga.com>
> > Signed-off-by: Sonny Rao <sonnyrao@linux.vnet.ibm.com>
> 
> Milton/Sonny,
> Did you happen to test this to see if the log output was the same? 

Well, I haven't tried the -1 but I suspect it is much more sane than
the 15 I had before because that was excessively verbose and I always
ended up turning it down.  I'll try it out and see.

-- 
Sonny Rao, LTC OzLabs, BML team

Re: [PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[David Miller]

From: Sonny Rao <sonnyrao@linux.vnet.ibm.com>
Date: Thu, 18 Nov 2010 04:25:03 -0600

> Driver appears to be mistaking the permission field with default value
> in the case of debug and qlge_irq_type.
> 
> Driver is also passing debug as a bitmask into netif_msg_init()
> which really wants a number of bits, so fix that.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> Signed-off-by: Sonny Rao <sonnyrao@linux.vnet.ibm.com>

Applied, thanks Sonny.

Re: [PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[David Miller]

Sonny, something is serious wrong with your email setup.

The number of Received headers in your outgoing emails is
enormous and as a result a large percentage of the lists
subscribers bounced your posting because it looks like it
is looping.

Re: [PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[Ron Mercer]

On Thu, Nov 18, 2010 at 02:25:03AM -0800, Sonny Rao wrote:
> Driver appears to be mistaking the permission field with default value
> in the case of debug and qlge_irq_type.
> 
> Driver is also passing debug as a bitmask into netif_msg_init()
> which really wants a number of bits, so fix that.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> Signed-off-by: Sonny Rao <sonnyrao@linux.vnet.ibm.com>
> ---
>  drivers/net/qlge/qlge_main.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
> index d9a7626..66878bb 100644
> --- a/drivers/net/qlge/qlge_main.c
> +++ b/drivers/net/qlge/qlge_main.c
> @@ -62,15 +62,15 @@ static const u32 default_msg =
>  /* NETIF_MSG_PKTDATA | */
>      NETIF_MSG_HW | NETIF_MSG_WOL | 0;
>  
> -static int debug = 0x00007fff;	/* defaults above */
> -module_param(debug, int, 0);
> +static int debug = 15;	/* defaults above */


I think what we want here is:

> +static int debug = -1;	/* defaults above */

This will correct it and maintain the current behavior.
By passing in the 0x00007ffff we were getting the passed in default
paramter from the 2nd compare in the first if statement below. If we set
debug to 15 then it looks like we would turn on all bits rather than
qlge default.

static inline u32 netif_msg_init(int debug_value, int default_msg_enable_bits)
{
        /* use default */
        if (debug_value < 0 || debug_value >= (sizeof(u32) * 8))
                return default_msg_enable_bits;
        if (debug_value == 0)   /* no output */
                return 0;
        /* set low N bits */
        return (1 << debug_value) - 1;
}

[PATCH] qlge: Fix incorrect usage of module parameters and netdev msg level

[Sonny Rao]

Driver appears to be mistaking the permission field with default value
in the case of debug and qlge_irq_type.

Driver is also passing debug as a bitmask into netif_msg_init()
which really wants a number of bits, so fix that.

Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Sonny Rao <sonnyrao@linux.vnet.ibm.com>
---
 drivers/net/qlge/qlge_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index d9a7626..66878bb 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -62,15 +62,15 @@ static const u32 default_msg =
 /* NETIF_MSG_PKTDATA | */
     NETIF_MSG_HW | NETIF_MSG_WOL | 0;

-static int debug = 0x00007fff;	/* defaults above */
-module_param(debug, int, 0);
+static int debug = 15;	/* defaults above */
+module_param(debug, int, 0664);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");

 #define MSIX_IRQ 0
 #define MSI_IRQ 1
 #define LEG_IRQ 2
 static int qlge_irq_type = MSIX_IRQ;
-module_param(qlge_irq_type, int, MSIX_IRQ);
+module_param(qlge_irq_type, int, 0664);
 MODULE_PARM_DESC(qlge_irq_type, "0 = MSI-X, 1 = MSI, 2 = Legacy.");

 static int qlge_mpi_coredump;
-- 
1.5.6.5