Hi,
to me it seems the thermal zone fan control model (see
https://www.kernel.org/doc/Documentation/devicetree/bindings/thermal/thermal.txt)
is inappropriate in many cases.
This model relies on a `cooling-levels` property of the fan being
present, which is then referenced by a thermal zone trip-point.
`cooling-levels` is essentially a map of 0-n fan power to 0-255 fan
power. It is probably meant to be used to make a power-cooling relation
more linear and cut off low-power setting which won't suffice to spin up
the fan ?!?
This setting can obviously only be configured for a specific fan. And to
do so one needs to recompile and install a device tree for the specific
fan.
To test the fan I added a fall-back code path to pwmfan.c, so the fan
doesn't depend on a `cooling-levels` setting (patch attached).
Now knowing that the 80mm fan does actually work and at what level it
will relieably spin up I could work out a sensible thermal-zone
configuration which I then added to the device tree (patch attached).
This diff won't help anybody with a different fan since other fans will
need more or less power to spin up.
What I would maybe like to add is a sysctl knob with which one can set
the cooling-levels map as a uint8_t cooling-levels[255]. That way the
fan could be controlled from userspace by setting the whole array to the
same value or the map can be used to adapt a given fan to the thermal
zones configuration of the device tree.
I'm not sure where to put it. Since this whole thermal-zone and
cooling-levels stuff is only an abstraction it should maybe be moved to
ofw_thermal.c and a generic "cooling-device" knob be added there.
Cooling devices would then need to pass their fdt_attach_args to
ofw_thermal so that ofw_thermal could then read the cooling-levels.
Does this make any sense? Is it a reasonable design?
Christopher
Index: pwmfan.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/pwmfan.c,v
retrieving revision 1.2
diff -u -p -r1.2 pwmfan.c
--- pwmfan.c 24 Oct 2021 17:52:26 -0000 1.2
+++ pwmfan.c 20 Dec 2021 18:08:48 -0000
@@ -76,23 +76,24 @@ pwmfan_attach(struct device *parent, str
return;
}
+ printf("\n");
+
sc->sc_pwm = malloc(len, M_DEVBUF, M_WAITOK);
OF_getpropintarray(faa->fa_node, "pwms", sc->sc_pwm, len);
sc->sc_pwm_len = len;
len = OF_getproplen(faa->fa_node, "cooling-levels");
- if (len < 0) {
- free(sc->sc_pwm, M_DEVBUF, sc->sc_pwm_len);
- printf(": no cooling levels\n");
- return;
+ if (len <= 0) {
+ len = 0;
+ sc->sc_levels = NULL;
+ sc->sc_nlevels = 255;
+ }
+ else {
+ sc->sc_levels = malloc(len, M_DEVBUF, M_WAITOK);
+ OF_getpropintarray(faa->fa_node, "cooling-levels",
+ sc->sc_levels, len);
+ sc->sc_nlevels = len / sizeof(uint32_t);
}
-
- sc->sc_levels = malloc(len, M_DEVBUF, M_WAITOK);
- OF_getpropintarray(faa->fa_node, "cooling-levels",
- sc->sc_levels, len);
- sc->sc_nlevels = len / sizeof(uint32_t);
-
- printf("\n");
sc->sc_cd.cd_node = faa->fa_node;
sc->sc_cd.cd_cookie = sc;
@@ -115,15 +116,22 @@ pwmfan_set_cooling_level(void *cookie, u
struct pwmfan_softc *sc = cookie;
struct pwm_state ps;
- if (level == sc->sc_curlevel || level > sc->sc_nlevels ||
- sc->sc_levels[level] > 255)
+ if (level >= sc->sc_nlevels)
+ level = sc->sc_nlevels - 1;
+
+ if (level == sc->sc_curlevel ||
+ (sc->sc_levels && sc->sc_levels[level] > 255))
return;
if (pwm_init_state(sc->sc_pwm, &ps))
return;
sc->sc_curlevel = level;
- level = sc->sc_levels[level];
+
+ if (sc->sc_levels)
+ level = sc->sc_levels[level];
+
+ printf("%s: setting level to %u\n", sc->sc_dev.dv_xname, level);
ps.ps_enabled = level ? 1 : 0;
ps.ps_pulse_width = (ps.ps_period * level) / 255;
Index: arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
--- arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi.orig
+++ arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -69,6 +69,7 @@
fan: pwm-fan {
compatible = "pwm-fan";
+ cooling-levels = <0 4 8 12 16 20 24 28 32 36 40 44 48 52 56 60 64 68 72 76 80 84 88 92 96 100 104 108 112 116 120 124 128 132 136 140 144 148 152 156 160 164 168 172 176 180 184 188 192 196 200 204 208 212 216 220 224 228 232 236 240 244 248 255>;
#cooling-cells = <2>;
fan-supply = <&vcc12v_dcin>;
pwms = <&pwm1 0 50000 0>;
@@ -243,6 +244,61 @@
&cpu_b1 {
cpu-supply = <&vdd_cpu_b>;
+};
+
+&cpu_thermal {
+ trips {
+ /*cpu_cold: cpu_cold {
+ temperature = <45000>;
+ hysteresis = <2000>;
+ type = "active";
+ };*/
+
+ cpu_warm: cpu_warm {
+ temperature = <60000>;
+ hysteresis = <5000>;
+ type = "active";
+ };
+
+ cpu_hot: cpu_hot {
+ temperature = <70000>;
+ hysteresis = <2000>;
+ type = "active";
+ };
+
+ /* 80°C is max. allowed temperature at full cpu freq. */
+ cpu_crit: cpu_crit {
+ temperature = <80000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ /* spin the fan just a little so the harddisks wont get too warm */
+ /*map_cold {
+ trip = <&cpu_cold>;
+ cooling-device = <&fan 0 0>;
+ };*/
+
+ /* keep it warm for effective cooling at silent fan speeds. */
+ map_warm {
+ trip = <&cpu_warm>;
+ cooling-device = <&fan 12 17>;
+ };
+
+ /* At hot temperature allow full fan speed. */
+ map_hot {
+ trip = <&cpu_hot>;
+ cooling-device = <&fan 12 THERMAL_NO_LIMIT>;
+ };
+
+ /* We are too hot now. Nothing but full fan speed immediately. */
+ map_crit {
+ trip = <&cpu_crit>;
+ cooling-device = <&fan 63 THERMAL_NO_LIMIT>;
+ };
+ };
};
&emmc_phy {
