Re: [PATCH] i8k: Add support for temperature sensor labels

[Guenter Roeck]

On 11/29/2014 11:07 AM, Pali RohÃr wrote:
> On Saturday 29 November 2014 19:58:28 Guenter Roeck wrote:
> > On 11/29/2014 10:27 AM, Gabriele Mazzotta wrote:
> > > On Saturday 29 November 2014 18:18:18 Pali RohÃr wrote:
> > > > On Saturday 29 November 2014 18:07:19 Gabriele Mazzotta
> wrote:
> > > > > On Saturday 29 November 2014 17:09:35 Pali RohÃr wrote:
> > > > > > On Saturday 29 November 2014 17:04:07 Pali RohÃr wrote:
> > > > > > > This patch adds labels for temperature sensors if SMM
> > > > > > > function with EAX register 0x11a3 reports it. These
> > > > > > > informations was taken from DOS binary NBSVC.MDM.
> > > > > > >
> > > > > > > Signed-off-by: Pali RohÃr <pali.rohar@xxxxxxxxx>
> > > > > > > ---
> > > > > > >
> > > > > > >    drivers/char/i8k.c |  110
> > > > > > >
> > > > > > > +++++++++++++++++++++++++++++++++++++++++----------- 1
> > > > > > > file changed, 88 insertions(+), 22 deletions(-)
> > > > > >
> > > > > > I tested patch on Latitude E6440 and i8k CPU & GPU temps
> > > > > > match intel coretemp & amd radeion temps.
> > > > > >
> > > > > > But I would like if somebody with other Dell laptop can
> > > > > > test if temperature labels are correct...
> > > > >
> > > > > I tested it on my XPS13 9333, here what sensors outputs:
> > > > >
> > > > > acpitz-virtual-0
> > > > > Adapter: Virtual device
> > > > > temp1:        +27.8ÂC  (crit = +105.0ÂC)
> > > > > temp2:        +29.8ÂC  (crit = +105.0ÂC)
> > > > >
> > > > > coretemp-isa-0000
> > > > > Adapter: ISA adapter
> > > > > Physical id 0:  +62.0ÂC  (high = +100.0ÂC, crit =
> > > > > +100.0ÂC) Core 0:         +62.0ÂC  (high = +100.0ÂC, crit
> > > > > = +100.0ÂC) Core 1:         +61.0ÂC  (high = +100.0ÂC,
> > > > > crit = +100.0ÂC)
> > > > >
> > > > > i8k-virtual-0
> > > > > Adapter: Virtual device
> > > > > fan2:           0 RPM
> > > > > CPU:          +62.0ÂC
> > > > > Ambient:      +49.0ÂC
> > > > > SODIMM:       +46.0ÂC
> > > > > temp4:            N/A
> > > > >
> > > > > CPU seems to be correct, but I can't say anything on
> > > > > Ambient and SODIMM. temp4 is constantly equal to SODIMM
> > > > > without this patch, so I'd say N/A is correct.
> > > > >
> > > > >
> > > > > Gabriele
> > > >
> > > > It is unknown for me how to directly read Ambient and
> > > > SODIMM temperatures (without Dell SMM functions). So we
> > > > can only trust Dell SMM that it reporting correct values
> > > > and type is really Ambient and SODIMM.
> > > >
> > > > And about temp4:
> > > >
> > > > Label is not set when SMM function fails. Original DOS
> > > > NBSVC.MDM just ignore all sensors for which SMM type
> > > > function fails.
> > > >
> > > > This patch should not disable any sensor, so if you
> > > > previously had some value (<= 128ÂC) and now not, then
> > > > there is some bug.
> > > >
> > > > Can you test this patch?
> > > > https://git.kernel.org/cgit/linux/kernel/git/gregkh/char-mi
> > > > sc.git/comm
> > > > it/?h=char-misc-testing&id=723493ca59c8d81fed3e7f261165fee
> > > > 493a29ffa
> > > >
> > > > It is possible that same value is caused by incorrect use
> > > > of prev[] array which should be fixed by above patch.
> > > >
> > > > Can you test i8k with and without above patch?
> > >
> > > I did some more tests. What I think is happening is that
> > > temp4_label returns -22, so I sensors prints N/A without
> > > actually reading temp4_input.
> > >
> > > I'm doing some tests to understand what's going on with
> > > temp4_input. It reports the value of the last temp*_input I
> > > read. If I read it right after I loaded i8k, I get an error
> > > (-22).
> > >
> > > The same doesn't happen for temp3_label, which constantly
> > > returns -22.
> > >
> > > I already had
> > > https://git.kernel.org/cgit/linux/kernel/git/gregkh/char-mi
> > > sc.git/commit/?h=char-misc-testing&id=723493ca59c8d81fed3e7f
> > > 261165fee493a29ffa applied. Without it, things seem not to
> > > change much. I either get an error (-22) or some incorrect
> > > values (for now always 1000) when I read temp4_input right
> > > after i8k was loaded. Once I read some other temp*_input, I
> > > always get the last value read.
> >
> > I am seeing exactly the same behavior on an XPS13.
> >
> > Guenter
>
> Original Dell DOS executable ignores all temperature sensors if
> type SMM function fails (if I decoded and understand that DOS
> assembler code correctly). So maybe we should do same...
>
> But because our i8k.c code ignores sensor only if it returns
> invalid temperature, there could be possible regression that on
> same machines type SMM function is not implemented or not
> working...
>
> What do you think?

Tested with XPS13, Studio 1555 (with GPU), and XPS M140.
Reading the type works with all of them. The Studio 1555
reports the GPU temperature in temp4. The M140 is quite old
(about 10 years), so I guess we can be reasonably sure that
all laptops currently in use support reporting the type.

Do you know what is returned for the type if the GPU is turned
off on a system with GPU ? I think that is the only open
question.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/