From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.4 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 25976C4338F for ; Thu, 29 Jul 2021 13:48:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 07C3660F00 for ; Thu, 29 Jul 2021 13:48:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237774AbhG2Nsw (ORCPT ); Thu, 29 Jul 2021 09:48:52 -0400 Received: from m-r2.th.seeweb.it ([5.144.164.171]:59517 "EHLO m-r2.th.seeweb.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237712AbhG2Nsv (ORCPT ); Thu, 29 Jul 2021 09:48:51 -0400 Received: from IcarusMOD.eternityproject.eu (unknown [2.237.20.237]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by m-r2.th.seeweb.it (Postfix) with ESMTPSA id 28A2A3F403; Thu, 29 Jul 2021 15:48:47 +0200 (CEST) Subject: Re: [PATCH 14/39] arm64: dts: qcom: sdm630: Add TSENS node To: Thara Gopinath , Konrad Dybcio , ~postmarketos/upstreaming@lists.sr.ht Cc: martin.botka@somainline.org, marijn.suijten@somainline.org, jamipkettunen@somainline.org, Andy Gross , Bjorn Andersson , Amit Kucheria , Zhang Rui , Daniel Lezcano , Rob Herring , linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: <20210728222542.54269-1-konrad.dybcio@somainline.org> <20210728222542.54269-15-konrad.dybcio@somainline.org> <860f1120-c5a4-f531-3ea9-aa90c6b063dc@linaro.org> <2318377c-959a-a42b-81b5-44e2629570d5@somainline.org> From: AngeloGioacchino Del Regno Message-ID: <2ffc4e54-a501-bd2d-3f29-a6df34023445@somainline.org> Date: Thu, 29 Jul 2021 15:48:46 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org Il 29/07/21 13:14, Thara Gopinath ha scritto: > > > On 7/29/21 6:55 AM, Konrad Dybcio wrote: >> >> On 29.07.2021 12:54, Thara Gopinath wrote: >>> >>> >>> On 7/29/21 6:52 AM, Konrad Dybcio wrote: >>>> >>>> On 29.07.2021 12:50, Thara Gopinath wrote: >>>>> Hi Konrad, >>>>> >>>>> On 7/28/21 6:25 PM, Konrad Dybcio wrote: >>>>>> This will enable temperature reporting for various SoC >>>>>> components. >>>>>> >>>>>> Signed-off-by: AngeloGioacchino Del Regno >>>>>> >>>>>> Signed-off-by: Konrad Dybcio >>>>>> --- >>>>>>     .../devicetree/bindings/thermal/qcom-tsens.yaml       |  1 + >>>>>>     arch/arm64/boot/dts/qcom/sdm630.dtsi                  | 11 +++++++++++ >>>>>>     2 files changed, 12 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >>>>>> b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >>>>>> index 4a2eaf28e3fd..d3b9e9b600a2 100644 >>>>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >>>>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >>>>>> @@ -48,6 +48,7 @@ properties: >>>>>>                   - qcom,sc7180-tsens >>>>>>                   - qcom,sc7280-tsens >>>>>>                   - qcom,sc8180x-tsens >>>>>> +              - qcom,sdm630-tsens >>>>>>                   - qcom,sdm845-tsens >>>>>>                   - qcom,sm8150-tsens >>>>>>                   - qcom,sm8250-tsens >>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi >>>>>> b/arch/arm64/boot/dts/qcom/sdm630.dtsi >>>>>> index 1e54828817d5..7e9c80e35fba 100644 >>>>>> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi >>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi >>>>>> @@ -627,6 +627,17 @@ mnoc: interconnect@1745000 { >>>>>>                      <&mmcc AHB_CLK_SRC>; >>>>>>             }; >>>>>>     +        tsens: thermal-sensor@10ae000 { >>>>>> +            compatible = "qcom,sdm630-tsens", "qcom,tsens-v2"; >>>>>> +            reg = <0x010ae000 0x1000>, /* TM */ >>>>>> +                  <0x010ad000 0x1000>; /* SROT */ >>>>>> +            #qcom,sensors = <12>; >>>>> >>>>> Are all 12 sensors used ? I see that in a later patch "arm64: dts: qcom: >>>>> sdm630: Add thermal-zones configuration" only 9 are used. >>>> >>>> Hi, >>>> >>>> if I recall correctly, they all give output but not all of the mappings were >>>> documented in the downstream sources and we have no documentation whatsoever :( >>> >>> Right. In that case, why not change #qcom,sensors to 9 and add rest of the >>> sensors if and when needed ? >>> >> I don't think it makes sense to describe the hardware incorrectly, even if some >> of it is unused. > > My thinking was more along the lines of don't expose unused h/w bits. > You're right about not exposing unused HW bits, but even PC x86 motherboards (I mean the smbus/i2c drivers for the big holy management/sensors chips) do have such a "base" configuration, where some lines are read as 0 because they are effectively not connected by hardware. In order to avoid confusion to other developers, in my personal opinion, it would be good go for the current value of 12 (which isn't incorrect, as that's what the SoC supports)... I don't think that anyone would be confused by seeing zero readings on some sensors (if their device don't support such sensor), as I think that everyone is used to that anyway, even if that's in other circumstances... In any case, luckily that's also safe, because there's no firmware that restricts the readings to a subset of sensors in this domain (nobody is going to get a hypervisor fault for that). I would also, in case, propose to see how things go: I would expect other developers to push device trees for many SDM630/636/660 devices, including but not limited to smartphones and SBCs.. so perhaps if we find out that really nobody uses the 12 sensors, or if the very vast majority uses a different amount, perhaps we may just transfer the value to device-specific configurations in one go, as to avoid unnecessary noise... I think :))) >> >> >> >