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 6/6/2023 11:44 AM, Krzysztof Kozlowski wrote:
On 05/06/2023 14:02, Manikanta Mylavarapu wrote:
+  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.

So it is now in email, but not in the code...

I will add this info, corresponding block diagram in driver code.

+
+    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.

It's the same device - WCSS - right? We do not create multiple nodes and
compatibles for the same devices. Bus suffixes are almost never parts of
compatibles.


No it's not the same device. WCSS on inside IPQ5018 and WCSS attached via pcie to IPQ5018. Here QDSP6 managing both WCSS's.

So for better clarity i will use attached SOC ID in compatible.
Below are the new compatible's.

- qcom,ipq5018-wcss-mpd //IPQ5018 internal radio
- qcom,ipq9574-wcss-mpd	//IPQ9574 internal radio
- qcom,qcn6122-wcss-mpd //IPQ5018 attached radio


Drop.

+
+unevaluatedProperties: false

This changed... why?


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

You don't have to explain me what are unevaluatedProperties or
additionalProperties. Let's assume that I know them. What you should
explain is why you changed it. Where is the reference to other schema?

I will go with previously used 'additionalProperties' itself and add
'unevaluatedProperties' in glink-edge.

Thanks & Regards,
Manikanta.

Best regards,
Krzysztof




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux