[PATCH 0/9] Remoteprocs (ADSP, CDSP, WPSS) for SC7280

[PATCH 0/9] Remoteprocs (ADSP, CDSP, WPSS) for SC7280

[Luca Weiss]

This series adds support for the ADSP, CDSP and WPSS remoteprocs found
on SC7280. And finally enable them and WiFi on the QCM6490-based
Fairphone 5 smartphone.

The first two patches are fixes for the MPSS to fix some dt validation
issues. They're included in this series to avoid conflicts with the
later patches and keep it simpler.

Please note, that the ChromeOS-based devices using SC7280 need different
driver and dts support, similar to how there's already
qcom,sc7280-mpss-pas for "standard" firmware and there's
qcom,sc7280-mss-pil for ChromeOS firmware.

I'm aware of the series also adding SC7280 ADSP support with the last
revision sent in June this year.

https://lore.kernel.org/linux-arm-msm/20230616103534.4031331-1-quic_mohs@quicinc.com/

However there's some differences since that series added the "pil"
variant for ChromeOS, not "pas" for standard firmware. Also it seems on
ChromeOS devices gpr+q6apm+q6prm is used. On my device it appears to be
instead apr+q6afe+q6asm+q6adm but I don't add either in this series to
keep it a bit simpler, and I couldn't test much of that yet.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Luca Weiss (9):
      dt-bindings: remoteproc: qcom: sc7180-pas: Fix SC7280 MPSS PD-names
      arm64: dts: qcom: sc7280: Remove unused second MPSS reg
      dt-bindings: remoteproc: qcom: sc7180-pas: Add SC7280 compatibles
      remoteproc: qcom_q6v5_pas: Add SC7280 ADSP, CDSP & WPSS
      arm64: dts: qcom: sc7280: Use WPSS PAS instead of PIL
      arm64: dts: qcom: sc7280: Add ADSP node
      arm64: dts: qcom: sc7280: Add CDSP node
      arm64: dts: qcom: qcm6490-fairphone-fp5: Enable various remoteprocs
      arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

 .../bindings/remoteproc/qcom,sc7180-pas.yaml       |  21 ++
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts |  24 +++
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |  24 ++-
 .../boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi    |   2 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 225 +++++++++++++++++++--
 drivers/remoteproc/qcom_q6v5_pas.c                 |  19 ++
 6 files changed, 300 insertions(+), 15 deletions(-)
---
base-commit: 6a0dad42244c987e3c12bfae728199e360acf079
change-id: 20231027-sc7280-remoteprocs-048208cc1e13

Best regards,
-- 
Luca Weiss <luca.weiss@fairphone.com>

[PATCH 1/9] dt-bindings: remoteproc: qcom: sc7180-pas: Fix SC7280 MPSS PD-names

[Luca Weiss]

The power domains for MPSS on SC7280 are actually named CX and MSS, and
not CX and MX. Adjust the name which also aligns the bindings with the
dts and fixes validation.

Fixes: 8bb92d6fd0b3 ("dt-bindings: remoteproc: qcom,sc7180-pas: split into separate file")
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml
index f10f329677d8..6f0bd6fa5d26 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml
@@ -114,11 +114,11 @@ allOf:
         power-domains:
           items:
             - description: CX power domain
-            - description: MX power domain
+            - description: MSS power domain
         power-domain-names:
           items:
             - const: cx
-            - const: mx
+            - const: mss
 
 unevaluatedProperties: false
 

-- 
2.42.0

Re: [PATCH 1/9] dt-bindings: remoteproc: qcom: sc7180-pas: Fix SC7280 MPSS PD-names

[Krzysztof Kozlowski]

On 27/10/2023 16:20, Luca Weiss wrote:
> The power domains for MPSS on SC7280 are actually named CX and MSS, and
> not CX and MX. Adjust the name which also aligns the bindings with the
> dts and fixes validation.
> 
> Fixes: 8bb92d6fd0b3 ("dt-bindings: remoteproc: qcom,sc7180-pas: split into separate file")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

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

Best regards,
Krzysztof

[PATCH 2/9] arm64: dts: qcom: sc7280: Remove unused second MPSS reg

[Luca Weiss]

The bindings for sc7280-mpss-pas neither expects a second reg nor a
reg-names property, which is only required by the sc7280-mss-pil
bindings.

Move it to sc7280-herobrine-lte-sku.dtsi, the only place where that
other compatible is used.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 2 ++
 arch/arm64/boot/dts/qcom/sc7280.dtsi                   | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi
index 95505549adcc..203274c10532 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi
@@ -33,6 +33,8 @@ &ipa {
 
 &remoteproc_mpss {
 	compatible = "qcom,sc7280-mss-pil";
+	reg = <0 0x04080000 0 0x10000>, <0 0x04180000 0 0x48>;
+	reg-names = "qdsp6", "rmb";
 
 	clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
 		 <&gcc GCC_MSS_OFFLINE_AXI_CLK>,
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 5db468d1a06e..0d9cc44066ce 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2848,8 +2848,7 @@ adreno_smmu: iommu@3da0000 {
 
 		remoteproc_mpss: remoteproc@4080000 {
 			compatible = "qcom,sc7280-mpss-pas";
-			reg = <0 0x04080000 0 0x10000>, <0 0x04180000 0 0x48>;
-			reg-names = "qdsp6", "rmb";
+			reg = <0 0x04080000 0 0x10000>;
 
 			interrupts-extended = <&intc GIC_SPI 264 IRQ_TYPE_LEVEL_HIGH>,
 					      <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,

-- 
2.42.0

Re: [PATCH 2/9] arm64: dts: qcom: sc7280: Remove unused second MPSS reg

[Krzysztof Kozlowski]

On 27/10/2023 16:20, Luca Weiss wrote:
> The bindings for sc7280-mpss-pas neither expects a second reg nor a
> reg-names property, which is only required by the sc7280-mss-pil
> bindings.
> 
> Move it to sc7280-herobrine-lte-sku.dtsi, the only place where that
> other compatible is used.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 2 ++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi                   | 3 +--
>  2 files changed, 3 insertions(+), 2 deletions(-)

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

Best regards,
Krzysztof

[PATCH 3/9] dt-bindings: remoteproc: qcom: sc7180-pas: Add SC7280 compatibles

[Luca Weiss]

Add the compatibles and constraints for the ADSP, CDSP and WPSS found on
the SC7280 SoC.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 .../bindings/remoteproc/qcom,sc7180-pas.yaml        | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml
index 6f0bd6fa5d26..c054b84fdcd5 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-pas.yaml
@@ -18,7 +18,10 @@ properties:
     enum:
       - qcom,sc7180-adsp-pas
       - qcom,sc7180-mpss-pas
+      - qcom,sc7280-adsp-pas
+      - qcom,sc7280-cdsp-pas
       - qcom,sc7280-mpss-pas
+      - qcom,sc7280-wpss-pas
 
   reg:
     maxItems: 1
@@ -75,6 +78,7 @@ allOf:
         compatible:
           enum:
             - qcom,sc7180-adsp-pas
+            - qcom,sc7280-adsp-pas
     then:
       properties:
         power-domains:
@@ -120,6 +124,23 @@ allOf:
             - const: cx
             - const: mss
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - qcom,sc7280-cdsp-pas
+            - qcom,sc7280-wpss-pas
+    then:
+      properties:
+        power-domains:
+          items:
+            - description: CX power domain
+            - description: MX power domain
+        power-domain-names:
+          items:
+            - const: cx
+            - const: mx
+
 unevaluatedProperties: false
 
 examples:

-- 
2.42.0

Re: [PATCH 3/9] dt-bindings: remoteproc: qcom: sc7180-pas: Add SC7280 compatibles

[Krzysztof Kozlowski]

On 27/10/2023 16:20, Luca Weiss wrote:
> Add the compatibles and constraints for the ADSP, CDSP and WPSS found on
> the SC7280 SoC.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  .../bindings/remoteproc/qcom,sc7180-pas.yaml        | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

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

Best regards,
Krzysztof

[PATCH 4/9] remoteproc: qcom_q6v5_pas: Add SC7280 ADSP, CDSP & WPSS

[Luca Weiss]

Add support for the ADSP, CDSP and WPSS remoteprocs found on the SC7280
SoC using the q6v5-pas driver.

This driver can be used on regular LA ("Linux Android") based releases,
however the SC7280 ChromeOS devices need different driver support due to
firmware differences.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 913a5d2068e8..a9dd58608052 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -1165,6 +1165,22 @@ static const struct adsp_data sm8550_mpss_resource = {
 	.region_assign_idx = 2,
 };
 
+static const struct adsp_data sc7280_wpss_resource = {
+	.crash_reason_smem = 626,
+	.firmware_name = "wpss.mdt",
+	.pas_id = 6,
+	.auto_boot = true,
+	.proxy_pd_names = (char*[]){
+		"cx",
+		"mx",
+		NULL
+	},
+	.load_state = "wpss",
+	.ssr_name = "wpss",
+	.sysmon_name = "wpss",
+	.ssctl_id = 0x19,
+};
+
 static const struct of_device_id adsp_of_match[] = {
 	{ .compatible = "qcom,msm8226-adsp-pil", .data = &adsp_resource_init},
 	{ .compatible = "qcom,msm8953-adsp-pil", .data = &msm8996_adsp_resource},
@@ -1178,7 +1194,10 @@ static const struct of_device_id adsp_of_match[] = {
 	{ .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init },
 	{ .compatible = "qcom,sc7180-adsp-pas", .data = &sm8250_adsp_resource},
 	{ .compatible = "qcom,sc7180-mpss-pas", .data = &mpss_resource_init},
+	{ .compatible = "qcom,sc7280-adsp-pas", .data = &sm8350_adsp_resource},
+	{ .compatible = "qcom,sc7280-cdsp-pas", .data = &sm6350_cdsp_resource},
 	{ .compatible = "qcom,sc7280-mpss-pas", .data = &mpss_resource_init},
+	{ .compatible = "qcom,sc7280-wpss-pas", .data = &sc7280_wpss_resource},
 	{ .compatible = "qcom,sc8180x-adsp-pas", .data = &sm8150_adsp_resource},
 	{ .compatible = "qcom,sc8180x-cdsp-pas", .data = &sm8150_cdsp_resource},
 	{ .compatible = "qcom,sc8180x-mpss-pas", .data = &sc8180x_mpss_resource},

-- 
2.42.0

[PATCH 5/9] arm64: dts: qcom: sc7280: Use WPSS PAS instead of PIL

[Luca Weiss]

The wpss-pil driver wants to manage too many resources that cannot be
touched with standard Qualcomm firmware.

Use the compatible from the PAS driver and move the ChromeOS-specific
bits to sc7280-chrome-common.dtsi.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 19 ++++++++++++++++++-
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 15 +++------------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index cd491e46666d..eb55616e0892 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -95,8 +95,25 @@ spi_flash: flash@0 {
 };
 
 &remoteproc_wpss {
-	status = "okay";
+	compatible = "qcom,sc7280-wpss-pil";
+	clocks = <&gcc GCC_WPSS_AHB_BDG_MST_CLK>,
+		 <&gcc GCC_WPSS_AHB_CLK>,
+		 <&gcc GCC_WPSS_RSCP_CLK>,
+		 <&rpmhcc RPMH_CXO_CLK>;
+	clock-names = "ahb_bdg",
+		      "ahb",
+		      "rscp",
+		      "xo";
+
+	resets = <&aoss_reset AOSS_CC_WCSS_RESTART>,
+		 <&pdc_reset PDC_WPSS_SYNC_RESET>;
+	reset-names = "restart", "pdc_sync";
+
+	qcom,halt-regs = <&tcsr_1 0x17000>;
+
 	firmware-name = "ath11k/WCN6750/hw1.0/wpss.mdt";
+
+	status = "okay";
 };
 
 &scm {
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 0d9cc44066ce..23bb6c41fca3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3579,7 +3579,7 @@ qspi: spi@88dc000 {
 		};
 
 		remoteproc_wpss: remoteproc@8a00000 {
-			compatible = "qcom,sc7280-wpss-pil";
+			compatible = "qcom,sc7280-wpss-pas";
 			reg = <0 0x08a00000 0 0x10000>;
 
 			interrupts-extended = <&intc GIC_SPI 587 IRQ_TYPE_EDGE_RISING>,
@@ -3591,12 +3591,8 @@ remoteproc_wpss: remoteproc@8a00000 {
 			interrupt-names = "wdog", "fatal", "ready", "handover",
 					  "stop-ack", "shutdown-ack";
 
-			clocks = <&gcc GCC_WPSS_AHB_BDG_MST_CLK>,
-				 <&gcc GCC_WPSS_AHB_CLK>,
-				 <&gcc GCC_WPSS_RSCP_CLK>,
-				 <&rpmhcc RPMH_CXO_CLK>;
-			clock-names = "ahb_bdg", "ahb",
-				      "rscp", "xo";
+			clocks = <&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "xo";
 
 			power-domains = <&rpmhpd SC7280_CX>,
 					<&rpmhpd SC7280_MX>;
@@ -3609,11 +3605,6 @@ remoteproc_wpss: remoteproc@8a00000 {
 			qcom,smem-states = <&wpss_smp2p_out 0>;
 			qcom,smem-state-names = "stop";
 
-			resets = <&aoss_reset AOSS_CC_WCSS_RESTART>,
-				 <&pdc_reset PDC_WPSS_SYNC_RESET>;
-			reset-names = "restart", "pdc_sync";
-
-			qcom,halt-regs = <&tcsr_1 0x17000>;
 
 			status = "disabled";
 

-- 
2.42.0

[PATCH 6/9] arm64: dts: qcom: sc7280: Add ADSP node

[Luca Weiss]

Add the node for the ADSP found on the SC7280 SoC, using standard
Qualcomm firmware.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 69 ++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 23bb6c41fca3..cc153f4e6979 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3578,6 +3578,75 @@ qspi: spi@88dc000 {
 			status = "disabled";
 		};
 
+		remoteproc_adsp: remoteproc@3700000 {
+			compatible = "qcom,sc7280-adsp-pas";
+			reg = <0 0x03700000 0 0x100>;
+
+			interrupts-extended = <&pdc 6 IRQ_TYPE_LEVEL_HIGH>,
+					      <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+					      <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+					      <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+					      <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
+					      <&adsp_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "wdog", "fatal", "ready", "handover",
+					  "stop-ack", "shutdown-ack";
+
+			clocks = <&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "xo";
+
+			power-domains = <&rpmhpd SC7280_LCX>,
+					<&rpmhpd SC7280_LMX>;
+			power-domain-names = "lcx", "lmx";
+
+			memory-region = <&adsp_mem>;
+
+			qcom,qmp = <&aoss_qmp>;
+
+			qcom,smem-states = <&adsp_smp2p_out 0>;
+			qcom,smem-state-names = "stop";
+
+			status = "disabled";
+
+			glink-edge {
+				interrupts-extended = <&ipcc IPCC_CLIENT_LPASS
+							     IPCC_MPROC_SIGNAL_GLINK_QMP
+							     IRQ_TYPE_EDGE_RISING>;
+
+				mboxes = <&ipcc IPCC_CLIENT_LPASS
+						IPCC_MPROC_SIGNAL_GLINK_QMP>;
+
+				label = "lpass";
+				qcom,remote-pid = <2>;
+
+				fastrpc {
+					compatible = "qcom,fastrpc";
+					qcom,glink-channels = "fastrpcglink-apps-dsp";
+					label = "adsp";
+					qcom,non-secure-domain;
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					compute-cb@3 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <3>;
+						iommus = <&apps_smmu 0x1803 0x0>;
+					};
+
+					compute-cb@4 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <4>;
+						iommus = <&apps_smmu 0x1804 0x0>;
+					};
+
+					compute-cb@5 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <5>;
+						iommus = <&apps_smmu 0x1805 0x0>;
+					};
+				};
+			};
+		};
+
 		remoteproc_wpss: remoteproc@8a00000 {
 			compatible = "qcom,sc7280-wpss-pas";
 			reg = <0 0x08a00000 0 0x10000>;

-- 
2.42.0

Re: [PATCH 6/9] arm64: dts: qcom: sc7280: Add ADSP node

[Krzysztof Kozlowski]

On 27/10/2023 16:20, Luca Weiss wrote:
> Add the node for the ADSP found on the SC7280 SoC, using standard
> Qualcomm firmware.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---

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

Best regards,
Krzysztof

[PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Luca Weiss]

Add the node for the ADSP found on the SC7280 SoC, using standard
Qualcomm firmware.

The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
yupik.dtsi since the other areas also seem to match that file there,
though I cannot be sure there.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
 2 files changed, 143 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index eb55616e0892..6e5a9d4c1fda 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
 			no-map;
 		};
 
+		cdsp_mem: memory@88f00000 {
+			reg = <0x0 0x88f00000 0x0 0x1e00000>;
+			no-map;
+		};
+
 		camera_mem: memory@8ad00000 {
 			reg = <0x0 0x8ad00000 0x0 0x500000>;
 			no-map;
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index cc153f4e6979..e15646289bf7 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3815,6 +3815,144 @@ nsp_noc: interconnect@a0c0000 {
 			qcom,bcm-voters = <&apps_bcm_voter>;
 		};
 
+		remoteproc_cdsp: remoteproc@a300000 {
+			compatible = "qcom,sc7280-cdsp-pas";
+			reg = <0 0x0a300000 0 0x10000>;
+
+			interrupts-extended = <&intc GIC_SPI 578 IRQ_TYPE_LEVEL_HIGH>,
+					      <&cdsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+					      <&cdsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+					      <&cdsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+					      <&cdsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
+					      <&cdsp_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "wdog", "fatal", "ready", "handover",
+					  "stop-ack", "shutdown-ack";
+
+			clocks = <&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "xo";
+
+			power-domains = <&rpmhpd SC7280_CX>,
+					<&rpmhpd SC7280_MX>;
+			power-domain-names = "cx", "mx";
+
+			interconnects = <&nsp_noc MASTER_CDSP_PROC 0 &mc_virt SLAVE_EBI1 0>;
+
+			memory-region = <&cdsp_mem>;
+
+			qcom,qmp = <&aoss_qmp>;
+
+			qcom,smem-states = <&cdsp_smp2p_out 0>;
+			qcom,smem-state-names = "stop";
+
+			status = "disabled";
+
+			glink-edge {
+				interrupts-extended = <&ipcc IPCC_CLIENT_CDSP
+							     IPCC_MPROC_SIGNAL_GLINK_QMP
+							     IRQ_TYPE_EDGE_RISING>;
+				mboxes = <&ipcc IPCC_CLIENT_CDSP
+						IPCC_MPROC_SIGNAL_GLINK_QMP>;
+
+				label = "cdsp";
+				qcom,remote-pid = <5>;
+
+				fastrpc {
+					compatible = "qcom,fastrpc";
+					qcom,glink-channels = "fastrpcglink-apps-dsp";
+					label = "cdsp";
+					qcom,non-secure-domain;
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					compute-cb@1 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <1>;
+						iommus = <&apps_smmu 0x11a1 0x0420>,
+							 <&apps_smmu 0x1181 0x0420>;
+					};
+
+					compute-cb@2 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <2>;
+						iommus = <&apps_smmu 0x11a2 0x0420>,
+							 <&apps_smmu 0x1182 0x0420>;
+					};
+
+					compute-cb@3 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <3>;
+						iommus = <&apps_smmu 0x11a3 0x0420>,
+							 <&apps_smmu 0x1183 0x0420>;
+					};
+
+					compute-cb@4 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <4>;
+						iommus = <&apps_smmu 0x11a4 0x0420>,
+							 <&apps_smmu 0x1184 0x0420>;
+					};
+
+					compute-cb@5 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <5>;
+						iommus = <&apps_smmu 0x11a5 0x0420>,
+							 <&apps_smmu 0x1185 0x0420>;
+					};
+
+					compute-cb@6 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <6>;
+						iommus = <&apps_smmu 0x11a6 0x0420>,
+							 <&apps_smmu 0x1186 0x0420>;
+					};
+
+					compute-cb@7 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <7>;
+						iommus = <&apps_smmu 0x11a7 0x0420>,
+							 <&apps_smmu 0x1187 0x0420>;
+					};
+
+					compute-cb@8 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <8>;
+						iommus = <&apps_smmu 0x11a8 0x0420>,
+							 <&apps_smmu 0x1188 0x0420>;
+					};
+
+					/* note: secure cb9 in downstream */
+
+					compute-cb@11 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <11>;
+						iommus = <&apps_smmu 0x11ab 0x0420>,
+							 <&apps_smmu 0x118b 0x0420>;
+					};
+
+					compute-cb@12 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <12>;
+						iommus = <&apps_smmu 0x11ac 0x0420>,
+							 <&apps_smmu 0x118c 0x0420>;
+					};
+
+					compute-cb@13 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <13>;
+						iommus = <&apps_smmu 0x11ad 0x0420>,
+							 <&apps_smmu 0x118d 0x0420>;
+					};
+
+					compute-cb@14 {
+						compatible = "qcom,fastrpc-compute-cb";
+						reg = <14>;
+						iommus = <&apps_smmu 0x11ae 0x0420>,
+							 <&apps_smmu 0x118e 0x0420>;
+					};
+				};
+			};
+		};
+
 		usb_1: usb@a6f8800 {
 			compatible = "qcom,sc7280-dwc3", "qcom,dwc3";
 			reg = <0 0x0a6f8800 0 0x400>;

-- 
2.42.0

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Krzysztof Kozlowski]

