linux-kernel - Re: [PATCH 1/3] thermal: qcom: lmh: Add support for sm8150

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 6 Dec 2021 09:22:21 -0500
From:   Thara Gopinath <thara.gopinath@...aro.org>
To:     Konrad Dybcio <konrad.dybcio@...ainline.org>, agross@...nel.org,
        bjorn.andersson@...aro.org, daniel.lezcano@...aro.org,
        rafael@...nel.org, rui.zhang@...el.com, robh+dt@...nel.org
Cc:     linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] thermal: qcom: lmh: Add support for sm8150

Hi Konrad,

Thanks for the review.

On 12/4/21 8:34 AM, Konrad Dybcio wrote:
> Hi,
> 
> On 02.12.2021 23:38, Thara Gopinath wrote:
>> Add compatible to support LMh for sm8150 SoC.
>> sm8150 does not require explicit enabling for various LMh subsystems.
>> Move this piece of code under condition that it is executed only
>> for sdm845 SoC.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@...aro.org>
>> ---
>>   drivers/thermal/qcom/lmh.c | 61 ++++++++++++++++++++------------------
>>   1 file changed, 32 insertions(+), 29 deletions(-)
> 
> [...]
> 
> 
>> -		return ret;
>> +	if (of_device_is_compatible(np, "qcom,sdm845-lmh")) {
>> +		if (!qcom_scm_lmh_dcvsh_available())
>> +			return -EINVAL;
> 
> I don't believe this is the correct approach, as different SoCs may
> 
> require different sequences of these writes (for example SDM660/MSM8998
> 
> seems to only enable the thermal algorithm), and there will (hopefully) be interest
> 
> in adding LMH support for more platforms, so perhaps separating this somehow
> 
> could keep this a bit cleaner and easier to work with for the next person..

I have not looked at SDM660/MSM8998. Are you telling me that these SoCs 
don't enable the current and BCL portion of LMh. Maybe they have an 
earlier version of Lmh which does not support all the features. The 
right approach in this case will be to add a match table with flags for 
init based on SoC. I can send v2, adding a match table with a flag to 
specify whether to do the init sequence or not. Since I am not adding 
the support for any other SoC at the moment, I cannot put in flags 
separating out thermal , current and BCL init.

> 
> 
> 
> Konrad
> 

-- 
Warm Regards
Thara (She/Her/Hers)