mbox series

[v4,0/3] ARM/hwlock: qcom: switch TCSR mutex to MMIO (msm8974)

Message ID 20220920150414.637634-1-krzysztof.kozlowski@linaro.org
Headers show
Series ARM/hwlock: qcom: switch TCSR mutex to MMIO (msm8974) | expand

Message

Krzysztof Kozlowski Sept. 20, 2022, 3:04 p.m. UTC
Hi,

Remaining patches from v3:
https://lore.kernel.org/all/20220909092035.223915-1-krzysztof.kozlowski@linaro.org/

Not tested on hardware. Please kindly provide tests.

Changes since v3
================
1. Drop applied patches - remaining is only msm8974.
2. Add syscon to TCSR mutex regs, after talk with Bjorn.
3. New patch: bindings.

Changes since v2
================
1. Rebase on current MFD changes.
2. Add Rb tag.
3. Split MFD patch to separate patchset:
https://lore.kernel.org/linux-devicetree/20220909091056.128949-1-krzysztof.kozlowski@linaro.org/T/#u

Changes since v1
================
1. Use existing qcom,tcsr-msm8974 compatible.
2. Fix few other TCSR syscon compatibles (new patches: ipq6018, msm8953,
   qcs404, msm8996).
3. New patch: dt-bindings: mfd: qcom,tcsr: drop simple-mfd from IPQ6018
4. New patch: dt-bindings: mfd: qcom,tcsr: add QCS404

Dependencies
============
1. DT bindings and driver patches can go via hwlock. DTS via Bjorn/Qualcomm.

2. The last five DTS commits (ARM and arm64) named "switch TCSR mutex to MMIO"
   depend on driver support. The changes are not bisectable, just like
   previously such changes were not bisectable:
   https://lore.kernel.org/all/20200622075956.171058-5-bjorn.andersson@linaro.org/
   Therefore these changes could wait for next release.

Best regards,
Krzysztof

Krzysztof Kozlowski (3):
  dt-bindings: hwlock: qcom-hwspinlock: add syscon to MSM8974
  ARM: dts: qcom: msm8974: add missing TCSR syscon compatible
  ARM: dts: qcom: msm8974: switch TCSR mutex to MMIO

 .../bindings/hwlock/qcom-hwspinlock.yaml         |  6 +++++-
 arch/arm/boot/dts/qcom-msm8974.dtsi              | 16 +++++-----------
 2 files changed, 10 insertions(+), 12 deletions(-)

Comments

Luca Weiss Sept. 21, 2022, 7:09 p.m. UTC | #1
Hi Krzysztof,

On Dienstag, 20. September 2022 17:04:11 CEST Krzysztof Kozlowski wrote:
> Hi,
> 
> Remaining patches from v3:
> https://lore.kernel.org/all/20220909092035.223915-1-krzysztof.kozlowski@lina
> ro.org/
> 
> Not tested on hardware. Please kindly provide tests.

With these patches on top of 5.19.9 everything incl. modem still seems to work 
fine on msm8974pro-fairphone-fp2:

(2/3 & 3/3 from this series)
ARM: dts: qcom: msm8974: add missing TCSR syscon compatible
ARM: dts: qcom: msm8974: switch TCSR mutex to MMIO

(picked from linux-next)
hwspinlock: qcom: Add support for mmio usage to sfpb-mutex
hwspinlock: qcom: correct MMIO max register for newer SoCs
hwspinlock: qcom: add support for MMIO on older SoCs

Tested-by: Luca Weiss <luca@z3ntu.xyz> # fairphone-fp2

Regards
Luca