On 27/10/2023 16:20, Luca Weiss wrote:
> Add the node for the ADSP found on the SC7280 SoC, using standard
> Qualcomm firmware.
> 
> The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
> yupik.dtsi since the other areas also seem to match that file there,
> though I cannot be sure there.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
>  arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
>  2 files changed, 143 insertions(+)

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

Best regards,
Krzysztof

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Mukesh Ojha]

On 10/27/2023 7:50 PM, Luca Weiss wrote:
> Add the node for the ADSP found on the SC7280 SoC, using standard
> Qualcomm firmware.
> 
> The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
> yupik.dtsi since the other areas also seem to match that file there,
> though I cannot be sure there.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>   arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
>   arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
>   2 files changed, 143 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> index eb55616e0892..6e5a9d4c1fda 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> @@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
>   			no-map;
>   		};
>   
> +		cdsp_mem: memory@88f00000 {
> +			reg = <0x0 0x88f00000 0x0 0x1e00000>;
> +			no-map;
> +		};
> +

Just a question, why to do it here, if chrome does not use this ?

-Mukesh

>   		camera_mem: memory@8ad00000 {
>   			reg = <0x0 0x8ad00000 0x0 0x500000>;
>   			no-map;
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index cc153f4e6979..e15646289bf7 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3815,6 +3815,144 @@ nsp_noc: interconnect@a0c0000 {
>   			qcom,bcm-voters = <&apps_bcm_voter>;
>   		};
>   
> +		remoteproc_cdsp: remoteproc@a300000 {
> +			compatible = "qcom,sc7280-cdsp-pas";
> +			reg = <0 0x0a300000 0 0x10000>;
> +
> +			interrupts-extended = <&intc GIC_SPI 578 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&cdsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> +					      <&cdsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> +					      <&cdsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> +					      <&cdsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
> +					      <&cdsp_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-names = "wdog", "fatal", "ready", "handover",
> +					  "stop-ack", "shutdown-ack";
> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> +			clock-names = "xo";
> +
> +			power-domains = <&rpmhpd SC7280_CX>,
> +					<&rpmhpd SC7280_MX>;
> +			power-domain-names = "cx", "mx";
> +
> +			interconnects = <&nsp_noc MASTER_CDSP_PROC 0 &mc_virt SLAVE_EBI1 0>;
> +
> +			memory-region = <&cdsp_mem>;
> +
> +			qcom,qmp = <&aoss_qmp>;
> +
> +			qcom,smem-states = <&cdsp_smp2p_out 0>;
> +			qcom,smem-state-names = "stop";
> +
> +			status = "disabled";
> +
> +			glink-edge {
> +				interrupts-extended = <&ipcc IPCC_CLIENT_CDSP
> +							     IPCC_MPROC_SIGNAL_GLINK_QMP
> +							     IRQ_TYPE_EDGE_RISING>;
> +				mboxes = <&ipcc IPCC_CLIENT_CDSP
> +						IPCC_MPROC_SIGNAL_GLINK_QMP>;
> +
> +				label = "cdsp";
> +				qcom,remote-pid = <5>;
> +
> +				fastrpc {
> +					compatible = "qcom,fastrpc";
> +					qcom,glink-channels = "fastrpcglink-apps-dsp";
> +					label = "cdsp";
> +					qcom,non-secure-domain;
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					compute-cb@1 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <1>;
> +						iommus = <&apps_smmu 0x11a1 0x0420>,
> +							 <&apps_smmu 0x1181 0x0420>;
> +					};
> +
> +					compute-cb@2 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <2>;
> +						iommus = <&apps_smmu 0x11a2 0x0420>,
> +							 <&apps_smmu 0x1182 0x0420>;
> +					};
> +
> +					compute-cb@3 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <3>;
> +						iommus = <&apps_smmu 0x11a3 0x0420>,
> +							 <&apps_smmu 0x1183 0x0420>;
> +					};
> +
> +					compute-cb@4 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <4>;
> +						iommus = <&apps_smmu 0x11a4 0x0420>,
> +							 <&apps_smmu 0x1184 0x0420>;
> +					};
> +
> +					compute-cb@5 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <5>;
> +						iommus = <&apps_smmu 0x11a5 0x0420>,
> +							 <&apps_smmu 0x1185 0x0420>;
> +					};
> +
> +					compute-cb@6 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <6>;
> +						iommus = <&apps_smmu 0x11a6 0x0420>,
> +							 <&apps_smmu 0x1186 0x0420>;
> +					};
> +
> +					compute-cb@7 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <7>;
> +						iommus = <&apps_smmu 0x11a7 0x0420>,
> +							 <&apps_smmu 0x1187 0x0420>;
> +					};
> +
> +					compute-cb@8 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <8>;
> +						iommus = <&apps_smmu 0x11a8 0x0420>,
> +							 <&apps_smmu 0x1188 0x0420>;
> +					};
> +
> +					/* note: secure cb9 in downstream */
> +
> +					compute-cb@11 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <11>;
> +						iommus = <&apps_smmu 0x11ab 0x0420>,
> +							 <&apps_smmu 0x118b 0x0420>;
> +					};
> +
> +					compute-cb@12 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <12>;
> +						iommus = <&apps_smmu 0x11ac 0x0420>,
> +							 <&apps_smmu 0x118c 0x0420>;
> +					};
> +
> +					compute-cb@13 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <13>;
> +						iommus = <&apps_smmu 0x11ad 0x0420>,
> +							 <&apps_smmu 0x118d 0x0420>;
> +					};
> +
> +					compute-cb@14 {
> +						compatible = "qcom,fastrpc-compute-cb";
> +						reg = <14>;
> +						iommus = <&apps_smmu 0x11ae 0x0420>,
> +							 <&apps_smmu 0x118e 0x0420>;
> +					};
> +				};
> +			};
> +		};
> +
>   		usb_1: usb@a6f8800 {
>   			compatible = "qcom,sc7280-dwc3", "qcom,dwc3";
>   			reg = <0 0x0a6f8800 0 0x400>;
> 

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Luca Weiss]

