Re: [PATCH v1 2/2] arm64: dts: qcom: Enable TSENS support for QCS615 SoC

[Gaurav Kohli]

On 4/14/2025 3:23 PM, Konrad Dybcio wrote:
> On 4/14/25 10:28 AM, Gaurav Kohli wrote:
> > thanks for review!
> >
> > On 4/12/2025 5:13 AM, Konrad Dybcio wrote:
> > > On 4/10/25 4:00 PM, Gaurav Kohli wrote:
> > > > Add TSENS and thermal devicetree node for QCS615 SoC.
> > > >
> > > > Signed-off-by: Gaurav Kohli <quic_gkohli@xxxxxxxxxxx>
> > > > ---
> > >
> > > subject: "arm64: dts: qcom: qcs615: ..">  arch/arm64/boot/dts/qcom/qcs615.dtsi | 281 +++++++++++++++++++++++++++
> > > >    1 file changed, 281 insertions(+)
> > will fix this.
> > > > diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> > > > index edfb796d8dd3..f0d8aed7da29 100644
> > > > --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> > > > @@ -3668,6 +3668,17 @@ usb_2_dwc3: usb@a800000 {
> > > >                    maximum-speed = "high-speed";
> > > >                };
> > > >            };
> > > > +
> > > > +        tsens0: tsens@c222000 {
> > > > +            compatible = "qcom,qcs615-tsens", "qcom,tsens-v2";
> > > > +            reg = <0x0 0xc263000 0x0 0x1ff>,
> > > > +                <0x0 0xc222000 0x0 0x8>;
> > > Pad the address part to 8 hex digits with leading zeroes> +            interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
> > >
> > > &pdc 26
> > >
> > > > +                    <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > &pdc 28
> > we don't want to mark this as wake up capable, so using this approach.
>
> Why not?
Intention was to avoid wake up, as system is already in lowest state, 
please let me know if you see any concern here.
> > > > +
> > > > +        cpuss-0-thermal {
> > > > +            thermal-sensors = <&tsens0 1>;
> > > > +
> > > > +            trips {
> > > > +
> > > > +                trip-point0 {
> > > > +                    temperature = <115000>;
> > > > +                    hysteresis = <5000>;
> > > > +                    type = "passive";
> > > > +                };
> > > > +
> > > > +                trip-point1 {
> > > > +                    temperature = <118000>;
> > > > +                    hysteresis = <5000>;
> > > > +                    type = "passive";
> > > > +                };
> > >
> > > Please drop the passive trip point for the *CPU* tzones, see
> >
> > we are using trip-point 0 for cpu idle injection mitigation which i will add in subsequent patches, if you are fine i will add cpu idle injection cooling map in this series only ?
>
> The folks working on qcs9xxx have made this point too, but I'm lukewarm
> on duplicating meaningless dt description everywhere. I've asked them to
> conduct some measurements on whether random default settings (that would
> be preset in the driver and require no additional dt fluff) show any
> significant difference - if not, we can save up on boilerplate.
>
> So let's wait to hear back from them on this.
Sure will wait for latest update.
> > > commit 06eadce936971dd11279e53b6dfb151804137836
> > > ("arm64: dts: qcom: x1e80100: Drop unused passive thermal trip points for CPU")
> > >
> > > and add a single critical point instead, see
> > As critical shutdown is already supported by hardware, so we are not defining here.
>
> The hardware critical shutdown will literally pull the plug out with the OS
> having no chance to sync the filesystem etc.
>
> Please define one that's like 5 degC below the hardware limit, so that the
> operating system can try to take some steps to avoid data loss

Sure will post critical in next patch.
> Konrad