[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1320436514.14409.309.camel@x61.thuisdomein>
Date: Fri, 04 Nov 2011 20:55:14 +0100
From: Paul Bolle <pebolle@...cali.nl>
To: Martyn Welch <martyn.welch@...com>
Cc: gregkh@...e.de, Grant Likely <grant.likely@...retlab.ca>,
devel@...verdev.osuosl.org, manohar.vanga@...n.ch, cota@...ap.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Driver for GE PIO2 VME Card
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?
I can't remember ever having seen guidelines for this. It's possible
they exist and my question is answered after reading those.
Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists