arm64: dts: qcom: sc7280: enable IPA in sc7280-herobrine-lte-sku.dtsi

PATCH Superseded Alex Elder 4 participants 4 messages Dec 8, 2022 – Dec 15, 2022

Subsystems (4)

Device tree bindings Arm Qualcomm mailing list Arm Qualcomm support Arm/Qualcomm chromebook support

lore

Share & tools

lore

Alex Elder

author

2022-12-08 18:38 UTC #0 lore raw

PATCH 1/1 arm64: dts: qcom: sc7280: enable IPA in sc7280-herobrine-lte-sku.dtsi

IPA is only needed on a platform if it includes a modem, and not all SC7280 SoC variants do. The file "sc7280-herobrine-lte-sku.dtsi" is used to encapsulate definitions related to Chrome OS SC7280 devices where a modem is present, and that's the proper place for the IPA node to be enabled.

Currently IPA is enabled in "sc7280-idp.dtsi", which is included by DTS files for Qualcomm reference platforms (all of which include the modem). That also includes "sc7280-herobrine-lte-sku.dtsi", so enabling IPA there would make it unnecessary for "sc7280-idp.dtsi" to enable it.

The only other place IPA is enabled is "sc7280-qcard.dtsi". That file is included only by "sc7280-herobrine.dtsi", which is (eventually) included only by these top-level DTS files:
sc7280-herobrine-crd.dts sc7280-herobrine-herobrine-r1.dts sc7280-herobrine-evoker.dts sc7280-herobrine-evoker-lte.dts sc7280-herobrine-villager-r0.dts sc7280-herobrine-villager-r1.dts sc7280-herobrine-villager-r1-lte.dts
All of but two of these include "sc7280-herobrine-lte-sku.dtsi", and for those cases, enabling IPA there means there is no need for it to be enabled in "sc7280-qcard.dtsi".

The two remaining cases will no longer enable IPA as a result of this change:
sc7280-herobrine-evoker.dts sc7280-herobrine-villager-r1.dts
Both of these have "lte" counterparts, and are meant to represent board variants that do *not* have a modem.

This is exactly the desired configuration.

Signed-off-by Alex Elder <[email protected]>

---
 arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 5 +++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi               | 5 -----
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi             | 5 -----
 3 files changed, 5 insertions(+), 10 deletions(-)

arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +5 -0

