| Message ID | 1463842366-17964-1-git-send-email-pali.rohar@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 05/21/2016 07:52 AM, Pali Rohár wrote: > Some Dell machines (e.g. Dell Precision M3800) have two fans, first with > index=0 and second with index=2. So export also attributes for third fan > device with index=2. > > Reported-by: Tolga Cakir <cevelnet@gmail.com> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com> > --- > drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > --- > > Hi Tolga! Can you test this patch if sensors see fan device correctly? > I assume this means I should wait for test results before applying the patch ? Thanks, Guenter > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c > index 577445f..7a2764a 100644 > --- a/drivers/hwmon/dell-smm-hwmon.c > +++ b/drivers/hwmon/dell-smm-hwmon.c > @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; > #define I8K_HWMON_HAVE_TEMP4 (1 << 3) > #define I8K_HWMON_HAVE_FAN1 (1 << 4) > #define I8K_HWMON_HAVE_FAN2 (1 << 5) > +#define I8K_HWMON_HAVE_FAN3 (1 << 6) > > MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); > MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); > @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) > static int i8k_get_fan_type(int fan) > { > /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */ > - static int types[2] = { INT_MIN, INT_MIN }; > + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; > > if (types[fan] == INT_MIN) > types[fan] = _i8k_get_fan_type(fan); > @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, > 1); > static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, > i8k_hwmon_set_pwm, 1); > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL, > + 2); > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, > + 2); > +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, > + i8k_hwmon_set_pwm, 2); > > static struct attribute *i8k_attrs[] = { > &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ > @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { > &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ > &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ > &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ > + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ > + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ > + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ > NULL > }; > > @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr, > if (index >= 11 && index <= 13 && > !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) > return 0; > + if (index >= 14 && index <= 16 && > + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) > + return 0; > > return attr->mode; > } > @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) > if (err >= 0) > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; > > + /* Third fan attributes, if fan status is OK */ > + err = i8k_get_fan_status(2); > + if (err >= 0) > + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; > + > i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm", > NULL, i8k_groups); > if (IS_ERR(i8k_hwmon_dev)) { > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 22 May 2016 02:21:46 Guenter Roeck wrote: > On 05/21/2016 07:52 AM, Pali Rohár wrote: > > Some Dell machines (e.g. Dell Precision M3800) have two fans, first > > with index=0 and second with index=2. So export also attributes > > for third fan device with index=2. > > > > Reported-by: Tolga Cakir <cevelnet@gmail.com> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com> > > --- > > > > drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > --- > > > > Hi Tolga! Can you test this patch if sensors see fan device > > correctly? > > I assume this means I should wait for test results before applying > the patch ? Yes, testing should be done. Do you have some pending/testing/next tree/branch for such patches? > Thanks, > Guenter > > > diff --git a/drivers/hwmon/dell-smm-hwmon.c > > b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644 > > --- a/drivers/hwmon/dell-smm-hwmon.c > > +++ b/drivers/hwmon/dell-smm-hwmon.c > > @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; > > > > #define I8K_HWMON_HAVE_TEMP4 (1 << 3) > > #define I8K_HWMON_HAVE_FAN1 (1 << 4) > > #define I8K_HWMON_HAVE_FAN2 (1 << 5) > > > > +#define I8K_HWMON_HAVE_FAN3 (1 << 6) > > > > MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); > > MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); > > > > @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) > > > > static int i8k_get_fan_type(int fan) > > { > > > > /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values > > */ > > > > - static int types[2] = { INT_MIN, INT_MIN }; > > + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; > > > > if (types[fan] == INT_MIN) > > > > types[fan] = _i8k_get_fan_type(fan); > > > > @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, > > i8k_hwmon_show_fan_label, NULL, > > > > 1); > > > > static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, > > i8k_hwmon_show_pwm, > > > > i8k_hwmon_set_pwm, 1); > > > > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, > > NULL, + 2); > > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, > > i8k_hwmon_show_fan_label, NULL, + 2); > > +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, > > i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, 2); > > > > static struct attribute *i8k_attrs[] = { > > > > &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ > > > > @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { > > > > &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ > > &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ > > &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ > > > > + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ > > + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ > > + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ > > > > NULL > > > > }; > > > > @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject > > *kobj, struct attribute *attr, > > > > if (index >= 11 && index <= 13 && > > > > !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) > > > > return 0; > > > > + if (index >= 14 && index <= 16 && > > + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) > > + return 0; > > > > return attr->mode; > > > > } > > > > @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) > > > > if (err >= 0) > > > > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; > > > > + /* Third fan attributes, if fan status is OK */ > > + err = i8k_get_fan_status(2); > > + if (err >= 0) > > + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; > > + > > > > i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, > > "dell_smm", > > > > NULL, i8k_groups); > > > > if (IS_ERR(i8k_hwmon_dev)) {
On 05/21/2016 05:31 PM, Pali Rohár wrote: > On Sunday 22 May 2016 02:21:46 Guenter Roeck wrote: >> On 05/21/2016 07:52 AM, Pali Rohár wrote: >>> Some Dell machines (e.g. Dell Precision M3800) have two fans, first >>> with index=0 and second with index=2. So export also attributes >>> for third fan device with index=2. >>> >>> Reported-by: Tolga Cakir <cevelnet@gmail.com> >>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com> >>> --- >>> >>> drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- >>> 1 file changed, 19 insertions(+), 1 deletion(-) >>> >>> --- >>> >>> Hi Tolga! Can you test this patch if sensors see fan device >>> correctly? >> >> I assume this means I should wait for test results before applying >> the patch ? > > Yes, testing should be done. > > Do you have some pending/testing/next tree/branch for such patches? > I have a hwmon-staging branch. I added your patches to it. Please use "hwmon: (dell-smm)" in the subject line of your patches. The subsystem name should come first, the driver name in () is a hwmon idiosyncrasy, and we don't need to repeat the "hwmon". Thanks, Guenter >> Thanks, >> Guenter >> >>> diff --git a/drivers/hwmon/dell-smm-hwmon.c >>> b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644 >>> --- a/drivers/hwmon/dell-smm-hwmon.c >>> +++ b/drivers/hwmon/dell-smm-hwmon.c >>> @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; >>> >>> #define I8K_HWMON_HAVE_TEMP4 (1 << 3) >>> #define I8K_HWMON_HAVE_FAN1 (1 << 4) >>> #define I8K_HWMON_HAVE_FAN2 (1 << 5) >>> >>> +#define I8K_HWMON_HAVE_FAN3 (1 << 6) >>> >>> MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); >>> MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); >>> >>> @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) >>> >>> static int i8k_get_fan_type(int fan) >>> { >>> >>> /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values >>> */ >>> >>> - static int types[2] = { INT_MIN, INT_MIN }; >>> + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; >>> >>> if (types[fan] == INT_MIN) >>> >>> types[fan] = _i8k_get_fan_type(fan); >>> >>> @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, >>> i8k_hwmon_show_fan_label, NULL, >>> >>> 1); >>> >>> static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, >>> i8k_hwmon_show_pwm, >>> >>> i8k_hwmon_set_pwm, 1); >>> >>> +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, >>> NULL, + 2); >>> +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, >>> i8k_hwmon_show_fan_label, NULL, + 2); >>> +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, >>> i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, 2); >>> >>> static struct attribute *i8k_attrs[] = { >>> >>> &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ >>> >>> @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { >>> >>> &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ >>> &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ >>> &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ >>> >>> + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ >>> + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ >>> + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ >>> >>> NULL >>> >>> }; >>> >>> @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject >>> *kobj, struct attribute *attr, >>> >>> if (index >= 11 && index <= 13 && >>> >>> !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) >>> >>> return 0; >>> >>> + if (index >= 14 && index <= 16 && >>> + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) >>> + return 0; >>> >>> return attr->mode; >>> >>> } >>> >>> @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) >>> >>> if (err >= 0) >>> >>> i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; >>> >>> + /* Third fan attributes, if fan status is OK */ >>> + err = i8k_get_fan_status(2); >>> + if (err >= 0) >>> + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; >>> + >>> >>> i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, >>> "dell_smm", >>> >>> NULL, i8k_groups); >>> >>> if (IS_ERR(i8k_hwmon_dev)) { > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 22 May 2016 17:11:18 Guenter Roeck wrote: > On 05/21/2016 05:31 PM, Pali Rohár wrote: > > On Sunday 22 May 2016 02:21:46 Guenter Roeck wrote: > >> On 05/21/2016 07:52 AM, Pali Rohár wrote: > >>> Some Dell machines (e.g. Dell Precision M3800) have two fans, > >>> first with index=0 and second with index=2. So export also > >>> attributes for third fan device with index=2. > >>> > >>> Reported-by: Tolga Cakir <cevelnet@gmail.com> > >>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com> > >>> --- > >>> > >>> drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- > >>> 1 file changed, 19 insertions(+), 1 deletion(-) > >>> > >>> --- > >>> > >>> Hi Tolga! Can you test this patch if sensors see fan device > >>> correctly? > >> > >> I assume this means I should wait for test results before applying > >> the patch ? > > > > Yes, testing should be done. > > > > Do you have some pending/testing/next tree/branch for such patches? > > I have a hwmon-staging branch. I added your patches to it. > > Please use "hwmon: (dell-smm)" in the subject line of your patches. > The subsystem name should come first, the driver name in () is a > hwmon idiosyncrasy, and we don't need to repeat the "hwmon". Ok, I use generic "driver_name: description" pattern as each subsystem use different format. But I can send in above format future hwmon patches...
Tolga, can you test this patch if is working for you correctly? On Saturday 21 May 2016 16:52:46 Pali Rohár wrote: > Some Dell machines (e.g. Dell Precision M3800) have two fans, first with > index=0 and second with index=2. So export also attributes for third fan > device with index=2. > > Reported-by: Tolga Cakir <cevelnet@gmail.com> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com> > --- > drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > --- > > Hi Tolga! Can you test this patch if sensors see fan device correctly? > > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c > index 577445f..7a2764a 100644 > --- a/drivers/hwmon/dell-smm-hwmon.c > +++ b/drivers/hwmon/dell-smm-hwmon.c > @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; > #define I8K_HWMON_HAVE_TEMP4 (1 << 3) > #define I8K_HWMON_HAVE_FAN1 (1 << 4) > #define I8K_HWMON_HAVE_FAN2 (1 << 5) > +#define I8K_HWMON_HAVE_FAN3 (1 << 6) > > MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); > MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); > @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) > static int i8k_get_fan_type(int fan) > { > /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */ > - static int types[2] = { INT_MIN, INT_MIN }; > + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; > > if (types[fan] == INT_MIN) > types[fan] = _i8k_get_fan_type(fan); > @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, > 1); > static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, > i8k_hwmon_set_pwm, 1); > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL, > + 2); > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, > + 2); > +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, > + i8k_hwmon_set_pwm, 2); > > static struct attribute *i8k_attrs[] = { > &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ > @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { > &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ > &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ > &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ > + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ > + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ > + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ > NULL > }; > > @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr, > if (index >= 11 && index <= 13 && > !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) > return 0; > + if (index >= 14 && index <= 16 && > + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) > + return 0; > > return attr->mode; > } > @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) > if (err >= 0) > i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; > > + /* Third fan attributes, if fan status is OK */ > + err = i8k_get_fan_status(2); > + if (err >= 0) > + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; > + > i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm", > NULL, i8k_groups); > if (IS_ERR(i8k_hwmon_dev)) {
Hi Pali, thanks for the patch and sorry for the delay! I've checked out https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-staging&id=86c050c82dd527b17cf47043831f03646f402a88 and ran it on my Dell M3800. It correctly works for identifying 2 fans, one listed as "Processor Fan" and the other as "Video Fan". The freeze only causes "Processor Fan" to run at maximum btw., "Video Fan" seems to be untouched. I don't get identical rpm values for both of them, so they are indeed 2 independent fans. E.g. at ~70°C, I get 2300rpm for CPU and 2400rpm for Video. Looks good to me. Cheers, Tolga Cakir 2016-05-31 13:03 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>: > Tolga, can you test this patch if is working for you correctly? > > On Saturday 21 May 2016 16:52:46 Pali Rohár wrote: >> Some Dell machines (e.g. Dell Precision M3800) have two fans, first with >> index=0 and second with index=2. So export also attributes for third fan >> device with index=2. >> >> Reported-by: Tolga Cakir <cevelnet@gmail.com> >> Signed-off-by: Pali Rohár <pali.rohar@gmail.com> >> --- >> drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- >> 1 file changed, 19 insertions(+), 1 deletion(-) >> --- >> >> Hi Tolga! Can you test this patch if sensors see fan device correctly? >> >> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c >> index 577445f..7a2764a 100644 >> --- a/drivers/hwmon/dell-smm-hwmon.c >> +++ b/drivers/hwmon/dell-smm-hwmon.c >> @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; >> #define I8K_HWMON_HAVE_TEMP4 (1 << 3) >> #define I8K_HWMON_HAVE_FAN1 (1 << 4) >> #define I8K_HWMON_HAVE_FAN2 (1 << 5) >> +#define I8K_HWMON_HAVE_FAN3 (1 << 6) >> >> MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); >> MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); >> @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) >> static int i8k_get_fan_type(int fan) >> { >> /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */ >> - static int types[2] = { INT_MIN, INT_MIN }; >> + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; >> >> if (types[fan] == INT_MIN) >> types[fan] = _i8k_get_fan_type(fan); >> @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, >> 1); >> static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, >> i8k_hwmon_set_pwm, 1); >> +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL, >> + 2); >> +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, >> + 2); >> +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, >> + i8k_hwmon_set_pwm, 2); >> >> static struct attribute *i8k_attrs[] = { >> &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ >> @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { >> &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ >> &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ >> &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ >> + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ >> + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ >> + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ >> NULL >> }; >> >> @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr, >> if (index >= 11 && index <= 13 && >> !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) >> return 0; >> + if (index >= 14 && index <= 16 && >> + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) >> + return 0; >> >> return attr->mode; >> } >> @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) >> if (err >= 0) >> i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; >> >> + /* Third fan attributes, if fan status is OK */ >> + err = i8k_get_fan_status(2); >> + if (err >= 0) >> + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; >> + >> i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm", >> NULL, i8k_groups); >> if (IS_ERR(i8k_hwmon_dev)) { > > -- > Pali Rohár > pali.rohar@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Guenter, I think you can take this patch also with Tolga's Tested-by. On Wednesday 01 June 2016 13:34:47 Tolga Cakir wrote: > Hi Pali, > > thanks for the patch and sorry for the delay! I've checked out > https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git > /commit/?h=hwmon-staging&id=86c050c82dd527b17cf47043831f03646f402a88 > and ran it on my Dell M3800. It correctly works for identifying 2 > fans, one listed as "Processor Fan" and the other as "Video Fan". > > The freeze only causes "Processor Fan" to run at maximum btw., "Video > Fan" seems to be untouched. I don't get identical rpm values for both > of them, so they are indeed 2 independent fans. E.g. at ~70°C, I get > 2300rpm for CPU and 2400rpm for Video. Looks good to me. > > Cheers, > Tolga Cakir > > 2016-05-31 13:03 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>: > > Tolga, can you test this patch if is working for you correctly? > > > > On Saturday 21 May 2016 16:52:46 Pali Rohár wrote: > >> Some Dell machines (e.g. Dell Precision M3800) have two fans, > >> first with index=0 and second with index=2. So export also > >> attributes for third fan device with index=2. > >> > >> Reported-by: Tolga Cakir <cevelnet@gmail.com> > >> Signed-off-by: Pali Rohár <pali.rohar@gmail.com> > >> --- > >> > >> drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- > >> 1 file changed, 19 insertions(+), 1 deletion(-) > >> > >> --- > >> > >> Hi Tolga! Can you test this patch if sensors see fan device > >> correctly? > >> > >> diff --git a/drivers/hwmon/dell-smm-hwmon.c > >> b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644 > >> --- a/drivers/hwmon/dell-smm-hwmon.c > >> +++ b/drivers/hwmon/dell-smm-hwmon.c > >> @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; > >> > >> #define I8K_HWMON_HAVE_TEMP4 (1 << 3) > >> #define I8K_HWMON_HAVE_FAN1 (1 << 4) > >> #define I8K_HWMON_HAVE_FAN2 (1 << 5) > >> > >> +#define I8K_HWMON_HAVE_FAN3 (1 << 6) > >> > >> MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); > >> MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); > >> > >> @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) > >> > >> static int i8k_get_fan_type(int fan) > >> { > >> > >> /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache > >> values */ > >> > >> - static int types[2] = { INT_MIN, INT_MIN }; > >> + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; > >> > >> if (types[fan] == INT_MIN) > >> > >> types[fan] = _i8k_get_fan_type(fan); > >> > >> @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, > >> S_IRUGO, i8k_hwmon_show_fan_label, NULL, > >> > >> 1); > >> > >> static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, > >> i8k_hwmon_show_pwm, > >> > >> i8k_hwmon_set_pwm, 1); > >> > >> +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, > >> i8k_hwmon_show_fan, NULL, + 2); > >> +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, > >> i8k_hwmon_show_fan_label, NULL, + 2); > >> +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, > >> i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, > >> 2); > >> > >> static struct attribute *i8k_attrs[] = { > >> > >> &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ > >> > >> @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { > >> > >> &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ > >> &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ > >> &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ > >> > >> + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ > >> + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ > >> + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ > >> > >> NULL > >> > >> }; > >> > >> @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject > >> *kobj, struct attribute *attr, > >> > >> if (index >= 11 && index <= 13 && > >> > >> !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) > >> > >> return 0; > >> > >> + if (index >= 14 && index <= 16 && > >> + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) > >> + return 0; > >> > >> return attr->mode; > >> > >> } > >> > >> @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) > >> > >> if (err >= 0) > >> > >> i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; > >> > >> + /* Third fan attributes, if fan status is OK */ > >> + err = i8k_get_fan_status(2); > >> + if (err >= 0) > >> + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; > >> + > >> > >> i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, > >> "dell_smm", > >> > >> NULL, i8k_groups); > >> > >> if (IS_ERR(i8k_hwmon_dev)) { > > > > -- > > Pali Rohár > > pali.rohar@gmail.com
On 06/13/2016 11:26 AM, Pali Rohár wrote: > Guenter, I think you can take this patch also with Tolga's Tested-by. > The patch on its own doesn't apply. It depends on 'Cache fan-type calls ...'. Should I take that patch as well ? Also, the test feedback was informal. I can not convert it to a formal Tested-by: without explicit permission (some people don't want to see their e-mail address in kernel logs). Guenter > On Wednesday 01 June 2016 13:34:47 Tolga Cakir wrote: >> Hi Pali, >> >> thanks for the patch and sorry for the delay! I've checked out >> https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git >> /commit/?h=hwmon-staging&id=86c050c82dd527b17cf47043831f03646f402a88 >> and ran it on my Dell M3800. It correctly works for identifying 2 >> fans, one listed as "Processor Fan" and the other as "Video Fan". >> >> The freeze only causes "Processor Fan" to run at maximum btw., "Video >> Fan" seems to be untouched. I don't get identical rpm values for both >> of them, so they are indeed 2 independent fans. E.g. at ~70°C, I get >> 2300rpm for CPU and 2400rpm for Video. Looks good to me. >> >> Cheers, >> Tolga Cakir >> >> 2016-05-31 13:03 GMT+02:00 Pali Rohár <pali.rohar@gmail.com>: >>> Tolga, can you test this patch if is working for you correctly? >>> >>> On Saturday 21 May 2016 16:52:46 Pali Rohár wrote: >>>> Some Dell machines (e.g. Dell Precision M3800) have two fans, >>>> first with index=0 and second with index=2. So export also >>>> attributes for third fan device with index=2. >>>> >>>> Reported-by: Tolga Cakir <cevelnet@gmail.com> >>>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com> >>>> --- >>>> >>>> drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- >>>> 1 file changed, 19 insertions(+), 1 deletion(-) >>>> >>>> --- >>>> >>>> Hi Tolga! Can you test this patch if sensors see fan device >>>> correctly? >>>> >>>> diff --git a/drivers/hwmon/dell-smm-hwmon.c >>>> b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644 >>>> --- a/drivers/hwmon/dell-smm-hwmon.c >>>> +++ b/drivers/hwmon/dell-smm-hwmon.c >>>> @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; >>>> >>>> #define I8K_HWMON_HAVE_TEMP4 (1 << 3) >>>> #define I8K_HWMON_HAVE_FAN1 (1 << 4) >>>> #define I8K_HWMON_HAVE_FAN2 (1 << 5) >>>> >>>> +#define I8K_HWMON_HAVE_FAN3 (1 << 6) >>>> >>>> MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); >>>> MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); >>>> >>>> @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) >>>> >>>> static int i8k_get_fan_type(int fan) >>>> { >>>> >>>> /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache >>>> values */ >>>> >>>> - static int types[2] = { INT_MIN, INT_MIN }; >>>> + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; >>>> >>>> if (types[fan] == INT_MIN) >>>> >>>> types[fan] = _i8k_get_fan_type(fan); >>>> >>>> @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, >>>> S_IRUGO, i8k_hwmon_show_fan_label, NULL, >>>> >>>> 1); >>>> >>>> static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, >>>> i8k_hwmon_show_pwm, >>>> >>>> i8k_hwmon_set_pwm, 1); >>>> >>>> +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, >>>> i8k_hwmon_show_fan, NULL, + 2); >>>> +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, >>>> i8k_hwmon_show_fan_label, NULL, + 2); >>>> +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, >>>> i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, >>>> 2); >>>> >>>> static struct attribute *i8k_attrs[] = { >>>> >>>> &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ >>>> >>>> @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { >>>> >>>> &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ >>>> &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ >>>> &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ >>>> >>>> + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ >>>> + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ >>>> + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ >>>> >>>> NULL >>>> >>>> }; >>>> >>>> @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject >>>> *kobj, struct attribute *attr, >>>> >>>> if (index >= 11 && index <= 13 && >>>> >>>> !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) >>>> >>>> return 0; >>>> >>>> + if (index >= 14 && index <= 16 && >>>> + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) >>>> + return 0; >>>> >>>> return attr->mode; >>>> >>>> } >>>> >>>> @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) >>>> >>>> if (err >= 0) >>>> >>>> i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; >>>> >>>> + /* Third fan attributes, if fan status is OK */ >>>> + err = i8k_get_fan_status(2); >>>> + if (err >= 0) >>>> + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; >>>> + >>>> >>>> i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, >>>> "dell_smm", >>>> >>>> NULL, i8k_groups); >>>> >>>> if (IS_ERR(i8k_hwmon_dev)) { >>> >>> -- >>> Pali Rohár >>> pali.rohar@gmail.com > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 13 June 2016 19:01:15 Guenter Roeck wrote: > On 06/13/2016 11:26 AM, Pali Rohár wrote: > >Guenter, I think you can take this patch also with Tolga's Tested-by. > > > > The patch on its own doesn't apply. It depends on 'Cache fan-type calls ...'. > Should I take that patch as well ? Ah right, so I will send all patches in new series. > Also, the test feedback was informal. I can not convert it to a formal > Tested-by: without explicit permission (some people don't want to see > their e-mail address in kernel logs). Tolga, can confirm or reject your Tested-by inclusion?
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c index 577445f..7a2764a 100644 --- a/drivers/hwmon/dell-smm-hwmon.c +++ b/drivers/hwmon/dell-smm-hwmon.c @@ -78,6 +78,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH; #define I8K_HWMON_HAVE_TEMP4 (1 << 3) #define I8K_HWMON_HAVE_FAN1 (1 << 4) #define I8K_HWMON_HAVE_FAN2 (1 << 5) +#define I8K_HWMON_HAVE_FAN3 (1 << 6) MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)"); MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); @@ -246,7 +247,7 @@ static int _i8k_get_fan_type(int fan) static int i8k_get_fan_type(int fan) { /* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */ - static int types[2] = { INT_MIN, INT_MIN }; + static int types[3] = { INT_MIN, INT_MIN, INT_MIN }; if (types[fan] == INT_MIN) types[fan] = _i8k_get_fan_type(fan); @@ -707,6 +708,12 @@ static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, 1); static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, i8k_hwmon_set_pwm, 1); +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL, + 2); +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL, + 2); +static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm, + i8k_hwmon_set_pwm, 2); static struct attribute *i8k_attrs[] = { &sensor_dev_attr_temp1_input.dev_attr.attr, /* 0 */ @@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = { &sensor_dev_attr_fan2_input.dev_attr.attr, /* 11 */ &sensor_dev_attr_fan2_label.dev_attr.attr, /* 12 */ &sensor_dev_attr_pwm2.dev_attr.attr, /* 13 */ + &sensor_dev_attr_fan3_input.dev_attr.attr, /* 14 */ + &sensor_dev_attr_fan3_label.dev_attr.attr, /* 15 */ + &sensor_dev_attr_pwm3.dev_attr.attr, /* 16 */ NULL }; @@ -747,6 +757,9 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr, if (index >= 11 && index <= 13 && !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2)) return 0; + if (index >= 14 && index <= 16 && + !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN3)) + return 0; return attr->mode; } @@ -788,6 +801,11 @@ static int __init i8k_init_hwmon(void) if (err >= 0) i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2; + /* Third fan attributes, if fan status is OK */ + err = i8k_get_fan_status(2); + if (err >= 0) + i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3; + i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm", NULL, i8k_groups); if (IS_ERR(i8k_hwmon_dev)) {
Some Dell machines (e.g. Dell Precision M3800) have two fans, first with index=0 and second with index=2. So export also attributes for third fan device with index=2. Reported-by: Tolga Cakir <cevelnet@gmail.com> Signed-off-by: Pali Rohár <pali.rohar@gmail.com> --- drivers/hwmon/dell-smm-hwmon.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) --- Hi Tolga! Can you test this patch if sensors see fan device correctly?