lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 21 Dec 2014 16:55:51 +0000 From: Steven Honeyman <stevenhoneyman@...il.com> To: Pali Rohár <pali.rohar@...il.com> Cc: Guenter Roeck <linux@...ck-us.net>, Arnd Bergmann <arnd@...db.de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-kernel@...r.kernel.org, Valdis Kletnieks <Valdis.Kletnieks@...edu>, Jean Delvare <jdelvare@...e.de>, Gabriele Mazzotta <gabriele.mzt@...il.com>, Jochen Eisinger <jochen@...guin-breeder.org> Subject: Re: [PATCH v3] i8k: Autodetect maximal fan speed and fan RPM multiplier On 21 December 2014 at 16:37, Pali Rohár <pali.rohar@...il.com> wrote: > On Sunday 21 December 2014 13:23:32 Guenter Roeck wrote: >> On 12/21/2014 04:09 AM, Pali Rohár wrote: >> > On Sunday 21 December 2014 12:57:08 Guenter Roeck wrote: >> >>> -#define I8K_FAN_MULT 30 >> >>> +#define I8K_FAN_MAX_RPM 30000 >> >>> >> >>> #define I8K_MAX_TEMP 127 >> >>> >> >>> #define I8K_FN_NONE 0x00 >> >>> >> >>> @@ -64,7 +66,7 @@ static DEFINE_MUTEX(i8k_mutex); >> >>> >> >>> static char bios_version[4]; >> >>> static struct device *i8k_hwmon_dev; >> >>> static u32 i8k_hwmon_flags; >> >>> >> >>> -static int i8k_fan_mult; >> >>> +static int i8k_fan_mult = 30; >> >> >> >> Why did you drop I8K_FAN_MULT ? >> > >> > Because I think it is not needed anymore... It is used only >> > in one place (there ^). But if you want I can revert it >> > back. >> >> That is not a reason to drop a define. >> >> >>> static int __init i8k_probe(void) >> >>> { >> >>> >> >>> + const struct i8k_config_data *conf; >> >> >> >> Why did you move this variable declaration ? >> > >> > Comes from previous version of patches where I moved all >> > variables to start of function. I will revert this change. >> > >> >>> - const struct i8k_config_data *conf = id->driver_data; >> >>> + conf = id->driver_data; >> >>> + if (fan_mult <= 0 && conf->fan_mult > 0) >> >> >> >> I still don't see the value in accepting fan_mult < 0 >> >> (compeared to == 0). >> > >> > Ok. What kernel driver should do if user load it with >> > negative parameter? We should not propagate negative value >> > to functions. >> >> You have multiple options: Ignore it (bad idea ;-), abort >> loading the module with -EINVAL, or make the module parameter >> an unsigned. >> > > And how to make module parameter as unsigned? It is possible? > > Code > > module_param(fan_mult, unsigned int, 0); > > cause compile error: > > i8k.c:99:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘int’ > i8k.c:99:1: error: ‘param_ops_unsigned’ undeclared here (not in a function) module_param(fan_mult, uint, 0); Steven. -- 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