On Mon Oct 30, 2023 at 10:04 AM CET, Mukesh Ojha wrote:
>
>
> On 10/27/2023 7:50 PM, Luca Weiss wrote:
> > Add the node for the ADSP found on the SC7280 SoC, using standard
> > Qualcomm firmware.
> > 
> > The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
> > yupik.dtsi since the other areas also seem to match that file there,
> > though I cannot be sure there.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >   arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
> >   arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
> >   2 files changed, 143 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > index eb55616e0892..6e5a9d4c1fda 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > @@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
> >   			no-map;
> >   		};
> >   
> > +		cdsp_mem: memory@88f00000 {
> > +			reg = <0x0 0x88f00000 0x0 0x1e00000>;
> > +			no-map;
> > +		};
> > +
>
> Just a question, why to do it here, if chrome does not use this ?

Other memory regions in sc7280.dtsi also get referenced but not actually
defined in that file, like mpss_mem and wpss_mem. Alternatively we can
also try and solve this differently, but then we should probably also
adjust mpss and wpss to be consistent.

Apart from either declaring cdsp_mem in sc7280.dtsi or
"/delete-property/ memory-region;" for CDSP I don't really have better
ideas though.

I also imagine these ChromeOS devices will want to enable cdsp at some
point but I don't know any plans there.

Regards
Luca

