Hello Heiko,
On 2024-03-02 12:25, Heiko Stuebner wrote:
Am Donnerstag, 29. Februar 2024, 20:26:32 CET schrieb Alexey Charkov:
Include thermal zones information in device tree for RK3588 variants.
This also enables the TSADC controller unconditionally on all boards
to ensure that thermal protections are in place via throttling and
emergency reset, once OPPs are added to enable CPU DVFS.
The default settings (using CRU as the emergency reset mechanism)
should work on all boards regardless of their wiring, as CRU resets
do not depend on any external components. Boards that have the TSHUT
signal wired to the reset line of the PMIC may opt to switch to GPIO
tshut mode instead (rockchip,hw-tshut-mode = <1>;)
It seems though that downstream kernels don't use that, even for
those boards where the wiring allows for GPIO based tshut, such as
Radxa Rock 5B [1], [2], [3]
[1]
https://github.com/radxa/kernel/blob/stable-5.10-rock5/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts#L540
[2]
https://github.com/radxa/kernel/blob/stable-5.10-rock5/arch/arm64/boot/dts/rockchip/rk3588s.dtsi#L5433
[3] https://dl.radxa.com/rock5/5b/docs/hw/radxa_rock_5b_v1423_sch.pdf
page 11 (TSADC_SHUT_H)
Signed-off-by: Alexey Charkov <alchark@xxxxxxxxx>
---
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 176
+++++++++++++++++++++++++++++-
1 file changed, 175 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 36b1b7acfe6a..9bf197358642 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -10,6 +10,7 @@
#include <dt-bindings/reset/rockchip,rk3588-cru.h>
#include <dt-bindings/phy/phy.h>
#include <dt-bindings/ata/ahci.h>
+#include <dt-bindings/thermal/thermal.h>
/ {
compatible = "rockchip,rk3588";
@@ -2225,7 +2226,180 @@ tsadc: tsadc@fec00000 {
pinctrl-1 = <&tsadc_shut>;
pinctrl-names = "gpio", "otpout";
#thermal-sensor-cells = <1>;
- status = "disabled";
+ status = "okay";
+ };
so I've skimmed over the general discussion, though don't have a hard
opinion in either direction yet. Still there are some low-hanging
fruit:
- having the thermal-zones addition in a separate patch would allow to
merge the obvious stuff, while this discussion is still ongoing
Very good suggestion.
- status=okay in a soc dtsi is wrong, because okay is the default
status
so if anything the status property should be removed
In general I'm not that much of a fan of things just working
implicitly.
So somehow, when someone submits a board devicetree, I expect them to
having ensured stuff is enabled somewhat ok. So even seeing a simple
&tsadc {
status = "okay"
};
suggests that they have at least noticed the existence of thermal
stuff.
I agree that having such additional "signed-off markers", so to speak,
in
a board dts is quite assuring. I mean, someone implementing a new dts
file
for a new board should simply know what needs to be done there, and
there
should be no excuses for not checking the thermal throttling stuff.