index ad66e5e9db4ed..956708397f035 100644
@@ -34,3 +34,8 @@ &remoteproc_mpss {
 &rmtfs_mem {
 	reg = <0x0 0x9c900000 0x0 0x800000>;
 };
+
+&ipa {
+	status = "okay";
+	modem-init;
+};

arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +0 -5

index f7efb9966afd1..0ddbe7f732bd9 100644
@@ -376,11 +376,6 @@ &gpi_dma1 {
 	status = "okay";
 };
 
-&ipa {
-	status = "okay";
-	modem-init;
-};
-
 &lpass_cpu {
 	status = "okay";

arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi +0 -5

index df49564ae6dc1..cd6ee84b36fd4 100644
@@ -336,11 +336,6 @@ vreg_bob: bob {
 
 /* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
 
-&ipa {
-	status = "okay";
-	modem-init;
-};
-
 &lpass_va_macro {
 	vdd-micb-supply = <&vreg_bob>;
 };

Signature · 1 line

2.34.1

Sibi Sankar

Reviewed-by Tested-by on v1 1/1

2022-12-15 17:38 UTC #1 lore raw

Alex,

Thanks for the patch.

Quoted 47 lines · Alex Elder

IPA is only needed on a platform if it includes a modem, and not all SC7280 SoC variants do. The file "sc7280-herobrine-lte-sku.dtsi" is used to encapsulate definitions related to Chrome OS SC7280 devices where a modem is present, and that's the proper place for the IPA node to be enabled.
Currently IPA is enabled in "sc7280-idp.dtsi", which is included by DTS files for Qualcomm reference platforms (all of which include the modem). That also includes "sc7280-herobrine-lte-sku.dtsi", so enabling IPA there would make it unnecessary for "sc7280-idp.dtsi" to enable it.
The only other place IPA is enabled is "sc7280-qcard.dtsi". That file is included only by "sc7280-herobrine.dtsi", which is (eventually) included only by these top-level DTS files:
   sc7280-herobrine-crd.dts
   sc7280-herobrine-herobrine-r1.dts
   sc7280-herobrine-evoker.dts
   sc7280-herobrine-evoker-lte.dts
   sc7280-herobrine-villager-r0.dts
   sc7280-herobrine-villager-r1.dts
   sc7280-herobrine-villager-r1-lte.dts
All of but two of these include "sc7280-herobrine-lte-sku.dtsi", and for those cases, enabling IPA there means there is no need for it to be enabled in "sc7280-qcard.dtsi".
The two remaining cases will no longer enable IPA as a result of this change:
   sc7280-herobrine-evoker.dts
   sc7280-herobrine-villager-r1.dts
Both of these have "lte" counterparts, and are meant to represent board variants that do *not* have a modem.
This is exactly the desired configuration.
Signed-off-by Alex Elder <[email protected]>
---
  arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 5 +++++
  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi               | 5 -----
  arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi             | 5 -----
  3 files changed, 5 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +0 -0
index ad66e5e9db4ed..956708397f035 100644

arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi &remoteproc_mpss { · line 41

Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +5 -0

@@ -34,3 +34,8 @@ &remoteproc_mpss {
  &rmtfs_mem {
  	reg = <0x0 0x9c900000 0x0 0x800000>;
  };
+
+&ipa {
+	status = "okay";
+	modem-init;
+};

nit; You'll probably want to move the ipa node above remoteproc node to maintain sorting.

Reviewed-by Sibi Sankar <[email protected]>

Tested-by Sibi Sankar <[email protected]>

Quoted 32 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +0 -5

index f7efb9966afd1..0ddbe7f732bd9 100644
@@ -376,11 +376,6 @@ &gpi_dma1 {
  	status = "okay";
  };
 
-&ipa {
-	status = "okay";
-	modem-init;
-};
-
  &lpass_cpu {
  	status = "okay";

arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi +0 -5

index df49564ae6dc1..cd6ee84b36fd4 100644
@@ -336,11 +336,6 @@ vreg_bob: bob {
 
  /* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
 
-&ipa {
-	status = "okay";
-	modem-init;
-};
-
  &lpass_va_macro {
  	vdd-micb-supply = <&vreg_bob>;
  };

Konrad Dybcio

2022-12-15 18:01 UTC #2 lore raw

Quoted 64 lines · Sibi Sankar

Alex,
Thanks for the patch.
Quoted 47 lines · Alex Elder
IPA is only needed on a platform if it includes a modem, and not all SC7280 SoC variants do. The file "sc7280-herobrine-lte-sku.dtsi" is used to encapsulate definitions related to Chrome OS SC7280 devices where a modem is present, and that's the proper place for the IPA node to be enabled.
Currently IPA is enabled in "sc7280-idp.dtsi", which is included by DTS files for Qualcomm reference platforms (all of which include the modem). That also includes "sc7280-herobrine-lte-sku.dtsi", so enabling IPA there would make it unnecessary for "sc7280-idp.dtsi" to enable it.
The only other place IPA is enabled is "sc7280-qcard.dtsi". That file is included only by "sc7280-herobrine.dtsi", which is (eventually) included only by these top-level DTS files:
   sc7280-herobrine-crd.dts sc7280-herobrine-herobrine-r1.dts sc7280-herobrine-evoker.dts sc7280-herobrine-evoker-lte.dts sc7280-herobrine-villager-r0.dts sc7280-herobrine-villager-r1.dts sc7280-herobrine-villager-r1-lte.dts
All of but two of these include "sc7280-herobrine-lte-sku.dtsi", and for those cases, enabling IPA there means there is no need for it to be enabled in "sc7280-qcard.dtsi".
The two remaining cases will no longer enable IPA as a result of this change:
   sc7280-herobrine-evoker.dts sc7280-herobrine-villager-r1.dts
Both of these have "lte" counterparts, and are meant to represent board variants that do *not* have a modem.
This is exactly the desired configuration.
Signed-off-by Alex Elder <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 5 +++++ arch/arm64/boot/dts/qcom/sc7280-idp.dtsi               | 5 ----- arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi             | 5 -----
  3 files changed, 5 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +0 -0
index ad66e5e9db4ed..956708397f035 100644
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi &remoteproc_mpss { · line 41
Quoted 8 lines
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +5 -0
@@ -34,3 +34,8 @@ &remoteproc_mpss {
   &rmtfs_mem {
       reg = <0x0 0x9c900000 0x0 0x800000>;
   };
+
+&ipa {
+    status = "okay";
+    modem-init;
+};
nit; You'll probably want to move the ipa node above remoteproc node to maintain sorting.

Quoted 2 lines

Reviewed-by Sibi Sankar <[email protected]>
Tested-by Sibi Sankar <[email protected]>

That + please flip the property order, as we're trying to keep status last wherever possible.

Konrad

Quoted 29 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +0 -4

index f7efb9966afd1..0ddbe7f732bd9 100644
@@ -376,11 +376,6 @@ &gpi_dma1 {
       status = "okay";
   };
   -&ipa {
-    status = "okay";
-    modem-init;
-};
-
   &lpass_cpu {
       status = "okay";

diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi index df49564ae6dc1..cd6ee84b36fd4 100644

arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi +0 -4

@@ -336,11 +336,6 @@ vreg_bob: bob {
     /* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
   -&ipa {
-    status = "okay";
-    modem-init;
-};
-
   &lpass_va_macro {
       vdd-micb-supply = <&vreg_bob>;
   };

Alex Elder

2022-12-15 18:11 UTC #3 lore raw

Quoted 67 lines · Konrad Dybcio

Quoted 64 lines · Sibi Sankar
Alex,
Thanks for the patch.
Quoted 47 lines · Alex Elder
IPA is only needed on a platform if it includes a modem, and not all SC7280 SoC variants do. The file "sc7280-herobrine-lte-sku.dtsi" is used to encapsulate definitions related to Chrome OS SC7280 devices where a modem is present, and that's the proper place for the IPA node to be enabled.
Currently IPA is enabled in "sc7280-idp.dtsi", which is included by DTS files for Qualcomm reference platforms (all of which include the modem). That also includes "sc7280-herobrine-lte-sku.dtsi", so enabling IPA there would make it unnecessary for "sc7280-idp.dtsi" to enable it.
The only other place IPA is enabled is "sc7280-qcard.dtsi". That file is included only by "sc7280-herobrine.dtsi", which is (eventually) included only by these top-level DTS files:
sc7280-herobrine-crd.dts sc7280-herobrine-herobrine-r1.dts sc7280-herobrine-evoker.dts sc7280-herobrine-evoker-lte.dts sc7280-herobrine-villager-r0.dts sc7280-herobrine-villager-r1.dts sc7280-herobrine-villager-r1-lte.dts
All of but two of these include "sc7280-herobrine-lte-sku.dtsi", and for those cases, enabling IPA there means there is no need for it to be enabled in "sc7280-qcard.dtsi".
The two remaining cases will no longer enable IPA as a result of this change:
sc7280-herobrine-evoker.dts sc7280-herobrine-villager-r1.dts
Both of these have "lte" counterparts, and are meant to represent board variants that do *not* have a modem.
This is exactly the desired configuration.
Signed-off-by Alex Elder <[email protected]>
---
   arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 5 +++++
   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi               | 5 -----
   arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi             | 5 -----
   3 files changed, 5 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +0 -0
index ad66e5e9db4ed..956708397f035 100644
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi &remoteproc_mpss { · line 41
Quoted 8 lines
arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +5 -0
@@ -34,3 +34,8 @@ &remoteproc_mpss {
   &rmtfs_mem {
       reg = <0x0 0x9c900000 0x0 0x800000>;
   };
+
+&ipa {
+    status = "okay";
+    modem-init;
+};
nit; You'll probably want to move the ipa node above remoteproc node to maintain sorting.

Quoted 4 lines

Quoted 2 lines
Reviewed-by Sibi Sankar <[email protected]>
Tested-by Sibi Sankar <[email protected]>
That + please flip the property order, as we're trying to keep status last wherever possible.

Thank you, I'll post version 2 this afternoon. -Alex

Quoted 31 lines

Konrad

Quoted 29 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +0 -4

index f7efb9966afd1..0ddbe7f732bd9 100644
@@ -376,11 +376,6 @@ &gpi_dma1 {
       status = "okay";
   };
   -&ipa {
-    status = "okay";
-    modem-init;
-};
-
   &lpass_cpu {
       status = "okay";
   diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi

index df49564ae6dc1..cd6ee84b36fd4 100644

arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi +0 -4

@@ -336,11 +336,6 @@ vreg_bob: bob {
     /* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
   -&ipa {
-    status = "okay";
-    modem-init;
-};
-
   &lpass_va_macro {
       vdd-micb-supply = <&vreg_bob>;
   };