[PATCH 0/3] Update firmware dt-bindings

[PATCH 0/3] Update firmware dt-bindings

[Ronak Jain]

The patch fixes/enhances below problems,
    1. Add missing compatible property under the example section for
    zynqmp_firmware node.
    2. Add conditional pinctrl schema
    3. Move FPGA schema references under the allOf schema to align
    with the expected structure.

Ronak Jain (3):
  dt-bindings: firmware: xilinx: Add xlnx,zynqmp-firmware compatible
  dt-bindings: firmware: xilinx: Add conditional pinctrl schema
  dt-bindings: firmware: xilinx: Move FPGA schema references under allOf

 .../firmware/xilinx/xlnx,zynqmp-firmware.yaml | 27 ++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/3] dt-bindings: firmware: xilinx: Add xlnx,zynqmp-firmware compatible

[Ronak Jain]

The absence of a compatible property caused dt_binding_check to skip
the zynqmp_firmware node.

To address this, add "xlnx,zynqmp-firmware" to the compatible property
in the example section for the zynqmp_firmware node.

Signed-off-by: Ronak Jain <ronak.jain@amd.com>
---
 .../bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml           | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml
index 9b6066c7ffe4..2b72fb9d3c22 100644
--- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml
+++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml
@@ -116,6 +116,7 @@ examples:
     #include <dt-bindings/power/xlnx-zynqmp-power.h>
     firmware {
       zynqmp_firmware: zynqmp-firmware {
+        compatible = "xlnx,zynqmp-firmware";
         #power-domain-cells = <1>;
         soc-nvmem {
           compatible = "xlnx,zynqmp-nvmem-fw";
-- 
2.34.1

[PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl schema

[Ronak Jain]

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#
+
 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

[PATCH 3/3] dt-bindings: firmware: xilinx: Move FPGA schema references under allOf

[Ronak Jain]

The "pcap" and "versal-fpga" schema references are moved under the
conditional schema "allOf" to align with the expected structure so
that versal-fpga is only available via the versal firmware and pcap
via zynqmp firmware.

Signed-off-by: Ronak Jain <ronak.jain@amd.com>
---
 .../bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml      | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml
index d50438b0fca8..46d18ce4a045 100644
--- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml
+++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml
@@ -69,7 +69,6 @@ properties:
     type: object
 
   pcap:
-    $ref: /schemas/fpga/xlnx,zynqmp-pcap-fpga.yaml
     description: The ZynqMP SoC uses the PCAP (Processor Configuration Port) to
       configure the Programmable Logic (PL). The configuration uses the
       firmware interface.
@@ -93,7 +92,6 @@ properties:
     type: object
 
   versal-fpga:
-    $ref: /schemas/fpga/xlnx,versal-fpga.yaml#
     description: Compatible of the FPGA device.
     type: object
 
@@ -115,10 +113,14 @@ allOf:
       properties:
         pinctrl:
           $ref: /schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
+        pcap:
+          $ref: /schemas/fpga/xlnx,zynqmp-pcap-fpga.yaml#
     else:
       properties:
         pinctrl:
           $ref: /schemas/pinctrl/xlnx,versal-pinctrl.yaml#
+        versal-fpga:
+          $ref: /schemas/fpga/xlnx,versal-fpga.yaml#
 
 required:
   - compatible
-- 
2.34.1

Re: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl schema

[Rob Herring]

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
> 

Re: [PATCH 1/3] dt-bindings: firmware: xilinx: Add xlnx,zynqmp-firmware compatible

[Rob Herring (Arm)]

On Thu, 06 Feb 2025 06:22:42 -0800, Ronak Jain wrote:
> The absence of a compatible property caused dt_binding_check to skip
> the zynqmp_firmware node.
> 
> To address this, add "xlnx,zynqmp-firmware" to the compatible property
> in the example section for the zynqmp_firmware node.
> 
> Signed-off-by: Ronak Jain <ronak.jain@amd.com>
> ---
>  .../bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml           | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

RE: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl schema

[Jain, Ronak]

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
> >

RE: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl schema

[Jain, Ronak]

[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
> > >

RE: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl schema

[Jain, Ronak]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi @Rob Herring,

Did you get a moment to go through the queries I asked for?

Thanks,
Ronak

> -----Original Message-----
> From: Jain, Ronak
> Sent: Thursday, February 20, 2025 5:48 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: 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
> > > >

RE: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl schema

[Jain, Ronak]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Rob,

I am following up on this thread as the query from April is still open.  Could you please take a look at my question in the inline mail below?

Thanks,
Ronak

> -----Original Message-----
> From: Jain, Ronak
> Sent: Tuesday, March 4, 2025 11:46 AM
> To: 'Rob Herring' <robh@kernel.org>
> Cc: 'krzk+dt@kernel.org' <krzk+dt@kernel.org>; 'conor+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'
> <devicetree@vger.kernel.org>; 'linux-arm-kernel@lists.infradead.org' <linux-
> arm-kernel@lists.infradead.org>; 'linux-kernel@vger.kernel.org' <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH 2/3] dt-bindings: firmware: xilinx: Add conditional pinctrl
> schema
>
> Hi @Rob Herring,
>
> Did you get a moment to go through the queries I asked for?
>
> Thanks,
> Ronak
>
> > -----Original Message-----
> > From: Jain, Ronak
> > Sent: Thursday, February 20, 2025 5:48 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: 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
> > > > >