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

From: Rob Herring
Date: Mon Nov 08 2021 - 13:05:00 EST


On Thu, Oct 28, 2021 at 06:15:16PM +0530, Pratyush Yadav wrote:
> Many SPI controllers need to add properties to slave devices. This could

Probably should replace 'slave' with 'peripheral' throughout[1].

> be the delay in clock or data lines, etc. These properties are
> controller specific but need to be defined in the slave node because
> they are per-slave and there can be multiple slaves attached to a
> controller.
>
> If these properties are not added to the slave binding, then the dtbs
> check emits a warning. But these properties do not make much sense in
> the slave binding because they are controller-specific and they will
> just pollute every slave binding. So this binding is added to collect
> all such properties from all such controllers. Slave 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 slave 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@xxxxxx>
>
> ---
>
> 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-slave-props.yaml | 91 +++++++++++++++++++
> 2 files changed, 93 insertions(+), 67 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/spi/spi-slave-props.yaml
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> index 8246891602e7..ff770657e214 100644
> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> @@ -94,73 +94,8 @@ patternProperties:
> "^.*@[0-9a-f]+$":
> type: object
>
> - properties:
> - compatible:
> - description:
> - Compatible of the SPI device.
> -
> - reg:
> - minItems: 1
> - maxItems: 256
> - items:
> - minimum: 0
> - maximum: 256
> - description:
> - Chip select used by the device.
> -
> - spi-3wire:
> - $ref: /schemas/types.yaml#/definitions/flag
> - description:
> - The device requires 3-wire mode.
> -
> - spi-cpha:
> - $ref: /schemas/types.yaml#/definitions/flag
> - description:
> - The device requires shifted clock phase (CPHA) mode.
> -
> - spi-cpol:
> - $ref: /schemas/types.yaml#/definitions/flag
> - description:
> - The device requires inverse clock polarity (CPOL) mode.
> -
> - spi-cs-high:
> - $ref: /schemas/types.yaml#/definitions/flag
> - description:
> - The device requires the chip select active high.
> -
> - spi-lsb-first:
> - $ref: /schemas/types.yaml#/definitions/flag
> - description:
> - The device requires the LSB first mode.
> -
> - spi-max-frequency:
> - $ref: /schemas/types.yaml#/definitions/uint32
> - description:
> - Maximum SPI clocking speed of the device in Hz.
> -
> - spi-rx-bus-width:
> - description:
> - Bus width to the SPI bus used for read transfers.
> - If 0 is provided, then no RX will be possible on this device.
> - $ref: /schemas/types.yaml#/definitions/uint32
> - enum: [0, 1, 2, 4, 8]
> - default: 1
> -
> - spi-rx-delay-us:
> - description:
> - Delay, in microseconds, after a read transfer.
> -
> - spi-tx-bus-width:
> - description:
> - Bus width to the SPI bus used for write transfers.
> - If 0 is provided, then no TX will be possible on this device.
> - $ref: /schemas/types.yaml#/definitions/uint32
> - enum: [0, 1, 2, 4, 8]
> - default: 1
> -
> - spi-tx-delay-us:
> - description:
> - Delay, in microseconds, after a write transfer.
> + allOf:
> + - $ref: spi-slave-props.yaml
>
> required:
> - compatible
> diff --git a/Documentation/devicetree/bindings/spi/spi-slave-props.yaml b/Documentation/devicetree/bindings/spi/spi-slave-props.yaml
> new file mode 100644
> index 000000000000..5166ec9b0353
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-slave-props.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/spi-slave-props.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Slave-specific properties for a SPI bus.
> +
> +description: |

Don't need '|' if no formatting to preserve.

> + Many SPI controllers need to add properties to slave devices. They could be
> + common properties like spi-max-frequency, spi-cpha, etc. or they could be
> + controller specific like delay in clock or data lines, etc. These properties
> + need to be defined in the slave node because they are per-slave and there can
> + be multiple slaves attached to a controller. All those properties are listed
> + here. The controller specific properties should go in their own separate
> + schema that should be referenced from here.
> +
> +maintainers:
> + - Pratyush Yadav <p.yadav@xxxxxx>
> +
> +properties:
> + compatible:
> + description:
> + Compatible of the SPI device.

You can drop 'compatible'.

> +
> + reg:
> + minItems: 1
> + maxItems: 256
> + items:
> + minimum: 0
> + maximum: 256
> + description:
> + Chip select used by the device.
> +
> + spi-3wire:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + The device requires 3-wire mode.
> +
> + spi-cpha:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + The device requires shifted clock phase (CPHA) mode.
> +
> + spi-cpol:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + The device requires inverse clock polarity (CPOL) mode.
> +
> + spi-cs-high:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + The device requires the chip select active high.
> +
> + spi-lsb-first:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + The device requires the LSB first mode.
> +
> + spi-max-frequency:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Maximum SPI clocking speed of the device in Hz.
> +
> + spi-rx-bus-width:
> + description:
> + Bus width to the SPI bus used for read transfers.
> + If 0 is provided, then no RX will be possible on this device.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 4, 8]
> + default: 1
> +
> + spi-rx-delay-us:
> + description:
> + Delay, in microseconds, after a read transfer.
> +
> + spi-tx-bus-width:
> + description:
> + Bus width to the SPI bus used for write transfers.
> + If 0 is provided, then no TX will be possible on this device.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 4, 8]
> + default: 1
> +
> + spi-tx-delay-us:
> + description:
> + Delay, in microseconds, after a write transfer.
> +
> +# The controller specific properties go here.
> +
> +additionalProperties: true
> --
> 2.33.1.835.ge9e5ba39a7
>
>