linux-kernel - Re: [PATCH v1] hwmon: drivetemp: support to be a platform driver for thermal

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: <5c805125-4cb5-cdf8-6b50-5a6ce7e97149@roeck-us.net>
Date:   Wed, 15 Mar 2023 22:07:52 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Phinex Hung <phinex@...ltek.com>,
        "jdelvare@...e.com" <jdelvare@...e.com>
Cc:     "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] hwmon: drivetemp: support to be a platform driver for
 thermal_of

On 3/15/23 20:29, Phinex Hung wrote:
> 
> On 3/16/23 11:02, Guenter Roeck" <groeck7@...il.com <mailto:groeck7@...il.com> <mailto:groeck7@...il.com <mailto:groeck7@...il.com>> wrote:
> 
> 
>> Sure. But your argument is inappropriate: You could as well argue that
>> this system with a single fan should bundle all its thermal sensors into
>> a single thermal zone, and that it should do so in the driver(s)
>> providing the thermal zone sensors to the thermal subsystem. This does
>> not take into account that there might be systems with dozens (or hundreds,
>> for that matter) of drives, in a system with multiple disk trays and fans
>> for each of those.
> 
>> I don't know if and how the thermal subsystem deals with the situation
>> of having N thermal zone sensors and M << N cooling devices. This is
>> a general problem, not limited to disk drives. Just as we won't bundle
>> multiple thermal sensors on a multi-channel thermal sensor chip into a
>> single thermal zone, we won't bundle multiple disk drives into a single
>> thermal zone.
> 
> It's true, there are several different combinations.
> 
> Keep a single sensor with a single thermal might give more flexibility.
> 
> My idea is inspired from the comment of the following patch..
> 
> https://www.spinics.net/lists/devicetree/msg537186.html
> 
> By the comment in this patch, it said that "Thermal zone works only with first disks".
> 
> But it has two hard disk, each describes a thermal-sensors-cells.
> 
>   /* Thermal zone works only with first disk */.
> 
> That is why I am trying to hack the original temp_read call back to support multiple sensors.
> 
> Anyway, keep a single mapping might be good and thanks for your comment.
> 
> 
>> In theory it should work just like described in the kirkwood devicetree
>> files. If that doesn't work, the question is how to find the sata port
>> nodes from the drivetemp driver. I don't have a system with such nodes,
>> so I have no means to find or know the answer.
> 
> 
>> I also don't know how to attach more than one thermal sensor to a
>> single thermal zone, or if that is even possible. If it isn't, it
>> is a limitation of the thermal subsystem, and trying to hack around
>> it in a driver providing thermal sensors would be inappropriate.
> 
> 
> Thank you for this great module to make thermal and cooling control simpler.
> 
> That is why I am trying to get it work under our kernel 4.19 since our SoC currently works under 4.19 only.
> 
> Sata ports can be found without any issue, and the corresponding hwmon entries can be found, such as.
> 
> root@...nWRT-Kylin:/sys/class/hwmon# ls -al
> total 0
> drwxr-xr-x    2 root     root             0 Mar 11 08:13 .
> drwxr-xr-x   42 root     root             0 Mar 11 08:13 ..
> lrwxrwxrwx    1 root     root             0 Mar 11 08:13 hwmon0 -> ../../devices/platform/soc/98000000.bus/9801b000.syscon/9801bc00.rtk_fan/hwmon/hwmon0
> lrwxrwxrwx    1 root     root             0 Mar 11 08:13 hwmon1 -> ../../devices/platform/soc/98000000.bus/9803f000.sata/ata1/host0/target0:0:0/0:0:0:0/hwmon/hwmon1
> lrwxrwxrwx    1 root     root             0 Mar 11 08:13 hwmon2 -> ../../devices/platform/soc/98000000.bus/9803f000.sata/ata2/host1/target1:0:0/1:0:0:0/hwmon/hwmon2
> 
> And I add the port number in the name of these hwmon devices so that I can differentiate them.
> 
> root@...nWRT-Kylin:/sys/class/hwmon# cat hwmon1/name hwmon1/temp1_input
> drivetemp_port0
> 52000
> root@...nWRT-Kylin:/sys/class/hwmon# cat hwmon2/name hwmon2/temp1_input
> drivetemp_port1
> 49000
> 
> My initial problem is that "how could I associated these hwmon devices with thermal zones"?
> 
> By tracing the source code, finding that I probably need to register thermal_zone using a platform device.
> 

Wrong conclusion. You have (or should have) devicetree node(s) such as

         sata: sata@...00 {
                 compatible = "marvell,orion-sata";
                 reg = <0x80000 0x5000>;
                 interrupts = <21>;
                 clocks = <&gate_clk 14>, <&gate_clk 15>;
                 clock-names = "0", "1";
                 phys = <&sata_phy0>, <&sata_phy1>;
                 phy-names = "port0", "port1";
                 status = "disabled";
         };

Those nodes should have devices associated with them (in this case instantiated
by drivers/ata/sata_mv.c). At the same time, the drivetemp driver callback
(drivetemp_add) gets called with a device pointer as parameter. The question
is how to get from the device pointer passed to drivetemp_add() to the device
created by the sata_mv driver. Is it dev ? or dev->parent ? Or dev->parent->parent ?
The devicetree node associated with that device is the one which should be used
to set the hwmon device devicetree node. Essentially you'll have to find out where
in the device list to find the of_node pointing to the above sata node. Then we
can discuss how to make that node available to the thermal subsystem.

Guenter