| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 15 Nov 2022 13:59:13 +0000
From: Vladimir Oltean <vladimir.oltean@....com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC: "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Vladimir Oltean <olteanv@...il.com>,
Mark Brown <broonie@...nel.org>,
Kuldeep Singh <singh.kuldeep87k@...il.com>,
Michael Walle <michael@...le.cc>,
Kuldeep Singh <kuldeep.singh@....com>
Subject: Re: [PATCH v2] dt-bindings: spi: convert Freescale DSPI to dt-schema
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@...il.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@...0000 {
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@...0000 {
> > + 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@...il.com>
> > L: linux-spi@...r.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...
Powered by blists - more mailing lists