2023-06-16 15:46:06

by Doug Anderson

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

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 <[email protected]>
---

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



2023-06-16 15:58:57

by Doug Anderson

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

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 <[email protected]>
---
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


2023-06-16 16:01:58

by Doug Anderson

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

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 <[email protected]>
---
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


2023-06-18 08:53:19

by Krzysztof Kozlowski

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

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 <[email protected]

2

Best regards,
Krzysztof


2023-06-19 10:17:48

by Krzysztof Kozlowski

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

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 <[email protected]
>>
>> 2
> Forgot to press alt or something

D'oh!

Best regards,
Krzysztof


2023-06-19 10:35:57

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---


Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2023-06-19 10:41:45

by Konrad Dybcio

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

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 <[email protected]
>
> 2
Forgot to press alt or something

Konrad
>
> Best regards,
> Krzysztof
>

2023-06-21 17:34:53

by Doug Anderson

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

Bjorn,

On Fri, Jun 16, 2023 at 8:18 AM Douglas Anderson <[email protected]> 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 <[email protected]>
> ---
>
> 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

2023-06-22 21:27:58

by Bjorn Andersson

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


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 <[email protected]>