lkml.org 
[lkml]   [2024]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] arm64: dts: rockchip: enable built-in thermal monitoring on rk3588
    Hello Alexey,

    Please see my comments below.

    On 2024-01-06 23:23, Alexey Charkov wrote:
    > Include thermal zones information in device tree for rk3588 variants
    > and enable the built-in thermal sensing ADC on RADXA Rock 5B
    >
    > Signed-off-by: Alexey Charkov <alchark@gmail.com>
    > ---
    > .../boot/dts/rockchip/rk3588-rock-5b.dts | 4 +
    > arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 143 ++++++++++++++++++
    > 2 files changed, 147 insertions(+)
    >
    > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
    > b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
    > index a5a104131403..f9d540000de3 100644
    > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
    > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
    > @@ -772,3 +772,7 @@ &usb_host1_ehci {
    > &usb_host1_ohci {
    > status = "okay";
    > };
    > +
    > +&tsadc {
    > + status = "okay";
    > +};
    > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
    > b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
    > index 8aa0499f9b03..8235991e3112 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";
    > @@ -2112,6 +2113,148 @@ tsadc: tsadc@fec00000 {
    > status = "disabled";
    > };
    >
    > + thermal_zones: thermal-zones {
    > + soc_thermal: soc-thermal {

    It should be better to name it cpu_thermal instead. In the end, that's
    what it is.

    > + polling-delay-passive = <20>; /* milliseconds */
    > + polling-delay = <1000>; /* milliseconds */
    > + sustainable-power = <2100>; /* milliwatts */

    These three comments above are pretty much redundant.

    > +
    > + thermal-sensors = <&tsadc 0>;

    An empty line should be added here.

    > + trips {
    > + threshold: trip-point-0 {

    It should be better to name it cpu_alert0 instead, because that's what
    other newer dtsi files already use.

    > + temperature = <75000>;
    > + hysteresis = <2000>;
    > + type = "passive";
    > + };
    > + target: trip-point-1 {

    It should be better to name it cpu_alert1 instead, because that's what
    other newer dtsi files already use.

    > + temperature = <85000>;
    > + hysteresis = <2000>;
    > + type = "passive";
    > + };
    > + soc_crit: soc-crit {

    It should be better to name it cpu_crit instead, because that's what
    other newer dtsi files already use.

    > + /* millicelsius */
    > + temperature = <115000>;
    > + /* millicelsius */

    These two comments above are pretty much redundant. It also applies to
    all other similar comments below.

    > + hysteresis = <2000>;
    > + type = "critical";
    > + };
    > + };
    > + cooling-maps {
    > + map0 {
    > + trip = <&target>;

    Shouldn't &threshold (i.e. &cpu_alert0) be referenced here instead?

    > + cooling-device = <&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

    Shouldn't all big CPU cores be listed here instead?

    > + contribution = <1024>;
    > + };
    > + map1 {
    > + trip = <&target>;

    Shouldn't &target (i.e. &cpu_alert1) be referenced here instead?

    > + cooling-device = <&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

    Shouldn't all little and big CPU cores be listed here instead?

    > + contribution = <1024>;
    > + };
    > + map2 {
    > + trip = <&target>;
    > + cooling-device = <&cpu_b2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
    > + contribution = <1024>;
    > + };

    Isn't this cooling map redundant?

    > + };
    > + };
    > +
    > + bigcore0_thermal: bigcore0-thermal {
    > + polling-delay-passive = <20>; /* milliseconds */
    > + polling-delay = <1000>; /* milliseconds */
    > + thermal-sensors = <&tsadc 1>;
    > +
    > + trips {
    > + big0_crit: big0-crit {
    > + /* millicelsius */
    > + temperature = <115000>;
    > + /* millicelsius */
    > + hysteresis = <2000>;
    > + type = "critical";
    > + };
    > + };
    > + };
    > +
    > + bigcore1_thermal: bigcore1-thermal {
    > + polling-delay-passive = <20>; /* milliseconds */
    > + polling-delay = <1000>; /* milliseconds */
    > + thermal-sensors = <&tsadc 2>;
    > +
    > + trips {
    > + big1_crit: big1-crit {
    > + /* millicelsius */
    > + temperature = <115000>;
    > + /* millicelsius */
    > + hysteresis = <2000>;
    > + type = "critical";
    > + };
    > + };
    > + };
    > +
    > + little_core_thermal: littlecore-thermal {
    > + polling-delay-passive = <20>; /* milliseconds */
    > + polling-delay = <1000>; /* milliseconds */
    > + thermal-sensors = <&tsadc 3>;
    > +
    > + trips {
    > + little_crit: little-crit {
    > + /* millicelsius */
    > + temperature = <115000>;
    > + /* millicelsius */
    > + hysteresis = <2000>;
    > + type = "critical";
    > + };
    > + };
    > + };
    > +
    > + center_thermal: center-thermal {
    > + polling-delay-passive = <20>; /* milliseconds */
    > + polling-delay = <1000>; /* milliseconds */
    > + thermal-sensors = <&tsadc 4>;
    > +
    > + trips {
    > + center_crit: center-crit {
    > + /* millicelsius */
    > + temperature = <115000>;
    > + /* millicelsius */
    > + hysteresis = <2000>;
    > + type = "critical";
    > + };
    > + };
    > + };
    > +
    > + gpu_thermal: gpu-thermal {
    > + polling-delay-passive = <20>; /* milliseconds */
    > + polling-delay = <1000>; /* milliseconds */
    > + thermal-sensors = <&tsadc 5>;
    > +
    > + trips {
    > + gpu_crit: gpu-crit {
    > + /* millicelsius */
    > + temperature = <115000>;
    > + /* millicelsius */
    > + hysteresis = <2000>;
    > + type = "critical";
    > + };
    > + };
    > + };
    > +
    > + npu_thermal: npu-thermal {
    > + polling-delay-passive = <20>; /* milliseconds */
    > + polling-delay = <1000>; /* milliseconds */
    > + thermal-sensors = <&tsadc 6>;
    > +
    > + trips {
    > + npu_crit: npu-crit {
    > + /* millicelsius */
    > + temperature = <115000>;
    > + /* millicelsius */
    > + hysteresis = <2000>;
    > + type = "critical";
    > + };
    > + };
    > + };
    > + };
    > +
    > saradc: adc@fec10000 {
    > compatible = "rockchip,rk3588-saradc";
    > reg = <0x0 0xfec10000 0x0 0x10000>;

    \
     
     \ /
      Last update: 2024-01-06 23:55    [W:4.473 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site