Re: [PATCH V2 01/13] dt-bindings: remoteproc: qcom: Add support for multipd model

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 5/30/2023 4:28 PM, Krzysztof Kozlowski wrote:
On 22/05/2023 00:28, Manikanta Mylavarapu wrote:
Add new binding document for multipd model remoteproc.
IPQ5018, IPQ9574 follows multipd model.

Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@xxxxxxxxxxx>
---
Changes in V2:
	- Fixed all comments and rebased for TOT.
	- Changed to BSD-3-Clause based on internal open source team
           suggestion.
	- Added firmware-name.

  .../bindings/remoteproc/qcom,multipd-pil.yaml | 265 ++++++++++++++++++
  1 file changed, 265 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml
new file mode 100644
index 000000000000..3257f27dc569
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml
@@ -0,0 +1,265 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,multipd-pil.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Multipd Secure Peripheral Image Loader
+
+maintainers:
+  - Bjorn Andersson <andersson@xxxxxxxxxx>
+  - Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
+
+description:
+  Multipd Peripheral Image Loader loads firmware and boots Q6 pd, WCSS pd

... boots Q6 Protection Domain (PD), WCSS PD ...

+  remoteproc's on the Qualcomm IPQ5018, IPQ9574 SoC.

Pd means protection
+  domain.

so you can skip this sentence as it is explained already.

It's similar to process in Linux. Here QDSP6 processor runs each
+  wifi radio functionality on a separate process. One process can't access
+  other process resources, so this is termed as PD i.e protection domain.
+
+properties:
+  compatible:
+    enum:
+      - qcom,ipq5018-q6-mpd
+      - qcom,ipq9574-q6-mpd
+
+  reg:
+    maxItems: 1
+
+  firmware-name:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: Firmware name of the Hexagon core

No need for ref and description. Instead maxItems.

+
+  interrupts-extended:
+    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
+
+  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 pd 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.
+
+patternProperties:
+  "^pd-1|pd-2|pd-3":
+    type: object
+    description:
+      In Multipd model, WCSS pd depends on Q6 pd i.e Q6 pd should be up before
+      WCSS. It can be achieved by keeping wcss pd node as subnode of Q6
+      device node.

That's not enough. Your description does not say what is this, why you
have two protection domains for same compatible. What's more, it a bit
deviates from hardware description.

WCSS means 'wireless connectivity sub system', in simple words it's a
wifi radio block.

IPQ5018 SOC has both internal (AHB) wifi radio/WCSS and external (PCIE)
wifi radio/WCSS. In Q6, Root protection domain will provide services to
both internal (AHB) and external (PCIE) wifi radio's protection domain.
So we have two protection domains for IPQ5018, one is for internal(AHB) and other is for external(PCIE) wifi radio.

+
+    properties:
+      compatible:
+        enum:
+          - qcom,ipq5018-wcss-ahb-mpd
+          - qcom,ipq9574-wcss-ahb-mpd
+          - qcom,ipq5018-wcss-pcie-mpd

Keep rather alphabetical order (so both 5018 together).

I also do not understand these at all. Why adding bus type to
compatible? This rarely is allowed (unless it is PCIe controller within
soc).

IPQ5018 SOC has in-built PCIE controller. Here QDSP6 will bring up
external(PCIE) and internal (AHB) wifi radio's. To separate AHB, PCIE radio's properties, i have added bus type to compatible.

+
+      firmware-name:
+        $ref: /schemas/types.yaml#/definitions/string-array
+        items:
+          - description: Firmware name of the Hexagon core

same comments

+
+      interrupts-extended:
+        items:
+          - description: Fatal interrupt
+          - description: Ready interrupt
+          - description: Spawn acknowledge interrupt
+          - description: Stop acknowledge interrupt

ditto

+
+      interrupt-names:
+        items:
+          - const: fatal
+          - const: ready
+          - const: spawn-ack
+          - const: stop-ack
+
+      qcom,smem-states:
+        $ref: /schemas/types.yaml#/definitions/phandle-array
+        description: States used by the AP to signal the remote processor
+        items:
+          - description: Shutdown WCSS pd
+          - description: Stop WCSS pd
+          - description: Spawn WCSS pd
+
+      qcom,smem-state-names:
+        description:
+          Names of the states used by the AP to signal the remote processor
+        items:
+          - const: shutdown
+          - const: stop
+          - const: spawn
+
+    required:
+      - compatible
+      - firmware-name
+      - interrupts-extended
+      - interrupt-names
+      - qcom,smem-states
+      - qcom,smem-state-names
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - firmware-name
+  - reg
+  - interrupts-extended
+  - interrupt-names
+  - qcom,smem-states
+  - qcom,smem-state-names
+  - memory-region
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq5018-q6-mpd
+    then:
+      properties:
+        firmware-name:
+          items:
+            - const: IPQ5018/q6_fw.mdt
+            - const: IPQ5018/m3_fw.mdt
+            - const: qcn6122/m3_fw.mdt

No, names are not part of bindings. Also paths do not look correct. Open
linux-firmware package and verify these are good...

+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq9574-q6-mpd
+    then:
+      properties:
+        firmware-name:
+          items:
+            - const: IPQ9574/q6_fw.mdt
+            - const: IPQ9574/m3_fw.mdt

Drop.

+
+unevaluatedProperties: false

This changed... why?


'unevaluatedProperties' is similar to 'additionalProperties' except
that it recognize properties declared in subschemas as well.

Thanks & Regards,
Manikanta.

Best regards,
Krzysztof




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux