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;
Date:   Mon, 11 Jul 2022 13:26:21 -0600
From:   Rob Herring <robh@kernel.org>
To:     Vidya Sagar <vidyas@nvidia.com>
Cc:     bhelgaas@google.com, lpieralisi@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, thierry.reding@gmail.com,
        jonathanh@nvidia.com, jingoohan1@gmail.com,
        gustavo.pimentel@synopsys.com, kw@linux.com, kishon@ti.com,
        linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
        linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
        kthota@nvidia.com, mmaddireddy@nvidia.com, sagar.tv@gmail.com
Subject: Re: [PATCH V4 2/9] dt-bindings: PCI: tegra234: Add schema for
 tegra234 rootport mode
Message-ID: <20220711192621.GA126971-robh@kernel.org>
References: <20220707081301.29961-1-vidyas@nvidia.com>
 <20220707081301.29961-3-vidyas@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220707081301.29961-3-vidyas@nvidia.com>
Precedence: bulk

On Thu, Jul 07, 2022 at 01:42:54PM +0530, Vidya Sagar wrote:
> Add support for PCIe controllers that operate in the rootport mode
> in tegra234 chipset.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V4:
> * Rebased on top of previous patch
> 
> V3:
> * New patch in this series
> 
>  .../bindings/pci/nvidia,tegra194-pcie.yaml    | 136 +++++++++++++++++-
>  1 file changed, 130 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> index 5d7cb56c86a2..c0dfe2e01112 100644
> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> @@ -24,6 +24,7 @@ properties:
>    compatible:
>      enum:
>        - nvidia,tegra194-pcie
> +      - nvidia,tegra234-pcie
>  
>    reg:
>      items:
> @@ -92,7 +93,8 @@ properties:
>        A phandle to the node that controls power to the respective PCIe
>        controller and a specifier name for the PCIe controller.
>  
> -      specifiers are defined in "include/dt-bindings/power/tegra194-powergate.h" file.
> +      Tegra194 specifiers are defined in "include/dt-bindings/power/tegra194-powergate.h" file.
> +      Tegra234 specifiers are defined in "include/dt-bindings/power/tegra234-powergate.h" file.
>  
>    interconnects:
>      items:
> @@ -112,17 +114,30 @@ properties:
>        Must contain a pair of phandle to BPMP controller node followed by
>        controller ID. Following are the controller IDs for each controller:
>  
> +      Tegra194
> +
>          0: C0
>          1: C1
>          2: C2
>          3: C3
>          4: C4
>          5: C5
> -    items:
> -      - items:
> -          - description: phandle to BPMP controller node
> -          - description: PCIe controller ID
> -            maximum: 5
> +
> +      Tegra234
> +
> +        0 : C0
> +        1 : C1
> +        2 : C2
> +        3 : C3
> +        4 : C4
> +        5 : C5
> +        6 : C6
> +        7 : C7
> +        8 : C8
> +        9 : C9
> +        10: C10
> +
> +      Platform constraints are described later
>  
>    nvidia,update-fc-fixup:
>      description: |
> @@ -131,6 +146,8 @@ properties:
>        of the following conditions thereby enabling root port to exchange
>        optimum number of FC (Flow Control) credits with downstream devices:
>  
> +      NOTE:- This is applicable only for Tegra194.
> +
>          1. If C0/C4/C5 run at x1/x2 link widths (irrespective of speed and MPS)
>          2. If C0/C1/C2/C3/C4/C5 operate at their respective max link widths and
>            a) speed is Gen-2 and MPS is 256B
> @@ -162,10 +179,56 @@ properties:
>        if the platform has one such slot. (Ex:- x16 slot owned by C5 controller
>        in p2972-0000 platform).
>  
> +  nvidia,enable-srns:
> +    description: |
> +      This boolean property needs to be present if the controller is configured
> +      to operate in SRNS (Separate Reference Clocks with No Spread-Spectrum Clocking).
> +      NOTE:- This is applicable only for Tegra234.
> +
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  nvidia,enable-ext-refclk:
> +    description: |
> +      This boolean property needs to be present if the controller is configured
> +      to use the reference clocking coming in from an external clock source instead of
> +      using the internal clock source.
> +
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
>  allOf:
>    - $ref: /schemas/pci/pci-bus.yaml#
>    - $ref: /schemas/pci/snps,dw-pcie.yaml#
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra194-pcie
> +    then:
> +      properties:
> +        nvidia,bpmp:
> +          items:
> +            - items:
> +                - description: phandle to BPMP controller node
> +                - description: PCIe controller ID
> +                  maximum: 5
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra234-pcie
> +    then:
> +      properties:
> +        nvidia,bpmp:
> +          items:
> +            - items:
> +                - description: phandle to BPMP controller node
> +                - description: PCIe controller ID
> +                  maximum: 10

I don't think this works because you still have a limit of 5 in the 
definition of nvidia,bpmp. I would just change it's limit to 10 and drop 
these if/then schemas. I don't think the extra checking is really worth 
it.

Rob