On Fri, Jun 21, 2024 at 03:42:11PM +0300, Vladimir Oltean wrote: > 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. I am not sure what should be added in comments? Ref part of Vladimir Oltean's work at https:// ... (I am not sure if allow http link). Or coworked-with Vladimir Oltean ... Or use seperated patch for your part. What do you like? Frank > > > --- > > .../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.