[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240621124211.pueymngpq5luokvj@skbuf>
Date: Fri, 21 Jun 2024 15:42:11 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Frank Li <Frank.Li@....com>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
imx@...ts.linux.dev
Subject: Re: [PATCH v3 2/3] spi: dt-bindings: fsl-dspi: Convert to yaml format
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@...2c000" to "spi@...2c000" 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@....com>
>
> ---
> Use part of Vladimir Oltean's work at
> https://lore.kernel.org/linux-spi/20221111224651.577729-1-vladimir.oltean@nxp.com/
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@....com>
> +
> +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@...2c000 {
> + 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.
Powered by blists - more mailing lists