[PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

[PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

[Alexey Minnekhanov]

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";
+			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";
 
-- 
2.41.0

Re: [PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

[Krzysztof Kozlowski]

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

Re: [PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

[Alexey Minnekhanov]

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.

-- 
Regards,
Alexey Minnekhanov
postmarketOS developer

Re: [PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

[Krzysztof Kozlowski]

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

Re: [PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

[Alexey Minnekhanov]

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/

-- 
Regards,
Alexey Minnekhanov
postmarketOS developer

Re: [PATCH v3] arm64: dts: qcom: sdm630: Add support for modem remoteproc

[Bjorn Andersson]

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,
-- 
Bjorn Andersson <andersson@kernel.org>