>
> -Mukesh
>
> >   		camera_mem: memory@8ad00000 {
> >   			reg = <0x0 0x8ad00000 0x0 0x500000>;
> >   			no-map;
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index cc153f4e6979..e15646289bf7 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -3815,6 +3815,144 @@ nsp_noc: interconnect@a0c0000 {
> >   			qcom,bcm-voters = <&apps_bcm_voter>;
> >   		};
> >   
> > +		remoteproc_cdsp: remoteproc@a300000 {
> > +			compatible = "qcom,sc7280-cdsp-pas";
> > +			reg = <0 0x0a300000 0 0x10000>;
> > +
> > +			interrupts-extended = <&intc GIC_SPI 578 IRQ_TYPE_LEVEL_HIGH>,
> > +					      <&cdsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> > +					      <&cdsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> > +					      <&cdsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> > +					      <&cdsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
> > +					      <&cdsp_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
> > +			interrupt-names = "wdog", "fatal", "ready", "handover",
> > +					  "stop-ack", "shutdown-ack";
> > +
> > +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> > +			clock-names = "xo";
> > +
> > +			power-domains = <&rpmhpd SC7280_CX>,
> > +					<&rpmhpd SC7280_MX>;
> > +			power-domain-names = "cx", "mx";
> > +
> > +			interconnects = <&nsp_noc MASTER_CDSP_PROC 0 &mc_virt SLAVE_EBI1 0>;
> > +
> > +			memory-region = <&cdsp_mem>;
> > +
> > +			qcom,qmp = <&aoss_qmp>;
> > +
> > +			qcom,smem-states = <&cdsp_smp2p_out 0>;
> > +			qcom,smem-state-names = "stop";
> > +
> > +			status = "disabled";
> > +
> > +			glink-edge {
> > +				interrupts-extended = <&ipcc IPCC_CLIENT_CDSP
> > +							     IPCC_MPROC_SIGNAL_GLINK_QMP
> > +							     IRQ_TYPE_EDGE_RISING>;
> > +				mboxes = <&ipcc IPCC_CLIENT_CDSP
> > +						IPCC_MPROC_SIGNAL_GLINK_QMP>;
> > +
> > +				label = "cdsp";
> > +				qcom,remote-pid = <5>;
> > +
> > +				fastrpc {
> > +					compatible = "qcom,fastrpc";
> > +					qcom,glink-channels = "fastrpcglink-apps-dsp";
> > +					label = "cdsp";
> > +					qcom,non-secure-domain;
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					compute-cb@1 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <1>;
> > +						iommus = <&apps_smmu 0x11a1 0x0420>,
> > +							 <&apps_smmu 0x1181 0x0420>;
> > +					};
> > +
> > +					compute-cb@2 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <2>;
> > +						iommus = <&apps_smmu 0x11a2 0x0420>,
> > +							 <&apps_smmu 0x1182 0x0420>;
> > +					};
> > +
> > +					compute-cb@3 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <3>;
> > +						iommus = <&apps_smmu 0x11a3 0x0420>,
> > +							 <&apps_smmu 0x1183 0x0420>;
> > +					};
> > +
> > +					compute-cb@4 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <4>;
> > +						iommus = <&apps_smmu 0x11a4 0x0420>,
> > +							 <&apps_smmu 0x1184 0x0420>;
> > +					};
> > +
> > +					compute-cb@5 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <5>;
> > +						iommus = <&apps_smmu 0x11a5 0x0420>,
> > +							 <&apps_smmu 0x1185 0x0420>;
> > +					};
> > +
> > +					compute-cb@6 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <6>;
> > +						iommus = <&apps_smmu 0x11a6 0x0420>,
> > +							 <&apps_smmu 0x1186 0x0420>;
> > +					};
> > +
> > +					compute-cb@7 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <7>;
> > +						iommus = <&apps_smmu 0x11a7 0x0420>,
> > +							 <&apps_smmu 0x1187 0x0420>;
> > +					};
> > +
> > +					compute-cb@8 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <8>;
> > +						iommus = <&apps_smmu 0x11a8 0x0420>,
> > +							 <&apps_smmu 0x1188 0x0420>;
> > +					};
> > +
> > +					/* note: secure cb9 in downstream */
> > +
> > +					compute-cb@11 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <11>;
> > +						iommus = <&apps_smmu 0x11ab 0x0420>,
> > +							 <&apps_smmu 0x118b 0x0420>;
> > +					};
> > +
> > +					compute-cb@12 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <12>;
> > +						iommus = <&apps_smmu 0x11ac 0x0420>,
> > +							 <&apps_smmu 0x118c 0x0420>;
> > +					};
> > +
> > +					compute-cb@13 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <13>;
> > +						iommus = <&apps_smmu 0x11ad 0x0420>,
> > +							 <&apps_smmu 0x118d 0x0420>;
> > +					};
> > +
> > +					compute-cb@14 {
> > +						compatible = "qcom,fastrpc-compute-cb";
> > +						reg = <14>;
> > +						iommus = <&apps_smmu 0x11ae 0x0420>,
> > +							 <&apps_smmu 0x118e 0x0420>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> >   		usb_1: usb@a6f8800 {
> >   			compatible = "qcom,sc7280-dwc3", "qcom,dwc3";
> >   			reg = <0 0x0a6f8800 0 0x400>;
> > 

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Doug Anderson]

Hi,

On Mon, Oct 30, 2023 at 2:12 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On Mon Oct 30, 2023 at 10:04 AM CET, Mukesh Ojha wrote:
> >
> >
> > On 10/27/2023 7:50 PM, Luca Weiss wrote:
> > > Add the node for the ADSP found on the SC7280 SoC, using standard
> > > Qualcomm firmware.
> > >
> > > The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
> > > yupik.dtsi since the other areas also seem to match that file there,
> > > though I cannot be sure there.
> > >
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > >   arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
> > >   arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
> > >   2 files changed, 143 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > index eb55616e0892..6e5a9d4c1fda 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > @@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
> > >                     no-map;
> > >             };
> > >
> > > +           cdsp_mem: memory@88f00000 {
> > > +                   reg = <0x0 0x88f00000 0x0 0x1e00000>;
> > > +                   no-map;
> > > +           };
> > > +
> >
> > Just a question, why to do it here, if chrome does not use this ?
>
> Other memory regions in sc7280.dtsi also get referenced but not actually
> defined in that file, like mpss_mem and wpss_mem. Alternatively we can
> also try and solve this differently, but then we should probably also
> adjust mpss and wpss to be consistent.
>
> Apart from either declaring cdsp_mem in sc7280.dtsi or
> "/delete-property/ memory-region;" for CDSP I don't really have better
> ideas though.
>
> I also imagine these ChromeOS devices will want to enable cdsp at some
> point but I don't know any plans there.

Given that "remoteproc_cdsp" has status "disabled" in the dtsi, it
feels like the dtsi shouldn't be reserving memory. I guess maybe
memory regions can't be status "disabled"?

-Doug

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Luca Weiss]

On Mon Oct 30, 2023 at 3:11 PM CET, Doug Anderson wrote:
> Hi,
>
> On Mon, Oct 30, 2023 at 2:12 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >
> > On Mon Oct 30, 2023 at 10:04 AM CET, Mukesh Ojha wrote:
> > >
> > >
> > > On 10/27/2023 7:50 PM, Luca Weiss wrote:
> > > > Add the node for the ADSP found on the SC7280 SoC, using standard
> > > > Qualcomm firmware.
> > > >
> > > > The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
> > > > yupik.dtsi since the other areas also seem to match that file there,
> > > > though I cannot be sure there.
> > > >
> > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > > ---
> > > >   arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
> > > >   arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
> > > >   2 files changed, 143 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > > index eb55616e0892..6e5a9d4c1fda 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > > @@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
> > > >                     no-map;
> > > >             };
> > > >
> > > > +           cdsp_mem: memory@88f00000 {
> > > > +                   reg = <0x0 0x88f00000 0x0 0x1e00000>;
> > > > +                   no-map;
> > > > +           };
> > > > +
> > >
> > > Just a question, why to do it here, if chrome does not use this ?
> >
> > Other memory regions in sc7280.dtsi also get referenced but not actually
> > defined in that file, like mpss_mem and wpss_mem. Alternatively we can
> > also try and solve this differently, but then we should probably also
> > adjust mpss and wpss to be consistent.
> >
> > Apart from either declaring cdsp_mem in sc7280.dtsi or
> > "/delete-property/ memory-region;" for CDSP I don't really have better
> > ideas though.
> >
> > I also imagine these ChromeOS devices will want to enable cdsp at some
> > point but I don't know any plans there.
>
> Given that "remoteproc_cdsp" has status "disabled" in the dtsi, it
> feels like the dtsi shouldn't be reserving memory. I guess maybe
> memory regions can't be status "disabled"?

Hi Doug,

That's how it works in really any qcom dtsi though. I think in most/all
cases normally the reserved-memory is already declared in the SoC dtsi
file and also used with the memory-region property.

I wouldn't be against adjusting sc7280.dtsi to match the way it's done
in the other dtsi files though, so to have all the required labels
already defined in the dtsi so it doesn't rely on these labels being
defined in the device dts.

In other words, currently if you include sc7280.dtsi and try to build,
you first have to define the labels mpss_mem and wpss_mem (after this
patch series also cdsp_mem and adsp_mem) for it to build.

I'm quite neutral either way, let me know :)

Regards
Luca

>
> -Doug

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Doug Anderson]

Hi,

