Re: [PATCH v5 2/4] arm64: dts: ti: j721e: Add VTM node

From: Vignesh Raghavendra
Date: Tue Apr 12 2022 - 13:10:17 EST


Hi Keerthy

On 12/04/22 3:44 pm, Keerthy wrote:
> VTM stands for Voltage Thermal Management
>
> Signed-off-by: Keerthy <j-keerthy@xxxxxx>
> ---
> .../boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 9 +++
> arch/arm64/boot/dts/ti/k3-j721e-thermal.dtsi | 73 +++++++++++++++++++
> arch/arm64/boot/dts/ti/k3-j721e.dtsi | 4 +
> 3 files changed, 86 insertions(+)
> create mode 100644 arch/arm64/boot/dts/ti/k3-j721e-thermal.dtsi
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
> index b4972dfb7da8..6290f563b8e7 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
> @@ -418,4 +418,13 @@
> interrupt-names = "int0", "int1";
> bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
> };
> +
> + wkup_vtm0: wkup_vtm0@42040000 {
> + compatible = "ti,j721e-vtm";
> + reg = <0x0 0x42040000 0x0 0x350>,
> + <0x0 0x42050000 0x0 0x350>,
> + <0x0 0x43000300 0x0 0x10>;

Please follow convention of using 0x00 as rest of the file:

reg = <0x00 0x42040000 0x00 0x350>,
....

> + power-domains = <&k3_pds 154 TI_SCI_PD_EXCLUSIVE>;
> + #thermal-sensor-cells = <1>;
> + };
> };
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-thermal.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-thermal.dtsi
> new file mode 100644
> index 000000000000..e922042f356f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-thermal.dtsi
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <dt-bindings/thermal/thermal.h>
> +
> +wkup_thermal: wkup_thermal {
> + polling-delay-passive = <250>; /* milliseconds */
> + polling-delay = <500>; /* milliseconds */
> + thermal-sensors = <&wkup_vtm0 0>;
> +
> + trips {
> + wkup_crit: wkup_crit {


Here and elsewhere no "-" in node names please.

You can use:

https://github.com/nmenon/kernel_patch_verify
kpv -C -n <no of commits>

This catches most issues like above

> + temperature = <125000>; /* milliCelsius */
> + hysteresis = <2000>; /* milliCelsius */
> + type = "critical";
> + };
> + };
> +};
> +
> +mpu_thermal: mpu_thermal {
> + polling-delay-passive = <250>; /* milliseconds */
> + polling-delay = <500>; /* milliseconds */
> + thermal-sensors = <&wkup_vtm0 1>;
> +
> + trips {
> + mpu_crit: mpu_crit {
> + temperature = <125000>; /* milliCelsius */
> + hysteresis = <2000>; /* milliCelsius */
> + type = "critical";
> + };
> + };
> +};
> +
> +c7x_thermal: c7x_thermal {
> + polling-delay-passive = <250>; /* milliseconds */
> + polling-delay = <500>; /* milliseconds */
> + thermal-sensors = <&wkup_vtm0 2>;
> +
> + trips {
> + c7x_crit: c7x_crit {
> + temperature = <125000>; /* milliCelsius */
> + hysteresis = <2000>; /* milliCelsius */
> + type = "critical";
> + };
> + };
> +};
> +
> +gpu_thermal: gpu_thermal {
> + polling-delay-passive = <250>; /* milliseconds */
> + polling-delay = <500>; /* milliseconds */
> + thermal-sensors = <&wkup_vtm0 3>;
> +
> + trips {
> + gpu_crit: gpu_crit {
> + temperature = <125000>; /* milliCelsius */
> + hysteresis = <2000>; /* milliCelsius */
> + type = "critical";
> + };
> + };
> +};
> +
> +r5f_thermal: r5f_thermal {
> + polling-delay-passive = <250>; /* milliseconds */
> + polling-delay = <500>; /* milliseconds */
> + thermal-sensors = <&wkup_vtm0 4>;
> +
> + trips {
> + r5f_crit: r5f_crit {
> + temperature = <125000>; /* milliCelsius */
> + hysteresis = <2000>; /* milliCelsius */
> + type = "critical";
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> index 0e23886c9fd1..6979863eb500 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> @@ -181,6 +181,10 @@
> <0x07 0x00000000 0x07 0x00000000 0x01 0x00000000>; /* FSS OSPI1 data region 3*/
> };
> };
> +
> + thermal_zones: thermal-zones {
> + #include "k3-j721e-thermal.dtsi"
> + };

Bit weird representation, any reason why thermal-zones {} cannot be
moved into k3-j721e-thermal.dtsi?

> };
>
> /* Now include the peripherals for each bus segments */


Regards
Vignesh