| Message ID | 20190109170010.15430-1-sibis@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
Hi, On Wed, Jan 9, 2019 at 9:00 AM Sibi Sankar <sibis@codeaurora.org> wrote: > > This patch adds Q6V5 MSS remoteproc node for SDM845 SoCs. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > --- > > v5: > * Use qmp_aop updated dt binding nit: since this is now a singleton patch in v5 (because patches #1 - #7 landed), the general policy is to drop the "8/8" in the subject. AKA I believe the subject of the patch ought to have been: [PATCH v5] arm64: dts: qcom: sdm845: Add Q6V5 MSS node > v3: > * with shutdown-ack irq redesign make it mandatory, > merge multiple patches into a single one > > v2: > * Fixed style changes > * Added missing clocks in the dt-bindings > * Split mss remoteproc node into a number of patches > > This patch depends on the following bindings: > https://patchwork.kernel.org/patch/10662089/ - mba/mpss reserved regions > https://patchwork.kernel.org/patch/10657325/ - pdc reset node > https://patchwork.kernel.org/patch/10753659/ - rpmhpd dt node > https://patchwork.kernel.org/patch/10749469/ - AOP QMP dt bindings > https://patchwork.kernel.org/patch/10751757/ - shutdown-irq binding > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 60 ++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 5da9fa1feb8a..e021b15f87fd 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -1366,6 +1366,66 @@ > }; > }; > > + remoteproc@4080000 { It would be handy if you added a label here, AKA: mss_pil: remoteproc@4080000 { It's expected that boards will need to refer to this node so that they can provide a firmware-name, so you need to give them a label to grab onto. ...and actually, I wonder if boards will also need to be able to set status = "okay"? Right now they don't because you don't have a status = "disabled" in sdm845.dtsi, but maybe you should? Are there ever going to be any boards with sdm845 that don't hook up the modem? -Doug
On Fri 11 Jan 13:06 PST 2019, Doug Anderson wrote: > Hi, > > On Wed, Jan 9, 2019 at 9:00 AM Sibi Sankar <sibis@codeaurora.org> wrote: > > > > This patch adds Q6V5 MSS remoteproc node for SDM845 SoCs. > > > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > --- > > > > v5: > > * Use qmp_aop updated dt binding > > nit: since this is now a singleton patch in v5 (because patches #1 - > #7 landed), the general policy is to drop the "8/8" in the subject. > AKA I believe the subject of the patch ought to have been: > > [PATCH v5] arm64: dts: qcom: sdm845: Add Q6V5 MSS node > > > > v3: > > * with shutdown-ack irq redesign make it mandatory, > > merge multiple patches into a single one > > > > v2: > > * Fixed style changes > > * Added missing clocks in the dt-bindings > > * Split mss remoteproc node into a number of patches > > > > This patch depends on the following bindings: > > https://patchwork.kernel.org/patch/10662089/ - mba/mpss reserved regions > > https://patchwork.kernel.org/patch/10657325/ - pdc reset node > > https://patchwork.kernel.org/patch/10753659/ - rpmhpd dt node > > https://patchwork.kernel.org/patch/10749469/ - AOP QMP dt bindings > > https://patchwork.kernel.org/patch/10751757/ - shutdown-irq binding > > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 60 ++++++++++++++++++++++++++++ > > 1 file changed, 60 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index 5da9fa1feb8a..e021b15f87fd 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -1366,6 +1366,66 @@ > > }; > > }; > > > > + remoteproc@4080000 { > > It would be handy if you added a label here, AKA: > > mss_pil: remoteproc@4080000 { > > It's expected that boards will need to refer to this node so that they > can provide a firmware-name, so you need to give them a label to grab > onto. > I agree. > ...and actually, I wonder if boards will also need to be able to set > status = "okay"? Right now they don't because you don't have a status > = "disabled" in sdm845.dtsi, but maybe you should? Are there ever > going to be any boards with sdm845 that don't hook up the modem? > The modem is status "ok" on my SDA845 device (but haven't verified upstream myself yet), so I think it's fine to leave it enabled. But merging this as is will cause the modem to crash repeatedly until rmtfs is present. Sibi is making progress on tying this to rmtfs, which would be accompanied by the following commit: https://lore.kernel.org/lkml/20180524192141.20323-1-ramon.fried@gmail.com/ I'm okay with merging that now, to unblock the merge of this patch. Would appreciate an Ack on this. Also, Sibi, the glink-edge binding doesn't define the mbox-names property, so please drop it. Regards, Bjorn
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 5da9fa1feb8a..e021b15f87fd 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -1366,6 +1366,66 @@ }; }; + remoteproc@4080000 { + compatible = "qcom,sdm845-mss-pil"; + reg = <0x04080000 0x408>, <0x04180000 0x48>; + reg-names = "qdsp6", "rmb"; + + interrupts-extended = + <&intc GIC_SPI 266 IRQ_TYPE_EDGE_RISING>, + <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, + <&modem_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, + <&modem_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, + <&modem_smp2p_in 3 IRQ_TYPE_EDGE_RISING>, + <&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "wdog", "fatal", "ready", + "handover", "stop-ack", + "shutdown-ack"; + + clocks = <&gcc GCC_MSS_CFG_AHB_CLK>, + <&gcc GCC_MSS_Q6_MEMNOC_AXI_CLK>, + <&gcc GCC_BOOT_ROM_AHB_CLK>, + <&gcc GCC_MSS_GPLL0_DIV_CLK_SRC>, + <&gcc GCC_MSS_SNOC_AXI_CLK>, + <&gcc GCC_MSS_MFAB_AXIS_CLK>, + <&gcc GCC_PRNG_AHB_CLK>, + <&rpmhcc RPMH_CXO_CLK>; + clock-names = "iface", "bus", "mem", "gpll0_mss", + "snoc_axi", "mnoc_axi", "prng", "xo"; + + qcom,smem-states = <&modem_smp2p_out 0>; + qcom,smem-state-names = "stop"; + + resets = <&aoss_reset AOSS_CC_MSS_RESTART>, + <&pdc_reset PDC_MODEM_SYNC_RESET>; + reset-names = "mss_restart", "pdc_reset"; + + qcom,halt-regs = <&tcsr_mutex_regs + 0x23000 0x25000 0x24000>; + + power-domains = <&aoss_qmp AOSS_QMP_LS_MODEM>, + <&rpmhpd SDM845_CX>, + <&rpmhpd SDM845_MX>, + <&rpmhpd SDM845_MSS>; + power-domain-names = "load_state", "cx", "mx", "mss"; + + mba { + memory-region = <&mba_region>; + }; + + mpss { + memory-region = <&mpss_region>; + }; + + glink-edge { + interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>; + label = "modem"; + qcom,remote-pid = <1>; + mboxes = <&apss_shared 12>; + mbox-names = "mpss_smem"; + }; + }; + usb_1_hsphy: phy@88e2000 { compatible = "qcom,sdm845-qusb2-phy"; reg = <0x88e2000 0x400>;