| Message ID | 20240820085517.435566-2-quic_gokulsri@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add new driver for WCSS secure PIL loading | expand |
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. > + 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. > + > + 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? > + > + clock-names: > + items: > + - const: im_sleep sleep? Are there different sleep clocks here? > + > + 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. > + 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. > + > + 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. > + > +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. > + 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 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>; + }; + };