lkml.org 
[lkml]   [2022]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH v2] dt-bindings: spi: convert Freescale DSPI to dt-schema
Date
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 <olteanv@gmail.com>
> > +
> > +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 <olteanv@gmail.com>
> > L: linux-spi@vger.kernel.org
> > 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...
\
 
 \ /
  Last update: 2022-11-15 15:00    [W:0.063 / U:0.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site