Re: [PATCH] Driver for GE PIO2 VME Card

[Greg KH <gregkh@suse.de>]

On Mon, Nov 07, 2011 at 10:37:51AM +0000, Martyn Welch wrote:
> On 04/11/11 20:00, Greg KH wrote:
> > On Fri, Nov 04, 2011 at 08:55:14PM +0100, Paul Bolle wrote:
> >> A few random remarks follow. Ie, things that came up while quickly
> >> scanning this.
> >>
> >> On Fri, 2011-11-04 at 17:33 +0000, Martyn Welch wrote:
> >>> diff --git a/drivers/staging/vme/devices/Kconfig b/drivers/staging/vme/devices/Kconfig
> >>> index ca5ba89..99f5414 100644
> >>> --- a/drivers/staging/vme/devices/Kconfig
> >>> +++ b/drivers/staging/vme/devices/Kconfig
> >>> @@ -6,3 +6,13 @@ config VME_USER
> >>>  	  If you say Y here you want to be able to access a limited number of
> >>>  	  VME windows in a manner at least semi-compatible with the interface
> >>>  	  provided with the original driver at http://vmelinux.org/.
> >>
> >> Blank line here.
> >>
> >>> +config VME_PIO2
> >>> +	tristate "GE PIO2 VME"
> >>> +	help
> >>> +	  If you say Y here you have a GE PIO2. The PIO2 is a 6U VME Card,
> >>
> >> Maybe something like: "Say Y here if you have a GE PIO2."?
> >>
> >>> +	  implementing 32 solid-state relay switched IO lines, in 4 groups of 8.
> >>> +	  The IO lines are provided as input, output or both as a build time
> >>> +	  option.
> >>
> >> What option would that be?
> >>
> >>> +	  Otherwise it is safe to say N here.
> >>> +
> >>
> >> A lot of modules have the "if you say M here the module will be called"
> >> boilerplate here (which actually doesn't match the line I just made up).
> >>
> >>> +/* These are required for each board */
> >>> +MODULE_PARM_DESC(bus, "Enumeration of VMEbus to which the board is connected");
> >>> +module_param_array(bus, int, &bus_num, 0);
> >>> +
> >>> +MODULE_PARM_DESC(base, "Base VME address for PIO2 Registers");
> >>> +module_param_array(base, long, &base_num, 0);
> >>> +
> >>> +MODULE_PARM_DESC(vector, "VME IRQ Vector (Lower 4 bits masked)");
> >>> +module_param_array(vector, int, &vector_num, 0);
> >>> +
> >>> +MODULE_PARM_DESC(level, "VME IRQ Level");
> >>> +module_param_array(level, int, &level_num, 0);
> >>> +
> >>> +MODULE_PARM_DESC(variant, "Last 4 characters of PIO2 board variant");
> >>> +module_param_array(variant, charp, &variant_num, 0);
> >>> +
> >>> +/* This is for debugging */
> >>> +MODULE_PARM_DESC(loopback, "Enable loopback mode on all cards");
> >>> +module_param(loopback, bool, 0);
> >>
> >> All module parameters have a sysfs visibility (or permission) of zero.
> >> Why is that? (This might very well be a naive question. But I often
> >> wonder why a certain parameter's permission isn't at least 400, just to
> >> allow a quick check of that parameter.) Are arrays tricky in sysfs?
> > 
> > Yes, arrays are tricky, and in the end, all of these need to be "real"
> > sysfs files, not module parameters, but for now, it's ok to leave it as
> > is until the code is cleaned up.
> > 
> 
> Hi Greg,
> 
> Am I right in thinking we wouldn't then be able to pass the parameters in the
> boot args?

If they are sysfs files and not module paramaters, then, no, you wouldn't.

greg k-h