Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters
On Wed, Jun 08, 2016 at 02:42:08PM +, Binder, David Anthony wrote: > > -Original Message- > > From: Neil Horman [mailto:nhor...@redhat.com] > > Sent: Wednesday, June 08, 2016 9:08 AM > > To: Binder, David Anthony > > Cc: Kershner, David A ; cor...@lwn.net; > > t...@linutronix.de; mi...@redhat.com; h...@zytor.com; > > gre...@linuxfoundation.org; Arfvidson, Erik ; > > Sell, Timothy C ; hof...@osadl.org; > > dzic...@redhat.com; jes.soren...@redhat.com; Curtin, Alexander Paul > > ; janani.rvchn...@gmail.com; > > sudipm.mukher...@gmail.com; pra...@redhat.com; > > dan.j.willi...@intel.com; linux-kernel@vger.kernel.org; linux- > > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > > Maintainer > > Subject: Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused > > module parameters > > > > On Wed, Jun 08, 2016 at 02:13:47AM +, Binder, David Anthony wrote: > > > > -Original Message- > > > > From: Neil Horman [mailto:nhor...@redhat.com] > > > > Sent: Tuesday, June 07, 2016 9:23 AM > > > > To: Kershner, David A > > > > Cc: cor...@lwn.net; t...@linutronix.de; mi...@redhat.com; > > > > h...@zytor.com; gre...@linuxfoundation.org; Arfvidson, Erik > > > > ; Sell, Timothy C > > ; > > > > hof...@osadl.org; dzic...@redhat.com; jes.soren...@redhat.com; > > Curtin, > > > > Alexander Paul ; > > > > janani.rvchn...@gmail.com; sudipm.mukher...@gmail.com; > > > > pra...@redhat.com; Binder, David Anthony ; > > > > dan.j.willi...@intel.com; linux-kernel@vger.kernel.org; linux- > > > > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > > > > Maintainer > > > > Subject: Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused > > > > module parameters > > > > > > > > On Sat, Jun 04, 2016 at 01:27:04PM -0400, David Kershner wrote: > > > > > From: David Binder > > > > > > > > > > Removes unused module parameters from visorbus_main.c, in > > response to > > > > > findings by SonarQube. > > > > > > > > > > Signed-off-by: David Binder > > > > > Signed-off-by: David Kershner > > > > > Reviewed-by: Tim Sell > > > > > --- > > > > > drivers/staging/unisys/visorbus/visorbus_main.c | 9 + > > > > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > > > > > > > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c > > > > b/drivers/staging/unisys/visorbus/visorbus_main.c > > > > > index 2ed9628..71bff07 100644 > > > > > --- a/drivers/staging/unisys/visorbus/visorbus_main.c > > > > > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c > > > > > @@ -27,10 +27,9 @@ > > > > > #define MYDRVNAME "visorbus" > > > > > > > > > > /* module parameters */ > > > > > -static int visorbus_debug; > > > > > static int visorbus_forcematch; > > > > > static int visorbus_forcenomatch; > > > > > -static int visorbus_debugref; > > > > > + > > > > > #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024) > > > > > > > > > > /* Display string that is guaranteed to be no longer the 99 > > > > > characters*/ > > > > > @@ -1332,9 +1331,6 @@ visorbus_exit(void) > > > > > remove_bus_type(); > > > > > } > > > > > > > > > > -module_param_named(debug, visorbus_debug, int, S_IRUGO); > > > > > -MODULE_PARM_DESC(visorbus_debug, "1 to debug"); > > > > > - > > > > > module_param_named(forcematch, visorbus_forcematch, int, > > S_IRUGO); > > > > > MODULE_PARM_DESC(visorbus_forcematch, > > > > >"1 to force a successful dev <--> drv match"); > > > > > @@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch, > > > > > module_param_named(forcenomatch, visorbus_forcenomatch, int, > > > > S_IRUGO); > > > > > MODULE_PARM_DESC(visorbus_forcenomatch, > > > > >"1 to force an UNsuccessful dev <--> drv match"); > > > > > - > > > > > -module_param_named(debugref, visorbus_debugref, int, S_IRUGO); > > > > > -MODULE_PARM_DESC(visorbus_debug
RE: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters
> -Original Message- > From: Neil Horman [mailto:nhor...@redhat.com] > Sent: Wednesday, June 08, 2016 9:08 AM > To: Binder, David Anthony > Cc: Kershner, David A ; cor...@lwn.net; > t...@linutronix.de; mi...@redhat.com; h...@zytor.com; > gre...@linuxfoundation.org; Arfvidson, Erik ; > Sell, Timothy C ; hof...@osadl.org; > dzic...@redhat.com; jes.soren...@redhat.com; Curtin, Alexander Paul > ; janani.rvchn...@gmail.com; > sudipm.mukher...@gmail.com; pra...@redhat.com; > dan.j.willi...@intel.com; linux-kernel@vger.kernel.org; linux- > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > Maintainer > Subject: Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused > module parameters > > On Wed, Jun 08, 2016 at 02:13:47AM +, Binder, David Anthony wrote: > > > -Original Message- > > > From: Neil Horman [mailto:nhor...@redhat.com] > > > Sent: Tuesday, June 07, 2016 9:23 AM > > > To: Kershner, David A > > > Cc: cor...@lwn.net; t...@linutronix.de; mi...@redhat.com; > > > h...@zytor.com; gre...@linuxfoundation.org; Arfvidson, Erik > > > ; Sell, Timothy C > ; > > > hof...@osadl.org; dzic...@redhat.com; jes.soren...@redhat.com; > Curtin, > > > Alexander Paul ; > > > janani.rvchn...@gmail.com; sudipm.mukher...@gmail.com; > > > pra...@redhat.com; Binder, David Anthony ; > > > dan.j.willi...@intel.com; linux-kernel@vger.kernel.org; linux- > > > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > > > Maintainer > > > Subject: Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused > > > module parameters > > > > > > On Sat, Jun 04, 2016 at 01:27:04PM -0400, David Kershner wrote: > > > > From: David Binder > > > > > > > > Removes unused module parameters from visorbus_main.c, in > response to > > > > findings by SonarQube. > > > > > > > > Signed-off-by: David Binder > > > > Signed-off-by: David Kershner > > > > Reviewed-by: Tim Sell > > > > --- > > > > drivers/staging/unisys/visorbus/visorbus_main.c | 9 + > > > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c > > > b/drivers/staging/unisys/visorbus/visorbus_main.c > > > > index 2ed9628..71bff07 100644 > > > > --- a/drivers/staging/unisys/visorbus/visorbus_main.c > > > > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c > > > > @@ -27,10 +27,9 @@ > > > > #define MYDRVNAME "visorbus" > > > > > > > > /* module parameters */ > > > > -static int visorbus_debug; > > > > static int visorbus_forcematch; > > > > static int visorbus_forcenomatch; > > > > -static int visorbus_debugref; > > > > + > > > > #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024) > > > > > > > > /* Display string that is guaranteed to be no longer the 99 > > > > characters*/ > > > > @@ -1332,9 +1331,6 @@ visorbus_exit(void) > > > > remove_bus_type(); > > > > } > > > > > > > > -module_param_named(debug, visorbus_debug, int, S_IRUGO); > > > > -MODULE_PARM_DESC(visorbus_debug, "1 to debug"); > > > > - > > > > module_param_named(forcematch, visorbus_forcematch, int, > S_IRUGO); > > > > MODULE_PARM_DESC(visorbus_forcematch, > > > > "1 to force a successful dev <--> drv match"); > > > > @@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch, > > > > module_param_named(forcenomatch, visorbus_forcenomatch, int, > > > S_IRUGO); > > > > MODULE_PARM_DESC(visorbus_forcenomatch, > > > > "1 to force an UNsuccessful dev <--> drv match"); > > > > - > > > > -module_param_named(debugref, visorbus_debugref, int, S_IRUGO); > > > > -MODULE_PARM_DESC(visorbus_debugref, "1 to debug reference > > > counting"); > > > > > > visorbus_forcematch and visorbus_forcenomatch appear to be > referenced in > > > visorbus_match (at least in the HEAD of linus' tree). If you're going to > > > remove > > > the module parameters, why not also remove those references and the > > > force[no]match variables themselves? > > > > > > Neil > > > > We presently use visorbus_forcematch and visorbus_forcenomatch for > > debugging purposes, and therefore decided to keep the variables in > > the driver. > > > Ok, thats fine, but this seems like a half measure then. This patch removes > the > module option for those features, which means that for you to use them, > you have > to rebuild the module. If you have intentions to use it, why not just leave > the > module option in place? If you don't, remove it all the way. > > Neil > > > David Binder Unless I'm missing something, the patch removes the module parameters for visorbus_debug and visorbus_debugref, but it leaves the parameters for visorbus_forcematch and visorbus_forcenomatch. David Binder
Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters
On Wed, Jun 08, 2016 at 02:13:47AM +, Binder, David Anthony wrote: > > -Original Message- > > From: Neil Horman [mailto:nhor...@redhat.com] > > Sent: Tuesday, June 07, 2016 9:23 AM > > To: Kershner, David A > > Cc: cor...@lwn.net; t...@linutronix.de; mi...@redhat.com; > > h...@zytor.com; gre...@linuxfoundation.org; Arfvidson, Erik > > ; Sell, Timothy C ; > > hof...@osadl.org; dzic...@redhat.com; jes.soren...@redhat.com; Curtin, > > Alexander Paul ; > > janani.rvchn...@gmail.com; sudipm.mukher...@gmail.com; > > pra...@redhat.com; Binder, David Anthony ; > > dan.j.willi...@intel.com; linux-kernel@vger.kernel.org; linux- > > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > > Maintainer > > Subject: Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused > > module parameters > > > > On Sat, Jun 04, 2016 at 01:27:04PM -0400, David Kershner wrote: > > > From: David Binder > > > > > > Removes unused module parameters from visorbus_main.c, in response to > > > findings by SonarQube. > > > > > > Signed-off-by: David Binder > > > Signed-off-by: David Kershner > > > Reviewed-by: Tim Sell > > > --- > > > drivers/staging/unisys/visorbus/visorbus_main.c | 9 + > > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > > > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c > > b/drivers/staging/unisys/visorbus/visorbus_main.c > > > index 2ed9628..71bff07 100644 > > > --- a/drivers/staging/unisys/visorbus/visorbus_main.c > > > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c > > > @@ -27,10 +27,9 @@ > > > #define MYDRVNAME "visorbus" > > > > > > /* module parameters */ > > > -static int visorbus_debug; > > > static int visorbus_forcematch; > > > static int visorbus_forcenomatch; > > > -static int visorbus_debugref; > > > + > > > #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024) > > > > > > /* Display string that is guaranteed to be no longer the 99 characters*/ > > > @@ -1332,9 +1331,6 @@ visorbus_exit(void) > > > remove_bus_type(); > > > } > > > > > > -module_param_named(debug, visorbus_debug, int, S_IRUGO); > > > -MODULE_PARM_DESC(visorbus_debug, "1 to debug"); > > > - > > > module_param_named(forcematch, visorbus_forcematch, int, S_IRUGO); > > > MODULE_PARM_DESC(visorbus_forcematch, > > >"1 to force a successful dev <--> drv match"); > > > @@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch, > > > module_param_named(forcenomatch, visorbus_forcenomatch, int, > > S_IRUGO); > > > MODULE_PARM_DESC(visorbus_forcenomatch, > > >"1 to force an UNsuccessful dev <--> drv match"); > > > - > > > -module_param_named(debugref, visorbus_debugref, int, S_IRUGO); > > > -MODULE_PARM_DESC(visorbus_debugref, "1 to debug reference > > counting"); > > > > visorbus_forcematch and visorbus_forcenomatch appear to be referenced in > > visorbus_match (at least in the HEAD of linus' tree). If you're going to > > remove > > the module parameters, why not also remove those references and the > > force[no]match variables themselves? > > > > Neil > > We presently use visorbus_forcematch and visorbus_forcenomatch for > debugging purposes, and therefore decided to keep the variables in > the driver. > Ok, thats fine, but this seems like a half measure then. This patch removes the module option for those features, which means that for you to use them, you have to rebuild the module. If you have intentions to use it, why not just leave the module option in place? If you don't, remove it all the way. Neil > David Binder
RE: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters
> -Original Message- > From: Neil Horman [mailto:nhor...@redhat.com] > Sent: Tuesday, June 07, 2016 9:23 AM > To: Kershner, David A > Cc: cor...@lwn.net; t...@linutronix.de; mi...@redhat.com; > h...@zytor.com; gre...@linuxfoundation.org; Arfvidson, Erik > ; Sell, Timothy C ; > hof...@osadl.org; dzic...@redhat.com; jes.soren...@redhat.com; Curtin, > Alexander Paul ; > janani.rvchn...@gmail.com; sudipm.mukher...@gmail.com; > pra...@redhat.com; Binder, David Anthony ; > dan.j.willi...@intel.com; linux-kernel@vger.kernel.org; linux- > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > Maintainer > Subject: Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused > module parameters > > On Sat, Jun 04, 2016 at 01:27:04PM -0400, David Kershner wrote: > > From: David Binder > > > > Removes unused module parameters from visorbus_main.c, in response to > > findings by SonarQube. > > > > Signed-off-by: David Binder > > Signed-off-by: David Kershner > > Reviewed-by: Tim Sell > > --- > > drivers/staging/unisys/visorbus/visorbus_main.c | 9 + > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c > b/drivers/staging/unisys/visorbus/visorbus_main.c > > index 2ed9628..71bff07 100644 > > --- a/drivers/staging/unisys/visorbus/visorbus_main.c > > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c > > @@ -27,10 +27,9 @@ > > #define MYDRVNAME "visorbus" > > > > /* module parameters */ > > -static int visorbus_debug; > > static int visorbus_forcematch; > > static int visorbus_forcenomatch; > > -static int visorbus_debugref; > > + > > #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024) > > > > /* Display string that is guaranteed to be no longer the 99 characters*/ > > @@ -1332,9 +1331,6 @@ visorbus_exit(void) > > remove_bus_type(); > > } > > > > -module_param_named(debug, visorbus_debug, int, S_IRUGO); > > -MODULE_PARM_DESC(visorbus_debug, "1 to debug"); > > - > > module_param_named(forcematch, visorbus_forcematch, int, S_IRUGO); > > MODULE_PARM_DESC(visorbus_forcematch, > > "1 to force a successful dev <--> drv match"); > > @@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch, > > module_param_named(forcenomatch, visorbus_forcenomatch, int, > S_IRUGO); > > MODULE_PARM_DESC(visorbus_forcenomatch, > > "1 to force an UNsuccessful dev <--> drv match"); > > - > > -module_param_named(debugref, visorbus_debugref, int, S_IRUGO); > > -MODULE_PARM_DESC(visorbus_debugref, "1 to debug reference > counting"); > > visorbus_forcematch and visorbus_forcenomatch appear to be referenced in > visorbus_match (at least in the HEAD of linus' tree). If you're going to > remove > the module parameters, why not also remove those references and the > force[no]match variables themselves? > > Neil We presently use visorbus_forcematch and visorbus_forcenomatch for debugging purposes, and therefore decided to keep the variables in the driver. David Binder
Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters
On Sat, Jun 04, 2016 at 01:27:04PM -0400, David Kershner wrote: > From: David Binder > > Removes unused module parameters from visorbus_main.c, in response to > findings by SonarQube. > > Signed-off-by: David Binder > Signed-off-by: David Kershner > Reviewed-by: Tim Sell > --- > drivers/staging/unisys/visorbus/visorbus_main.c | 9 + > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c > b/drivers/staging/unisys/visorbus/visorbus_main.c > index 2ed9628..71bff07 100644 > --- a/drivers/staging/unisys/visorbus/visorbus_main.c > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c > @@ -27,10 +27,9 @@ > #define MYDRVNAME "visorbus" > > /* module parameters */ > -static int visorbus_debug; > static int visorbus_forcematch; > static int visorbus_forcenomatch; > -static int visorbus_debugref; > + > #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024) > > /* Display string that is guaranteed to be no longer the 99 characters*/ > @@ -1332,9 +1331,6 @@ visorbus_exit(void) > remove_bus_type(); > } > > -module_param_named(debug, visorbus_debug, int, S_IRUGO); > -MODULE_PARM_DESC(visorbus_debug, "1 to debug"); > - > module_param_named(forcematch, visorbus_forcematch, int, S_IRUGO); > MODULE_PARM_DESC(visorbus_forcematch, >"1 to force a successful dev <--> drv match"); > @@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch, > module_param_named(forcenomatch, visorbus_forcenomatch, int, S_IRUGO); > MODULE_PARM_DESC(visorbus_forcenomatch, >"1 to force an UNsuccessful dev <--> drv match"); > - > -module_param_named(debugref, visorbus_debugref, int, S_IRUGO); > -MODULE_PARM_DESC(visorbus_debugref, "1 to debug reference counting"); visorbus_forcematch and visorbus_forcenomatch appear to be referenced in visorbus_match (at least in the HEAD of linus' tree). If you're going to remove the module parameters, why not also remove those references and the force[no]match variables themselves? Neil
