Re: [PATCH v3 12/14] arm64: dts: qcom: Add initial support for MSM8917

From: barnabas . czeman
Date: Sat Nov 09 2024 - 06:37:09 EST


On 2024-11-08 18:03, Konrad Dybcio wrote:
On 7.11.2024 6:02 PM, Barnabás Czémán wrote:
From: Otto Pflüger <otto.pflueger@xxxxxxxxx>

Add initial support for MSM8917 SoC.

Signed-off-by: Otto Pflüger <otto.pflueger@xxxxxxxxx>
[reword commit, rebase, fix schema errors]
Signed-off-by: Barnabás Czémán <barnabas.czeman@xxxxxxxxxxxxxx>
---

[...]

+ domain-idle-states {
+ cluster_pwrdn: cluster-gdhs {

Please rename these to cluster-sleep-<n> and sort from shallowest to
deepest sleep state, in this case: ret, pwrdn, pc

[...]

+
+ l2_0: l2-cache {
+ compatible = "cache";
+ cache-level = <2>;
+ cache-unified;
+ };

Please put this under the cpu0 node

[...]

+ restart@4ab000 {
+ compatible = "qcom,pshold";
+ reg = <0x4ab000 0x4>;

Please also pad all address parts to 8 hex digits with leading zeroes

[...]

+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-598000000 {
+ opp-hz = /bits/ 64 <598000000>;
+ };
+
+ opp-523200000 {
+ opp-hz = /bits/ 64 <523200000>;
+ };
+
+ opp-484800000 {
+ opp-hz = /bits/ 64 <484800000>;
+ };
+
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ };
+
+ opp-270000000 {
+ opp-hz = /bits/ 64 <270000000>;
+ };
+
+ opp-19200000 {
+ opp-hz = /bits/ 64 <19200000>;
+ };

Does the GPU actually function at 19.2 MHz? You can check this by removing
all other entries and starting some gpu workload
Yes

[...]

+ cpuss1-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;

You can remove polling-delay (not -passive), as we have an interrupt
that fires on threshold crossing

+
+ thermal-sensors = <&tsens 4>;
+
+ cooling-maps {
+ map0 {
+ trip = <&cpuss1_alert0>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+
+ trips {
+ cpuss1_crit: cpuss1-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+
+ cpuss1_alert0: trip-point0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpuss1_alert1: trip-point1 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };

Sorting these by temperature, rising would be nice

Konrad