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 <[email protected]>
---
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/[email protected]/T/#meeacc5c57a9610b19758d313e5b2d17ab470f646
.../mailbox/xlnx,zynqmp-ipi-mailbox.yaml | 172 ++++++++++++++----
1 file changed, 138 insertions(+), 34 deletions(-)
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>;
+ };
+ };
+ };
...
base-commit: abb240f7a2bd14567ab53e602db562bb683391e6
prerequisite-patch-id: 70017c8eaded5fc85749995b9cf093c6c625fab3
--
2.25.1
On 13/12/2023 00:03, Tanmay Shah wrote:
> 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 <[email protected]>
> ---
>
> 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
> -
Independent change. Please do not mix logical changes in one patch.
> $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
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?
>
> 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
Same concern.
>
> '#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
Were these described in old binding? If not, it's a separate change.
> +
> + 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.
> - 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
> +
> +
Just one blank line.
> 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;
How is this related to Versal?
>
> 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 {
mailbox@
> + 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;
> +
Best regards,
Krzysztof
Hi Krzysztof,
Thanks for reviews. Please find my comments below inline.
On 12/13/23 12:35 AM, Krzysztof Kozlowski wrote:
> On 13/12/2023 00:03, Tanmay Shah wrote:
> > 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 <[email protected]>
> > ---
> >
>
>
> > 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
> > -
>
> Independent change. Please do not mix logical changes in one patch.
>
> > $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
>
> 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.
Should I add "unevaluatedProperties: false" after "additionalProperties: false" for parent node below [1] ?
>
> >
> > 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
>
> Same concern.
This one, reg and reg-names are available for both old and new devices but, with different
definition. And this is checked based on compatible of child node below [2].
>
> >
> > '#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
[2] Here based on compatible string, "reg" and "reg-names" are defined.
> > + 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..)
2) Add versal platforms bindings doc. (This will add if else cases and Versal platform 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
"reg" and "reg-names" for parent node ?
If not, could you please suggest how to achieve this?
Or is there a way to undefine "reg" and "reg-names" based on compatible property ?
Based on your feedback, I will post v3.
> > +
> > +
>
> Just one blank line.
Ack.
>
> > 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;
>
> How is this related to Versal?
Yeah its not, will go in cleanup patch.
>
> >
> > 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 {
>
> mailbox@
Ack.
>
> > + 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;
> > +
>
>
> Best regards,
> Krzysztof
>
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?
>
> 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)
>
>
>>> +
>>> + 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
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
>