linux-kernel - Re: [PATCH v2] hwmon: Add thermal sensor driver for Surface Aggregator Module

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <39eecb79-0b1a-4a17-865f-dcb60a20a893@gmail.com>
Date: Sat, 10 Aug 2024 03:14:29 +0200
From: Konrad Dybcio <konradybcio@...il.com>
To: Maximilian Luz <luzmaximilian@...il.com>,
 Konrad Dybcio <konradybcio@...nel.org>, Guenter Roeck <linux@...ck-us.net>
Cc: Jean Delvare <jdelvare@...e.com>, Hans de Goede <hdegoede@...hat.com>,
 Ivor Wanders <ivor@...nders.net>, linux-kernel@...r.kernel.org,
 linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: Add thermal sensor driver for Surface
 Aggregator Module

On 10.08.2024 3:04 AM, Maximilian Luz wrote:
> On 8/10/24 1:35 AM, Konrad Dybcio wrote:
>> On 5.08.2024 1:08 AM, Maximilian Luz wrote:
>>> Some of the newer Microsoft Surface devices (such as the Surface Book
>>> 3 and Pro 9) have thermal sensors connected via the Surface Aggregator
>>> Module (the embedded controller on those devices). Add a basic driver
>>> to read out the temperature values of those sensors.
>>>
>>> The EC can have up to 16 thermal sensors connected via a single
>>> sub-device, each providing temperature readings and a label string.
>>>
>>> Link: https://github.com/linux-surface/surface-aggregator-module/issues/59
>>> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
>>> Co-developed-by: Ivor Wanders <ivor@...nders.net>
>>> Signed-off-by: Ivor Wanders <ivor@...nders.net>
>>> Signed-off-by: Maximilian Luz <luzmaximilian@...il.com>
>>>
>>> ---
>>
>> Gave it a shot on SL7, some names are repeated and one sensor is
>> totally busted
>>
>> /sys/class/hwmon/hwmon66/name:surface_thermal
>> /sys/class/hwmon/hwmon66/temp10_input:32200
>> /sys/class/hwmon/hwmon66/temp10_label:I_RTS2
>> /sys/class/hwmon/hwmon66/temp11_input:31600
>> /sys/class/hwmon/hwmon66/temp11_label:I_RTS3
>> /sys/class/hwmon/hwmon66/temp12_input:38000
>> /sys/class/hwmon/hwmon66/temp12_label:I_RTS4
>> /sys/class/hwmon/hwmon66/temp1_input:43900
>> /sys/class/hwmon/hwmon66/temp1_label:I_RTS1
>> /sys/class/hwmon/hwmon66/temp2_input:44000
>> /sys/class/hwmon/hwmon66/temp2_label:I_RTS2
>> /sys/class/hwmon/hwmon66/temp3_input:47300
>> /sys/class/hwmon/hwmon66/temp3_label:I_RTS3
>> /sys/class/hwmon/hwmon66/temp4_input:-273100
>> /sys/class/hwmon/hwmon66/temp4_label:I_RTS4
>> /sys/class/hwmon/hwmon66/temp5_input:31300
>> /sys/class/hwmon/hwmon66/temp5_label:I_RTS5
>> /sys/class/hwmon/hwmon66/temp9_input:37100
>> /sys/class/hwmon/hwmon66/temp9_label:I_RTS1
> 
> Hmm, on the SPX it looks like this:
> 
> I_RTS1:       +31.9°C
> I_RTS2:       +31.3°C
> I_RTS3:       +31.4°C
> I_RTS4:       +28.3°C
> I_RTS5:       +29.3°C
> I_RTS6:       +29.3°C
> I_RTS7:       +29.3°C
> I_RTS8:       +29.3°C
> VTS1:         +30.2°C
> VTS2:          +0.0°C
> VTS3:          +0.0°C
> VTS4:          +0.0°C
> VTS5:          +0.0°C
> 
> So VTS2-5 seem like they may not actually be connected, but the rest at
> least look somewhat sensible. I'd probably still keep the names as they
> at least might give an indication what the sensors could be for.
> 
> But there's a good chance that we're missing something on how MS
> envisions these sensors to work exactly.

I think the takeaway here is that I'll keep the sensors disabled for
now on SL7 until we have a better idea and not block this driver

Konrad