Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

2016-06-13 Thread Guenter Roeck 

On 06/13/2016 11:52 AM, Pali Rohár wrote:

On Sunday 22 May 2016 02:28:00 Pali Rohár wrote:

On Sunday 22 May 2016 02:19:48 Guenter Roeck wrote:

On 05/21/2016 07:46 AM, Pali Rohár wrote:

On more Dell machines (e.g. Dell Precision M3800)
I8K_SMM_GET_FAN_TYPE call is too expensive (CPU is too long in
SMM mode) and cause kernel to hang. This patch cache type for
each fan (as it should not change) and change the way how fan
presense is detected. It revert and use function fan_status() as
was before commit f989e55452c7 ("i8k: Add support for fan
labels").

Moreover, kernel hangs for 2 - 3 seconds only sometimes and only
on some Dell machines. When kernel hangs fan speed is at max. So
it was hard to debug and bisect where is root of this problem.
It looks like this is bug in Dell BIOS which implement fan type
SMM code... and there is no way how to fix it in kernel.

Signed-off-by: Pali Rohár 
Reviewed-by: Jean Delvare 
Reported-and-tested-by: Tolga Cakir 
Fixes: f989e55452c7 ("i8k: Add support for fan labels")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=112021
Link: https://bugzilla.kernel.org/show_bug.cgi?id=100121
Cc: sta...@vger.kernel.org # v4.0+, will need backport


Should this patch be applied, or do you wait for more testing ?


I would like to hear some confirmation from people with affected
machine.



Ok, now after testing we know that kernel should prevent calling
I8K_SMM_GET_FAN_TYPE on affected buggy Dell machines.

Looks like there are two different bugs in Dell SMM with
I8K_SMM_GET_FAN_TYPE call.

First bug cause that kernel freeze for 2 - 3 seconds when
I8K_SMM_GET_FAN_TYPE is issued.

Second bug cause that fan goes randomly up and down (that is controlled
by Dell SMM) when I8K_SMM_GET_FAN_TYPE is issued. Normal behaviour is
returned after machine reboots.

Some Dell machines are affected by first bug, some by second bug. And
there are Dell machines without both bugs.

This my patch just partially fix first bug and prevent calling that call
at boot time. But can be issued by sysfs (+value is cached, so it is
called only once).

So question is: is my patch enough for fixing first bug?

And second question: how to fix second bug? I see only one option:
Create machine blacklist with broken Dell SMM firmware and disallow
calling I8K_SMM_GET_FAN_TYPE for them.


Or maybe a whitelist with known working systems ?

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dell-smm-hwmon: Detect fan with index=2

2016-06-13 Thread Guenter Roeck 

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=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 :

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 
Signed-off-by: Pali Rohár 
---

  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 (d...@debian.org)");
  MODULE_AUTHOR("Pali Rohár ");

@@ -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[] = {

   _dev_attr_temp1_input.dev_attr.attr, /* 0 */

@@ -723,6 +730,9 @@ static struct attribute *i8k_attrs[] = {

   _dev_attr_fan2_input.dev_attr.attr,  /* 11 */
   _dev_attr_fan2_label.dev_attr.attr,  /* 12 */
   _dev_attr_pwm2.dev_attr.attr,/* 13 */

+ _dev_attr_fan3_input.dev_attr.attr,  /* 14 */
+ _dev_attr_fan3_label.dev_attr.attr,  /* 15 */
+ _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.ro...@gmail.com




--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dell-smm-hwmon: Cache fan_type() calls and use fan_status() for fan detection

2016-06-13 Thread Pali Rohár 
On Monday 30 May 2016 13:45:51 Thorsten Leemhuis wrote:
> Pali Rohár wrote on 27.05.2016 15:21:
> > On Friday 27 May 2016 15:05:54 Thorsten Leemhuis wrote:
> >> Pali Rohár wrote on 27.05.2016 12:45:
> >> So I tried a few things
> >> and came to the conclusion: the problem shows up as soon as
> >> i8k_get_fan_type() (introduced in f989e55452) is called somewhere.
> > 
> > So, once kernel call i8k_get_fan_type() function, then fan speed
> > going up/down?
> 
> Yes.
> 
> > To make sure that this is root of your problem, can you take some
> > older kernel version (where is i8k working fine) and try to
> > patch+call that i8k_get_fan_type() function? To check that
> > something else cannot interference with it...
> 
> I just tried with 3.19.8 (had to install a older distro first :-/ ),
> where the problem does not show up (I verified just to be sure). Then
> I applied below patch and voila: the fan speed starts going up/down.
> 
> IOW: From what I can see that SMM call that i8k_get_fan_type()
> triggers the problem on my Studio 8000
> 
> CU, knurd
> 

Thank you for your testing! I believe now we definitely know root of 
this problem. It is buggy Dell BIOS/SMM code and we need to avoid 
calling I8K_SMM_GET_FAN_TYPE on affected buggy Dell machines.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.