Re: [PATCH v3 1/3] spi: dt-bindings: add schema listing peripheral-specific properties

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
Date: Thu, 18 Nov 2021 16:23:20 -0600	[thread overview]
Message-ID: <YZbSWA5kSfORSzky@robh.at.kernel.org> (raw)
In-Reply-To: <20211109181911.2251-2-p.yadav@ti.com>

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>