Re: [PATCH v3 2/3] spi: dt-bindings: fsl-dspi: Convert to yaml format

[Vladimir Oltean <olteanv@xxxxxxxxx>]

On Thu, Jun 20, 2024 at 12:58:28PM -0400, Frank Li wrote:
> Convert dt-binding spi-fsl-dspi.txt to yaml format.
> 
> Addtional changes during convert:
> - compatible string "fsl,ls1028a-dspi" can be followed by
> fsl,ls1021a-v1.0-dspi.
> - Change "dspi0@4002c000" to "spi@4002c000" in example.
> - Reorder properties in example.
> - Use GIC include in example.
> - Remove fsl,spi-cs-sck-delay and fsl,spi-sck-cs-delay by use common SPI
> property.
> - Use compatible string 'jedec,spi-nor' in example.
> - Split peripheral part to fsl,spi-dspi-peripheral-props.yaml
> 
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> 
> ---
> Use part of Vladimir Oltean's work at
> https://lore.kernel.org/linux-spi/20221111224651.577729-1-vladimir.oltean@xxxxxxx/

Hm, you took part of that but gave no attribution? The portion below ---
is also discarded when the patch is applied, so even the link is lost,
FYI.

> ---
>  .../devicetree/bindings/spi/fsl,dspi.yaml          | 115 +++++++++++++++++++++
>  .../spi/fsl,spi-dspi-peripheral-props.yaml         |  28 +++++

For consistency, could you name this fsl,dspi-peripheral-props.yaml?

>  .../devicetree/bindings/spi/spi-fsl-dspi.txt       |  65 ------------
>  .../bindings/spi/spi-peripheral-props.yaml         |   1 +

No MAINTAINERS change for the schema path? There was a discussion with
Krzysztof in the old thread.

>  4 files changed, 144 insertions(+), 65 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/fsl,dspi.yaml b/Documentation/devicetree/bindings/spi/fsl,dspi.yaml
> new file mode 100644
> index 0000000000000..924ba19aea017
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/fsl,dspi.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/fsl,dspi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Freescale DSPI controller
> +
> +maintainers:
> +  - Frank Li <Frank.Li@xxxxxxx>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - fsl,vf610-dspi
> +          - fsl,ls1021a-v1.0-dspi
> +          - fsl,ls1012a-dspi
> +          - fsl,ls1028a-dspi
> +          - fsl,ls1043a-dspi
> +          - fsl,ls1046a-dspi
> +          - fsl,ls1088a-dspi
> +          - fsl,ls2080a-dspi
> +          - fsl,ls2085a-dspi
> +          - fsl,lx2160a-dspi
> +      - items:
> +          - enum:
> +              - fsl,ls1012a-dspi
> +              - fsl,ls1028a-dspi
> +              - fsl,ls1043a-dspi
> +              - fsl,ls1046a-dspi
> +              - fsl,ls1088a-dspi
> +          - const: fsl,ls1021a-v1.0-dspi
> +      - items:
> +          - const: fsl,ls2080a-dspi
> +          - const: fsl,ls2085a-dspi
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: dspi
> +
> +  pinctrl-0: true
> +
> +  pinctrl-names:
> +    items:
> +      - const: default

I don't think that pinctrl properties need to be specified in the
schema. Somehow, I think dt-schema applies
dtschema/schemas/pinctrl/pinctrl-consumer.yaml by default every time.

> +
> +  spi-num-chipselects:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: the number of the chipselect signals.

Worth mentioning that this is about _native_ chip select signals.
cs-gpios don't count against this number.

> +
> +  big-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      If present the dspi device's registers are implemented
> +      in big endian mode.

I'm not sure that this needs an explanation, it is an absolutely generic
property with a universal meaning.

> +
> +  bus-num:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: the slave chip chipselect signal number.

In fact, no, this is not a chip select number, the old documentation is
wrong. It just gets assigned to the struct spi_controller :: bus_num.
In my last submitted version I wrote "SoC-specific identifier for the
SPI controller", that seems perfectly adequate.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - pinctrl-0
> +  - pinctrl-names

interrupts and pinctrl are not required.

> +  - spi-num-chipselects
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/vf610-clock.h>
> +
> +    spi@4002c000 {
> +        compatible = "fsl,vf610-dspi";
> +        reg = <0x4002c000 0x1000>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&clks VF610_CLK_DSPI0>;
> +        clock-names = "dspi";
> +        spi-num-chipselects = <5>;
> +        bus-num = <0>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&pinctrl_dspi0_1>;
> +        big-endian;
> +
> +        flash@0 {
> +                compatible = "jedec,spi-nor";
> +                reg = <0>;
> +                spi-max-frequency = <16000000>;
> +                spi-cpol;
> +                spi-cpha;
> +                spi-cs-setup-delay-ns = <100>;
> +                spi-cs-hold-delay-ns = <50>;
> +        };
> +    };
> +

Please remove newline at end of file.