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
