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 <[email protected]>
> ---
>
> V3: Use memory-region property to specify memory regions, instead
> of using "mba" and "mpss" subnodes.
>
> V2 link: https://lore.kernel.org/lkml/[email protected]/
>
> 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 <[email protected]>
>> ---
>>
>> V3: Use memory-region property to specify memory regions, instead
>> of using "mba" and "mpss" subnodes.
>>
>> V2 link: https://lore.kernel.org/lkml/[email protected]/
>>
>> 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
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 <[email protected]>
>>> ---
>>>
>>> V3: Use memory-region property to specify memory regions, instead
>>> of using "mba" and "mpss" subnodes.
>>>
>>> V2 link: https://lore.kernel.org/lkml/[email protected]/
>>>
>>> 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/[email protected]/
--
Regards,
Alexey Minnekhanov
postmarketOS developer