Date:   Thu, 18 Nov 2021 16:23:20 -0600
From:   Rob Herring <robh@kernel.org>
To:     Pratyush Yadav <p.yadav@ti.com>
Cc:     Mark Brown <broonie@kernel.org>, Nishanth Menon <nm@ti.com>,
        Michael Walle <michael@walle.cc>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        Tudor Ambarus <tudor.ambarus@microchip.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org
Subject: Re: [PATCH v3 1/3] spi: dt-bindings: add schema listing
 peripheral-specific properties
Message-ID: <YZbSWA5kSfORSzky@robh.at.kernel.org>
References: <20211109181911.2251-1-p.yadav@ti.com>
 <20211109181911.2251-2-p.yadav@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211109181911.2251-2-p.yadav@ti.com>
Precedence: bulk

On Tue, Nov 09, 2021 at 11:49:09PM +0530, Pratyush Yadav wrote:
> Many SPI controllers need to add properties to peripheral devices. This
> could be the delay in clock or data lines, etc. These properties are
> controller specific but need to be defined in the peripheral node
> because they are per-peripheral and there can be multiple peripherals
> attached to a controller.
> 
> If these properties are not added to the peripheral binding, then the
> dtbs check emits a warning. But these properties do not make much sense
> in the peripheral binding because they are controller-specific and they
> will just pollute every peripheral binding. So this binding is added to
> collect all such properties from all such controllers. Peripheral
> bindings should simply refer to this binding and they should be rid of
> the warnings.
> 
> There are some limitations with this approach. Firstly, there is no way
> to specify required properties. The schema contains properties for all
> controllers and there is no way to know which controller is being used.
> Secondly, there is no way to restrict additional properties. Since this
> schema will be used with an allOf operator, additionalProperties needs
> to be true. In addition, the peripheral schema will have to set
> unevaluatedProperties: false.
> 
> Despite these limitations, this appears to be the best solution to this
> problem that doesn't involve modifying existing tools or schema specs.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> 
> ---
> 
> Changes in v3:
> - s/slave/peripheral/g
> - Drop the | on description.
> - Drop the compatible property.
> 
> Changes in v2:
> - Move other subnode properties listed in spi-controller.yaml to
>   spi-slave-props.yaml
> - Move the Cadence controller-specific properties out of
>   spi-slave-props.yaml. They will be added in a separate file.
> - Add a reference to spi-slave-props.yaml in spi-controller.yaml.
> - Update description.
> 
>  .../bindings/spi/spi-controller.yaml          | 69 +--------------
>  .../bindings/spi/spi-peripheral-props.yaml    | 87 +++++++++++++++++++
>  2 files changed, 89 insertions(+), 67 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml

Reviewed-by: Rob Herring <robh@kernel.org>