| Message ID | 20221208183837.505454-1-elder@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: sc7280: enable IPA in sc7280-herobrine-lte-sku.dtsi | expand |
On 15.12.2022 18:38, Sibi Sankar wrote: > Alex, > > Thanks for the patch. > > On 12/9/22 00:08, Alex Elder wrote: >> 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 <elder@linaro.org> >> --- >> 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(-) >> >> 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 ad66e5e9db4ed..956708397f035 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >> @@ -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 <quic_sibis@quicinc.com> > Tested-by: Sibi Sankar <quic_sibis@quicinc.com> That + please flip the property order, as we're trying to keep status last wherever possible. Konrad > >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> index f7efb9966afd1..0ddbe7f732bd9 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> @@ -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 >> --- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi >> @@ -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>; >> };
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 ad66e5e9db4ed..956708397f035 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi @@ -34,3 +34,8 @@ &remoteproc_mpss { &rmtfs_mem { reg = <0x0 0x9c900000 0x0 0x800000>; }; + +&ipa { + status = "okay"; + modem-init; +}; diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi index f7efb9966afd1..0ddbe7f732bd9 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi @@ -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 --- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi @@ -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>; };
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 <elder@linaro.org> --- 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(-)