On Mon, Oct 30, 2023 at 7:43 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On Mon Oct 30, 2023 at 3:11 PM CET, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Oct 30, 2023 at 2:12 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> > >
> > > On Mon Oct 30, 2023 at 10:04 AM CET, Mukesh Ojha wrote:
> > > >
> > > >
> > > > On 10/27/2023 7:50 PM, Luca Weiss wrote:
> > > > > Add the node for the ADSP found on the SC7280 SoC, using standard
> > > > > Qualcomm firmware.
> > > > >
> > > > > The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
> > > > > yupik.dtsi since the other areas also seem to match that file there,
> > > > > though I cannot be sure there.
> > > > >
> > > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > > > ---
> > > > >   arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
> > > > >   arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
> > > > >   2 files changed, 143 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > > > index eb55616e0892..6e5a9d4c1fda 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > > > +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> > > > > @@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
> > > > >                     no-map;
> > > > >             };
> > > > >
> > > > > +           cdsp_mem: memory@88f00000 {
> > > > > +                   reg = <0x0 0x88f00000 0x0 0x1e00000>;
> > > > > +                   no-map;
> > > > > +           };
> > > > > +
> > > >
> > > > Just a question, why to do it here, if chrome does not use this ?
> > >
> > > Other memory regions in sc7280.dtsi also get referenced but not actually
> > > defined in that file, like mpss_mem and wpss_mem. Alternatively we can
> > > also try and solve this differently, but then we should probably also
> > > adjust mpss and wpss to be consistent.
> > >
> > > Apart from either declaring cdsp_mem in sc7280.dtsi or
> > > "/delete-property/ memory-region;" for CDSP I don't really have better
> > > ideas though.
> > >
> > > I also imagine these ChromeOS devices will want to enable cdsp at some
> > > point but I don't know any plans there.
> >
> > Given that "remoteproc_cdsp" has status "disabled" in the dtsi, it
> > feels like the dtsi shouldn't be reserving memory. I guess maybe
> > memory regions can't be status "disabled"?
>
> Hi Doug,
>
> That's how it works in really any qcom dtsi though. I think in most/all
> cases normally the reserved-memory is already declared in the SoC dtsi
> file and also used with the memory-region property.
>
> I wouldn't be against adjusting sc7280.dtsi to match the way it's done
> in the other dtsi files though, so to have all the required labels
> already defined in the dtsi so it doesn't rely on these labels being
> defined in the device dts.
>
> In other words, currently if you include sc7280.dtsi and try to build,
> you first have to define the labels mpss_mem and wpss_mem (after this
> patch series also cdsp_mem and adsp_mem) for it to build.
>
> I'm quite neutral either way, let me know :)

I haven't done a ton of thinking about this, so if I'm spouting
gibberish then feel free to ignore me. :-P It just feels weird that
when all the "dtsi" files are combined and you look at what you end up
on a sc7280 Chrome board that you'll be reserving 32MB of memory for a
device that's set (in the same device tree) to be "disabled", right?
...the 32MB is completely wasted, I think. If we wanted to enable the
CDSP we'd have to modify the device tree anyway, so it seems like that
same modification would set the CDSP to "okay" and also reserve the
memory...

In that vein, it seems like maybe you could move the "cdsp_mem" to the
SoC .dsti file with a status of "disabled". . I guess we don't do that
elsewhere, but maybe we should be? As far as I can tell without
testing it (just looking at fdt_scan_reserved_mem()) this should
work...

-Doug

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Mukesh Ojha]

On 10/30/2023 8:33 PM, Doug Anderson wrote:
> Hi,
> 
> On Mon, Oct 30, 2023 at 7:43 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>>
>> On Mon Oct 30, 2023 at 3:11 PM CET, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Mon, Oct 30, 2023 at 2:12 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>>
>>>> On Mon Oct 30, 2023 at 10:04 AM CET, Mukesh Ojha wrote:
>>>>>
>>>>>
>>>>> On 10/27/2023 7:50 PM, Luca Weiss wrote:
>>>>>> Add the node for the ADSP found on the SC7280 SoC, using standard
>>>>>> Qualcomm firmware.
>>>>>>
>>>>>> The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
>>>>>> yupik.dtsi since the other areas also seem to match that file there,
>>>>>> though I cannot be sure there.
>>>>>>
>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>> ---
>>>>>>    arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
>>>>>>    arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
>>>>>>    2 files changed, 143 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
>>>>>> index eb55616e0892..6e5a9d4c1fda 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
>>>>>> @@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
>>>>>>                      no-map;
>>>>>>              };
>>>>>>
>>>>>> +           cdsp_mem: memory@88f00000 {
>>>>>> +                   reg = <0x0 0x88f00000 0x0 0x1e00000>;
>>>>>> +                   no-map;
>>>>>> +           };
>>>>>> +
>>>>>
>>>>> Just a question, why to do it here, if chrome does not use this ?
>>>>
>>>> Other memory regions in sc7280.dtsi also get referenced but not actually
>>>> defined in that file, like mpss_mem and wpss_mem. Alternatively we can
>>>> also try and solve this differently, but then we should probably also
>>>> adjust mpss and wpss to be consistent.
>>>>
>>>> Apart from either declaring cdsp_mem in sc7280.dtsi or
>>>> "/delete-property/ memory-region;" for CDSP I don't really have better
>>>> ideas though.
>>>>
>>>> I also imagine these ChromeOS devices will want to enable cdsp at some
>>>> point but I don't know any plans there.
>>>
>>> Given that "remoteproc_cdsp" has status "disabled" in the dtsi, it
>>> feels like the dtsi shouldn't be reserving memory. I guess maybe
>>> memory regions can't be status "disabled"?
>>
>> Hi Doug,
>>
>> That's how it works in really any qcom dtsi though. I think in most/all
>> cases normally the reserved-memory is already declared in the SoC dtsi
>> file and also used with the memory-region property.
>>
>> I wouldn't be against adjusting sc7280.dtsi to match the way it's done
>> in the other dtsi files though, so to have all the required labels
>> already defined in the dtsi so it doesn't rely on these labels being
>> defined in the device dts.
>>
>> In other words, currently if you include sc7280.dtsi and try to build,
>> you first have to define the labels mpss_mem and wpss_mem (after this
>> patch series also cdsp_mem and adsp_mem) for it to build.
>>
>> I'm quite neutral either way, let me know :)
> 
> I haven't done a ton of thinking about this, so if I'm spouting
> gibberish then feel free to ignore me. :-P It just feels weird that
> when all the "dtsi" files are combined and you look at what you end up
> on a sc7280 Chrome board that you'll be reserving 32MB of memory for a
> device that's set (in the same device tree) to be "disabled", right?
> ...the 32MB is completely wasted, I think. If we wanted to enable the
> CDSP we'd have to modify the device tree anyway, so it seems like that
> same modification would set the CDSP to "okay" and also reserve the
> memory...
> 
> In that vein, it seems like maybe you could move the "cdsp_mem" to the
> SoC .dsti file with a status of "disabled". . I guess we don't do that
> elsewhere, but maybe we should be? As far as I can tell without
> testing it (just looking at fdt_scan_reserved_mem()) this should
> work...

What do you think about moving present reserve memory block from
sc7280-chrome-common to sc7280.dtsi and delete the stuff which
chrome does not need it sc7280-chrome-common ?

-Mukesh
> 
> -Doug

Re: [PATCH 7/9] arm64: dts: qcom: sc7280: Add CDSP node

[Luca Weiss]

