diff mbox series

[v2] dt-bindings: mailbox: add Versal IPI bindings

Message ID 20231212230353.888714-1-tanmay.shah@amd.com
State New
Headers show
Series [v2] dt-bindings: mailbox: add Versal IPI bindings | expand

Commit Message

Tanmay Shah Dec. 12, 2023, 11:03 p.m. UTC
Add documentation for AMD-Xilinx Versal platform Inter Processor Interrupt
controller. Versal IPI controller contains buffer-less IPI which do not
have buffers for message passing. For such IPI channels message buffers
are not expected and only notification to/from remote agent is expected.

Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
---

Changes in v2:
  - Add versal bindings to existing bindings doc instead of separate
    file.
  - Sort required list same as properties list
  - Add minimum and maximum range for xlnx,ipi-id vendor property
  - Move vendor property last in the list
  - Fix description of child node reg property for versal bindings
  - Change commit text

depends on: https://lore.kernel.org/linux-arm-kernel/79f65b96-9015-41c4-b4ee-a82526c9eefc@linaro.org/T/#meeacc5c57a9610b19758d313e5b2d17ab470f646

 .../mailbox/xlnx,zynqmp-ipi-mailbox.yaml      | 172 ++++++++++++++----
 1 file changed, 138 insertions(+), 34 deletions(-)


base-commit: abb240f7a2bd14567ab53e602db562bb683391e6
prerequisite-patch-id: 70017c8eaded5fc85749995b9cf093c6c625fab3

Comments

Tanmay Shah Dec. 13, 2023, 4:35 p.m. UTC | #1
On 12/13/23 10:21 AM, Krzysztof Kozlowski wrote:
> On 13/12/2023 17:18, Tanmay Shah wrote:
>
>
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>
> >> I don't understand why this change is here. Previously you did not have
> >> MMIO address space? If yes, then where do you restrict the old device to
> >> disallow these?
> > 
> > 
> > Hardware description is different between ZynqMP and Versal. And so, we have to design
> > 
> > new bindings for Versal. reg and reg-names for parent node, is only available for new devices.
> > 
> > The new device is checked with compatible string after "required" section.
>
> Hm? What does that mean?


Following [1]. I think You answered what needs to be done. I will have reg: false for old devices.

Thanks.


>
> > 
> > Should I add "unevaluatedProperties: false" after "additionalProperties: false" for parent node below [1] ?
>
> No, you should disallow reg/reg-names for older device.
>
> ...
>
> > 
> >>> +        then:
> >>> +          properties:
> >>> +            reg:
> >>> +              items:
> >>> +                - description: Host agent request message buffer
> >>> +                - description: Host agent response message buffer
> >>> +                - description: Remote agent request message buffer
> >>> +                - description: Remote agent response message buffer
> >>> +
> >>> +            reg-names:
> >>> +              items:
> >>> +                - const: local_request_region
> >>> +                - const: local_response_region
> >>> +                - const: remote_request_region
> >>> +                - const: remote_response_region
> >>> +        else:
> >>> +          properties:
> >>> +            reg:
> >>> +              minItems: 1
> >>> +              items:
> >>> +                - description: Remote IPI agent control register
> >>> +                - description: Remote IPI agent optional message buffer
> >>
> >> Were these described in old binding? If not, it's a separate change.
> > 
> > Okay, so I will split this in two patches:
> > 
> > 1) Clean up current bindings (like remove redundant descriptino, sort "required" property order etc..)
>
> If you want to combine cleanup and functional changes, then this would
> be two patches.
>
> > 
> > 2) Add versal platforms bindings doc. (This will add if else cases and Versal platform support)

In that case, can I just post single patch that adds new device support ?

Won't touch anything that is not needed for new device support.


