| 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
| ||
|
Message-ID: <ZnWQpa+rSXUhxCn7@lizhi-Precision-Tower-5810>
Date: Fri, 21 Jun 2024 10:39:33 -0400
From: Frank Li <Frank.li@....com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
imx@...ts.linux.dev
Subject: Re: [PATCH v3 2/3] spi: dt-bindings: fsl-dspi: Convert to yaml format
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@...2c000" to "spi@...2c000" 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@....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@....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@...2c000 {
> > + 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.
Powered by blists - more mailing lists