Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1417396

Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters

Show all headers | View raw

On Wed, Jun 08, 2016 at 02:13:47AM +0000, Binder, David Anthony wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@redhat.com]
> > Sent: Tuesday, June 07, 2016 9:23 AM
> > To: Kershner, David A <David.Kershner@unisys.com>
> > Cc: corbet@lwn.net; tglx@linutronix.de; mingo@redhat.com;
> > hpa@zytor.com; gregkh@linuxfoundation.org; Arfvidson, Erik
> > <Erik.Arfvidson@unisys.com>; Sell, Timothy C <Timothy.Sell@unisys.com>;
> > hofrat@osadl.org; dzickus@redhat.com; jes.sorensen@redhat.com; Curtin,
> > Alexander Paul <Alexander.Curtin@unisys.com>;
> > janani.rvchndrn@gmail.com; sudipm.mukherjee@gmail.com;
> > prarit@redhat.com; Binder, David Anthony <David.Binder@unisys.com>;
> > dan.j.williams@intel.com; linux-kernel@vger.kernel.org; linux-
> > doc@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> > Maintainer <SParMaintainer@unisys.com>
> > 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 <david.binder@unisys.com>
> > >
> > > Removes unused module parameters from visorbus_main.c, in response to
> > > findings by SonarQube.
> > >
> > > Signed-off-by: David Binder <david.binder@unisys.com>
> > > Signed-off-by: David Kershner <david.kershner@unisys.com>
> > > Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
> > > ---
> > >  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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 04/30] staging: unisys: visorbus: remove unused module parameters David Kershner <david.kershner@unisys.com> - 2016-06-04 19:30 +0200
  Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module  parameters Neil Horman <nhorman@redhat.com> - 2016-06-07 15:30 +0200
    RE: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module  parameters "Binder, David Anthony" <David.Binder@unisys.com> - 2016-06-08 04:30 +0200
      Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module  parameters Neil Horman <nhorman@redhat.com> - 2016-06-08 15:10 +0200
        RE: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module  parameters "Binder, David Anthony" <David.Binder@unisys.com> - 2016-06-08 16:50 +0200
          Re: [PATCH v3 04/30] staging: unisys: visorbus: remove unused module  parameters Neil Horman <nhorman@redhat.com> - 2016-06-08 17:10 +0200

csiph-web