Re: [PATCH v1 02/36] dt-bindings: spi: support non-spi bindings as SPI slaves

[Maxime Ripard <maxime@xxxxxxxxxx>]

Hi Sam,

On Sun, Mar 15, 2020 at 02:43:42PM +0100, Sam Ravnborg wrote:
> Independent bindings can be SPI slaves which for example is
> the case for several panel bindings.
>
> Move SPI slave properties to spi-slave.yaml so the independent
> SPI slave bindings can include spi-slave.yaml rather than
> duplicating the properties.
>
> Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: linux-spi@xxxxxxxxxxxxxxx
> ---
>  .../bindings/spi/spi-controller.yaml          | 63 +-------------
>  .../devicetree/bindings/spi/spi-slave.yaml    | 83 +++++++++++++++++++
>  2 files changed, 86 insertions(+), 60 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-slave.yaml
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> index 1e0ca6ccf64b..99531c8d10dd 100644
> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> @@ -67,71 +67,14 @@ patternProperties:
>    "^.*@[0-9a-f]+$":
>      type: object
>
> +    allOf:
> +      - $ref: spi-slave.yaml#
> +
>      properties:
>        compatible:
>          description:
>            Compatible of the SPI device.
>
> -      reg:
> -        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:
> -        allOf:
> -          - $ref: /schemas/types.yaml#/definitions/uint32
> -          - enum: [ 1, 2, 4, 8 ]
> -          - default: 1
> -        description:
> -          Bus width to the SPI bus used for MISO.
> -
> -      spi-rx-delay-us:
> -        description:
> -          Delay, in microseconds, after a read transfer.
> -
> -      spi-tx-bus-width:
> -        allOf:
> -          - $ref: /schemas/types.yaml#/definitions/uint32
> -          - enum: [ 1, 2, 4, 8 ]
> -          - default: 1
> -        description:
> -          Bus width to the SPI bus used for MOSI.
> -
> -      spi-tx-delay-us:
> -        description:
> -          Delay, in microseconds, after a write transfer.
> -

I can see what you're trying to do, but you don't really need to.

All the SPI devices will be declared under a spi controller node that
will validate its child nodes (and thus the devices) already.

Doing it this way would actually make all the checks happen twice,
once as part of the SPI controller, once as part of the SPI device
binding, without any good reason.

Maxime