[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <s5hk45ym2xr.wl%tiwai@suse.de>
Date: Thu, 15 Dec 2011 13:31:28 +0100
From: Takashi Iwai <tiwai@...e.de>
To: Rusty Russell <rusty@...tcorp.com.au>
Cc: linux-kernel@...r.kernel.org, Pawel Moll <pawel.moll@....com>,
Steve Glendinning <steve.glendinning@...c.com>,
Jean Delvare <khali@...ux-fr.org>,
Guenter Roeck <guenter.roeck@...csson.com>,
Hoang-Nam Nguyen <hnguyen@...ibm.com>,
Christoph Raisch <raisch@...ibm.com>,
Roland Dreier <roland@...nel.org>,
Sean Hefty <sean.hefty@...el.com>,
Hal Rosenstock <hal.rosenstock@...il.com>, linux390@...ibm.com,
Anton Altaparmakov <anton@...era.com>,
Jaroslav Kysela <perex@...ex.cz>, lm-sensors@...sensors.org,
linux-rdma@...r.kernel.org, linux-s390@...r.kernel.org,
linux-ntfs-dev@...ts.sourceforge.net, alsa-devel@...a-project.org
Subject: Re: [PATCH 2/15] module_param: avoid bool abuse, add bint for special cases.
At Thu, 15 Dec 2011 13:32:26 +1030,
Rusty Russell wrote:
>
> For historical reasons, we allow module_param(bool) to take an int (or
> an unsigned int). That's going away.
>
> A few drivers really want an int: they set it to -1 and a parameter
> will set it to 0 or 1. This sucks: reading them from sysfs will give
> 'Y' for both -1 and 1, but if we change it to an int, then the users
> might be broken (if they did "param" instead of "param=1").
>
> Use a new 'bint' parser for them.
>
> (ntfs has a different problem: it needs an int for debug_msgs because
> it's also exposed via sysctl.)
>
> Cc: Steve Glendinning <steve.glendinning@...c.com>
> Cc: Jean Delvare <khali@...ux-fr.org>
> Cc: Guenter Roeck <guenter.roeck@...csson.com>
> Cc: Hoang-Nam Nguyen <hnguyen@...ibm.com>
> Cc: Christoph Raisch <raisch@...ibm.com>
> Cc: Roland Dreier <roland@...nel.org>
> Cc: Sean Hefty <sean.hefty@...el.com>
> Cc: Hal Rosenstock <hal.rosenstock@...il.com>
> Cc: linux390@...ibm.com
> Cc: Anton Altaparmakov <anton@...era.com>
> Cc: Jaroslav Kysela <perex@...ex.cz>
> Cc: Takashi Iwai <tiwai@...e.de>
For the sound part,
Acked-by: Takashi Iwai <tiwai@...e.de>
It's a nice feature. I know many drivers can be replaced with bint
gracefully now (e.g. i915 driver).
thanks,
Takashi
> Cc: lm-sensors@...sensors.org
> Cc: linux-rdma@...r.kernel.org
> Cc: linux-s390@...r.kernel.org
> Cc: linux-ntfs-dev@...ts.sourceforge.net
> Cc: alsa-devel@...a-project.org
> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
> ---
> drivers/hwmon/emc2103.c | 2 +-
> drivers/infiniband/hw/ehca/ehca_main.c | 2 +-
> drivers/s390/cio/cmf.c | 2 +-
> fs/ntfs/super.c | 2 +-
> include/linux/moduleparam.h | 6 ++++++
> kernel/params.c | 24 ++++++++++++++++++++++++
> sound/pci/intel8x0.c | 4 ++--
> 7 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/emc2103.c b/drivers/hwmon/emc2103.c
> --- a/drivers/hwmon/emc2103.c
> +++ b/drivers/hwmon/emc2103.c
> @@ -55,7 +55,7 @@ static const u8 REG_TEMP_MAX[4] = { 0x34
> * it. Default is to leave the device in the state it's already in (-1).
> * This parameter allows APD mode to be optionally forced on or off */
> static int apd = -1;
> -module_param(apd, bool, 0);
> +module_param(apd, bint, 0);
> MODULE_PARM_DESC(init, "Set to zero to disable anti-parallel diode mode");
>
> struct temperature {
> diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c
> --- a/drivers/infiniband/hw/ehca/ehca_main.c
> +++ b/drivers/infiniband/hw/ehca/ehca_main.c
> @@ -82,7 +82,7 @@ module_param_named(port_act_time, ehca_p
> module_param_named(poll_all_eqs, ehca_poll_all_eqs, bool, S_IRUGO);
> module_param_named(static_rate, ehca_static_rate, int, S_IRUGO);
> module_param_named(scaling_code, ehca_scaling_code, bool, S_IRUGO);
> -module_param_named(lock_hcalls, ehca_lock_hcalls, bool, S_IRUGO);
> +module_param_named(lock_hcalls, ehca_lock_hcalls, bint, S_IRUGO);
> module_param_named(number_of_cqs, ehca_max_cq, int, S_IRUGO);
> module_param_named(number_of_qps, ehca_max_qp, int, S_IRUGO);
>
> diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
> --- a/drivers/s390/cio/cmf.c
> +++ b/drivers/s390/cio/cmf.c
> @@ -98,7 +98,7 @@ enum cmb_format {
> * enum cmb_format.
> */
> static int format = CMF_AUTODETECT;
> -module_param(format, bool, 0444);
> +module_param(format, bint, 0444);
>
> /**
> * struct cmb_operations - functions to use depending on cmb_format
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -3198,7 +3198,7 @@ MODULE_DESCRIPTION("NTFS 1.2/3.x driver
> MODULE_VERSION(NTFS_VERSION);
> MODULE_LICENSE("GPL");
> #ifdef DEBUG
> -module_param(debug_msgs, bool, 0);
> +module_param(debug_msgs, bint, 0);
> MODULE_PARM_DESC(debug_msgs, "Enable debug messages.");
> #endif
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -367,6 +367,12 @@ extern int param_set_invbool(const char
> extern int param_get_invbool(char *buffer, const struct kernel_param *kp);
> #define param_check_invbool(name, p) __param_check(name, p, bool)
>
> +/* An int, which can only be set like a bool (though it shows as an int). */
> +extern struct kernel_param_ops param_ops_bint;
> +extern int param_set_bint(const char *val, const struct kernel_param *kp);
> +#define param_get_bint param_get_int
> +#define param_check_bint param_check_int
> +
> /**
> * module_param_array - a parameter which is an array of some type
> * @name: the name of the array variable
> diff --git a/kernel/params.c b/kernel/params.c
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -369,6 +369,30 @@ struct kernel_param_ops param_ops_invboo
> };
> EXPORT_SYMBOL(param_ops_invbool);
>
> +int param_set_bint(const char *val, const struct kernel_param *kp)
> +{
> + struct kernel_param boolkp;
> + bool v;
> + int ret;
> +
> + /* Match bool exactly, by re-using it. */
> + boolkp = *kp;
> + boolkp.arg = &v;
> + boolkp.flags |= KPARAM_ISBOOL;
> +
> + ret = param_set_bool(val, &boolkp);
> + if (ret == 0)
> + *(int *)kp->arg = v;
> + return ret;
> +}
> +EXPORT_SYMBOL(param_set_bint);
> +
> +struct kernel_param_ops param_ops_bint = {
> + .set = param_set_bint,
> + .get = param_get_int,
> +};
> +EXPORT_SYMBOL(param_ops_bint);
> +
> /* We break the rule and mangle the string. */
> static int param_array(const char *name,
> const char *val,
> diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
> --- a/sound/pci/intel8x0.c
> +++ b/sound/pci/intel8x0.c
> @@ -95,13 +95,13 @@ module_param(ac97_quirk, charp, 0444);
> MODULE_PARM_DESC(ac97_quirk, "AC'97 workaround for strange hardware.");
> module_param(buggy_semaphore, bool, 0444);
> MODULE_PARM_DESC(buggy_semaphore, "Enable workaround for hardwares with problematic codec semaphores.");
> -module_param(buggy_irq, bool, 0444);
> +module_param(buggy_irq, bint, 0444);
> MODULE_PARM_DESC(buggy_irq, "Enable workaround for buggy interrupts on some motherboards.");
> module_param(xbox, bool, 0444);
> MODULE_PARM_DESC(xbox, "Set to 1 for Xbox, if you have problems with the AC'97 codec detection.");
> module_param(spdif_aclink, int, 0444);
> MODULE_PARM_DESC(spdif_aclink, "S/PDIF over AC-link.");
> -module_param(inside_vm, bool, 0444);
> +module_param(inside_vm, bint, 0444);
> MODULE_PARM_DESC(inside_vm, "KVM/Parallels optimization.");
>
> /* just for backward compatibility */
>
--
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