On 12/17/25 1:55 AM, Krzysztof Kozlowski wrote:
On 17/12/2025 06:01, Alex G. wrote:
Filename based on the compatible, so for example:
qcom,ipq8074-wcss-pil.yaml
Okay.
@@ -0,0 +1,167 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,ipq9574-wcss-pil.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ WCSS Peripheral Image Loader
+
+maintainers:
+ - Placeholder Maintainer <[email protected]>
This must be a real person. Fallback is your SoC maintainer.
I can't find an official maintainer for IPQ8074 or IPQ9574. I could list
I don't think you looked then. get_maintainers gives you clear answer.
get_maintainers on qcom,q6v5.txt gives five generic subsystem
maintainers, and you are on of them! I'll take something more specific,
but just defaulting to get_maintainers is not helpful here.
myself, but you know a lot about these bindings. Is it okay if I list
you as the maintainer of this binding, Krzysztof?
No. I am not the maintainer of this SoC.
Thank you for confirming you do not wish to be listed as the maintainer
here.
+
+ reg-names:
+ items:
+ - const: qdsp6
+ - const: rmb
+
+ interrupts-extended:
No, you only need interrupts. Please look at other bindings - how they
write this.
I thought I needed interrupts-extended if the interrupts use more than
one interrupt controller. Is that not the case?
Instead of asking the same question again, which would give you the same
answer "No, you only need interrupts" please rather think on the rest of
the answer - look at other bindings.
No. It's a clarifying question with additional context. I think I should
be expected to ask them when I still have doubts.
+ qcom,smem-states:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
That's incomplete - missing constraints. Are you sure you wrote this
code the same way we already did for other devices?
I am not sure. It seems to match qcom,qcs404-cdsp-pil.yaml or
No, it does not.
Look at these files even - they have maxItems. Where do you see maxItems
here? So how this can be done the same way ("match")?
qcom,wcnss.yaml. What constraints are you expecting here?
So you take the latest binding, e.g. pas-common for all new platforms.
Or qcom,qcs404-cdsp-pil.yaml. You need maxItems here and list of items
for the names.
Okay. I wasn't sure if that's the appropriate solution when QCS404 and
IPQ8074 take a different number of smem-states.
+ description: States used by the AP to signal the remote processor
+
+ qcom,smem-state-names:
+ description:
+ Names of the states used by the AP to signal the remote processor
+
+ glink-edge:
+ $ref: /schemas/remoteproc/qcom,glink-edge.yaml#
+ description:
+ Qualcomm G-Link subnode which represents communication edge, channels
+ and devices related to the Modem.
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts-extended
+ - interrupt-names
+ - memory-region
+ - qcom,halt-regs
+ - qcom,smem-states
+ - qcom,smem-state-names
+
+allOf:
Seems you do not reference other schemas. I am going to repeat myself
for 10th time: are you sure you followed other devices?
It's the sixth time, but I see your point. Comparing to
qcom,qcs404-cdsp-pil.yaml or qcom,wcnss.yaml, I can't see what's
missing. What do I need here?
In previous cases you did not look at other binding, so I am questioning
now everything.I resent the accusation. I looked at several other bindings to see the
best way to write this one in a manner that also works with "make
dt_binding_check". I have done my homework, and think it is unproductive
to accuse me of not doing it because I did not do it to your standards
or liking.
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq8074-wcss-pil
+ then:
+ properties:
+ qcom,smem-states:
+ items:
+ - description: Shutdown Q6
+ - description: Stop Q6
+ qcom,smem-state-names:
+ items:
+ - const: shutdown
+ - const: stop
Missing clocks
The text binding that this replaces implies no clocks for IPQ8074. What
would you like me to add instead?
Disallow the clocks. See example-schema.
Okay. Makes sense (clocks: false).
Missing blank line
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,qcs404-wcss-pil
+ then:
+ properties:
+ qcom,smem-states:
+ maxItems: 1
+ qcom,smem-state-names:
+ items:
+ - const: stop
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,qcs404-wcss-pil
+ then:
+ properties:
+ clocks:
+ minItems: 10
+ maxItems: 10
+ clock-names:
+ items:
+ - const: xo
+ - const: gcc_abhs_cbcr
+ - const: gcc_axim_cbcr
+ - const: lcc_ahbfabric_cbc
+ - const: tcsr_lcc_cbc
+ - const: lcc_abhs_cbc
+ - const: lcc_tcm_slave_cbc
+ - const: lcc_abhm_cbc
+ - const: lcc_axim_cbc
+ - const: lcc_bcr_sleep
All this goes to previous if.
Okay
+ required:
+ - clocks
+ - clock-names
+ - cx-supply
+
+additionalProperties: false
Missing example.
I plan to add the example in the next patch in the series that adds
IPQ9547 binding. I don't have the resources to test IPQ8074 or QCS404,
and I want to be sure that the example I add is tested.
I don't understand what example has anything to do with testing. We
speak here ONLY about this binding. I do not review other code. This
patch should have the example, but if you cannot come with one, because
it does not exist in any existing sources, then you should just say
that. You have entire commit msg to explain every unusual thing. And
testing something on a device is not a reason, because you do not test
the binding on a device.
I will mention this discrepant in the commit message.
Best regards,
Krzysztof
