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

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] [day] [month] [year] [list]

Message-ID: <3df9f397-e97c-4224-a388-df6fe211778d@quicinc.com>
Date: Thu, 17 Apr 2025 10:47:25 +0530
From: Gaurav Kohli <quic_gkohli@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, <amitk@...nel.org>,
        <rafael@...nel.org>, <daniel.lezcano@...aro.org>,
        <rui.zhang@...el.com>, <lukasz.luba@....com>, <robh@...nel.org>,
        <krzk+dt@...nel.org>, <andersson@...nel.org>, <konradybcio@...nel.org>
CC: <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <quic_manafm@...cinc.com>
Subject: Re: [PATCH v1 2/2] arm64: dts: qcom: Enable TSENS support for QCS615
 SoC



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@...cinc.com>
>>>> ---
>>>
>>> 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@...0000 {
>>>>                    maximum-speed = "high-speed";
>>>>                };
>>>>            };
>>>> +
>>>> +        tsens0: tsens@...2000 {
>>>> +            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