[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2b6b43b3-c99b-4aac-b1fb-24f6e5e562ce@quicinc.com>
Date: Thu, 22 Aug 2024 16:17:25 +0530
From: Gokul Sriram P <quic_gokulsri@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
<q@...k-bin.smtp.subspace.kernel.org>
CC: <andersson@...nel.org>, <krzk+dt@...nel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<quic_viswanat@...cinc.com>, <quic_mmanikan@...cinc.com>,
<quic_varada@...cinc.com>, <quic_srichara@...cinc.com>
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: qcom: document hexagon based
WCSS secure PIL
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@...cinc.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@...cinc.com>
>> Signed-off-by: Gokul Sriram Palanisamy<quic_gokulsri@...cinc.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@...cinc.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@...0000 {
> 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
>
Powered by blists - more mailing lists