Re: [PATCH v10] arm64: dts: qcom: ipq5018: Add tsens node

[George Moussalem]

On 6/10/25 17:08, Konrad Dybcio wrote:
> On 6/10/25 3:03 PM, George Moussalem via B4 Relay wrote:
> > From: Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx>
> >
> > IPQ5018 has tsens V1.0 IP with 5 sensors, though 4 are in use.
> > There is no RPM, so tsens has to be manually enabled. Adding the tsens
> > and nvmem nodes and adding 4 thermal sensors (zones). With the
> > critical temperature being 120'C and action is to reboot.
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > Signed-off-by: Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx>
> > Signed-off-by: George Moussalem <george.moussalem@xxxxxxxxxxx>
> > ---
>
> [...]
>
> > +		tsens: thermal-sensor@4a9000 {
> > +			compatible = "qcom,ipq5018-tsens";
> > +			reg = <0x004a9000 0x1000>, /* TM */
> > +			      <0x004a8000 0x1000>; /* SROT */
>
> Please drop these comments

will drop

> [...]
>
> > +	thermal-zones {
> > +		cpu-thermal {
> > +			polling-delay-passive = <0>;
> > +			polling-delay = <0>;
> > +			thermal-sensors = <&tsens 2>;
> > +
> > +			trips {
> > +				cpu-critical {
> > +					temperature = <120000>;
> > +					hysteresis = <2>;
> > +					type = "critical";
> > +				};
> > +			};
> > +		};
> > +
> > +		gephy-thermal {
> > +			polling-delay-passive = <0>;
> > +			polling-delay = <0>;
> > +			thermal-sensors = <&tsens 4>;
> > +
> > +			trips {
> > +				gephy-critical {
> > +					temperature = <120000>;
>
> I'm not sure whether there's any firmware/hardware measure to shut
> down these beforehand. It's better to have a software trip at 120C
> than to not have any at all, but you may want to try and find a
> case_therm or so thermistor for your devices..

these are the temps as defined for the critical trips by qcom. IPQ8074 
has the temp set at 110C, while IPQ6018 even higher at 125C.
They've also defined lower temp trips in the downstream DTS files.
Should I try and add those? I could check if the driver supports them..

FWIW, devices I've tested don't go above 70C even under load.

> > +					hysteresis = <2>;
> > +					type = "critical";
> > +				};
> > +			};
> > +		};
> > +
> > +		top-glue-thermal {
> > +			polling-delay-passive = <0>;
> > +			polling-delay = <0>;
>
> 0 is default here, you can drop all such entries

will drop

> > +			thermal-sensors = <&tsens 3>;
> > +
> > +			trips {
> > +				top_glue-critical {
>
> underscores are not allowed in DTs (see:
> Documentation/devicetree/bindings/dts-coding-style.rst)
>
> use a hyphen instead

will update to hyphen, thanks!

> Konrad

Best regards,
George