> 
> Changes since v3
> ================
> 1. Drop applied patches - remaining is only msm8974.
> 2. Add syscon to TCSR mutex regs, after talk with Bjorn.
> 3. New patch: bindings.
> 
> Changes since v2
> ================
> 1. Rebase on current MFD changes.
> 2. Add Rb tag.
> 3. Split MFD patch to separate patchset:
> https://lore.kernel.org/linux-devicetree/20220909091056.128949-1-krzysztof.k
> ozlowski@linaro.org/T/#u
> 
> Changes since v1
> ================
> 1. Use existing qcom,tcsr-msm8974 compatible.
> 2. Fix few other TCSR syscon compatibles (new patches: ipq6018, msm8953,
>    qcs404, msm8996).
> 3. New patch: dt-bindings: mfd: qcom,tcsr: drop simple-mfd from IPQ6018
> 4. New patch: dt-bindings: mfd: qcom,tcsr: add QCS404
> 
> Dependencies
> ============
> 1. DT bindings and driver patches can go via hwlock. DTS via Bjorn/Qualcomm.
> 
> 2. The last five DTS commits (ARM and arm64) named "switch TCSR mutex to
> MMIO" depend on driver support. The changes are not bisectable, just like
> previously such changes were not bisectable:
>   
> https://lore.kernel.org/all/20200622075956.171058-5-bjorn.andersson@linaro.
> org/ Therefore these changes could wait for next release.
> 
> Best regards,
> Krzysztof
> 
> Krzysztof Kozlowski (3):
>   dt-bindings: hwlock: qcom-hwspinlock: add syscon to MSM8974
>   ARM: dts: qcom: msm8974: add missing TCSR syscon compatible
>   ARM: dts: qcom: msm8974: switch TCSR mutex to MMIO
> 
>  .../bindings/hwlock/qcom-hwspinlock.yaml         |  6 +++++-
>  arch/arm/boot/dts/qcom-msm8974.dtsi              | 16 +++++-----------
>  2 files changed, 10 insertions(+), 12 deletions(-)
Krzysztof Kozlowski Sept. 21, 2022, 7:18 p.m. UTC | #2
On 21/09/2022 21:09, Luca Weiss wrote:
> Hi Krzysztof,
> 
> On Dienstag, 20. September 2022 17:04:11 CEST Krzysztof Kozlowski wrote:
>> Hi,
>>
>> Remaining patches from v3:
>> https://lore.kernel.org/all/20220909092035.223915-1-krzysztof.kozlowski@lina
>> ro.org/
>>
>> Not tested on hardware. Please kindly provide tests.
> 
> With these patches on top of 5.19.9 everything incl. modem still seems to work 
> fine on msm8974pro-fairphone-fp2:
> 
> (2/3 & 3/3 from this series)
> ARM: dts: qcom: msm8974: add missing TCSR syscon compatible
> ARM: dts: qcom: msm8974: switch TCSR mutex to MMIO
> 
> (picked from linux-next)
> hwspinlock: qcom: Add support for mmio usage to sfpb-mutex
> hwspinlock: qcom: correct MMIO max register for newer SoCs
> hwspinlock: qcom: add support for MMIO on older SoCs
> 
> Tested-by: Luca Weiss <luca@z3ntu.xyz> # fairphone-fp2
> 

Thanks!

Best regards,
Krzysztof
Alexey Minnekhanov Sept. 21, 2022, 8:55 p.m. UTC | #3
Hi,

On 20.09.2022 18:04, Krzysztof Kozlowski wrote:

> -	tcsr_mutex: tcsr-mutex {
> -		compatible = "qcom,tcsr-mutex";
> -		syscon = <&tcsr_mutex_block 0 0x80>;

I'm looking and don't understand where does this information go, is it 
lost in the conversion? I mean those "0 0x80" parameters to syscon 
reference.

Looking at the code of qcom_hwspinlock driver those seem to be read by 
qcom_hwspinlock_probe_syscon() [1] using  of_property_read_u32_index() 
as base and stride values and those would be 0 nad 0x80 respectively as 
is now.

But without syscon reference, in mmio mode, code goes through 
qcom_hwspinlock_probe_mmio() few lines below, which says

	/* All modern platform has offset 0 and stride of 4k */
	*offset = 0;
	*stride = 0x1000;

So after this conversion stride value will jump from 0x80  to 0x1000, 
which does not seem to be 1 to 1 identical conversion to me, unless I am 
missing something.

Perhaps msm8974 does not fall into category of "All modern platform"?


[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/hwspinlock/qcom_hwspinlock.c#L73
Krzysztof Kozlowski Sept. 22, 2022, 6:44 a.m. UTC | #4
On 21/09/2022 22:55, Alexey Minnekhanov wrote:
> Hi,
> 
> On 20.09.2022 18:04, Krzysztof Kozlowski wrote:
> 
>> -	tcsr_mutex: tcsr-mutex {
>> -		compatible = "qcom,tcsr-mutex";
>> -		syscon = <&tcsr_mutex_block 0 0x80>;
> 
> I'm looking and don't understand where does this information go, is it 
> lost in the conversion? I mean those "0 0x80" parameters to syscon 
> reference.

This compatible is using:
of_msm8226_tcsr_mutex
.stride = 0x80,

Best regards,
Krzysztof
Rob Herring (Arm) Sept. 26, 2022, 7:49 p.m. UTC | #5
On Tue, 20 Sep 2022 17:04:12 +0200, Krzysztof Kozlowski wrote:
> The TCSR_MUTEX region contains two set of registers: mutex and halt.
> Add syscon, so the TCSR mutex device (hwspinlock) can use MMIO based
> method and in the same time share regmap with other devices for the halt
> regs.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/hwlock/qcom-hwspinlock.yaml         | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>