On Tue Oct 31, 2023 at 7:44 AM CET, Mukesh Ojha wrote:
>
>
> On 10/30/2023 8:33 PM, Doug Anderson wrote:
> > Hi,
> > 
> > On Mon, Oct 30, 2023 at 7:43 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>
> >> On Mon Oct 30, 2023 at 3:11 PM CET, Doug Anderson wrote:
> >>> Hi,
> >>>
> >>> On Mon, Oct 30, 2023 at 2:12 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>>>
> >>>> On Mon Oct 30, 2023 at 10:04 AM CET, Mukesh Ojha wrote:
> >>>>>
> >>>>>
> >>>>> On 10/27/2023 7:50 PM, Luca Weiss wrote:
> >>>>>> Add the node for the ADSP found on the SC7280 SoC, using standard
> >>>>>> Qualcomm firmware.
> >>>>>>
> >>>>>> The memory region for sc7280-chrome-common.dtsi is taken from msm-5.4
> >>>>>> yupik.dtsi since the other areas also seem to match that file there,
> >>>>>> though I cannot be sure there.
> >>>>>>
> >>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >>>>>> ---
> >>>>>>    arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   5 +
> >>>>>>    arch/arm64/boot/dts/qcom/sc7280.dtsi               | 138 +++++++++++++++++++++
> >>>>>>    2 files changed, 143 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> >>>>>> index eb55616e0892..6e5a9d4c1fda 100644
> >>>>>> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> >>>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> >>>>>> @@ -29,6 +29,11 @@ adsp_mem: memory@86700000 {
> >>>>>>                      no-map;
> >>>>>>              };
> >>>>>>
> >>>>>> +           cdsp_mem: memory@88f00000 {
> >>>>>> +                   reg = <0x0 0x88f00000 0x0 0x1e00000>;
> >>>>>> +                   no-map;
> >>>>>> +           };
> >>>>>> +
> >>>>>
> >>>>> Just a question, why to do it here, if chrome does not use this ?
> >>>>
> >>>> Other memory regions in sc7280.dtsi also get referenced but not actually
> >>>> defined in that file, like mpss_mem and wpss_mem. Alternatively we can
> >>>> also try and solve this differently, but then we should probably also
> >>>> adjust mpss and wpss to be consistent.
> >>>>
> >>>> Apart from either declaring cdsp_mem in sc7280.dtsi or
> >>>> "/delete-property/ memory-region;" for CDSP I don't really have better
> >>>> ideas though.
> >>>>
> >>>> I also imagine these ChromeOS devices will want to enable cdsp at some
> >>>> point but I don't know any plans there.
> >>>
> >>> Given that "remoteproc_cdsp" has status "disabled" in the dtsi, it
> >>> feels like the dtsi shouldn't be reserving memory. I guess maybe
> >>> memory regions can't be status "disabled"?
> >>
> >> Hi Doug,
> >>
> >> That's how it works in really any qcom dtsi though. I think in most/all
> >> cases normally the reserved-memory is already declared in the SoC dtsi
> >> file and also used with the memory-region property.
> >>
> >> I wouldn't be against adjusting sc7280.dtsi to match the way it's done
> >> in the other dtsi files though, so to have all the required labels
> >> already defined in the dtsi so it doesn't rely on these labels being
> >> defined in the device dts.
> >>
> >> In other words, currently if you include sc7280.dtsi and try to build,
> >> you first have to define the labels mpss_mem and wpss_mem (after this
> >> patch series also cdsp_mem and adsp_mem) for it to build.
> >>
> >> I'm quite neutral either way, let me know :)
> > 
> > I haven't done a ton of thinking about this, so if I'm spouting
> > gibberish then feel free to ignore me. :-P It just feels weird that
> > when all the "dtsi" files are combined and you look at what you end up
> > on a sc7280 Chrome board that you'll be reserving 32MB of memory for a
> > device that's set (in the same device tree) to be "disabled", right?
> > ...the 32MB is completely wasted, I think. If we wanted to enable the
> > CDSP we'd have to modify the device tree anyway, so it seems like that
> > same modification would set the CDSP to "okay" and also reserve the
> > memory...
> > 
> > In that vein, it seems like maybe you could move the "cdsp_mem" to the
> > SoC .dsti file with a status of "disabled". . I guess we don't do that
> > elsewhere, but maybe we should be? As far as I can tell without
> > testing it (just looking at fdt_scan_reserved_mem()) this should
> > work...
>
> What do you think about moving present reserve memory block from
> sc7280-chrome-common to sc7280.dtsi and delete the stuff which
> chrome does not need it sc7280-chrome-common ?

Hi Mukesh,

I'll do that in v2, thanks!

Regards
Luca

>
> -Mukesh
> > 
> > -Doug

