Convert the QCS404 and IPQ WCSS Peripheral Image Loader bindings to DT
schema. The text bindngs incorrectly implied that IPQ8074 needs only
one qcom,smem-states entry. This is only true for QCS404. IPQ8074
requires both "stop" and "shutdown".
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
.../remoteproc/qcom,ipq9574-wcss-pil.yaml | 167 ++++++++++++++++++
.../bindings/remoteproc/qcom,q6v5.txt | 102 -----------
2 files changed, 167 insertions(+), 102 deletions(-)
create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml
delete mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml
new file mode 100644
index 0000000000000..d28f42661d084
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml
@@ -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 <placeholder@kernel.org>
+
+description:
+ The IPQ WCSS peripheral image loader is used to load firmware on the Qualcomm
+ Q6 processor that exposes WiFi-6 devices to the OS via the AHB bus. It is
+ generally used by ath11k to start up the wireless firmware.
+
+properties:
+ compatible:
+ enum:
+ - qcom,ipq8074-wcss-pil
+ - qcom,qcs404-wcss-pil
+
+ reg:
+ minItems: 2
+ maxItems: 2
+ description:
+ The base address and size of the QDSP6, and RMB register blocks
+
+ reg-names:
+ items:
+ - const: qdsp6
+ - const: rmb
+
+ interrupts-extended:
+ minItems: 5
+ maxItems: 5
+
+ interrupt-names:
+ items:
+ - const: wdog
+ - const: fatal
+ - const: ready
+ - const: handover
+ - const: stop-ack
+
+ resets:
+ minItems: 3
+ maxItems: 3
+
+ reset-names:
+ items:
+ - const: wcss_aon_reset
+ - const: wcss_reset
+ - const: wcss_q6_reset
+
+ clocks:
+ minItems: 10
+ maxItems: 13
+
+ clock-names:
+ minItems: 10
+ maxItems: 13
+
+ cx-supply:
+ description:
+ reference to the regulators used for the booting of the Hexagon core
+
+ memory-region:
+ description: Reference to wcss reserved-memory region
+
+ qcom,halt-regs:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ A phandle reference to a syscon representing TCSR followed by the three
+ offsets within syscon for q6, wcss and nc halt registers.
+ items:
+ - items:
+ - description: phandle to TCSR_MUTEX registers
+ - description: offset to the Q6 halt register
+ - description: offset to the wcss halt register
+ - description: offset to the nc halt register
+
+ qcom,smem-states:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ 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:
+ - 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
+ - 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
+ required:
+ - clocks
+ - clock-names
+ - cx-supply
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
deleted file mode 100644
index 573a88b606773..0000000000000
--- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
+++ /dev/null
@@ -1,102 +0,0 @@
-Qualcomm Hexagon Peripheral Image Loader
-
-This document defines the binding for a component that loads and boots firmware
-on the Qualcomm Hexagon core.
-
-- compatible:
- Usage: required
- Value type: <string>
- Definition: must be one of:
- "qcom,ipq8074-wcss-pil"
- "qcom,qcs404-wcss-pil"
-
-- reg:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: must specify the base address and size of the qdsp6 and
- rmb register blocks
-
-- reg-names:
- Usage: required
- Value type: <stringlist>
- Definition: must be "q6dsp" and "rmb"
-
-- interrupts-extended:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: reference to the interrupts that match interrupt-names
-
-- interrupt-names:
- Usage: required
- Value type: <stringlist>
- Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack"
-
-- clocks:
- Usage: required
- Value type: <phandle>
- Definition: reference to the clocks that match clock-names
-
-- clock-names:
- Usage: required
- Value type: <stringlist>
- Definition: The clocks needed depend on the compatible string:
- qcom,ipq8074-wcss-pil:
- no clock names required
- qcom,qcs404-wcss-pil:
- must be "xo", "gcc_abhs_cbcr", "gcc_abhs_cbcr",
- "gcc_axim_cbcr", "lcc_ahbfabric_cbc", "tcsr_lcc_cbc",
- "lcc_abhs_cbc", "lcc_tcm_slave_cbc", "lcc_abhm_cbc",
- "lcc_axim_cbc", "lcc_bcr_sleep"
-
-- resets:
- Usage: required
- Value type: <phandle>
- Definition: reference to the list of 3 reset-controllers for the
- wcss sub-system
-
-- reset-names:
- Usage: required
- Value type: <stringlist>
- Definition: must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
- for the wcss sub-system
-
-- memory-region:
- Usage: required
- Value type: <phandle>
- Definition: reference to wcss reserved-memory region.
-
-For the compatible string below the following supplies are required:
- "qcom,qcs404-wcss-pil"
-- cx-supply:
- Usage: required
- Value type: <phandle>
- Definition: reference to the regulators to be held on behalf of the
- booting of the Hexagon core
-
-- qcom,smem-states:
- Usage: required
- Value type: <phandle>
- Definition: reference to the smem state for requesting the Hexagon to
- shut down
-
-- qcom,smem-state-names:
- Usage: required
- Value type: <stringlist>
- Definition: must be "stop"
-
-- qcom,halt-regs:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: a phandle reference to a syscon representing TCSR followed
- by the three offsets within syscon for q6, wcss and nc
- halt registers.
-
-- memory-region:
- Usage: required
- Value type: <phandle>
- Definition: reference to the reserved-memory for the region
-
-The Hexagon node may also have an subnode named either "smd-edge" or
-"glink-edge" that describes the communication edge, channels and devices
-related to the Hexagon. See ../soc/qcom/qcom,smd.yaml and
-../soc/qcom/qcom,glink.txt for details on how to describe these.
--
2.45.1On Tue, Dec 09, 2025 at 06:37:23PM -0600, Alexandru Gagniuc wrote: > Convert the QCS404 and IPQ WCSS Peripheral Image Loader bindings to DT > schema. The text bindngs incorrectly implied that IPQ8074 needs only > one qcom,smem-states entry. This is only true for QCS404. IPQ8074 > requires both "stop" and "shutdown". > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Don't add fake addresses to CC. I could not respond to this email because of that! > --- > .../remoteproc/qcom,ipq9574-wcss-pil.yaml | 167 ++++++++++++++++++ > .../bindings/remoteproc/qcom,q6v5.txt | 102 ----------- > 2 files changed, 167 insertions(+), 102 deletions(-) > create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml > delete mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml > new file mode 100644 > index 0000000000000..d28f42661d084 > --- /dev/null > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml Filename based on the compatible, so for example: qcom,ipq8074-wcss-pil.yaml > @@ -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 <placeholder@kernel.org> This must be a real person. Fallback is your SoC maintainer. > + > +description: > + The IPQ WCSS peripheral image loader is used to load firmware on the Qualcomm > + Q6 processor that exposes WiFi-6 devices to the OS via the AHB bus. It is > + generally used by ath11k to start up the wireless firmware. > + > +properties: > + compatible: > + enum: > + - qcom,ipq8074-wcss-pil > + - qcom,qcs404-wcss-pil > + > + reg: > + minItems: 2 Drop > + maxItems: 2 > + description: > + The base address and size of the QDSP6, and RMB register blocks Drop description. Look at other bindings how this is written. > + > + reg-names: > + items: > + - const: qdsp6 > + - const: rmb > + > + interrupts-extended: No, you only need interrupts. Please look at other bindings - how they write this. > + minItems: 5 Drop > + maxItems: 5 > + > + interrupt-names: > + items: > + - const: wdog > + - const: fatal > + - const: ready > + - const: handover > + - const: stop-ack > + > + resets: > + minItems: 3 Drop > + maxItems: 3 > + > + reset-names: > + items: > + - const: wcss_aon_reset > + - const: wcss_reset > + - const: wcss_q6_reset > + > + clocks: > + minItems: 10 > + maxItems: 13 Why is this flexible? Wasn't in the old binding and nothing in the commit msg explained a change in the binding. > + > + clock-names: > + minItems: 10 > + maxItems: 13 > + > + cx-supply: > + description: > + reference to the regulators used for the booting of the Hexagon core > + > + memory-region: > + description: Reference to wcss reserved-memory region Drop description. Missing maxItems, please look at other bindings. Don't write your own style, but look how we already wrote remoteproc bindings (the latest). > + > + qcom,halt-regs: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + A phandle reference to a syscon representing TCSR followed by the three > + offsets within syscon for q6, wcss and nc halt registers. > + items: > + - items: > + - description: phandle to TCSR_MUTEX registers > + - description: offset to the Q6 halt register > + - description: offset to the wcss halt register > + - description: offset to the nc halt register > + > + 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? > + 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? > + - 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 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. > + required: > + - clocks > + - clock-names > + - cx-supply > + > +additionalProperties: false Missing example. Best regards, Krzysztof
On 12/15/25 11:53 PM, Krzysztof Kozlowski wrote: > On Tue, Dec 09, 2025 at 06:37:23PM -0600, Alexandru Gagniuc wrote: >> Convert the QCS404 and IPQ WCSS Peripheral Image Loader bindings to DT >> schema. The text bindngs incorrectly implied that IPQ8074 needs only >> one qcom,smem-states entry. This is only true for QCS404. IPQ8074 >> requires both "stop" and "shutdown". >> >> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> > > Don't add fake addresses to CC. I could not respond to this email > because of that! Okay. >> --- >> .../remoteproc/qcom,ipq9574-wcss-pil.yaml | 167 ++++++++++++++++++ >> .../bindings/remoteproc/qcom,q6v5.txt | 102 ----------- >> 2 files changed, 167 insertions(+), 102 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml >> delete mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt >> >> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml >> new file mode 100644 >> index 0000000000000..d28f42661d084 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,ipq9574-wcss-pil.yaml > > 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 <placeholder@kernel.org> > > 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 myself, but you know a lot about these bindings. Is it okay if I list you as the maintainer of this binding, Krzysztof? >> + >> + 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? >> + minItems: 5 > > Drop > >> + maxItems: 5 >> + >> + interrupt-names: >> + items: >> + - const: wdog >> + - const: fatal >> + - const: ready >> + - const: handover >> + - const: stop-ack >> + >> + resets: >> + minItems: 3 > > Drop I will drop all the items you identified as excessive.> >> + maxItems: 3 >> + >> + reset-names: >> + items: >> + - const: wcss_aon_reset >> + - const: wcss_reset >> + - const: wcss_q6_reset >> + >> + clocks: >> + minItems: 10 >> + maxItems: 13 > > Why is this flexible? Wasn't in the old binding and nothing in the > commit msg explained a change in the binding. I was thinking ahead to the next patch in the series that adds IPQ9574 binding. It makes more sense to keep it at 10 fot this patch, like you suggest. >> + >> + clock-names: >> + minItems: 10 >> + maxItems: 13 >> + >> + cx-supply: >> + description: >> + reference to the regulators used for the booting of the Hexagon core >> + >> + memory-region: >> + description: Reference to wcss reserved-memory region > > Drop description. Missing maxItems, please look at other bindings. Don't > write your own style, but look how we already wrote remoteproc bindings > (the latest). Is "maxItems: 1" the correct thing to add here? >> + >> + qcom,halt-regs: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: >> + A phandle reference to a syscon representing TCSR followed by the three >> + offsets within syscon for q6, wcss and nc halt registers. >> + items: >> + - items: >> + - description: phandle to TCSR_MUTEX registers >> + - description: offset to the Q6 halt register >> + - description: offset to the wcss halt register >> + - description: offset to the nc halt register >> + >> + 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 qcom,wcnss.yaml. What constraints are you expecting here? >> + 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? > >> + - 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? > 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. Alex
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 <placeholder@kernel.org>
>>
>> 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.
> 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.
>
>>> +
>>> + 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.
..
>>> + 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.
>
>>> + 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.
>
>>
>>> + - 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.
>
>> 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.
Best regards,
Krzysztof
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 <placeholder@kernel.org>
>>>
>>> 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
On 17/12/2025 23:22, mr.nuke.me@gmail.com wrote: > > > 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 <placeholder@kernel.org> >>>> >>>> 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 That's a binding and I did not say maintainer of this file. I said SoC maintainer - it's easy to find the name of the soc here and the maintainers of that SoC. Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.