Supporting thermal sensors on ipq806x

For Developers
1

Hi everyone,

I got a Linksys EA8500 recently, built LEDE r21114 myself and it runs pretty well. But when I was trying to compare thermal status of this new platform with my old Netgear R7000, I found that there were nothing under /sys/class/thermal, nor /sys/class/hwmon. lm-sensors gave nothing as well.

After some digging I discovered that Qualcomm had sent relevant patches to mainline Linux (for e.g., the DTS change), but they were not accepted for some reason. I tried picking the changes into my local sources and it works as expected:

root@lede:~# ls /sys/class/thermal
cooling_device0  cooling_device1  thermal_zone0    thermal_zone1    thermal_zone2

root@lede:~# cat /sys/class/thermal/*/temp
54696
54696
53080

My changes are here. Patch 10 needs to be applied in the root directory of LEDE source, while other 10 patches should be applied to kernel source directly.

I'm thinking about adjusting the patches for LEDE and submitting them, but given that upstream did not accept those patches, I'm wondering whether it is appropriate to do so.

Furthermore, I found that Qualcomm thermal driver was merged in a newer kernel version (4.6 maybe?), but there is still no sensor data in qcom-ipq8064.dts after that version.

What should I do with these patches?

2

submit them, see https://github.com/lede-project/source/pull/204 for an example of thermal sensor patch that was accepted and what they asked to the submitter.

They will ask you to also submit upstream, so show proof of what was upstreamed already.

if the driver was upstreamed I think it should be OK, changing the dts to add the thermal sensor is kind of a consequence to that.

3

Thanks. I just checked Linux kernel archive, and found that the driver was officially merged in 4.9. Maybe I should backport drivers from 4.9, or just wait for LEDE to switch to the new release (when it's ready) and send the DTS patch.

4

[quote="updateing, post:3, topic:205"]
found that the driver was officially merged in 4.9. Maybe I should backport drivers from 4.9, or just wait for LEDE to switch to the new release (when it's ready) and send the DTS patch.
[/quote]If there is an officially merged driver version in 4.9, backporting it to 4.4 (the current ipq806x kernel) is the logical option. It may take quite a long time before 4.9 is the mainstream kernel in LEDE.

5

I see. Thanks for the reply.

6

What Hnyman said. Backport it. Could be year or two before the kernel in LEDE gets updated. Also, nice work on this! I'm excited to get thermal stats from my router.

7

So with that patch you can get correct temp of CPU?
I've been also playing with sensors driver a week ago but with newer version without qfprom include, but I couldn't confirm that it was working. Seems that I have been missing the "stride" patch
The upstream tsens patch unfortunately doesn't support ipq806x.
More than that, the twin socs to ipq806x are apq8064/msm8960 have support for tsens in GCC driver, so to enable it you have to add according entries into DT...

Ipq806x is a dedicated case - a lot of patches were submitted for various functions but later with every iteration they were loosing support for ipq806x:(

The patch that you proposed duplicates nvmem driver with embedded qfprom support that is upstream.
I'll adjust my work and provide PR if everything works ok

8

Apply this patch to your tree and check please if it works ok
https://github.com/dissent1/r7800/commit/64c7ed01777c7c48ac45f14a24b9ed77ad82f9b3.patch

9

Ok, works as expected

10

How does the sensor data manifest itself in the running system?
A device to be read somewhere in /sys or /proc ?
Or a "real" sensor to be read with lm-sensors or such?

I am just thinking if there is an easy way to read data via collectd-mod-thremal or collectd-mod-sensors...

11

The excessive info is in 1st post :slight_smile:

12

Sorry, I didn't realize that your patch has similar output as the patch from updateing.

But in any case I applied your patch (from the PR) and my R7800 nicely shows data from 11 thermal zones (of ipq8065).

And collectd-mod-thermal reads data ok, but for some reason the plugin divides the temperature by 1000. Strange. (But based on sources that has been behaviour of the plugin since 2008)

13

I figured that out.
Linux documentation says the "temp" is millicelcius, but your patch produces full celcius degrees. (or actually in my case the already existing dts for 8065 uses the tsens_factor 1000, too)

https://www.kernel.org/doc/Documentation/thermal/sysfs-api.txt

temp
Current temperature as reported by thermal zone (sensor).
Unit: millidegree Celsius

14

Are there 11 different thermal sensors in that chip? :astonished:

15

No idea. But values are shown for 11 different zones (0-10)... And the values did change when I generated CPU load via openssl benchmark, so I guess that they are real :wink:

root@lede:/# cat /sys/class/thermal/thermal_zone*/temp
56
55
54
53
55
53
56
54
58
52
58

Note that the values are full Celsius degrees, not millicelsius that they apparently should be (by Linux standards)

EDIT: and creating LuCI statistics supports was rather straightforward. It is not yet finalised but works ok in my local router. Values are shows as 1/1000 of the real, because collectd expects millicelsius values and divides the input by 1000 when reading values from /sys/class/thermal/:

image.png691×172 2.24 KB

16

Well, that's the way it's represented in oem gpl.
Have you tried setting tsens factor to 1 000 000?

17

Not yet. I just wanted to highlight that it looks like a deviation from the Linux standard practice, but if it is really coming from OEM upstream, I can live with it. (Actually, having full normal Celsius degrees '56' there is much more userfriendly than having millicelsius like '56000')

I already committed the Luci statistics extension for thermal data. For ipq806x that will show the correct number as above picture, but the unit 'm' will be wrong. But that is no big deal.

18

You can try the upstream version instead the one in PR

Not sure if it is going work though.

19

It doesn't build, I'll check for depends

20


Builds, but doesn't work