Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220309120025.6721-1-bharat.kumar.gogada@xilinx.com> <20220309120025.6721-2-bharat.kumar.gogada@xilinx.com>
In-Reply-To: <20220309120025.6721-2-bharat.kumar.gogada@xilinx.com>
From:   Rob Herring <robh@kernel.org>
Date:   Thu, 10 Mar 2022 13:13:18 -0600
Message-ID: <CAL_JsqL3CSERnTihfc_CKB96bbdN2xxkb78H383pMfC_bGg+Xw@mail.gmail.com>
Subject: Re: [PATCH v3 1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port
To:     Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
Cc:     PCI <linux-pci@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        devicetree@vger.kernel.org,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Michal Simek <michals@xilinx.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Mar 9, 2022 at 6:00 AM Bharat Kumar Gogada
<bharat.kumar.gogada@xilinx.com> wrote:
>
> Xilinx Versal Premium series has CPM5 block which supports Root Port
> functioning at Gen5 speed.
>
> Add support for YAML schemas documentation for Versal CPM5 Root Port driver.
>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
>  .../bindings/pci/xilinx-versal-cpm.yaml       | 47 ++++++++++++++++---
>  1 file changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> index 32f4641085bc..97c7229d7f91 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -14,17 +14,21 @@ allOf:
>
>  properties:
>    compatible:
> -    const: xlnx,versal-cpm-host-1.00
> +    contains:

Nope. That means 'compatible = "foo", "xlnx,versal-cpm-host-1.00",
"bar";' would be valid.

> +      enum:
> +        - xlnx,versal-cpm-host-1.00
> +        - xlnx,versal-cpm5-host-1.00

Where does 1.00 come from? My guess is you or whoever did the original
binding just made it up. Version numbers are only used when they
correspond to something documented for the h/w. In the case of Xilinx,
that should be soft IP (which I assume has versioned releases) and
nothing else. If 'versal' is not specific enough to identify a
specific SoC, then add to it.


>    reg:
> -    items:
> -      - description: Configuration space region and bridge registers.
> -      - description: CPM system level control and status registers.
> +    description: |
> +      Should contain cpm_slcr, cfg registers location and length.
> +      For xlnx,versal-cpm5-host-1.00, it should also contain cpm_csr.

Not an improvement in defining what each entry is.

> +    minItems: 2
> +    maxItems: 3