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

[Krzysztof Kozlowski]

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@fc880000 {
>>  			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@fd484000 {
>>  			compatible = "syscon";
>> -			reg = <0xfd484000 0x2000>;
>> +			reg = <0xfd484000 0x1000>;
>>  		};
>>  
>> -		tcsr: syscon@fd4a0000 {
>> +		tcsr_1: syscon@fd485000 {
> 
> 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@fd484000
tcsr_mutex_2: syscon@fd485000
tcsr: syscon@fd4a0000
?

> 
> 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@fd4a0000 {
> 
> And I would like to keep this as "tcsr".



Best regards,
Krzysztof