Ok, I will describe my problem. Guenter, maybe you can find another solution/fix for it.
Calling i8k_get_temp(3) on my laptop without I8K_TEMPERATURE_BUG
always returns value 193 (which is above I8K_MAX_TEMP).
When I8K_TEMPERATURE_BUG is enabled (by default) then
i8k_get_temp(3) returns value from prev[3] and store new value
I8K_TEMPERATURE_BUG to prev[3]. Value in prev[3] is initialized
to 0.
What I want to achieve is: when i8k_get_temp() for particular
sensor id always returns invalid value (> I8K_MAX_TEMP) then we
should totally ignore sensor with that id and do not export it
via hwmon.
My solution is: initialize prev[id] to I8K_MAX_TEMP, so on
invalid data first call to i8k_get_temp(id) returns I8K_MAX_TEMP.
Then in i8k_init_hwmon check if value is < I8K_MAX_TEMP and if
not ignore sensor id.
Guenter, it is clear now? Are you ok that we should ignore sensor
if always report value above I8K_MAX_TEMP? If you do not like my
solution/patch for it, can you specify how other can it be fixed?
On Sunday 19 October 2014 17:13:29 Guenter Roeck wrote:
> On 10/19/2014 07:46 AM, Pali Rohár wrote:
> > On some machines some temperature sensors report too high
> > values which are
>
> What is "too high", and what is "some machines" ?
> Would be great if you can be more specific.
>
> > invalid. When value is invalid function i8k_get_temp()
> > returns previous value and at next call it returns
> > I8K_MAX_TEMP.
> >
> > With this patch also firt i8k_get_temp() call returns
> > I8K_MAX_TEMP and
>
> fix ? Also, I am not entirely sure I understand what exactly
> you are fixing here.
>
> > i8k_init_hwmon() will ignore all sensor ids which report
> > incorrect values.
> >
> > Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> > ---
> >
> > drivers/char/i8k.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
> > index 7272b08..bc327fa 100644
> > --- a/drivers/char/i8k.c
> > +++ b/drivers/char/i8k.c
> > @@ -298,7 +298,7 @@ static int i8k_get_temp(int sensor)
> >
> > int temp;
> >
> > #ifdef I8K_TEMPERATURE_BUG
> >
> > - static int prev[4];
> > + static int prev[4] = { I8K_MAX_TEMP, I8K_MAX_TEMP,
> > I8K_MAX_TEMP, I8K_MAX_TEMP };
>
> I am not sure I understand what this change is expected to
> accomplish. Please explain.
>
> > #endif
> >
> > regs.ebx = sensor & 0xff;
> > rc = i8k_smm(®s);
> >
> > @@ -610,17 +610,17 @@ static int __init i8k_init_hwmon(void)
> >
> > /* CPU temperature attributes, if temperature reading is
> > OK */ err = i8k_get_temp(0);
> >
> > - if (err >= 0)
> > + if (err >= 0 && err < I8K_MAX_TEMP)
>
> I8K_MAX_TEMP is, at least in theory, a valid temperature, so
> this should be "<=".
>
> It would be important to understand what the "too high"
> temperature is to possibly be able to distinguish it from the
> buggy temperature that the code is trying to fix.
>
> Thanks,
> Guenter
>
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP1;
> >
> > /* check for additional temperature sensors */
> > err = i8k_get_temp(1);
> >
> > - if (err >= 0)
> > + if (err >= 0 && err < I8K_MAX_TEMP)
> >
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP2;
> >
> > err = i8k_get_temp(2);
> >
> > - if (err >= 0)
> > + if (err >= 0 && err < I8K_MAX_TEMP)
> >
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP3;
> >
> > err = i8k_get_temp(3);
> >
> > - if (err >= 0)
> > + if (err >= 0 && err < I8K_MAX_TEMP)
> >
> > i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
> >
> > /* Left fan attributes, if left fan is present */
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
