| Message ID | 20250206142244.2553237-3-ronak.jain@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Update firmware dt-bindings | expand |
On Thu, Feb 06, 2025 at 06:22:43AM -0800, Ronak Jain wrote: > Updates the Device Tree bindings for Xilinx firmware by introducing > conditional schema references for the pinctrl node. > > Previously, the pinctrl node directly referenced > xlnx,zynqmp-pinctrl.yaml. However, this patch modifies the schema to > conditionally apply the correct pinctrl schema based on the compatible > property. Specifically: > - If compatible contains "xlnx,zynqmp-pinctrl", reference > xlnx,zynqmp-pinctrl.yaml. > - If compatible contains "xlnx,versal-pinctrl", reference > xlnx,versal-pinctrl.yaml. > > Additionally, an example entry for "xlnx,versal-pinctrl" has been > added under the examples section. > > Signed-off-by: Ronak Jain <ronak.jain@amd.com> > --- > .../firmware/xilinx/xlnx,zynqmp-firmware.yaml | 20 ++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml > index 2b72fb9d3c22..d50438b0fca8 100644 > --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml > @@ -76,7 +76,6 @@ properties: > type: object > > pinctrl: > - $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# > description: The pinctrl node provides access to pinconfig and pincontrol > functionality available in firmware. > type: object > @@ -106,6 +105,21 @@ properties: > type: object > deprecated: true > > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: xlnx,zynqmp-firmware > + then: > + properties: > + pinctrl: > + $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# > + else: > + properties: > + pinctrl: > + $ref: /schemas/pinctrl/xlnx,versal-pinctrl.yaml# The somewhat preferred way to do this would be to do this in the top level: pinctrl: type: object additionalProperties: true properties: compatible: contains: enum: - xlnx,zynqmp-pinctrl - xlnx,versal-pinctrl required: - compatible Otherwise, the pinctrl schema ends up being applied twice. > + > required: > - compatible > > @@ -164,6 +178,10 @@ examples: > compatible = "xlnx,versal-fpga"; > }; > > + pinctrl { > + compatible = "xlnx,versal-pinctrl"; > + }; > + > xlnx_aes: zynqmp-aes { > compatible = "xlnx,zynqmp-aes"; > }; > -- > 2.34.1 >
Hi Rob, > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Wednesday, February 12, 2025 3:24 AM > To: Jain, Ronak <ronak.jain@amd.com> > Cc: krzk+dt@kernel.org; conor+dt@kernel.org; Simek, Michal > <michal.simek@amd.com>; Manne, Nava kishore > <nava.kishore.manne@amd.com>; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl > schema > > On Thu, Feb 06, 2025 at 06:22:43AM -0800, Ronak Jain wrote: > > Updates the Device Tree bindings for Xilinx firmware by introducing > > conditional schema references for the pinctrl node. > > > > Previously, the pinctrl node directly referenced > > xlnx,zynqmp-pinctrl.yaml. However, this patch modifies the schema to > > conditionally apply the correct pinctrl schema based on the compatible > > property. Specifically: > > - If compatible contains "xlnx,zynqmp-pinctrl", reference > > xlnx,zynqmp-pinctrl.yaml. > > - If compatible contains "xlnx,versal-pinctrl", reference > > xlnx,versal-pinctrl.yaml. > > > > Additionally, an example entry for "xlnx,versal-pinctrl" has been > > added under the examples section. > > > > Signed-off-by: Ronak Jain <ronak.jain@amd.com> > > --- > > .../firmware/xilinx/xlnx,zynqmp-firmware.yaml | 20 ++++++++++++++++++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > firmware.yaml b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > firmware.yaml > > index 2b72fb9d3c22..d50438b0fca8 100644 > > --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > firmware.yaml > > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > firmware.yaml > > @@ -76,7 +76,6 @@ properties: > > type: object > > > > pinctrl: > > - $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# > > description: The pinctrl node provides access to pinconfig and pincontrol > > functionality available in firmware. > > type: object > > @@ -106,6 +105,21 @@ properties: > > type: object > > deprecated: true > > > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: xlnx,zynqmp-firmware > > + then: > > + properties: > > + pinctrl: > > + $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# > > + else: > > + properties: > > + pinctrl: > > + $ref: /schemas/pinctrl/xlnx,versal-pinctrl.yaml# > > The somewhat preferred way to do this would be to do this in the top > level: > > pinctrl: > type: object > additionalProperties: true > properties: > compatible: > contains: > enum: > - xlnx,zynqmp-pinctrl > - xlnx,versal-pinctrl > required: > - compatible > > Otherwise, the pinctrl schema ends up being applied twice. Thanks for the patch review and inputs. I'll update and send the next version. Thanks, Ronak > > > + > > required: > > - compatible > > > > @@ -164,6 +178,10 @@ examples: > > compatible = "xlnx,versal-fpga"; > > }; > > > > + pinctrl { > > + compatible = "xlnx,versal-pinctrl"; > > + }; > > + > > xlnx_aes: zynqmp-aes { > > compatible = "xlnx,zynqmp-aes"; > > }; > > -- > > 2.34.1 > >
[AMD Official Use Only - AMD Internal Distribution Only] Hi Rob, > -----Original Message----- > From: Jain, Ronak > Sent: Thursday, February 13, 2025 4:46 PM > To: Rob Herring <robh@kernel.org> > Cc: krzk+dt@kernel.org; conor+dt@kernel.org; Simek, Michal > <michal.simek@amd.com>; Manne, Nava kishore > <nava.kishore.manne@amd.com>; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl > schema > > Hi Rob, > > > -----Original Message----- > > From: Rob Herring <robh@kernel.org> > > Sent: Wednesday, February 12, 2025 3:24 AM > > To: Jain, Ronak <ronak.jain@amd.com> > > Cc: krzk+dt@kernel.org; conor+dt@kernel.org; Simek, Michal > > <michal.simek@amd.com>; Manne, Nava kishore > > <nava.kishore.manne@amd.com>; devicetree@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl > > schema > > > > On Thu, Feb 06, 2025 at 06:22:43AM -0800, Ronak Jain wrote: > > > Updates the Device Tree bindings for Xilinx firmware by introducing > > > conditional schema references for the pinctrl node. > > > > > > Previously, the pinctrl node directly referenced > > > xlnx,zynqmp-pinctrl.yaml. However, this patch modifies the schema to > > > conditionally apply the correct pinctrl schema based on the compatible > > > property. Specifically: > > > - If compatible contains "xlnx,zynqmp-pinctrl", reference > > > xlnx,zynqmp-pinctrl.yaml. > > > - If compatible contains "xlnx,versal-pinctrl", reference > > > xlnx,versal-pinctrl.yaml. > > > > > > Additionally, an example entry for "xlnx,versal-pinctrl" has been > > > added under the examples section. > > > > > > Signed-off-by: Ronak Jain <ronak.jain@amd.com> > > > --- > > > .../firmware/xilinx/xlnx,zynqmp-firmware.yaml | 20 ++++++++++++++++++- > > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > > firmware.yaml b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > > firmware.yaml > > > index 2b72fb9d3c22..d50438b0fca8 100644 > > > --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > > firmware.yaml > > > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > > firmware.yaml > > > @@ -76,7 +76,6 @@ properties: > > > type: object > > > > > > pinctrl: > > > - $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# > > > description: The pinctrl node provides access to pinconfig and pincontrol > > > functionality available in firmware. > > > type: object > > > @@ -106,6 +105,21 @@ properties: > > > type: object > > > deprecated: true > > > > > > +allOf: > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + const: xlnx,zynqmp-firmware > > > + then: > > > + properties: > > > + pinctrl: > > > + $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# > > > + else: > > > + properties: > > > + pinctrl: > > > + $ref: /schemas/pinctrl/xlnx,versal-pinctrl.yaml# > > > > The somewhat preferred way to do this would be to do this in the top > > level: > > > > pinctrl: > > type: object > > additionalProperties: true > > properties: > > compatible: > > contains: > > enum: > > - xlnx,zynqmp-pinctrl > > - xlnx,versal-pinctrl > > required: > > - compatible > > > > Otherwise, the pinctrl schema ends up being applied twice. > > Thanks for the patch review and inputs. I'll update and send the next version. > In your suggested code, the schema allows either xlnx,zynqmp-pinctrl or xlnx,versal-pinctrl on any platform, which is incorrect. This means that if a user mistakenly assigns xlnx,versal-pinctrl to a ZynqMP platform or xlnx,zynqmp-pinctrl to a Versal platform, the wrong reference will be used, but no error is reported. The dt-binding check still passes instead of flagging this as an issue. By using a conditional schema, we can enforce platform-specific compatibility, ensuring that the correct compatible string is used for the corresponding platform. This would also generate an error if an incorrect compatible string is provided, preventing misconfigurations. Please let me know your thoughts. Thanks, Ronak > Thanks, > Ronak > > > > > + > > > required: > > > - compatible > > > > > > @@ -164,6 +178,10 @@ examples: > > > compatible = "xlnx,versal-fpga"; > > > }; > > > > > > + pinctrl { > > > + compatible = "xlnx,versal-pinctrl"; > > > + }; > > > + > > > xlnx_aes: zynqmp-aes { > > > compatible = "xlnx,zynqmp-aes"; > > > }; > > > -- > > > 2.34.1 > > >
diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml index 2b72fb9d3c22..d50438b0fca8 100644 --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml @@ -76,7 +76,6 @@ properties: type: object pinctrl: - $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# description: The pinctrl node provides access to pinconfig and pincontrol functionality available in firmware. type: object @@ -106,6 +105,21 @@ properties: type: object deprecated: true +allOf: + - if: + properties: + compatible: + contains: + const: xlnx,zynqmp-firmware + then: + properties: + pinctrl: + $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml# + else: + properties: + pinctrl: + $ref: /schemas/pinctrl/xlnx,versal-pinctrl.yaml# + required: - compatible @@ -164,6 +178,10 @@ examples: compatible = "xlnx,versal-fpga"; }; + pinctrl { + compatible = "xlnx,versal-pinctrl"; + }; + xlnx_aes: zynqmp-aes { compatible = "xlnx,zynqmp-aes"; };
Updates the Device Tree bindings for Xilinx firmware by introducing conditional schema references for the pinctrl node. Previously, the pinctrl node directly referenced xlnx,zynqmp-pinctrl.yaml. However, this patch modifies the schema to conditionally apply the correct pinctrl schema based on the compatible property. Specifically: - If compatible contains "xlnx,zynqmp-pinctrl", reference xlnx,zynqmp-pinctrl.yaml. - If compatible contains "xlnx,versal-pinctrl", reference xlnx,versal-pinctrl.yaml. Additionally, an example entry for "xlnx,versal-pinctrl" has been added under the examples section. Signed-off-by: Ronak Jain <ronak.jain@amd.com> --- .../firmware/xilinx/xlnx,zynqmp-firmware.yaml | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)