[PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[Douglas Anderson]

Trogdor devices use firmware backed by TF-A instead of Qualcomm's
normal TZ. On TF-A we end up mapping memory as cacheable. Specifically,
you can see in Trogdor's TF-A code [1] in qti_sip_mem_assign() that we
call qti_mmap_add_dynamic_region() with MT_RO_DATA. This translates
down to MT_MEMORY instead of MT_NON_CACHEABLE or MT_DEVICE.

Let's allow devices like trogdor to be described properly by allowing
"dma-coherent" in the SCM node.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- Bindings change new for v2.

 Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
index 367d04ad1923..83381f3a1341 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
@@ -71,6 +71,8 @@ properties:
     minItems: 1
     maxItems: 3
 
+  dma-coherent: true
+
   interconnects:
     maxItems: 1
 
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH v2 2/4] arm64: dts: qcom: sc7180: Mark SCM as dma-coherent for IDP

[Douglas Anderson]

sc7180-idp is, for most intents and purposes, a trogdor device.
Specifically, sc7180-idp is designed to run the same style of firmware
as trogdor devices. This can be seen from the fact that IDP has the
same "Reserved memory changes" in its device tree that trogdor has.

Recently it was realized that we need to mark SCM as dma-coherent to
match what trogdor's style of firmware (based on TF-A) does [1]. That
means we need this dma-coherent tag on IDP as well.

Without this, on newer versions of Linux, specifically those with
commit 7bd6680b47fa ("Revert "Revert "arm64: dma: Drop cache
invalidation from arch_dma_prep_coherent()"""), WiFi will fail to
work. At bootup you'll see:

  qcom_scm firmware:scm: Assign memory protection call failed -22
  qcom_rmtfs_mem 94600000.memory: assign memory failed
  qcom_rmtfs_mem: probe of 94600000.memory failed with error -22

[1] https://lore.kernel.org/r/20230615145253.1.Ic62daa649b47b656b313551d646c4de9a7da4bd4@changeid

Fixes: 7bd6680b47fa ("Revert "Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()""")
Fixes: f5ab220d162c ("arm64: dts: qcom: sc7180: Add remoteproc enablers")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
I realized that this needed to be in IDP as well and that the IDP
patch actually needed to come _before_ the trogdor one given the order
that things landed upstream. I still left most of the description of
the problem in the trogdor patch, though. Hopefully that's OK.

Changes in v2:
- sc7180-IDP patch added for v2.

 arch/arm64/boot/dts/qcom/sc7180-idp.dts | 5 +++++
 arch/arm64/boot/dts/qcom/sc7180.dtsi    | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 9f052270e090..299ef5dc225a 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -393,6 +393,11 @@ &remoteproc_mpss {
 	qcom,spare-regs = <&tcsr_regs_2 0xb3e4>;
 };
 
