linux-kernel - Re: [PATCH v4 4/6] arm64: dts: qcom: Add initial support for MSM8937

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <93ea35691deaa1ff38d229225e26cf41@mainlining.org>
Date: Thu, 17 Apr 2025 08:20:36 +0200
From: barnabas.czeman@...nlining.org
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>, Michael Turquette
 <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Stephan Gerhold <stephan@...hold.net>,
 Otto Pflüger <otto.pflueger@...cue.de>, Linus Walleij
 <linus.walleij@...aro.org>, Lee Jones <lee@...nel.org>, Srinivas Kandagatla
 <srinivas.kandagatla@...aro.org>, Joerg Roedel <joro@...tes.org>, Will
 Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>, Konrad Dybcio
 <konradybcio@...nel.org>, Rob Clark <robdclark@...il.com>, Sean Paul
 <sean@...rly.run>, Abhinav Kumar <quic_abhinavk@...cinc.com>, Marijn Suijten
 <marijn.suijten@...ainline.org>, David Airlie <airlied@...il.com>, Simona
 Vetter <simona@...ll.ch>, Maarten Lankhorst
 <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>, Dmitry Baryshkov
 <lumag@...nel.org>, linux-arm-msm@...r.kernel.org,
 linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
 iommu@...ts.linux.dev, dri-devel@...ts.freedesktop.org,
 freedreno@...ts.freedesktop.org, phone-devel@...r.kernel.org,
 ~postmarketos/upstreaming@...ts.sr.ht, linux@...nlining.org, Dang Huynh
 <danct12@...eup.net>
Subject: Re: [PATCH v4 4/6] arm64: dts: qcom: Add initial support for MSM8937

On 2025-04-14 22:55, Konrad Dybcio wrote:
> On 3/15/25 3:57 PM, Barnabás Czémán wrote:
>> From: Dang Huynh <danct12@...eup.net>
>> 
>> Add initial support for MSM8937 SoC.
>> 
>> Signed-off-by: Dang Huynh <danct12@...eup.net>
>> Co-developed-by: Barnabás Czémán <barnabas.czeman@...nlining.org>
>> Signed-off-by: Barnabás Czémán <barnabas.czeman@...nlining.org>
>> ---
> 
> [...]
> 
>> +			power-domains = <&cpu_pd0>;
>> +			power-domain-names = "psci";
> 
> So CPU4-7 get "nicer" idle, but 0-3 don't?
> 
> [...]
> 
>> +		cpu-map {
>> +			/* The MSM8937 has 2 cluster A53 setup. */
> 
> This comment seems superfluous
> 
> [...]
> 
>> +	timer {
> 
> 'p' < 't', please sort top-level nodes alphabetically
> 
> [...]
> 
>> +				wcss-wlan2-pins {
>> +					pins = "gpio76";
>> +					function = "wcss_wlan2";
>> +					drive-strength = <6>;
> 
> please unify this order (drive-strength before bias)
> 
>> +					bias-pull-up;
>> +
>> +				};
> 
> Extra newline
> 
> [...]
> 
>> +		gpu: gpu@...0000 {
>> +			compatible = "qcom,adreno-505.0", "qcom,adreno";
>> +			reg = <0x1c00000 0x40000>;
>> +			reg-names = "kgsl_3d0_reg_memory";
>> +			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "kgsl_3d0_irq";
>> +			#cooling-cells = <2>;
>> +			clocks = <&gcc GCC_OXILI_GFX3D_CLK>,
>> +				<&gcc GCC_OXILI_AHB_CLK>,
>> +				<&gcc GCC_BIMC_GFX_CLK>,
>> +				<&gcc GCC_BIMC_GPU_CLK>,
>> +				<&gcc GCC_OXILI_TIMER_CLK>,
>> +				<&gcc GCC_OXILI_AON_CLK>;
> 
> Please align the <s
> 
>> +			clock-names = "core",
>> +				      "iface",
>> +				      "mem_iface",
>> +				      "alt_mem_iface",
>> +				      "rbbmtimer",
>> +				      "alwayson";
>> +			operating-points-v2 = <&gpu_opp_table>;
>> +			power-domains = <&gcc OXILI_GX_GDSC>;
>> +
>> +			iommus = <&adreno_smmu 0>;
>> +
>> +			status = "disabled";
>> +
>> +			gpu_opp_table: opp-table {
>> +				compatible = "operating-points-v2";
>> +
>> +				opp-19200000 {
>> +					opp-hz = /bits/ 64 <19200000>;
>> +					opp-supported-hw = <0xFF>;
> 
> 0xff is overly broad, please document the existing known speed bins
There are no speedbins for 8937 gpu. 8940 have but 8940 is not scope of 
this series.
> 
> [...]
> 
>> +		adreno_smmu: iommu@...0000 {
>> +			compatible = "qcom,msm8996-smmu-v2",
>> +				     "qcom,adreno-smmu",
>> +				     "qcom,smmu-v2";
>> +			reg = <0x1c40000 0x10000>;
> 
> Does it work as-is, without iommu changes?
> 
> [...]
> 
>> +	thermal_zones: thermal-zones {
>> +		aoss-thermal {
>> +			polling-delay-passive = <250>;
> 
> There are no passive trip points> +
>> +			thermal-sensors = <&tsens 0>;
>> +
>> +			trips {
>> +				aoss_alert0: trip-point0 {
>> +					temperature = <85000>;
>> +					hysteresis = <2000>;
>> +					type = "hot";
>> +				};
> 
> Please convert these to 'critical' instead
> 
> [...]
> 
>> +		cpuss1-thermal {
>> +			polling-delay-passive = <250>;
> 
> You can drop polling-delay-passive under CPU tzones, as threshold
> crossing is interrupt-driven
> 
>> +
>> +			thermal-sensors = <&tsens 4>;
>> +
>> +			cooling-maps {
>> +				map0 {
>> +					trip = <&cpuss1_alert0>;
>> +					cooling-device = <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +				};
>> +			};
>> +
>> +			trips {
>> +				cpuss1_alert0: trip-point0 {
>> +					temperature = <75000>;
>> +					hysteresis = <2000>;
>> +					type = "passive";
>> +				};
>> +
>> +				cpuss1_alert1: trip-point1 {
>> +					temperature = <85000>;
>> +					hysteresis = <2000>;
>> +					type = "hot";
>> +				};
> 
> On newer platforms we rely on LMH to shut down the device if it
> were to reach the junction temperature, but let's leave them here
> as probably no one remembers for sure how reliable that is on these
> older platforms and you're most likely not willing to test that
> 
> Konrad