>
> > 
> > 
> >>> +
> >>> +            reg-names:
> >>> +              minItems: 1
> >>> +              items:
> >>> +                - const: ctrl
> >>> +                - const: msg
> >>
> >> Blank line
> >>
> >>>  required:
> >>>    - compatible
> >>> -  - interrupts
> >>>    - '#address-cells'
> >>>    - '#size-cells'
> >>> +  - interrupts
> >>
> >> Separate change with its own rationale. Trivial cleanups can be
> >> organized in one patch, but should not be mixed with adding new devices.
> > 
> > Ack
> > 
> > 
> >>>    - xlnx,ipi-id
> >>>  
> >>> +allOf:
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - xlnx,versal-ipi-mailbox
> >>> +    then:
> >>> +      properties:
> >>> +        reg:
> >>> +          items:
> >>> +            - description: Host IPI agent control registers
> >>> +            - description: Host IPI agent optional message buffers
> >>> +
> >>> +        reg-names:
> >>> +          items:
> >>> +            - const: ctrl
> >>> +            - const: msg
> >>> +
> >>> +      required:
> >>> +        - reg
> >>> +        - reg-names
> >>> +
> >>> +additionalProperties: false
> > 
> > [1] Here, if I add unevaluatedProperties: false then is it enough for old device to disallow
>
> No, you need reg:false
>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.yaml
index 8b15a0532120..95146adb9631 100644
--- a/Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.yaml
+++ b/Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.yaml
@@ -8,11 +8,11 @@  title: Xilinx IPI(Inter Processor Interrupt) mailbox controller
 
 description: |
   The Xilinx IPI(Inter Processor Interrupt) mailbox controller is to manage
-  messaging between two Xilinx Zynq UltraScale+ MPSoC IPI agents. Each IPI
-  agent owns registers used for notification and buffers for message.
+  messaging between two Xilinx Zynq UltraScale+ MPSoC and Versal IPI agents.
+  Each IPI agent owns registers used for notification and buffers for message.
 
                +-------------------------------------+
-               | Xilinx ZynqMP IPI Controller        |
+               | Xilinx IPI Controller               |
                +-------------------------------------+
     +--------------------------------------------------+
   TF-A                   |                     |
@@ -37,15 +37,13 @@  maintainers:
 
 properties:
   compatible:
-    const: xlnx,zynqmp-ipi-mailbox
+    enum:
+      - xlnx,zynqmp-ipi-mailbox
+      - xlnx,versal-ipi-mailbox
 
   method:
     description: |
       The method of calling the PM-API firmware layer.
-      Permitted values are.
-      - "smc" : SMC #0, following the SMCCC
-      - "hvc" : HVC #0, following the SMCCC
-
     $ref: /schemas/types.yaml#/definitions/string
     enum:
       - smc
@@ -58,16 +56,26 @@  properties:
   '#size-cells':
     const: 2
 
-  xlnx,ipi-id:
-    description: |
-      Remote Xilinx IPI agent ID of which the mailbox is connected to.
-    $ref: /schemas/types.yaml#/definitions/uint32
+  reg:
+    minItems: 1
+    maxItems: 2
+
+  reg-names:
+    minItems: 1
+    maxItems: 2
 
   interrupts:
     maxItems: 1
 
   ranges: true
 
+  xlnx,ipi-id:
+    description: |
+      Remote Xilinx IPI agent ID of which the mailbox is connected to.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 64
+
 patternProperties:
   '^mailbox@[0-9a-f]+$':
     description: Internal ipi mailbox node
@@ -76,57 +84,116 @@  patternProperties:
     properties:
 
       compatible:
-        const: xlnx,zynqmp-ipi-dest-mailbox
+        enum:
+          - xlnx,zynqmp-ipi-dest-mailbox
+          - xlnx,versal-ipi-dest-mailbox
 
-      xlnx,ipi-id:
-        description:
-          Remote Xilinx IPI agent ID of which the mailbox is connected to.
-        $ref: /schemas/types.yaml#/definitions/uint32
+      reg:
+        minItems: 1
+        maxItems: 4
+
+      reg-names:
+        minItems: 1
+        maxItems: 4
 
       '#mbox-cells':
         const: 1
         description:
           It contains tx(0) or rx(1) channel IPI id number.
 
