linux-kernel - Re: (bug report) HWMON & Thermal interactions

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: <8005acfe-da2d-8d38-0e87-a96c438eeab1@roeck-us.net>
Date:   Sun, 23 Oct 2022 14:23:00 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Cristian Marussi <cristian.marussi@....com>,
        daniel.lezcano@...aro.org
Cc:     sudeep.holla@....com, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: (bug report) HWMON & Thermal interactions

On 10/23/22 11:27, Cristian Marussi wrote:
> Hi,
> 
> Starting with v6.1-rc1 the SCMI HWMON driver failed probing on my JUNO due
> to the fact that no trip points were (ever !) defined in the DT; bisecting it
> looks like that after:
> 
> https://lore.kernel.org/all/20220804224349.1926752-28-daniel.lezcano@linexp.org/
> 
> the presence of the mandatory trips node within thermal zones is now
> enforced.
> 
> So, this is NOT what this bug report is about (I'll post soon patches for
> the JUNO DT missing trips) BUT once this problem was solved in the DT,
> another issue appeared:
> 
> [    1.921929] hwmon hwmon0: temp2_input not attached to any thermal zone
> 
> that despite having now a goodi/valid DT describing 2 sensors and 2 thermal zones
> embedding that sensors, only the first one is found as belonging to one ThermZ.
> (this happens ALSO with v6.0 once I added the trips...)
> 
> Digging deep into this, it turned out that inside the call chain
> 
> devm_hwmon_device_register_with_info
>    hwmon_device_register_with_info
>      __hwmon_device_register
> 	hwmon_thermal_register_sensors(dev)
> 		--> hwmon_thermal_add_sensor(dev, j)
> 			--> devm_thermal_of_zone_register(dev, sensor_id, tdata, )
> 
> the HWMON channel index j is passed to the Thermal framework in order to
> search and bind sensors with defined thermal zone, but this lead to the
> assumption that sequential HWMON channel indexes corresponds one-to-one to the
> underlying real sensor IDs that the ThermalFramework uses for matching
> within the DT.
> 
> On a system like my SCMI-based DT where I have 2 temp-sensors bound to 2
> thermal zones like:
> 
> thernal_zones {
> 	pmic {
> 		...
> 		thermal-sensors = <&scmi_sensors0 0>;
> 		...
> 		trips {
> 			...
> 		}
> 	soc {
> 		...
> 		thermal-sensors = <&scmi_sensors0 3>;
> 		...
> 		trips {
> 			...
> 		}
> 	}
> }
> 
> This works fine by chance for the pmic (j=0, sensor_id=0) BUT cannot work for
> the soc where J=1 BUT the real sensor ID is 3.
> 
> Note that there can be a number of sensors, not all of them of a type handled
> by HWMON, and enumerated by SCMI in different ways depending on the
> platform.
> 
> I suppose this is not an SCMI-only related issue, but maybe in non-SCMI
> context, where sensors are purely defined in the DT, the solution can be
> more easily attained (i.e. renumber the sensors).
> 
> At first I tried to solve this inside scmi-hwmon.c BUT I could not find
> a way to present to the HWMON subsystem the list of sensors preserving
> the above index/sensor_id matching (not even with a hack like passing
> down dummy sensors to the HWMON subsystem to fill the 'holes' in the
> numbering)
> 
> My tentative solution, which works fine for me in my context, was to add
> an optional HWMON hwops, so that the core hwmon can retrieve if needed the
> real sensor ID if different from the channel index (using an optional hwops
> instead of some static hwinfo var let me avoid to have to patch all the
> existent hwmon drivers that happens to just work fine as of today...but
> maybe it is not necessarily the proper final solution...)
> 
> i.e.
> 
> ----8<----
> 
> Author: Cristian Marussi <cristian.marussi@....com>
> Date:   Fri Oct 21 17:24:04 2022 +0100
> 
>      hwmon: Add new .get_sensor_id hwops
>      
>      Add a new optional helper which can be defined to allow an hwmon chip to
>      provide the logic to map hwmon indexes to the real underlying sensor IDs.
>      

Maybe I am missing something, but ...

The driver isn't supposed to know anything about thermal devices and
thermal zones. If that no longer works, and drivers have to know about
thermal zones and thermal zone device index values anyway, we might
as well pull thermal device support from the hwmon core and implement
it in drivers.

Guenter