[PATCH 8/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable various remoteprocs

[Luca Weiss]

Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index cc092735ce17..d65eef30091b 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -490,6 +490,26 @@ &qupv3_id_1 {
 	status = "okay";
 };
 
+&remoteproc_adsp {
+	firmware-name = "qcom/qcm6490/fairphone5/adsp.mdt";
+	status = "okay";
+};
+
+&remoteproc_cdsp {
+	firmware-name = "qcom/qcm6490/fairphone5/cdsp.mdt";
+	status = "okay";
+};
+
+&remoteproc_mpss {
+	firmware-name = "qcom/qcm6490/fairphone5/modem.mdt";
+	status = "okay";
+};
+
+&remoteproc_wpss {
+	firmware-name = "qcom/qcm6490/fairphone5/wpss.mdt";
+	status = "okay";
+};
+
 &sdc2_clk {
 	drive-strength = <16>;
 	bias-disable;

-- 
2.42.0

Re: [PATCH 8/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable various remoteprocs

[Konrad Dybcio]

On 27.10.2023 16:20, Luca Weiss wrote:
> Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> index cc092735ce17..d65eef30091b 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -490,6 +490,26 @@ &qupv3_id_1 {
>  	status = "okay";
>  };
>  
> +&remoteproc_adsp {
> +	firmware-name = "qcom/qcm6490/fairphone5/adsp.mdt";
> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> +	firmware-name = "qcom/qcm6490/fairphone5/cdsp.mdt";
> +	status = "okay";
> +};
> +
> +&remoteproc_mpss {
> +	firmware-name = "qcom/qcm6490/fairphone5/modem.mdt";
> +	status = "okay";
> +};
> +
> +&remoteproc_wpss {
> +	firmware-name = "qcom/qcm6490/fairphone5/wpss.mdt";
mbn?

Konrad

Re: [PATCH 8/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable various remoteprocs

[Luca Weiss]

On Mon Oct 30, 2023 at 8:26 PM CET, Konrad Dybcio wrote:
> On 27.10.2023 16:20, Luca Weiss wrote:
> > Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > index cc092735ce17..d65eef30091b 100644
> > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > @@ -490,6 +490,26 @@ &qupv3_id_1 {
> >  	status = "okay";
> >  };
> >  
> > +&remoteproc_adsp {
> > +	firmware-name = "qcom/qcm6490/fairphone5/adsp.mdt";
> > +	status = "okay";
> > +};
> > +
> > +&remoteproc_cdsp {
> > +	firmware-name = "qcom/qcm6490/fairphone5/cdsp.mdt";
> > +	status = "okay";
> > +};
> > +
> > +&remoteproc_mpss {
> > +	firmware-name = "qcom/qcm6490/fairphone5/modem.mdt";
> > +	status = "okay";
> > +};
> > +
> > +&remoteproc_wpss {
> > +	firmware-name = "qcom/qcm6490/fairphone5/wpss.mdt";
> mbn?

Downstream ships mdt but if preferred I can change to .mbn and use
pil-squasher. Not sure what's the correct thing nowadays :)

Regards
Luca

>
> Konrad

[PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Luca Weiss]

Now that the WPSS remoteproc is enabled, enable wifi so we can use it.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index d65eef30091b..e7e20f73cbe6 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -713,3 +713,7 @@ &venus {
 	firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
 	status = "okay";
 };
+
+&wifi {
+	status = "okay";
+};

-- 
2.42.0

Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Konrad Dybcio]

On 27.10.2023 16:20, Luca Weiss wrote:
> Now that the WPSS remoteproc is enabled, enable wifi so we can use it.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> index d65eef30091b..e7e20f73cbe6 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -713,3 +713,7 @@ &venus {
>  	firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
>  	status = "okay";
>  };
> +
> +&wifi {
> +	status = "okay";
qcom,ath11k-calibration-variant?

Konrad

Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Luca Weiss]

On Mon Oct 30, 2023 at 8:26 PM CET, Konrad Dybcio wrote:
> On 27.10.2023 16:20, Luca Weiss wrote:
> > Now that the WPSS remoteproc is enabled, enable wifi so we can use it.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > index d65eef30091b..e7e20f73cbe6 100644
> > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > @@ -713,3 +713,7 @@ &venus {
> >  	firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
> >  	status = "okay";
> >  };
> > +
> > +&wifi {
> > +	status = "okay";
> qcom,ath11k-calibration-variant?

What value would I put there for my device? Based on existing usages
(mostly for ath10k) I'd say "Fairphone_5"?

And you mean I should add this property in dts before even looking into
the firmware/calibration side of it?

Regards
Luca

>
> Konrad

Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Konrad Dybcio]

On 31.10.2023 11:31, Luca Weiss wrote:
> On Mon Oct 30, 2023 at 8:26 PM CET, Konrad Dybcio wrote:
>> On 27.10.2023 16:20, Luca Weiss wrote:
>>> Now that the WPSS remoteproc is enabled, enable wifi so we can use it.
>>>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>> index d65eef30091b..e7e20f73cbe6 100644
>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>> @@ -713,3 +713,7 @@ &venus {
>>>  	firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
>>>  	status = "okay";
>>>  };
>>> +
>>> +&wifi {
>>> +	status = "okay";
>> qcom,ath11k-calibration-variant?
> 
> What value would I put there for my device? Based on existing usages
> (mostly for ath10k) I'd say "Fairphone_5"?
> 
> And you mean I should add this property in dts before even looking into
> the firmware/calibration side of it?
This is basically a "compatible" for the board file, I think Fairphone_5
makes sense here, perhaps Dmitry can confirm

Konrad

Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Dmitry Baryshkov]

[Added Kalle to the CC list]

On Tue, 31 Oct 2023 at 12:31, Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On Mon Oct 30, 2023 at 8:26 PM CET, Konrad Dybcio wrote:
> > On 27.10.2023 16:20, Luca Weiss wrote:
> > > Now that the WPSS remoteproc is enabled, enable wifi so we can use it.
> > >
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > > index d65eef30091b..e7e20f73cbe6 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > > @@ -713,3 +713,7 @@ &venus {
> > >     firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
> > >     status = "okay";
> > >  };
> > > +
> > > +&wifi {
> > > +   status = "okay";
> > qcom,ath11k-calibration-variant?
>
> What value would I put there for my device? Based on existing usages
> (mostly for ath10k) I'd say "Fairphone_5"?

I think this is fine.

> And you mean I should add this property in dts before even looking into
> the firmware/calibration side of it?

From my experience some (most?) of the device manufacturers do the
wrong thing here. They do not program a sensible board_id, leaving it
as 0xff or some other semi-random value. The calibration variant is
the only way for the kernel to distinguish between such poor devices.

The kernel will do a smart thing though. If the device-specific
calibration data is not present, it will try to fall back to the
generic data.

-- 
With best wishes
Dmitry

Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Kalle Valo]

(adding ath11k list)

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> [Added Kalle to the CC list]
>
> On Tue, 31 Oct 2023 at 12:31, Luca Weiss <luca.weiss@fairphone.com> wrote:
>>
>> On Mon Oct 30, 2023 at 8:26 PM CET, Konrad Dybcio wrote:
>> > On 27.10.2023 16:20, Luca Weiss wrote:
>> > > Now that the WPSS remoteproc is enabled, enable wifi so we can use it.
>> > >
>> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> > > ---
>> > >  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4 ++++
>> > >  1 file changed, 4 insertions(+)
>> > >
>> > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> > > index d65eef30091b..e7e20f73cbe6 100644
>> > > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> > > @@ -713,3 +713,7 @@ &venus {
>> > >     firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
>> > >     status = "okay";
>> > >  };
>> > > +
>> > > +&wifi {
>> > > +   status = "okay";
>> > qcom,ath11k-calibration-variant?
>>
>> What value would I put there for my device? Based on existing usages
>> (mostly for ath10k) I'd say "Fairphone_5"?
>
> I think this is fine.

From style point of view I would prefer lower case and dashes, for
example "fairphone-5" but I'm just nitpicking, uppercase and underscores
work fine as well.

If you have different SKUs or similar which need different ath11k board
files being more specific like "fairphone-5-eu" and "fairphone-5-us" is
one option. But I'm sure Luca knows best what is needed for Fairphone,
just throwing out ideas here.

>> And you mean I should add this property in dts before even looking into
>> the firmware/calibration side of it?
>
> From my experience some (most?) of the device manufacturers do the
> wrong thing here. They do not program a sensible board_id, leaving it
> as 0xff or some other semi-random value. The calibration variant is
> the only way for the kernel to distinguish between such poor devices.
>
> The kernel will do a smart thing though. If the device-specific
> calibration data is not present, it will try to fall back to the
> generic data.

You are correct, just to be specific it's ath11k which will choose which
board file to use. I recommend always setting
qcom,ath11k-calibration-variant in DTS if you can.

Back in the day I have tried to push for the firmware team to improve
the board file selection but no success. So the only practical solution
we have is qcom,ath11k-calibration-variant in DTS.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Luca Weiss]

On Mon Nov 13, 2023 at 1:22 PM CET, Kalle Valo wrote:
> (adding ath11k list)
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > [Added Kalle to the CC list]
> >
> > On Tue, 31 Oct 2023 at 12:31, Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>
> >> On Mon Oct 30, 2023 at 8:26 PM CET, Konrad Dybcio wrote:
> >> > On 27.10.2023 16:20, Luca Weiss wrote:
> >> > > Now that the WPSS remoteproc is enabled, enable wifi so we can use it.
> >> > >
> >> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >> > > ---
> >> > >  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 4 ++++
> >> > >  1 file changed, 4 insertions(+)
> >> > >
> >> > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> >> > > index d65eef30091b..e7e20f73cbe6 100644
> >> > > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> >> > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> >> > > @@ -713,3 +713,7 @@ &venus {
> >> > >     firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
> >> > >     status = "okay";
> >> > >  };
> >> > > +
> >> > > +&wifi {
> >> > > +   status = "okay";
> >> > qcom,ath11k-calibration-variant?
> >>
> >> What value would I put there for my device? Based on existing usages
> >> (mostly for ath10k) I'd say "Fairphone_5"?
> >
> > I think this is fine.
>
> From style point of view I would prefer lower case and dashes, for
> example "fairphone-5" but I'm just nitpicking, uppercase and underscores
> work fine as well.

I really don't mind, but I used "Fairphone_5" in v2 now, but I can
change it for v3 if that happens if you wish.

>
> If you have different SKUs or similar which need different ath11k board
> files being more specific like "fairphone-5-eu" and "fairphone-5-us" is
> one option. But I'm sure Luca knows best what is needed for Fairphone,
> just throwing out ideas here.

As far as I am aware, there's only one hardware variant, so nothing
extra should be needed there. (We also only really sell in Europe, apart
from a small rollout in Taiwan and a trial in the US, but same HW there)

Regards
Luca

>
> >> And you mean I should add this property in dts before even looking into
> >> the firmware/calibration side of it?
> >
> > From my experience some (most?) of the device manufacturers do the
> > wrong thing here. They do not program a sensible board_id, leaving it
> > as 0xff or some other semi-random value. The calibration variant is
> > the only way for the kernel to distinguish between such poor devices.
> >
> > The kernel will do a smart thing though. If the device-specific
> > calibration data is not present, it will try to fall back to the
> > generic data.
>
> You are correct, just to be specific it's ath11k which will choose which
> board file to use. I recommend always setting
> qcom,ath11k-calibration-variant in DTS if you can.
>
> Back in the day I have tried to push for the firmware team to improve
> the board file selection but no success. So the only practical solution
> we have is qcom,ath11k-calibration-variant in DTS.

Re: [PATCH 9/9] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable WiFi

[Kalle Valo]

"Luca Weiss" <luca.weiss@fairphone.com> writes:

>> >> > > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> >> > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> >> > > @@ -713,3 +713,7 @@ &venus {
>> >> > >     firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
>> >> > >     status = "okay";
>> >> > >  };
>> >> > > +
>> >> > > +&wifi {
>> >> > > +   status = "okay";
>> >> > qcom,ath11k-calibration-variant?
>> >>
>> >> What value would I put there for my device? Based on existing usages
>> >> (mostly for ath10k) I'd say "Fairphone_5"?
>> >
>> > I think this is fine.
>>
>> From style point of view I would prefer lower case and dashes, for
>> example "fairphone-5" but I'm just nitpicking, uppercase and underscores
>> work fine as well.
>
> I really don't mind, but I used "Fairphone_5" in v2 now, but I can
> change it for v3 if that happens if you wish.

Nah, no need to resend. That's fine.

But in the future please try to CC the ath11k list for patches like
this, easier to follow what's happening.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches