Hey Konrad,
Thanks for the review!
On Mon, 22 Mar 2021 at 18:27, Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxx> 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.