+&scm {
+	/* TF-A firmware maps memory cached so mark dma-coherent to match. */
+	dma-coherent;
+};
+
 &sdhc_1 {
 	status = "okay";
 
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index f479cab8ab45..a65be760d1a7 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -369,7 +369,7 @@ CLUSTER_SLEEP_0: cluster-sleep-0 {
 	};
 
 	firmware {
-		scm {
+		scm: scm {
 			compatible = "qcom,scm-sc7180", "qcom,scm";
 		};
 	};
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH v2 3/4] arm64: dts: qcom: sc7180: Mark SCM as dma-coherent for trogdor

[Douglas Anderson]

Trogdor devices use firmware backed by TF-A instead of Qualcomm's
normal TZ. On TF-A we end up mapping memory as cacheable.
Specifically, you can see in Trogdor's TF-A code [1] in
qti_sip_mem_assign() that we call qti_mmap_add_dynamic_region() with
MT_RO_DATA. This translates down to MT_MEMORY instead of
MT_NON_CACHEABLE or MT_DEVICE. Apparently Qualcomm's normal TZ
implementation maps the memory as non-cacheable.

Let's add the "dma-coherent" attribute to the SCM for trogdor.

Adding "dma-coherent" like this fixes WiFi on sc7180-trogdor
devices. WiFi was broken as of commit 7bd6680b47fa ("Revert "Revert
"arm64: dma: Drop cache invalidation from
arch_dma_prep_coherent()"""). Specifically at bootup we'd get:

 qcom_scm firmware:scm: Assign memory protection call failed -22
 qcom_rmtfs_mem 94600000.memory: assign memory failed
 qcom_rmtfs_mem: probe of 94600000.memory failed with error -22

From discussion on the mailing lists [2] and over IRC [3], it was
determined that we should always have been tagging the SCM as
dma-coherent on trogdor but that the old "invalidate" happened to make
things work most of the time. Tagging it properly like this is a much
more robust solution.

[1] https://chromium.googlesource.com/chromiumos/third_party/arm-trusted-firmware/+/refs/heads/firmware-trogdor-13577.B/plat/qti/common/src/qti_syscall.c
[2] https://lore.kernel.org/r/20230614165904.1.I279773c37e2c1ed8fbb622ca6d1397aea0023526@changeid
[3] https://oftc.irclog.whitequark.org/linux-msm/2023-06-15

Fixes: 7bd6680b47fa ("Revert "Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()""")
Fixes: 7ec3e67307f8 ("arm64: dts: qcom: sc7180-trogdor: add initial trogdor and lazor dt")
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- Add comment in dts.
- Moved scm node out of pinctrl section to the proper place.

 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index ca6920de7ea8..1472e7f10831 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -892,6 +892,11 @@ &remoteproc_mpss {
 	qcom,spare-regs = <&tcsr_regs_2 0xb3e4>;
 };
 
+&scm {
+	/* TF-A firmware maps memory cached so mark dma-coherent to match. */
+	dma-coherent;
+};
+
 &sdhc_1 {
 	status = "okay";
 
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH v2 4/4] arm64: dts: qcom: sc7280: Mark SCM as dma-coherent for chrome devices

[Douglas Anderson]

Just like for sc7180 devices using the Chrome bootflow (AKA trogdor
and IDP), sc7280 devices using the Chrome bootflow also need their
firmware marked dma-coherent. On sc7280 this wasn't causing WiFi to
fail to startup, since WiFi works differently there. However, on
sc7280 devices we were still getting the message at bootup after
commit 7bd6680b47fa ("Revert "Revert "arm64: dma: Drop cache
invalidation from arch_dma_prep_coherent()"""):

 qcom_scm firmware:scm: Assign memory protection call failed -22
 qcom_rmtfs_mem 9c900000.memory: assign memory failed
 qcom_rmtfs_mem: probe of 9c900000.memory failed with error -22

We should mark SCM properly just like we did for trogdor.

Fixes: 7bd6680b47fa ("Revert "Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()""")
Fixes: 7a1f4e7f740d ("arm64: dts: qcom: sc7280: Add basic dts/dtsi files for sc7280 soc")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
I marked this as "Fixes" for the patch that first added the SCM node
to sc7280. Given all the reorganization of the files it wouldn't be
all that easy to really backport it to there, but that should be
OK. Things seemed to work fine before commit 7bd6680b47fa ("Revert
"Revert "arm64: dma: Drop cache invalidation from
arch_dma_prep_coherent()""") anyway.

Changes in v2:
- sc7280 patch new for v2.

 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 5 +++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index f562e4d2b655..2e1cd219fc18 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -79,6 +79,11 @@ &remoteproc_wpss {
 	firmware-name = "ath11k/WCN6750/hw1.0/wpss.mdt";
 };
 
+&scm {
+	/* TF-A firmware maps memory cached so mark dma-coherent to match. */
+	dma-coherent;
+};
+
 &wifi {
 	status = "okay";
 
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 2fd1d3c0eb34..36f0bb9b3cbb 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -656,7 +656,7 @@ memory@80000000 {
 	};
 
 	firmware {
-		scm {
+		scm: scm {
 			compatible = "qcom,scm-sc7280", "qcom,scm";
 		};
 	};
-- 
2.41.0.162.gfafddb0af9-goog

Re: [PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[Krzysztof Kozlowski]

On 16/06/2023 17:14, Douglas Anderson wrote:
> Trogdor devices use firmware backed by TF-A instead of Qualcomm's
> normal TZ. On TF-A we end up mapping memory as cacheable. Specifically,
> you can see in Trogdor's TF-A code [1] in qti_sip_mem_assign() that we
> call qti_mmap_add_dynamic_region() with MT_RO_DATA. This translates
> down to MT_MEMORY instead of MT_NON_CACHEABLE or MT_DEVICE.
> 
> Let's allow devices like trogdor to be described properly by allowing
> "dma-coherent" in the SCM node.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org

2

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[Konrad Dybcio]

On 18.06.2023 10:07, Krzysztof Kozlowski wrote:
> On 16/06/2023 17:14, Douglas Anderson wrote:
>> Trogdor devices use firmware backed by TF-A instead of Qualcomm's
>> normal TZ. On TF-A we end up mapping memory as cacheable. Specifically,
>> you can see in Trogdor's TF-A code [1] in qti_sip_mem_assign() that we
>> call qti_mmap_add_dynamic_region() with MT_RO_DATA. This translates
>> down to MT_MEMORY instead of MT_NON_CACHEABLE or MT_DEVICE.
>>
>> Let's allow devices like trogdor to be described properly by allowing
>> "dma-coherent" in the SCM node.
>>
>> Signed-off-by: Douglas Anderson <dianders@chromium.org
> 
> 2
Forgot to press alt or something

Konrad
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[Krzysztof Kozlowski]

On 16/06/2023 17:14, Douglas Anderson wrote:
> Trogdor devices use firmware backed by TF-A instead of Qualcomm's
> normal TZ. On TF-A we end up mapping memory as cacheable. Specifically,
> you can see in Trogdor's TF-A code [1] in qti_sip_mem_assign() that we
> call qti_mmap_add_dynamic_region() with MT_RO_DATA. This translates
> down to MT_MEMORY instead of MT_NON_CACHEABLE or MT_DEVICE.
> 
> Let's allow devices like trogdor to be described properly by allowing
> "dma-coherent" in the SCM node.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[Krzysztof Kozlowski]

On 19/06/2023 12:00, Konrad Dybcio wrote:
> On 18.06.2023 10:07, Krzysztof Kozlowski wrote:
>> On 16/06/2023 17:14, Douglas Anderson wrote:
>>> Trogdor devices use firmware backed by TF-A instead of Qualcomm's
>>> normal TZ. On TF-A we end up mapping memory as cacheable. Specifically,
>>> you can see in Trogdor's TF-A code [1] in qti_sip_mem_assign() that we
>>> call qti_mmap_add_dynamic_region() with MT_RO_DATA. This translates
>>> down to MT_MEMORY instead of MT_NON_CACHEABLE or MT_DEVICE.
>>>
>>> Let's allow devices like trogdor to be described properly by allowing
>>> "dma-coherent" in the SCM node.
>>>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org
>>
>> 2
> Forgot to press alt or something

D'oh!

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[Doug Anderson]

Bjorn,

On Fri, Jun 16, 2023 at 8:18 AM Douglas Anderson <dianders@chromium.org> wrote:
>
> Trogdor devices use firmware backed by TF-A instead of Qualcomm's
> normal TZ. On TF-A we end up mapping memory as cacheable. Specifically,
> you can see in Trogdor's TF-A code [1] in qti_sip_mem_assign() that we
> call qti_mmap_add_dynamic_region() with MT_RO_DATA. This translates
> down to MT_MEMORY instead of MT_NON_CACHEABLE or MT_DEVICE.
>
> Let's allow devices like trogdor to be described properly by allowing
> "dma-coherent" in the SCM node.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - Bindings change new for v2.
>
>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 2 ++
>  1 file changed, 2 insertions(+)

Without this series v6.4 will have a regression where WiFi / LTE won't
work at all on trogdor devices. Any chance you can send up a "Fixes"
pull request with the 4 patches in it? ...or I could try to convince
someone on the SoC tree to land them directly?

Thanks!

-Doug

Re: (subset) [PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent

[Bjorn Andersson]

On Fri, 16 Jun 2023 08:14:38 -0700, Douglas Anderson wrote:
> Trogdor devices use firmware backed by TF-A instead of Qualcomm's
> normal TZ. On TF-A we end up mapping memory as cacheable. Specifically,
> you can see in Trogdor's TF-A code [1] in qti_sip_mem_assign() that we
> call qti_mmap_add_dynamic_region() with MT_RO_DATA. This translates
> down to MT_MEMORY instead of MT_NON_CACHEABLE or MT_DEVICE.
> 
> Let's allow devices like trogdor to be described properly by allowing
> "dma-coherent" in the SCM node.
> 
> [...]

Applied, thanks!

[1/4] dt-bindings: firmware: qcom,scm: Document that SCM can be dma-coherent
      commit: c0877829ada0406233aee5bd54f6813db79d5f1f
[2/4] arm64: dts: qcom: sc7180: Mark SCM as dma-coherent for IDP
      commit: 9a5f0b11e49e27f0a01a73c31d05df4a95bea3fa
[3/4] arm64: dts: qcom: sc7180: Mark SCM as dma-coherent for trogdor
      commit: a54b7fa6b9ab6b4ecb7d9aba6b1a0ce1bcc961e3
[4/4] arm64: dts: qcom: sc7280: Mark SCM as dma-coherent for chrome devices
      commit: 7b59e8ae92fe089fed8ff1b23e53442ae5b204c9

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>