-      reg:
-        maxItems: 4
-
-      reg-names:
-        items:
-          - const: local_request_region
-          - const: local_response_region
-          - const: remote_request_region
-          - const: remote_response_region
+      xlnx,ipi-id:
+        description:
+          Remote Xilinx IPI agent ID of which the mailbox is connected to.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 64
 
     required:
       - compatible
       - reg
       - reg-names
       - "#mbox-cells"
-
-additionalProperties: false
-
+      - xlnx,ipi-id
+
+    allOf:
+      - if:
+          properties:
+            compatible:
+              contains:
+                enum:
+                  - xlnx,zynqmp-ipi-dest-mailbox
+        then:
+          properties:
+            reg:
+              items:
+                - description: Host agent request message buffer
+                - description: Host agent response message buffer
+                - description: Remote agent request message buffer
+                - description: Remote agent response message buffer
+
+            reg-names:
+              items:
+                - const: local_request_region
+                - const: local_response_region
+                - const: remote_request_region
+                - const: remote_response_region
+        else:
+          properties:
+            reg:
+              minItems: 1
+              items:
+                - description: Remote IPI agent control register
+                - description: Remote IPI agent optional message buffer
+
+            reg-names:
+              minItems: 1
+              items:
+                - const: ctrl
+                - const: msg
 required:
   - compatible
-  - interrupts
   - '#address-cells'
   - '#size-cells'
+  - interrupts
   - xlnx,ipi-id
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - xlnx,versal-ipi-mailbox
+    then:
+      properties:
+        reg:
+          items:
+            - description: Host IPI agent control registers
+            - description: Host IPI agent optional message buffers
+
+        reg-names:
+          items:
+            - const: ctrl
+            - const: msg
+
+      required:
+        - reg
+        - reg-names
+
+additionalProperties: false
+
+
 examples:
   - |
     #include<dt-bindings/interrupt-controller/arm-gic.h>
 
-    amba {
-      #address-cells = <0x2>;
-      #size-cells = <0x2>;
+    bus {
       zynqmp-mailbox {
         compatible = "xlnx,zynqmp-ipi-mailbox";
         interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
         xlnx,ipi-id = <0>;
         #address-cells = <2>;
         #size-cells = <2>;
-        ranges;
 
         mailbox: mailbox@ff9905c0 {
           compatible = "xlnx,zynqmp-ipi-dest-mailbox";
@@ -144,4 +211,41 @@  examples:
       };
     };
 
+  - |
+    #include<dt-bindings/interrupt-controller/arm-gic.h>
+
+    bus {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      zynqmp-mailbox@ff300000 {
+        compatible = "xlnx,versal-ipi-mailbox";
+        interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <2>;
+        #size-cells = <2>;
+        reg = <0x0 0xff300000 0x0 0x1000>,
+              <0x0 0xff990000 0x0 0x1ff>;
+        reg-names = "ctrl", "msg";
+        xlnx,ipi-id = <0>;
+        ranges;
+
+        /* buffered IPI */
+        mailbox@ff340000 {
+          compatible = "xlnx,versal-ipi-dest-mailbox";
+          reg = <0x0 0xff340000 0x0 0x1000>,
+                <0x0 0xff990400 0x0 0x1ff>;
+          reg-names = "ctrl", "msg";
+          #mbox-cells = <1>;
+          xlnx,ipi-id = <4>;
+        };
+
+        /* bufferless IPI */
+        mailbox@ff370000 {
+          compatible = "xlnx,versal-ipi-dest-mailbox";
+          reg = <0x0 0xff370000 0x0 0x1000>;
+          reg-names = "ctrl";
+          #mbox-cells = <1>;
+          xlnx,ipi-id = <7>;
+        };
+      };
+    };
 ...