.../mailbox/xlnx,zynqmp-ipi-mailbox.yaml | 172 ++++++++++++++---- 1 file changed, 138 insertions(+), 34 deletions(-)
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(-)
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.1On 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 <tanmay.shah@amd.com>
> ---
>
> 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 <tanmay.shah@amd.com>
> > ---
> >
>
>
> > 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 >
© 2016 - 2025 Red Hat, Inc.