Re: [Patch V2 2/3] spi: dt-bindings: split peripheral prods

From: Rob Herring <robh@kernel.org>
To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
Cc: broonie@kernel.org, thierry.reding@gmail.com,
	jonathanh@nvidia.com, linux-spi@vger.kernel.org,
	linux-tegra@vger.kernel.org, ashishsingha@nvidia.com,
	skomatineni@nvidia.com, ldewangan@nvidia.com,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [Patch V2 2/3] spi: dt-bindings: split peripheral prods
Date: Mon, 16 May 2022 12:25:52 -0500	[thread overview]
Message-ID: <20220516172552.GA2928615-robh@kernel.org> (raw)
In-Reply-To: <20220513080828.22079-3-kyarlagadda@nvidia.com>

On Fri, May 13, 2022 at 01:38:27PM +0530, Krishna Yarlagadda wrote:
> Move peripheral properties for Tegra QSPI controller to
> nvidia,tegra210-quad-peripheral-props.yaml and add reference
> to spi-peripheral-props.yaml file.
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  ...nvidia,tegra210-quad-peripheral-props.yaml | 45 +++++++++++++++++++
>  .../bindings/spi/nvidia,tegra210-quad.yaml    | 32 -------------
>  .../bindings/spi/spi-peripheral-props.yaml    |  1 +
>  3 files changed, 46 insertions(+), 32 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> new file mode 100644
> index 000000000000..f91347759c65
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad-peripheral-props.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad-peripheral-props.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Peripheral properties for Tegra Quad SPI Controller
> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Jonathan Hunter <jonathanh@nvidia.com>
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +
> +  spi-rx-bus-width:
> +    enum: [1, 2, 4]
> +
> +  spi-tx-bus-width:
> +    enum: [1, 2, 4]

You've just set the constraints for everyone. This needs to stay in your 
controller schema as it's an additional constraint on your controller.

> +
> +  nvidia,tx-clk-tap-delay:
> +    description:
> +      Delays the clock going out to device with this tap value.
> +      Tap value varies based on platform design trace lengths from Tegra
> +      QSPI to corresponding slave device.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 31
> +
> +  nvidia,rx-clk-tap-delay:
> +    description:
> +      Delays the clock coming in from the device with this tap value.
> +      Tap value varies based on platform design trace lengths from Tegra
> +      QSPI to corresponding slave device.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +
> +required:
> +      - reg

Pretty sure we captured that elsewhere.

> +
> +unevaluatedProperties: false

This is why this patch makes everything fail. This means the only 
properties allowed for any SPI child are the above ones. You need 
'additionalProperties: true' here.

> +
> diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
> index 0296edd1de22..7d60bb1af047 100644
> --- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
> +++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
> @@ -21,9 +21,6 @@ properties:
>        - nvidia,tegra194-qspi
>        - nvidia,tegra234-qspi
>  
> -  reg:
> -    maxItems: 1

Umm, this is for your controller registers. I think you need them.

> -
>    interrupts:
>      maxItems: 1
>  
> @@ -50,37 +47,8 @@ patternProperties:
>    "@[0-9a-f]+":
>      type: object
>  
> -    properties:
> -      spi-rx-bus-width:
> -        enum: [1, 2, 4]
> -
> -      spi-tx-bus-width:
> -        enum: [1, 2, 4]
> -
> -      nvidia,tx-clk-tap-delay:
> -        description:
> -          Delays the clock going out to device with this tap value.
> -          Tap value varies based on platform design trace lengths from Tegra
> -          QSPI to corresponding slave device.
> -        $ref: /schemas/types.yaml#/definitions/uint32
> -        minimum: 0
> -        maximum: 31
> -
> -      nvidia,rx-clk-tap-delay:
> -        description:
> -          Delays the clock coming in from the device with this tap value.
> -          Tap value varies based on platform design trace lengths from Tegra
> -          QSPI to corresponding slave device.
> -        $ref: /schemas/types.yaml#/definitions/uint32
> -        minimum: 0
> -        maximum: 255
> -
> -    required:
> -      - reg
> -
>  required:
>    - compatible
> -  - reg
>    - interrupts
>    - clock-names
>    - clocks
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index 5e32928c4fc3..cf589d17b0f5 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -112,5 +112,6 @@ properties:
>  allOf:
>    - $ref: cdns,qspi-nor-peripheral-props.yaml#
>    - $ref: samsung,spi-peripheral-props.yaml#
> +  - $ref: nvidia,tegra210-quad-peripheral-props.yaml#
>  
>  additionalProperties: true
> -- 
> 2.17.1
> 
> 