| Message ID | 20240820085517.435566-2-quic_gokulsri@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add new driver for WCSS secure PIL loading | expand |
On 8/20/2024 4:50 PM, Krzysztof Kozlowski wrote: > On Tue, Aug 20, 2024 at 02:25:14PM +0530, Gokul Sriram Palanisamy wrote: >> From: Manikanta Mylavarapu<quic_mmanikan@quicinc.com> >> >> Add new binding document for hexagon based WCSS secure PIL remoteproc. >> IPQ5332, IPQ9574 follows secure PIL remoteproc. >> >> Signed-off-by: Manikanta Mylavarapu<quic_mmanikan@quicinc.com> >> Signed-off-by: Gokul Sriram Palanisamy<quic_gokulsri@quicinc.com> >> --- >> .../remoteproc/qcom,wcss-sec-pil.yaml | 125 ++++++++++++++++++ >> 1 file changed, 125 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,wcss-sec-pil.yaml >> >> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,wcss-sec-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,wcss-sec-pil.yaml >> new file mode 100644 >> index 000000000000..c69401b6cec1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcss-sec-pil.yaml >> @@ -0,0 +1,125 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id:http://devicetree.org/schemas/remoteproc/qcom,wcss-sec-pil.yaml# >> +$schema:http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm WCSS Secure Peripheral Image Loader > ... > >> + >> +maintainers: >> + - Manikanta Mylavarapu<quic_mmanikan@quicinc.com> >> + >> +description: >> + WCSS Secure Peripheral Image Loader loads firmware and power up QDSP6 > What is WCSS? Don't add random acronyms without any explanation. WCSS/WCNSS - Wireless Connectivity subsystem. will update. >> + remoteproc's on the Qualcomm IPQ9574, IPQ5332 SoC. >> + >> +properties: >> + compatible: >> + enum: >> + - qcom,ipq5332-wcss-sec-pil >> + - qcom,ipq9574-wcss-sec-pil >> + >> + reg: >> + maxItems: 1 >> + >> + firmware-name: >> + $ref: /schemas/types.yaml#/definitions/string >> + description: Firmware name for the Hexagon core > No, look how other bindings are doing it. > > It looks like you develop patches on some old kernel, so please start > from scratch on newest kernel. will update. >> + >> + interrupts: >> + items: >> + - description: Watchdog interrupt >> + - description: Fatal interrupt >> + - description: Ready interrupt >> + - description: Handover interrupt >> + - description: Stop acknowledge interrupt >> + >> + interrupt-names: >> + items: >> + - const: wdog >> + - const: fatal >> + - const: ready >> + - const: handover >> + - const: stop-ack >> + >> + clocks: >> + items: >> + - description: IM SLEEP clock > What is IM? Explain all acronyms. > > What is SLEEP? IM_SLEEP_CLK - Internal Module sleep clock needed for Q6 reset. SLEEP is not an acronym here. >> + >> + clock-names: >> + items: >> + - const: im_sleep > sleep? Are there different sleep clocks here? We have different branches of sleep clk each enabled separately. im_sleep is one of those branches that q6 uses. >> + >> + qcom,smem-states: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: States used by the AP to signal the remote processor >> + items: >> + - description: Shutdown Q6 >> + - description: Stop Q6 >> + > Do not introduce other order. First stop, then shutdown. will update >> + qcom,smem-state-names: >> + description: >> + Names of the states used by the AP to signal the remote processor >> + items: >> + - const: shutdown >> + - const: stop > The same. will update. >> + >> + memory-region: >> + items: >> + - description: Q6 reserved region >> + >> + 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. >> + unevaluatedProperties: false >> + >> +required: >> + - compatible >> + - firmware-name >> + - reg >> + - interrupts >> + - interrupt-names >> + - qcom,smem-states >> + - qcom,smem-state-names >> + - memory-region > Keep the same order as in properties. ok. >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/clock/qcom,ipq5332-gcc.h> >> + q6v5_wcss: remoteproc@d100000 { > Drop unused label. ok. Regards, Gokul >> + compatible = "qcom,ipq5332-wcss-sec-pil"; >> + reg = <0xd100000 0x4040>; >> + firmware-name = "ath12k/IPQ5332/hw1.0/q6_fw0.mdt"; >> + interrupts-extended = <&intc GIC_SPI 291 IRQ_TYPE_EDGE_RISING>, >> + <&wcss_smp2p_in 0 IRQ_TYPE_NONE>, >> + <&wcss_smp2p_in 1 IRQ_TYPE_NONE>, > Best regards, > Krzysztof >
On 22/08/2024 12:47, Gokul Sriram P wrote: >>> + >>> + interrupts: >>> + items: >>> + - description: Watchdog interrupt >>> + - description: Fatal interrupt >>> + - description: Ready interrupt >>> + - description: Handover interrupt >>> + - description: Stop acknowledge interrupt >>> + >>> + interrupt-names: >>> + items: >>> + - const: wdog >>> + - const: fatal >>> + - const: ready >>> + - const: handover >>> + - const: stop-ack >>> + >>> + clocks: >>> + items: >>> + - description: IM SLEEP clock >> What is IM? Explain all acronyms. >> >> What is SLEEP? > > IM_SLEEP_CLK - Internal Module sleep clock needed for Q6 reset. > > SLEEP is not an acronym here. Then probably you mean "Internal sleep", although "internal" is also confusing. Devices do not receive as input something which is internal to them. > >>> + >>> + clock-names: >>> + items: >>> + - const: im_sleep >> sleep? Are there different sleep clocks here? > > We have different branches of sleep clk each enabled separately. > > im_sleep is one of those branches that q6 uses. So this device misses other branches? Then provide them. Otherwise it is just "sleep". Best regards, Krzysztof
On 23/08/2024 11:47, Gokul Sriram P wrote: > > On 8/22/2024 5:00 PM, Krzysztof Kozlowski wrote: >>> IM_SLEEP_CLK - Internal Module sleep clock needed for Q6 reset. >>> >>> SLEEP is not an acronym here. >> Then probably you mean "Internal sleep", although "internal" is also >> confusing. Devices do not receive as input something which is internal >> to them. >> >>>>> + >>>>> + clock-names: >>>>> + items: >>>>> + - const: im_sleep >>>> sleep? Are there different sleep clocks here? >>> We have different branches of sleep clk each enabled separately. >>> >>> im_sleep is one of those branches that q6 uses. >> So this device misses other branches? Then provide them. Otherwise it is >> just "sleep". > > ok, I shall keep it as simply "sleep" and move on? Other branches of > sleep clk are irrelevant to remoteproc. Then it is "sleep". The name here describes the clock input in this device, not the clock in other places. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,wcss-sec-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,wcss-sec-pil.yaml new file mode 100644 index 000000000000..c69401b6cec1 --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcss-sec-pil.yaml @@ -0,0 +1,125 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/remoteproc/qcom,wcss-sec-pil.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm WCSS Secure Peripheral Image Loader + +maintainers: + - Manikanta Mylavarapu <quic_mmanikan@quicinc.com> + +description: + WCSS Secure Peripheral Image Loader loads firmware and power up QDSP6 + remoteproc's on the Qualcomm IPQ9574, IPQ5332 SoC. + +properties: + compatible: + enum: + - qcom,ipq5332-wcss-sec-pil + - qcom,ipq9574-wcss-sec-pil + + reg: + maxItems: 1 + + firmware-name: + $ref: /schemas/types.yaml#/definitions/string + description: Firmware name for the Hexagon core + + interrupts: + items: + - description: Watchdog interrupt + - description: Fatal interrupt + - description: Ready interrupt + - description: Handover interrupt + - description: Stop acknowledge interrupt + + interrupt-names: + items: + - const: wdog + - const: fatal + - const: ready + - const: handover + - const: stop-ack + + clocks: + items: + - description: IM SLEEP clock + + clock-names: + items: + - const: im_sleep + + qcom,smem-states: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: States used by the AP to signal the remote processor + items: + - description: Shutdown Q6 + - description: Stop Q6 + + qcom,smem-state-names: + description: + Names of the states used by the AP to signal the remote processor + items: + - const: shutdown + - const: stop + + memory-region: + items: + - description: Q6 reserved region + + 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. + unevaluatedProperties: false + +required: + - compatible + - firmware-name + - reg + - interrupts + - interrupt-names + - qcom,smem-states + - qcom,smem-state-names + - memory-region + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,ipq5332-gcc.h> + q6v5_wcss: remoteproc@d100000 { + compatible = "qcom,ipq5332-wcss-sec-pil"; + reg = <0xd100000 0x4040>; + firmware-name = "ath12k/IPQ5332/hw1.0/q6_fw0.mdt"; + interrupts-extended = <&intc GIC_SPI 291 IRQ_TYPE_EDGE_RISING>, + <&wcss_smp2p_in 0 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 1 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 2 IRQ_TYPE_NONE>, + <&wcss_smp2p_in 3 IRQ_TYPE_NONE>; + interrupt-names = "wdog", + "fatal", + "ready", + "handover", + "stop-ack"; + + clocks = <&gcc GCC_IM_SLEEP_CLK>; + clock-names = "im_sleep"; + + qcom,smem-states = <&wcss_smp2p_out 0>, + <&wcss_smp2p_out 1>; + qcom,smem-state-names = "shutdown", + "stop"; + + memory-region = <&q6_region>; + + glink-edge { + interrupts = <GIC_SPI 417 IRQ_TYPE_EDGE_RISING>; + label = "rtr"; + qcom,remote-pid = <1>; + mboxes = <&apcs_glb 8>; + }; + };