| Message ID | 1615269111-25559-7-git-send-email-sibis@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable miscellaneous hardware blocks to boot WPSS | expand |
Quoting Sibi Sankar (2021-03-08 21:51:51) > Add miscellaneous nodes to boot the Wireless Processor Subsystem on Maybe add (WPSS) after the name so we know they're related. > SC7280 SoCs. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > > https://patchwork.kernel.org/project/linux-arm-msm/list/?series=438217 > Depends on ipcc dt node enablement from ^^ > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 143 +++++++++++++++++++++++++++++++++++ > 1 file changed, 143 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 18637c369c1d..4f03c468df51 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -244,12 +251,131 @@ > reg = <0 0x80000000 0 0>; > }; > > + tcsr_mutex: hwlock { > + compatible = "qcom,tcsr-mutex"; > + syscon = <&tcsr_mutex_regs 0 0x1000>; > + #hwlock-cells = <1>; > + }; Is this node in the right place? I think the node above it is 'memory'? In which case 'hwlock' comes before 'memory' alphabetically. > + > + smem { > + compatible = "qcom,smem"; > + memory-region = <&smem_mem>; > + hwlocks = <&tcsr_mutex 3>; > + }; > + > firmware { > scm { > compatible = "qcom,scm-sc7280", "qcom,scm"; > }; > }; > > + smp2p-adsp { > + compatible = "qcom,smp2p"; > + qcom,smem = <443>, <429>; > + interrupts-extended = <&ipcc IPCC_CLIENT_LPASS > + IPCC_MPROC_SIGNAL_SMP2P > + IRQ_TYPE_EDGE_RISING>; > + mboxes = <&ipcc IPCC_CLIENT_LPASS > + IPCC_MPROC_SIGNAL_SMP2P>; > + > + qcom,local-pid = <0>; > + qcom,remote-pid = <2>; > + > + adsp_smp2p_out: master-kernel { > + qcom,entry-name = "master-kernel"; > + #qcom,smem-state-cells = <1>; > + }; > + > + adsp_smp2p_in: slave-kernel { > + qcom,entry-name = "slave-kernel"; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + }; > + > + smp2p-cdsp { > + compatible = "qcom,smp2p"; > + qcom,smem = <94>, <432>; > + interrupts-extended = <&ipcc IPCC_CLIENT_CDSP > + IPCC_MPROC_SIGNAL_SMP2P > + IRQ_TYPE_EDGE_RISING>; > + mboxes = <&ipcc IPCC_CLIENT_CDSP > + IPCC_MPROC_SIGNAL_SMP2P>; > + > + qcom,local-pid = <0>; > + qcom,remote-pid = <5>; > + > + cdsp_smp2p_out: master-kernel { > + qcom,entry-name = "master-kernel"; > + #qcom,smem-state-cells = <1>; > + }; > + > + cdsp_smp2p_in: slave-kernel { > + qcom,entry-name = "slave-kernel"; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + }; > + > + smp2p-mpss { > + compatible = "qcom,smp2p"; > + qcom,smem = <435>, <428>; > + interrupts-extended = <&ipcc IPCC_CLIENT_MPSS > + IPCC_MPROC_SIGNAL_SMP2P > + IRQ_TYPE_EDGE_RISING>; > + mboxes = <&ipcc IPCC_CLIENT_MPSS > + IPCC_MPROC_SIGNAL_SMP2P>; > + > + qcom,local-pid = <0>; > + qcom,remote-pid = <1>; > + > + modem_smp2p_out: master-kernel { > + qcom,entry-name = "master-kernel"; > + #qcom,smem-state-cells = <1>; > + }; > + > + modem_smp2p_in: slave-kernel { > + qcom,entry-name = "slave-kernel"; Do these names need to have 'master' and 'slave' in them? We're trying to avoid these terms. See Documentation/process/coding-style.rst Section 4 naming. > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + > + ipa_smp2p_out: ipa-ap-to-modem { > + qcom,entry-name = "ipa"; > + #qcom,smem-state-cells = <1>; > + }; > + > + ipa_smp2p_in: ipa-modem-to-ap { > + qcom,entry-name = "ipa"; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + }; > +
On Sat 13 Mar 15:46 CST 2021, Stephen Boyd wrote: > Quoting Sibi Sankar (2021-03-08 21:51:51) > > Add miscellaneous nodes to boot the Wireless Processor Subsystem on > > Maybe add (WPSS) after the name so we know they're related. > > > SC7280 SoCs. > > > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > > --- > > > > https://patchwork.kernel.org/project/linux-arm-msm/list/?series=438217 > > Depends on ipcc dt node enablement from ^^ > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 143 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 143 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > index 18637c369c1d..4f03c468df51 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > @@ -244,12 +251,131 @@ > > reg = <0 0x80000000 0 0>; > > }; > > > > + tcsr_mutex: hwlock { > > + compatible = "qcom,tcsr-mutex"; > > + syscon = <&tcsr_mutex_regs 0 0x1000>; > > + #hwlock-cells = <1>; > > + }; > > Is this node in the right place? I think the node above it is 'memory'? > In which case 'hwlock' comes before 'memory' alphabetically. > Thanks for spotting this, as it's no longer acceptable to have a standalone "syscon" node I was asked to rewrite the binding for this a few months ago. So the tcsr_mutex should now be represented with a reg under /soc. > > + > > + smem { > > + compatible = "qcom,smem"; > > + memory-region = <&smem_mem>; > > + hwlocks = <&tcsr_mutex 3>; > > + }; > > + > > firmware { > > scm { > > compatible = "qcom,scm-sc7280", "qcom,scm"; > > }; > > }; > > > > + smp2p-adsp { > > + compatible = "qcom,smp2p"; > > + qcom,smem = <443>, <429>; > > + interrupts-extended = <&ipcc IPCC_CLIENT_LPASS > > + IPCC_MPROC_SIGNAL_SMP2P > > + IRQ_TYPE_EDGE_RISING>; > > + mboxes = <&ipcc IPCC_CLIENT_LPASS > > + IPCC_MPROC_SIGNAL_SMP2P>; > > + > > + qcom,local-pid = <0>; > > + qcom,remote-pid = <2>; > > + > > + adsp_smp2p_out: master-kernel { > > + qcom,entry-name = "master-kernel"; > > + #qcom,smem-state-cells = <1>; > > + }; > > + > > + adsp_smp2p_in: slave-kernel { > > + qcom,entry-name = "slave-kernel"; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + }; > > + > > + smp2p-cdsp { > > + compatible = "qcom,smp2p"; > > + qcom,smem = <94>, <432>; > > + interrupts-extended = <&ipcc IPCC_CLIENT_CDSP > > + IPCC_MPROC_SIGNAL_SMP2P > > + IRQ_TYPE_EDGE_RISING>; > > + mboxes = <&ipcc IPCC_CLIENT_CDSP > > + IPCC_MPROC_SIGNAL_SMP2P>; > > + > > + qcom,local-pid = <0>; > > + qcom,remote-pid = <5>; > > + > > + cdsp_smp2p_out: master-kernel { > > + qcom,entry-name = "master-kernel"; > > + #qcom,smem-state-cells = <1>; > > + }; > > + > > + cdsp_smp2p_in: slave-kernel { > > + qcom,entry-name = "slave-kernel"; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + }; > > + > > + smp2p-mpss { > > + compatible = "qcom,smp2p"; > > + qcom,smem = <435>, <428>; > > + interrupts-extended = <&ipcc IPCC_CLIENT_MPSS > > + IPCC_MPROC_SIGNAL_SMP2P > > + IRQ_TYPE_EDGE_RISING>; > > + mboxes = <&ipcc IPCC_CLIENT_MPSS > > + IPCC_MPROC_SIGNAL_SMP2P>; > > + > > + qcom,local-pid = <0>; > > + qcom,remote-pid = <1>; > > + > > + modem_smp2p_out: master-kernel { > > + qcom,entry-name = "master-kernel"; > > + #qcom,smem-state-cells = <1>; > > + }; > > + > > + modem_smp2p_in: slave-kernel { > > + qcom,entry-name = "slave-kernel"; > > Do these names need to have 'master' and 'slave' in them? We're trying > to avoid these terms. See Documentation/process/coding-style.rst Section > 4 naming. > They need to match the naming in the firmware, but I would welcome a future change to something in line with the coding style and simply more descriptive. Regards, Bjorn > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + > > + ipa_smp2p_out: ipa-ap-to-modem { > > + qcom,entry-name = "ipa"; > > + #qcom,smem-state-cells = <1>; > > + }; > > + > > + ipa_smp2p_in: ipa-modem-to-ap { > > + qcom,entry-name = "ipa"; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + }; > > +
Quoting Bjorn Andersson (2021-03-13 20:16:39) > On Sat 13 Mar 15:46 CST 2021, Stephen Boyd wrote: > > > Quoting Sibi Sankar (2021-03-08 21:51:51) > > > Add miscellaneous nodes to boot the Wireless Processor Subsystem on > > > > Maybe add (WPSS) after the name so we know they're related. > > > > > SC7280 SoCs. > > > > > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > > > --- > > > > > > https://patchwork.kernel.org/project/linux-arm-msm/list/?series=438217 > > > Depends on ipcc dt node enablement from ^^ > > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 143 +++++++++++++++++++++++++++++++++++ > > > 1 file changed, 143 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > index 18637c369c1d..4f03c468df51 100644 > > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > @@ -244,12 +251,131 @@ > > > reg = <0 0x80000000 0 0>; > > > }; > > > > > > + tcsr_mutex: hwlock { > > > + compatible = "qcom,tcsr-mutex"; > > > + syscon = <&tcsr_mutex_regs 0 0x1000>; > > > + #hwlock-cells = <1>; > > > + }; > > > > Is this node in the right place? I think the node above it is 'memory'? > > In which case 'hwlock' comes before 'memory' alphabetically. > > > > Thanks for spotting this, as it's no longer acceptable to have a > standalone "syscon" node I was asked to rewrite the binding for this a > few months ago. So the tcsr_mutex should now be represented with a reg > under /soc. Oh nice, I wasn't aware. > > > + #interrupt-cells = <2>; > > > + }; > > > + }; > > > + > > > + smp2p-mpss { > > > + compatible = "qcom,smp2p"; > > > + qcom,smem = <435>, <428>; > > > + interrupts-extended = <&ipcc IPCC_CLIENT_MPSS > > > + IPCC_MPROC_SIGNAL_SMP2P > > > + IRQ_TYPE_EDGE_RISING>; > > > + mboxes = <&ipcc IPCC_CLIENT_MPSS > > > + IPCC_MPROC_SIGNAL_SMP2P>; > > > + > > > + qcom,local-pid = <0>; > > > + qcom,remote-pid = <1>; > > > + > > > + modem_smp2p_out: master-kernel { > > > + qcom,entry-name = "master-kernel"; > > > + #qcom,smem-state-cells = <1>; > > > + }; > > > + > > > + modem_smp2p_in: slave-kernel { > > > + qcom,entry-name = "slave-kernel"; > > > > Do these names need to have 'master' and 'slave' in them? We're trying > > to avoid these terms. See Documentation/process/coding-style.rst Section > > 4 naming. > > > > They need to match the naming in the firmware, but I would welcome a > future change to something in line with the coding style and simply more > descriptive. > Sibi can this be done? I think it's still pretty early days for the firmware so hopefully the terms can be replaced with something different.
On 2021-03-24 12:19, Sibi Sankar wrote: > On 2021-03-24 03:36, Stephen Boyd wrote: >> Quoting Bjorn Andersson (2021-03-13 20:16:39) >>> On Sat 13 Mar 15:46 CST 2021, Stephen Boyd wrote: >>> >>> > Quoting Sibi Sankar (2021-03-08 21:51:51) >>> > > Add miscellaneous nodes to boot the Wireless Processor Subsystem on >>> > >>> > Maybe add (WPSS) after the name so we know they're related. >>> > >>> > > SC7280 SoCs. >>> > > >>> > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >>> > > --- >>> > > >>> > > https://patchwork.kernel.org/project/linux-arm-msm/list/?series=438217 >>> > > Depends on ipcc dt node enablement from ^^ >>> > > >>> > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 143 +++++++++++++++++++++++++++++++++++ >>> > > 1 file changed, 143 insertions(+) >>> > > >>> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> > > index 18637c369c1d..4f03c468df51 100644 >>> > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> > > @@ -244,12 +251,131 @@ >>> > > reg = <0 0x80000000 0 0>; >>> > > }; >>> > > >>> > > + tcsr_mutex: hwlock { >>> > > + compatible = "qcom,tcsr-mutex"; >>> > > + syscon = <&tcsr_mutex_regs 0 0x1000>; >>> > > + #hwlock-cells = <1>; >>> > > + }; >>> > >>> > Is this node in the right place? I think the node above it is 'memory'? >>> > In which case 'hwlock' comes before 'memory' alphabetically. >>> > >>> >>> Thanks for spotting this, as it's no longer acceptable to have a >>> standalone "syscon" node I was asked to rewrite the binding for this >>> a >>> few months ago. So the tcsr_mutex should now be represented with a >>> reg >>> under /soc. >> >> Oh nice, I wasn't aware. >> >>> > > + #interrupt-cells = <2>; >>> > > + }; >>> > > + }; >>> > > + >>> > > + smp2p-mpss { >>> > > + compatible = "qcom,smp2p"; >>> > > + qcom,smem = <435>, <428>; >>> > > + interrupts-extended = <&ipcc IPCC_CLIENT_MPSS >>> > > + IPCC_MPROC_SIGNAL_SMP2P >>> > > + IRQ_TYPE_EDGE_RISING>; >>> > > + mboxes = <&ipcc IPCC_CLIENT_MPSS >>> > > + IPCC_MPROC_SIGNAL_SMP2P>; >>> > > + >>> > > + qcom,local-pid = <0>; >>> > > + qcom,remote-pid = <1>; >>> > > + >>> > > + modem_smp2p_out: master-kernel { >>> > > + qcom,entry-name = "master-kernel"; >>> > > + #qcom,smem-state-cells = <1>; >>> > > + }; >>> > > + >>> > > + modem_smp2p_in: slave-kernel { >>> > > + qcom,entry-name = "slave-kernel"; >>> > >>> > Do these names need to have 'master' and 'slave' in them? We're trying >>> > to avoid these terms. See Documentation/process/coding-style.rst Section >>> > 4 naming. >>> > >>> >>> They need to match the naming in the firmware, but I would welcome a >>> future change to something in line with the coding style and simply >>> more >>> descriptive. >>> >> >> Sibi can this be done? I think it's still pretty early days for the >> firmware so hopefully the terms can be replaced with something >> different. > > I'll discuss the ask with the modem fw team and > get back. Sorry for the delayed response. Looks like it's something Qualcomm wouldn't want to do mid-project since a number of the fw images are re-used across platforms. But this is something that will be taken up on newer SoCs.
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 18637c369c1d..4f03c468df51 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -10,6 +10,8 @@ #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/mailbox/qcom-ipcc.h> #include <dt-bindings/power/qcom-rpmpd.h> +#include <dt-bindings/reset/qcom,sdm845-aoss.h> +#include <dt-bindings/reset/qcom,sdm845-pdc.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> / { @@ -50,6 +52,11 @@ no-map; }; + smem_mem: memory@80900000 { + reg = <0x0 0x80900000 0x0 0x200000>; + no-map; + }; + cpucp_mem: memory@80b00000 { no-map; reg = <0x0 0x80b00000 0x0 0x100000>; @@ -244,12 +251,131 @@ reg = <0 0x80000000 0 0>; }; + tcsr_mutex: hwlock { + compatible = "qcom,tcsr-mutex"; + syscon = <&tcsr_mutex_regs 0 0x1000>; + #hwlock-cells = <1>; + }; + + smem { + compatible = "qcom,smem"; + memory-region = <&smem_mem>; + hwlocks = <&tcsr_mutex 3>; + }; + firmware { scm { compatible = "qcom,scm-sc7280", "qcom,scm"; }; }; + smp2p-adsp { + compatible = "qcom,smp2p"; + qcom,smem = <443>, <429>; + interrupts-extended = <&ipcc IPCC_CLIENT_LPASS + IPCC_MPROC_SIGNAL_SMP2P + IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_LPASS + IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <2>; + + adsp_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + adsp_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-cdsp { + compatible = "qcom,smp2p"; + qcom,smem = <94>, <432>; + interrupts-extended = <&ipcc IPCC_CLIENT_CDSP + IPCC_MPROC_SIGNAL_SMP2P + IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_CDSP + IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <5>; + + cdsp_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + cdsp_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-mpss { + compatible = "qcom,smp2p"; + qcom,smem = <435>, <428>; + interrupts-extended = <&ipcc IPCC_CLIENT_MPSS + IPCC_MPROC_SIGNAL_SMP2P + IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_MPSS + IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <1>; + + modem_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + modem_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + + ipa_smp2p_out: ipa-ap-to-modem { + qcom,entry-name = "ipa"; + #qcom,smem-state-cells = <1>; + }; + + ipa_smp2p_in: ipa-modem-to-ap { + qcom,entry-name = "ipa"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + smp2p-wpss { + compatible = "qcom,smp2p"; + qcom,smem = <617>, <616>; + interrupts-extended = <&ipcc IPCC_CLIENT_WPSS + IPCC_MPROC_SIGNAL_SMP2P + IRQ_TYPE_EDGE_RISING>; + mboxes = <&ipcc IPCC_CLIENT_WPSS + IPCC_MPROC_SIGNAL_SMP2P>; + + qcom,local-pid = <0>; + qcom,remote-pid = <13>; + + wpss_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,smem-state-cells = <1>; + }; + + wpss_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + pmu { compatible = "arm,armv8-pmuv3"; interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>; @@ -315,6 +441,11 @@ }; }; + tcsr_mutex_regs: syscon@1f40000 { + compatible = "syscon"; + reg = <0 0x01f40000 0 0x40000>; + }; + pdc: interrupt-controller@b220000 { compatible = "qcom,sc7280-pdc", "qcom,pdc"; reg = <0 0x0b220000 0 0x30000>; @@ -328,6 +459,18 @@ interrupt-controller; }; + pdc_reset: reset-controller@b5e0000 { + compatible = "qcom,sc7280-pdc-global"; + reg = <0 0x0b5e0000 0 0x20000>; + #reset-cells = <1>; + }; + + aoss_reset: reset-controller@c2a0000 { + compatible = "qcom,sc7280-aoss-cc", "qcom,sdm845-aoss-cc"; + reg = <0 0x0c2a0000 0 0x31000>; + #reset-cells = <1>; + }; + spmi_bus: spmi@c440000 { compatible = "qcom,spmi-pmic-arb"; reg = <0 0x0c440000 0 0x1100>,
Add miscellaneous nodes to boot the Wireless Processor Subsystem on SC7280 SoCs. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- https://patchwork.kernel.org/project/linux-arm-msm/list/?series=438217 Depends on ipcc dt node enablement from ^^ arch/arm64/boot/dts/qcom/sc7280.dtsi | 143 +++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+)