linux-kernel - Re: [PATCH v3 10/15] ARM: dts: qcom: msm8974: split TCSR halt regs out of mutex

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:   Thu, 15 Sep 2022 15:49:37 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Bjorn Andersson <andersson@...nel.org>
Cc:     Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Ohad Ben-Cohen <ohad@...ery.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Vinod Koul <vkoul@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-remoteproc@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 10/15] ARM: dts: qcom: msm8974: split TCSR halt regs
 out of mutex

On 13/09/2022 23:44, Bjorn Andersson wrote:
> On Fri, Sep 09, 2022 at 11:20:30AM +0200, Krzysztof Kozlowski wrote:
> [..]
>> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> index 90a6d4b7605c..ada232bed2c8 100644
>> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
>> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> @@ -1189,7 +1189,7 @@ remoteproc_mss: remoteproc@...80000 {
>>  			resets = <&gcc GCC_MSS_RESTART>;
>>  			reset-names = "mss_restart";
>>  
>> -			qcom,halt-regs = <&tcsr_mutex_block 0x1180 0x1200 0x1280>;
>> +			qcom,halt-regs = <&tcsr_1 0x180 0x200 0x280>;
>>  
>>  			qcom,smem-states = <&modem_smp2p_out 0>;
>>  			qcom,smem-state-names = "stop";
>> @@ -1230,10 +1230,15 @@ smd-edge {
>>  
>>  		tcsr_mutex_block: syscon@...84000 {
>>  			compatible = "syscon";
>> -			reg = <0xfd484000 0x2000>;
>> +			reg = <0xfd484000 0x1000>;
>>  		};
>>  
>> -		tcsr: syscon@...a0000 {
>> +		tcsr_1: syscon@...85000 {
> 
> While the accessed registers look general purpose in nature, I would
> prefer that we stick with naming it based on the register blocks - and
> this is part of what's named "tcsr_mutex".

Then everything would be like:

tcsr_mutex_1: syscon@...84000
tcsr_mutex_2: syscon@...85000
tcsr: syscon@...a0000
?

> 
> Is it not possible to claim that this region is a
> "qcom,msm8974-tcsr-mutex" and a "syscon"?

Hm, yes, that's another approach. We can go this way, but it has one
drawback - you could have two different devices (mutex and syscon user)
poking to the same registers. The regmap makes it safe from concurrency
point of view, but not safe from logic point of view.

Splitting these makes it sure, that no one touches hwlock registers,
except the hwlock driver.

Any preference?

> 
>> +			compatible = "qcom,tcsr-msm8974", "syscon";
>> +			reg = <0xfd485000 0x1000>;
>> +		};
>> +
>> +		tcsr_2: syscon@...a0000 {
> 
> And I would like to keep this as "tcsr".



Best regards,
Krzysztof