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 >
