| Message ID | 20240620-ls_qspi-v3-2-1a2afcf417e4@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: fsl-dspi: Convert to yaml format and use common SPI property | expand |
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@nxp.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@nxp.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@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.
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@nxp.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. 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@nxp.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@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.
On Fri, Jun 21, 2024 at 10:39:33AM -0400, Frank Li wrote: > On Fri, Jun 21, 2024 at 03:42:11PM +0300, Vladimir Oltean wrote: > > > 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. > > 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 Well, it depends on how much from that work you end up taking. I would recommend taking all of it :) and completing the unfinalized review feedback with your work, plus some additions like the standard CS timing parameters. Then, you could keep that sign off chain and add yours at the bottom. At least that was my approach when I tried to continue Kuldeep's work. If you take a very small portion, I guess you could at least move the link inside the portion of the commit message that remains visible in the commit log. And for a significant portion, Documentation/process/submitting-patches.rst says you could add Co-developed-by + Signed-off-by, followed by your sign off.
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@nxp.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 + + spi-num-chipselects: + $ref: /schemas/types.yaml#/definitions/uint32 + description: the number of the chipselect signals. + + big-endian: + $ref: /schemas/types.yaml#/definitions/flag + description: + If present the dspi device's registers are implemented + in big endian mode. + + bus-num: + $ref: /schemas/types.yaml#/definitions/uint32 + description: the slave chip chipselect signal number. + +required: + - compatible + - reg + - clocks + - clock-names + - interrupts + - pinctrl-0 + - pinctrl-names + - 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>; + }; + }; + diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-dspi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-dspi-peripheral-props.yaml new file mode 100644 index 0000000000000..ea9c7c52c1883 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/fsl,spi-dspi-peripheral-props.yaml @@ -0,0 +1,28 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/fsl,spi-dspi-peripheral-props.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Peripheral-specific properties for Freescale DSPI controller + +maintainers: + - Vladimir Oltean <olteanv@gmail.com> + +description: + See spi-peripheral-props.yaml for more info. + +properties: + fsl,spi-cs-sck-delay: + description: + Delay in nanoseconds between activating chip select and the start of + clock signal, at the start of a transfer. + $ref: /schemas/types.yaml#/definitions/uint32 + + fsl,spi-sck-cs-delay: + description: + Delay in nanoseconds between stopping the clock signal and + deactivating chip select, at the end of a transfer. + $ref: /schemas/types.yaml#/definitions/uint32 + +additionalProperties: true diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt deleted file mode 100644 index 30a79da9c039d..0000000000000 --- a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt +++ /dev/null @@ -1,65 +0,0 @@ -ARM Freescale DSPI controller - -Required properties: -- compatible : must be one of: - "fsl,vf610-dspi", - "fsl,ls1021a-v1.0-dspi", - "fsl,ls1012a-dspi" (optionally followed by "fsl,ls1021a-v1.0-dspi"), - "fsl,ls1028a-dspi", - "fsl,ls1043a-dspi" (optionally followed by "fsl,ls1021a-v1.0-dspi"), - "fsl,ls1046a-dspi" (optionally followed by "fsl,ls1021a-v1.0-dspi"), - "fsl,ls1088a-dspi" (optionally followed by "fsl,ls1021a-v1.0-dspi"), - "fsl,ls2080a-dspi" (optionally followed by "fsl,ls2085a-dspi"), - "fsl,ls2085a-dspi", - "fsl,lx2160a-dspi", -- reg : Offset and length of the register set for the device -- interrupts : Should contain SPI controller interrupt -- clocks: from common clock binding: handle to dspi clock. -- clock-names: from common clock binding: Shall be "dspi". -- pinctrl-0: pin control group to be used for this controller. -- pinctrl-names: must contain a "default" entry. -- spi-num-chipselects : the number of the chipselect signals. - -Optional property: -- big-endian: If present the dspi device's registers are implemented - in big endian mode. -- bus-num : the slave chip chipselect signal number. - -Optional SPI slave node properties: -- fsl,spi-cs-sck-delay: a delay in nanoseconds between activating chip - select and the start of clock signal, at the start of a transfer. -- fsl,spi-sck-cs-delay: a delay in nanoseconds between stopping the clock - signal and deactivating chip select, at the end of a transfer. - -Example: - -dspi0@4002c000 { - #address-cells = <1>; - #size-cells = <0>; - compatible = "fsl,vf610-dspi"; - reg = <0x4002c000 0x1000>; - interrupts = <0 67 0x04>; - 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; - - sflash: at26df081a@0 { - #address-cells = <1>; - #size-cells = <1>; - compatible = "atmel,at26df081a"; - spi-max-frequency = <16000000>; - spi-cpol; - spi-cpha; - reg = <0>; - linux,modalias = "m25p80"; - modal = "at26df081a"; - fsl,spi-cs-sck-delay = <100>; - fsl,spi-sck-cs-delay = <50>; - }; -}; - - diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml index 15938f81fdce2..fcc39a04a8b7a 100644 --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml @@ -122,6 +122,7 @@ properties: allOf: - $ref: arm,pl022-peripheral-props.yaml# - $ref: cdns,qspi-nor-peripheral-props.yaml# + - $ref: fsl,spi-dspi-peripheral-props.yaml# - $ref: samsung,spi-peripheral-props.yaml# - $ref: nvidia,tegra210-quad-peripheral-props.yaml#
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@nxp.com> --- Use part of Vladimir Oltean's work at https://lore.kernel.org/linux-spi/20221111224651.577729-1-vladimir.oltean@nxp.com/ --- .../devicetree/bindings/spi/fsl,dspi.yaml | 115 +++++++++++++++++++++ .../spi/fsl,spi-dspi-peripheral-props.yaml | 28 +++++ .../devicetree/bindings/spi/spi-fsl-dspi.txt | 65 ------------ .../bindings/spi/spi-peripheral-props.yaml | 1 + 4 files changed, 144 insertions(+), 65 deletions(-)