| Message ID | 20210322100420.125616-1-robert.foss@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] dt-bindings: thermal: qcom-tsens: Add compatible for sm8350 | expand |
On 22-03-21, 11:04, Robert Foss wrote: > sm8350 has 29 thermal sensors split across two tsens controllers. Add > the thermal zones to expose them and wireup the cpus to throttle their > frequencies on crossing passive temperature thresholds. Reviewed-by: Vinod Koul <vkoul@kernel.org>
Hi! > + tsens0: thermal-sensor@c222000 { > + compatible = "qcom,sm8350-tsens", "qcom,tsens-v2"; > + reg = <0 0x0C263000 0 0x1ff>, /* TM */ > + <0 0x0C222000 0 0x8>; /* SROT */ Please use lowercase hex > + tsens1: thermal-sensor@c223000 { > + compatible = "qcom,sm8350-tsens", "qcom,tsens-v2"; > + reg = <0 0x0C265000 0 0x1ff>, /* TM */ > + <0 0x0c223000 0 0x8>; /* SROT */ Ditto > + trips { > + cpu0_alert0: trip-point0 { > + temperature = <90000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu0_alert1: trip-point1 { > + temperature = <95000>; > + hysteresis = <2000>; > + type = "passive"; Shouldn't this be "hot"? Possibly ditto for all cpu*alert1-labeled nodes. > + }; > + > + cpu0_crit: cpu_crit { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; These values seem, err.. scorching hot.. Are they alright? > + // TODO: What is the NSP subsystem? Please use C-style comments (/* foo */) Konrad
Hey Konrad, Thanks for the review! On Mon, 22 Mar 2021 at 18:27, Konrad Dybcio <konrad.dybcio@somainline.org> wrote: > > Hi! > > > > + tsens0: thermal-sensor@c222000 { > > + compatible = "qcom,sm8350-tsens", "qcom,tsens-v2"; > > + reg = <0 0x0C263000 0 0x1ff>, /* TM */ > > + <0 0x0C222000 0 0x8>; /* SROT */ > > Please use lowercase hex Ack > > > > + tsens1: thermal-sensor@c223000 { > > + compatible = "qcom,sm8350-tsens", "qcom,tsens-v2"; > > + reg = <0 0x0C265000 0 0x1ff>, /* TM */ > > + <0 0x0c223000 0 0x8>; /* SROT */ > > Ditto Ack > > > > + trips { > > + cpu0_alert0: trip-point0 { > > + temperature = <90000>; > > + hysteresis = <2000>; > > + type = "passive"; > > + }; > > + > > + cpu0_alert1: trip-point1 { > > + temperature = <95000>; > > + hysteresis = <2000>; > > + type = "passive"; > > Shouldn't this be "hot"? Possibly ditto for all cpu*alert1-labeled nodes. I based this patch on the upstream DTS for sm8250 & sdm845, and this is what they use. However, if you think it is incorrect I'm happy to do a little digging. > > > > + }; > > + > > + cpu0_crit: cpu_crit { > > + temperature = <110000>; > > + hysteresis = <1000>; > > + type = "critical"; > > + }; > > + }; > > These values seem, err.. scorching hot.. Are they alright? I agree :) This is what the vendor ships in their downstream DTS. > > > > > + // TODO: What is the NSP subsystem? > Please use C-style comments (/* foo */) Removing comment.
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml index 95462e071ab4..e788378eff8d 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml @@ -43,6 +43,7 @@ properties: - qcom,sdm845-tsens - qcom,sm8150-tsens - qcom,sm8250-tsens + - qcom,sm8350-tsens - const: qcom,tsens-v2 reg:
Add tsens bindings for sm8350. Signed-off-by: Robert Foss <robert.foss@linaro.org> --- Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 1 + 1 file changed, 1 insertion(+) -- 2.27.0