| Message ID | 20230719093458.2668842-1-alexeymin@postmarketos.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc | expand |
On 19/07/2023 11:34, Alexey Minnekhanov wrote: > Modem subsystem in SDM630/660 is similar to MSM8998 and > device tree node for it is based on the one from msm8998.dtsi. > > Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org> > --- > > V3: Use memory-region property to specify memory regions, instead > of using "mba" and "mpss" subnodes. > > V2 link: https://lore.kernel.org/lkml/20230621175046.61521-3-alexeymin@postmarketos.org/ > > arch/arm64/boot/dts/qcom/sdm630.dtsi | 59 ++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi > index 2136ded22f7e..797625cf78ac 100644 > --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi > @@ -1032,6 +1032,65 @@ data-pins { > }; > }; > > + remoteproc_mss: remoteproc@4080000 { > + compatible = "qcom,sdm660-mss-pil"; Missing bindings so standard comment: It does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Best regards, Krzysztof
On 19.07.2023 12:45, Krzysztof Kozlowski wrote: > On 19/07/2023 11:34, Alexey Minnekhanov wrote: >> Modem subsystem in SDM630/660 is similar to MSM8998 and >> device tree node for it is based on the one from msm8998.dtsi. >> >> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org> >> --- >> >> V3: Use memory-region property to specify memory regions, instead >> of using "mba" and "mpss" subnodes. >> >> V2 link: https://lore.kernel.org/lkml/20230621175046.61521-3-alexeymin@postmarketos.org/ >> >> arch/arm64/boot/dts/qcom/sdm630.dtsi | 59 ++++++++++++++++++++++++++++ >> 1 file changed, 59 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi >> index 2136ded22f7e..797625cf78ac 100644 >> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi >> @@ -1032,6 +1032,65 @@ data-pins { >> }; >> }; >> >> + remoteproc_mss: remoteproc@4080000 { >> + compatible = "qcom,sdm660-mss-pil"; > > Missing bindings so standard comment: > > It does not look like you tested the DTS against bindings. Please run > `make dtbs_check` (see > Documentation/devicetree/bindings/writing-schema.rst or > https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ > for instructions). > > Best regards, > Krzysztof > Bindings are already in linux-next/master since https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=ed50ac266f67829d4732c8ca61ef1953c2cc63d0 . And I did check DTS against bindings, it spat 2 warnings about new remoteproc node, which do not make much sense to me: DTC_CHK arch/arm64/boot/dts/qcom/sdm660-xiaomi-lavender.dtb .../arch/arm64/boot/dts/qcom/sdm660-xiaomi-lavender.dtb: remoteproc@4080000: qcom,halt-regs:0: [43] is too short From schema: .../Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml .../arch/arm64/boot/dts/qcom/sdm660-xiaomi-lavender.dtb: remoteproc@4080000: memory-region: [[45], [46]] is too short From schema: .../Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml because I do not understand where the limitation to number of items for "qcom,halt-regs" or "memory-regions" come from.
On 19/07/2023 12:10, Alexey Minnekhanov wrote: > On 19.07.2023 12:45, Krzysztof Kozlowski wrote: >> On 19/07/2023 11:34, Alexey Minnekhanov wrote: >>> Modem subsystem in SDM630/660 is similar to MSM8998 and >>> device tree node for it is based on the one from msm8998.dtsi. >>> >>> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org> >>> --- >>> >>> V3: Use memory-region property to specify memory regions, instead >>> of using "mba" and "mpss" subnodes. >>> >>> V2 link: https://lore.kernel.org/lkml/20230621175046.61521-3-alexeymin@postmarketos.org/ >>> >>> arch/arm64/boot/dts/qcom/sdm630.dtsi | 59 ++++++++++++++++++++++++++++ >>> 1 file changed, 59 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi >>> index 2136ded22f7e..797625cf78ac 100644 >>> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi >>> @@ -1032,6 +1032,65 @@ data-pins { >>> }; >>> }; >>> >>> + remoteproc_mss: remoteproc@4080000 { >>> + compatible = "qcom,sdm660-mss-pil"; >> >> Missing bindings so standard comment: >> >> It does not look like you tested the DTS against bindings. Please run >> `make dtbs_check` (see >> Documentation/devicetree/bindings/writing-schema.rst or >> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ >> for instructions). >> >> Best regards, >> Krzysztof >> > > Bindings are already in linux-next/master since > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=ed50ac266f67829d4732c8ca61ef1953c2cc63d0 Hm, I checked a two days older next. Provide explanations or links if you resend DTS in such case. > . > > And I did check DTS against bindings, it spat 2 warnings about new > remoteproc node, which do not make much sense to me: > > DTC_CHK arch/arm64/boot/dts/qcom/sdm660-xiaomi-lavender.dtb > .../arch/arm64/boot/dts/qcom/sdm660-xiaomi-lavender.dtb: > remoteproc@4080000: qcom,halt-regs:0: [43] is too short That's ok. > From schema: > .../Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml > .../arch/arm64/boot/dts/qcom/sdm660-xiaomi-lavender.dtb: > remoteproc@4080000: memory-region: [[45], [46]] is too short > From schema: > .../Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml This needs fixes. The binding lists expected memory regions and you do not have three of them. Best regards, Krzysztof
On 19.07.2023 13:18, Krzysztof Kozlowski wrote:>> From schema: >> .../Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml >> .../arch/arm64/boot/dts/qcom/sdm660-xiaomi-lavender.dtb: >> remoteproc@4080000: memory-region: [[45], [46]] is too short >> From schema: >> .../Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml > > This needs fixes. The binding lists expected memory regions and you do > not have three of them. > > Best regards, > Krzysztof > According to driver code, dedicated reserved metadata memory region is optional. If it is not provided, it is allocated by driver itself using dma_alloc_attr() and then passed to modem processor. I see that MSM8996 and MSM8998 (nearest sdm660 siblings) got their dedicated metadata reserved regions in patchset [1]. SDM660 MSS was not present at that time and I'm not sure if it is needed for this SoC, because downstream device tree for my device does not have this region. I can either do similar trick for sdm630.dtsi, or alternatively maybe we should fix bindings to indicate that metadata region is optional? I did quick test: added dedicated reserved metadata region, and everything still works fine, it seems, and one DTB check warning was gone. [1] https://lore.kernel.org/all/20230117085840.32356-1-quic_sibis@quicinc.com/
On Wed, 19 Jul 2023 12:34:58 +0300, Alexey Minnekhanov wrote: > Modem subsystem in SDM630/660 is similar to MSM8998 and > device tree node for it is based on the one from msm8998.dtsi. > > Applied, thanks! [1/1] arm64: dts: qcom: sdm630: Add support for modem remoteproc commit: 09f1642eca6eb6d25a630214098350dc02917954 Best regards,
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi index 2136ded22f7e..797625cf78ac 100644 --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi @@ -1032,6 +1032,65 @@ data-pins { }; }; + remoteproc_mss: remoteproc@4080000 { + compatible = "qcom,sdm660-mss-pil"; + reg = <0x04080000 0x100>, <0x04180000 0x40>; + reg-names = "qdsp6", "rmb"; + + interrupts-extended = <&intc GIC_SPI 448 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_BIMC_MSS_Q6_AXI_CLK>, + <&gcc GCC_BOOT_ROM_AHB_CLK>, + <&gcc GPLL0_OUT_MSSCC>, + <&gcc GCC_MSS_SNOC_AXI_CLK>, + <&gcc GCC_MSS_MNOC_BIMC_AXI_CLK>, + <&rpmcc RPM_SMD_QDSS_CLK>, + <&rpmcc RPM_SMD_XO_CLK_SRC>; + clock-names = "iface", + "bus", + "mem", + "gpll0_mss", + "snoc_axi", + "mnoc_axi", + "qdss", + "xo"; + + qcom,smem-states = <&modem_smp2p_out 0>; + qcom,smem-state-names = "stop"; + + resets = <&gcc GCC_MSS_RESTART>; + reset-names = "mss_restart"; + + qcom,halt-regs = <&tcsr_regs_1 0x3000 0x5000 0x4000>; + + power-domains = <&rpmpd SDM660_VDDCX>, + <&rpmpd SDM660_VDDMX>; + power-domain-names = "cx", "mx"; + + memory-region = <&mba_region>, <&mpss_region>; + + status = "disabled"; + + glink-edge { + interrupts = <GIC_SPI 452 IRQ_TYPE_EDGE_RISING>; + label = "modem"; + qcom,remote-pid = <1>; + mboxes = <&apcs_glb 15>; + }; + }; + adreno_gpu: gpu@5000000 { compatible = "qcom,adreno-508.0", "qcom,adreno";
Modem subsystem in SDM630/660 is similar to MSM8998 and device tree node for it is based on the one from msm8998.dtsi. Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org> --- V3: Use memory-region property to specify memory regions, instead of using "mba" and "mpss" subnodes. V2 link: https://lore.kernel.org/lkml/20230621175046.61521-3-alexeymin@postmarketos.org/ arch/arm64/boot/dts/qcom/sdm630.dtsi | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)