From: Kuldeep Singh <[email protected]>
Convert the Freescale DSPI binding to DT schema format.
Signed-off-by: Kuldeep Singh <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1 (from more than 1 year ago) at:
https://patchwork.kernel.org/project/spi-devel-general/patch/[email protected]/
.../fsl,spi-fsl-dspi-peripheral-props.yaml | 28 +++++
.../bindings/spi/fsl,spi-fsl-dspi.yaml | 118 ++++++++++++++++++
.../devicetree/bindings/spi/spi-fsl-dspi.txt | 65 ----------
.../bindings/spi/spi-peripheral-props.yaml | 1 +
MAINTAINERS | 3 +-
5 files changed, 149 insertions(+), 66 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi-peripheral-props.yaml
create mode 100644 Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
delete mode 100644 Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi-peripheral-props.yaml
new file mode 100644
index 000000000000..d15f77c040d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-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-fsl-dspi-peripheral-props.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Peripheral-specific properties for Freescale DSPI controller
+
+maintainers:
+ - Vladimir Oltean <[email protected]>
+
+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/fsl,spi-fsl-dspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
new file mode 100644
index 000000000000..8a790c0ed95f
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
@@ -0,0 +1,118 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/fsl,spi-fsl-dspi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale DSPI Controller
+
+maintainers:
+ - Vladimir Oltean <[email protected]>
+
+allOf:
+ - $ref: "spi-controller.yaml#"
+
+properties:
+ compatible:
+ description:
+ Some integrations can have a single compatible string containing their
+ SoC name (LS1012A, LS1021A, ...). Others require their SoC compatible
+ string, plus a fallback compatible string (either on LS1021A or on
+ LS2085A).
+ oneOf:
+ - enum:
+ - fsl,ls1012a-dspi
+ - fsl,ls1021a-v1.0-dspi
+ - fsl,ls1028a-dspi
+ - fsl,ls2085a-dspi
+ - fsl,lx2160a-dspi
+ - fsl,vf610-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:
+ - enum:
+ - fsl,ls2080a-dspi
+ - fsl,lx2160a-dspi
+ - const: fsl,ls2085a-dspi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: dspi
+
+ dmas:
+ maxItems: 2
+
+ dma-names:
+ items:
+ - const: tx
+ - const: rx
+
+ spi-num-chipselects:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Number of available native Chip Select signals
+
+ bus-num:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: SoC-specific identifier for the SPI controller
+
+ little-endian: true
+ big-endian: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - spi-num-chipselects
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/fsl,qoriq-clockgen.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ spi@2100000 {
+ compatible = "fsl,ls1028a-dspi", "fsl,ls1021a-v1.0-dspi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x0 0x2100000 0x0 0x10000>;
+ interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+ clock-names = "dspi";
+ clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL QORIQ_CLK_PLL_DIV(2)>;
+ dmas = <&edma0 0 62>, <&edma0 0 60>;
+ dma-names = "tx", "rx";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_dspi0_1>;
+ spi-num-chipselects = <4>;
+ little-endian;
+
+ flash@0 {
+ compatible = "jedec,spi-nor";
+ spi-max-frequency = <10000000>;
+ fsl,spi-cs-sck-delay = <100>;
+ fsl,spi-sck-cs-delay = <100>;
+ reg = <0>;
+ };
+ };
+ };
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 30a79da9c039..000000000000
--- 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 dca677f9e1b9..a475e757f8da 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -101,6 +101,7 @@ properties:
# The controller specific properties go here.
allOf:
- $ref: cdns,qspi-nor-peripheral-props.yaml#
+ - $ref: fsl,spi-fsl-dspi-peripheral-props.yaml#
- $ref: samsung,spi-peripheral-props.yaml#
- $ref: nvidia,tegra210-quad-peripheral-props.yaml#
diff --git a/MAINTAINERS b/MAINTAINERS
index c242098a34f9..c75ae49c85b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8160,7 +8160,8 @@ FREESCALE DSPI DRIVER
M: Vladimir Oltean <[email protected]>
L: [email protected]
S: Maintained
-F: Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
+F: Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
+F: Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi-peripheral-props.yaml
F: drivers/spi/spi-fsl-dspi.c
F: include/linux/spi/spi-fsl-dspi.h
--
2.34.1
On 11/11/2022 23:46, Vladimir Oltean wrote:
> From: Kuldeep Singh <[email protected]>
>
> Convert the Freescale DSPI binding to DT schema format.
>
> Signed-off-by: Kuldeep Singh <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> v1 (from more than 1 year ago) at:
> https://patchwork.kernel.org/project/spi-devel-general/patch/[email protected]/
>
> .../fsl,spi-fsl-dspi-peripheral-props.yaml | 28 +++++
> .../bindings/spi/fsl,spi-fsl-dspi.yaml | 118 ++++++++++++++++++
> .../devicetree/bindings/spi/spi-fsl-dspi.txt | 65 ----------
> .../bindings/spi/spi-peripheral-props.yaml | 1 +
> MAINTAINERS | 3 +-
> 5 files changed, 149 insertions(+), 66 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi-peripheral-props.yaml
> create mode 100644 Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
> delete mode 100644 Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
>
> diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi-peripheral-props.yaml
> new file mode 100644
> index 000000000000..d15f77c040d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-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-fsl-dspi-peripheral-props.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Peripheral-specific properties for Freescale DSPI controller
> +
> +maintainers:
> + - Vladimir Oltean <[email protected]>
> +
> +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/fsl,spi-fsl-dspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
> new file mode 100644
> index 000000000000..8a790c0ed95f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
> @@ -0,0 +1,118 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/fsl,spi-fsl-dspi.yaml#
Why second "fsl" in file name? It does not patch compatibles and
duplicates the vendor. We do not have compatibles "nxp,imx6-nxp".
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale DSPI Controller
> +
> +maintainers:
> + - Vladimir Oltean <[email protected]>
> +
> +allOf:
> + - $ref: "spi-controller.yaml#"
Drop quotes.
> +
> +properties:
> + compatible:
> + description:
> + Some integrations can have a single compatible string containing their
> + SoC name (LS1012A, LS1021A, ...). Others require their SoC compatible
> + string, plus a fallback compatible string (either on LS1021A or on
> + LS2085A).
Why? The fsl,ls1012a-dspi device is either compatible with
fsl,ls1021a-v1.0-dspi or not. It cannot be both - compatible and not
compatible.
> + oneOf:
> + - enum:
> + - fsl,ls1012a-dspi
> + - fsl,ls1021a-v1.0-dspi
> + - fsl,ls1028a-dspi
> + - fsl,ls2085a-dspi
> + - fsl,lx2160a-dspi
> + - fsl,vf610-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:
> + - enum:
> + - fsl,ls2080a-dspi
> + - fsl,lx2160a-dspi
> + - const: fsl,ls2085a-dspi
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: dspi
> +
> + dmas:
> + maxItems: 2
> +
> + dma-names:
> + items:
> + - const: tx
> + - const: rx
> +
> + spi-num-chipselects:
Would be nice to deprecated it in separate patches. There is num-cs
property.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Number of available native Chip Select signals
> +
> + bus-num:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: SoC-specific identifier for the SPI controller
> +
> + little-endian: true
> + big-endian: true
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - spi-num-chipselects
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/fsl,qoriq-clockgen.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + spi@2100000 {
> + compatible = "fsl,ls1028a-dspi", "fsl,ls1021a-v1.0-dspi";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x0 0x2100000 0x0 0x10000>;
reg by convention is a second property.
> + interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "dspi";
> + clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL QORIQ_CLK_PLL_DIV(2)>;
> + dmas = <&edma0 0 62>, <&edma0 0 60>;
> + dma-names = "tx", "rx";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_dspi0_1>;
> + spi-num-chipselects = <4>;
> + little-endian;
> +
> + flash@0 {
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <10000000>;
> + fsl,spi-cs-sck-delay = <100>;
> + fsl,spi-sck-cs-delay = <100>;
> + reg = <0>;
Ditto.
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
> deleted file mode 100644
(...)
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index dca677f9e1b9..a475e757f8da 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -101,6 +101,7 @@ properties:
> # The controller specific properties go here.
> allOf:
> - $ref: cdns,qspi-nor-peripheral-props.yaml#
> + - $ref: fsl,spi-fsl-dspi-peripheral-props.yaml#
> - $ref: samsung,spi-peripheral-props.yaml#
> - $ref: nvidia,tegra210-quad-peripheral-props.yaml#
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c242098a34f9..c75ae49c85b5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8160,7 +8160,8 @@ FREESCALE DSPI DRIVER
> M: Vladimir Oltean <[email protected]>
> L: [email protected]
> S: Maintained
> -F: Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
> +F: Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
Instead: Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi*
Best regards,
Krzysztof
On Tue, Nov 15, 2022 at 02:46:21PM +0100, Krzysztof Kozlowski wrote:
> > +$id: http://devicetree.org/schemas/spi/fsl,spi-fsl-dspi.yaml
>
> Why second "fsl" in file name? It does not patch compatibles and
> duplicates the vendor. We do not have compatibles "nxp,imx6-nxp".
Ok, which file name would be good then? There are 9 different (all SoC
specific) compatible strings, surely the convention of naming the file
after a compatible string has some limitations...
> > +$schema: http://devicetree.org/meta-schemas/core.yaml
> > +
> > +title: Freescale DSPI Controller
> > +
> > +maintainers:
> > + - Vladimir Oltean <[email protected]>
> > +
> > +allOf:
> > + - $ref: "spi-controller.yaml#"
>
> Drop quotes.
>
> > +
> > +properties:
> > + compatible:
> > + description:
> > + Some integrations can have a single compatible string containing their
> > + SoC name (LS1012A, LS1021A, ...). Others require their SoC compatible
> > + string, plus a fallback compatible string (either on LS1021A or on
> > + LS2085A).
>
> Why? The fsl,ls1012a-dspi device is either compatible with
> fsl,ls1021a-v1.0-dspi or not. It cannot be both - compatible and not
> compatible.
LS1012A is compatible with LS1021A to the extent that it works when
treated like a LS1021A. LS1012A has a FIFO size of 8 SPI words, LS1021A
of just 4. Treating it like LS1021A means roughly half the performance,
but it still works.
I didn't invent any of this. When I took over the driver, there were
device trees like this all over the place:
dspi: spi@2100000 {
compatible = "fsl,ls1012a-dspi", "fsl,ls1021a-v1.0-dspi";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x0 0x2100000 0x0 0x10000>;
interrupts = <0 64 IRQ_TYPE_LEVEL_HIGH>;
clock-names = "dspi";
clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
QORIQ_CLK_PLL_DIV(1)>;
spi-num-chipselects = <5>;
big-endian;
status = "disabled";
};
but the Linux driver pre-~5.7 always relied on the fallback compatible
string (LS1021A in this case). I'm working with what's out in the field,
haven't changed a thing there.
> > + oneOf:
> > + - enum:
> > + - fsl,ls1012a-dspi
> > + - fsl,ls1021a-v1.0-dspi
> > + - fsl,ls1028a-dspi
> > + - fsl,ls2085a-dspi
> > + - fsl,lx2160a-dspi
> > + - fsl,vf610-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:
> > + - enum:
> > + - fsl,ls2080a-dspi
> > + - fsl,lx2160a-dspi
> > + - const: fsl,ls2085a-dspi
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + items:
> > + - const: dspi
> > +
> > + dmas:
> > + maxItems: 2
> > +
> > + dma-names:
> > + items:
> > + - const: tx
> > + - const: rx
> > +
> > + spi-num-chipselects:
>
> Would be nice to deprecated it in separate patches. There is num-cs
> property.
Will add this on my TODO list. Right now I'm just converting what exists.
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/fsl,qoriq-clockgen.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + spi@2100000 {
> > + compatible = "fsl,ls1028a-dspi", "fsl,ls1021a-v1.0-dspi";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0x0 0x2100000 0x0 0x10000>;
>
> reg by convention is a second property.
ok.
> > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > index dca677f9e1b9..a475e757f8da 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > @@ -101,6 +101,7 @@ properties:
> > # The controller specific properties go here.
> > allOf:
> > - $ref: cdns,qspi-nor-peripheral-props.yaml#
> > + - $ref: fsl,spi-fsl-dspi-peripheral-props.yaml#
> > - $ref: samsung,spi-peripheral-props.yaml#
> > - $ref: nvidia,tegra210-quad-peripheral-props.yaml#
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c242098a34f9..c75ae49c85b5 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -8160,7 +8160,8 @@ FREESCALE DSPI DRIVER
> > M: Vladimir Oltean <[email protected]>
> > L: [email protected]
> > S: Maintained
> > -F: Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
> > +F: Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi.yaml
>
> Instead: Documentation/devicetree/bindings/spi/fsl,spi-fsl-dspi*
ok...
On 15/11/2022 14:59, Vladimir Oltean wrote:
> On Tue, Nov 15, 2022 at 02:46:21PM +0100, Krzysztof Kozlowski wrote:
>>> +$id: http://devicetree.org/schemas/spi/fsl,spi-fsl-dspi.yaml
>>
>> Why second "fsl" in file name? It does not patch compatibles and
>> duplicates the vendor. We do not have compatibles "nxp,imx6-nxp".
>
> Ok, which file name would be good then? There are 9 different (all SoC
> specific) compatible strings, surely the convention of naming the file
> after a compatible string has some limitations...
If all DSPI blocks fit here, then maybe: fsl,dspi.yaml
fsl,spi-dspi.yaml is also a bit redundant.
>
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml
>>> +
>>> +title: Freescale DSPI Controller
>>> +
>>> +maintainers:
>>> + - Vladimir Oltean <[email protected]>
>>> +
>>> +allOf:
>>> + - $ref: "spi-controller.yaml#"
>>
>> Drop quotes.
>>
>>> +
>>> +properties:
>>> + compatible:
>>> + description:
>>> + Some integrations can have a single compatible string containing their
>>> + SoC name (LS1012A, LS1021A, ...). Others require their SoC compatible
>>> + string, plus a fallback compatible string (either on LS1021A or on
>>> + LS2085A).
>>
>> Why? The fsl,ls1012a-dspi device is either compatible with
>> fsl,ls1021a-v1.0-dspi or not. It cannot be both - compatible and not
>> compatible.
>
> LS1012A is compatible with LS1021A to the extent that it works when
> treated like a LS1021A. LS1012A has a FIFO size of 8 SPI words, LS1021A
> of just 4. Treating it like LS1021A means roughly half the performance,
> but it still works.
>
> I didn't invent any of this. When I took over the driver, there were
> device trees like this all over the place:
>
> dspi: spi@2100000 {
> compatible = "fsl,ls1012a-dspi", "fsl,ls1021a-v1.0-dspi";
Which looks ok...
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0x0 0x2100000 0x0 0x10000>;
> interrupts = <0 64 IRQ_TYPE_LEVEL_HIGH>;
> clock-names = "dspi";
> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
> QORIQ_CLK_PLL_DIV(1)>;
> spi-num-chipselects = <5>;
> big-endian;
> status = "disabled";
> };
>
> but the Linux driver pre-~5.7 always relied on the fallback compatible
> string (LS1021A in this case). I'm working with what's out in the field,
> haven't changed a thing there.
The driver matters less (except ABI), but anyway it confirms the case -
fallback is expected always. Why the fallback should be removed if the
devices are compatible (including halved performance)?
>
>>> + oneOf:
>>> + - enum:
>>> + - fsl,ls1012a-dspi
>>> + - fsl,ls1021a-v1.0-dspi
>>> + - fsl,ls1028a-dspi
>>> + - fsl,ls2085a-dspi
>>> + - fsl,lx2160a-dspi
>>> + - fsl,vf610-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:
>>> + - enum:
>>> + - fsl,ls2080a-dspi
>>> + - fsl,lx2160a-dspi
>>> + - const: fsl,ls2085a-dspi
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + clocks:
>>> + maxItems: 1
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: dspi
>>> +
>>> + dmas:
>>> + maxItems: 2
>>> +
>>> + dma-names:
>>> + items:
>>> + - const: tx
>>> + - const: rx
>>> +
>>> + spi-num-chipselects:
>>
>> Would be nice to deprecated it in separate patches. There is num-cs
>> property.
>
> Will add this on my TODO list. Right now I'm just converting what exists.
Sure.
Best regards,
Krzysztof
On Tue, Nov 15, 2022 at 03:08:37PM +0100, Krzysztof Kozlowski wrote:
> On 15/11/2022 14:59, Vladimir Oltean wrote:
> > On Tue, Nov 15, 2022 at 02:46:21PM +0100, Krzysztof Kozlowski wrote:
> >>> +$id: http://devicetree.org/schemas/spi/fsl,spi-fsl-dspi.yaml
> >>
> >> Why second "fsl" in file name? It does not patch compatibles and
> >> duplicates the vendor. We do not have compatibles "nxp,imx6-nxp".
> >
> > Ok, which file name would be good then? There are 9 different (all SoC
> > specific) compatible strings, surely the convention of naming the file
> > after a compatible string has some limitations...
>
> If all DSPI blocks fit here, then maybe: fsl,dspi.yaml
>
> fsl,spi-dspi.yaml is also a bit redundant.
Ok, fsl,dspi.yaml and fsl,dspi-peripheral-props.yaml, and MAINTAINERS
entry for fsl,dspi*.yaml?
> >>> +properties:
> >>> + compatible:
> >>> + description:
> >>> + Some integrations can have a single compatible string containing their
> >>> + SoC name (LS1012A, LS1021A, ...). Others require their SoC compatible
> >>> + string, plus a fallback compatible string (either on LS1021A or on
> >>> + LS2085A).
> >>
> >> Why? The fsl,ls1012a-dspi device is either compatible with
> >> fsl,ls1021a-v1.0-dspi or not. It cannot be both - compatible and not
> >> compatible.
> >
> > LS1012A is compatible with LS1021A to the extent that it works when
> > treated like a LS1021A. LS1012A has a FIFO size of 8 SPI words, LS1021A
> > of just 4. Treating it like LS1021A means roughly half the performance,
> > but it still works.
> >
> > I didn't invent any of this. When I took over the driver, there were
> > device trees like this all over the place:
> >
> > dspi: spi@2100000 {
> > compatible = "fsl,ls1012a-dspi", "fsl,ls1021a-v1.0-dspi";
>
> Which looks ok...
>
> > #address-cells = <1>;
> > #size-cells = <0>;
> > reg = <0x0 0x2100000 0x0 0x10000>;
> > interrupts = <0 64 IRQ_TYPE_LEVEL_HIGH>;
> > clock-names = "dspi";
> > clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
> > QORIQ_CLK_PLL_DIV(1)>;
> > spi-num-chipselects = <5>;
> > big-endian;
> > status = "disabled";
> > };
> >
> > but the Linux driver pre-~5.7 always relied on the fallback compatible
> > string (LS1021A in this case). I'm working with what's out in the field,
> > haven't changed a thing there.
>
> The driver matters less (except ABI), but anyway it confirms the case -
> fallback is expected always. Why the fallback should be removed if the
> devices are compatible (including halved performance)?
I don't think I said the fallback should be removed? I think you're
talking about a typo/braino I made, which puts the LS1012A both in the
bucket of SoCs with a single compatible strings required, as well as in
that with fallback required. Obviously both can't be true... I didn't
mean LS1012A but VF610.
On 15/11/2022 15:19, Vladimir Oltean wrote:
> On Tue, Nov 15, 2022 at 03:08:37PM +0100, Krzysztof Kozlowski wrote:
>> On 15/11/2022 14:59, Vladimir Oltean wrote:
>>> On Tue, Nov 15, 2022 at 02:46:21PM +0100, Krzysztof Kozlowski wrote:
>>>>> +$id: http://devicetree.org/schemas/spi/fsl,spi-fsl-dspi.yaml
>>>>
>>>> Why second "fsl" in file name? It does not patch compatibles and
>>>> duplicates the vendor. We do not have compatibles "nxp,imx6-nxp".
>>>
>>> Ok, which file name would be good then? There are 9 different (all SoC
>>> specific) compatible strings, surely the convention of naming the file
>>> after a compatible string has some limitations...
>>
>> If all DSPI blocks fit here, then maybe: fsl,dspi.yaml
>>
>> fsl,spi-dspi.yaml is also a bit redundant.
>
> Ok, fsl,dspi.yaml and fsl,dspi-peripheral-props.yaml, and MAINTAINERS
> entry for fsl,dspi*.yaml?
Yes.
>
>>>>> +properties:
>>>>> + compatible:
>>>>> + description:
>>>>> + Some integrations can have a single compatible string containing their
>>>>> + SoC name (LS1012A, LS1021A, ...). Others require their SoC compatible
>>>>> + string, plus a fallback compatible string (either on LS1021A or on
>>>>> + LS2085A).
>>>>
>>>> Why? The fsl,ls1012a-dspi device is either compatible with
>>>> fsl,ls1021a-v1.0-dspi or not. It cannot be both - compatible and not
>>>> compatible.
>>>
>>> LS1012A is compatible with LS1021A to the extent that it works when
>>> treated like a LS1021A. LS1012A has a FIFO size of 8 SPI words, LS1021A
>>> of just 4. Treating it like LS1021A means roughly half the performance,
>>> but it still works.
>>>
>>> I didn't invent any of this. When I took over the driver, there were
>>> device trees like this all over the place:
>>>
>>> dspi: spi@2100000 {
>>> compatible = "fsl,ls1012a-dspi", "fsl,ls1021a-v1.0-dspi";
>>
>> Which looks ok...
>>
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>> reg = <0x0 0x2100000 0x0 0x10000>;
>>> interrupts = <0 64 IRQ_TYPE_LEVEL_HIGH>;
>>> clock-names = "dspi";
>>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>>> QORIQ_CLK_PLL_DIV(1)>;
>>> spi-num-chipselects = <5>;
>>> big-endian;
>>> status = "disabled";
>>> };
>>>
>>> but the Linux driver pre-~5.7 always relied on the fallback compatible
>>> string (LS1021A in this case). I'm working with what's out in the field,
>>> haven't changed a thing there.
>>
>> The driver matters less (except ABI), but anyway it confirms the case -
>> fallback is expected always. Why the fallback should be removed if the
>> devices are compatible (including halved performance)?
>
> I don't think I said the fallback should be removed? I think you're
> talking about a typo/braino I made, which puts the LS1012A both in the
> bucket of SoCs with a single compatible strings required, as well as in
> that with fallback required. Obviously both can't be true... I didn't
> mean LS1012A but VF610.
To be clear: ls1012a, ls1028a and lx2160a should be either followed by
compatible or not. Cannot be both.
Best regards,
Krzysztof
On Tue, Nov 15, 2022 at 03:26:11PM +0100, Krzysztof Kozlowski wrote:
> To be clear: ls1012a, ls1028a and lx2160a should be either followed by
> compatible or not. Cannot be both.
LS1012A should be followed by fallback compatible for practical reasons
(Linux kernel worked that way up to 5.7, time during which LS1012A was
supported).
LS1028A and LX2160A device trees were both introduced after the Linux
kernel started looking at specific device trees, so I believe that Linux
never relied on the fallback compatible string and it could be removed.
The fallback is present in device trees in circulation, even if the .txt
schema says it isn't required. I don't know what the BSDs do about this,
so I'd be tempted to leave them in the camp with required fallbacks,
just because it's not worth risking a regression.