On 25/02/20 11:54 am, Ramuthevar, Vadivel MuruganX wrote: > Hi Rob, > > Thank you for the review comments... > > On 24/2/2020 11:54 PM, Rob Herring wrote: >> On Tue, Feb 18, 2020 at 8:29 PM Ramuthevar,Vadivel MuruganX >> <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx> wrote: >>> From: Ramuthevar Vadivel Murugan >>> <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx> >> Cc the DT list if you want this reviewed. > Sorry, next patch will add DT list in CC. >>> Add dt-bindings documentation for Cadence-QSPI controller to support >>> spi based flash memories. >>> >>> Signed-off-by: Ramuthevar Vadivel Murugan >>> <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx> >>> --- >>> .../devicetree/bindings/spi/cdns,qspi-nor.yaml | 147 >>> +++++++++++++++++++++ >>> 1 file changed, 147 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml >>> b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml >>> new file mode 100644 >>> index 000000000000..1a4d6e8d0d0b >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml >>> @@ -0,0 +1,147 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: "http://devicetree.org/schemas/spi/cdns,qspi-nor.yaml#" >>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >>> + >>> +title: Cadence QSPI Flash Controller support >>> + >>> +maintainers: >>> + - Ramuthevar Vadivel Murugan >>> <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx> >>> + >>> +allOf: >>> + - $ref: "spi-controller.yaml#" >>> + >>> +description: | >>> + Binding Documentation for Cadence QSPI controller,This controller is >>> + present in the Intel LGM, Altera SoCFPGA and TI SoCs and this driver >>> + has been tested On Intel's LGM SoC. >>> + >>> + - compatible : should be one of the following: >>> + Generic default - "cdns,qspi-nor". >>> + For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor". >>> + For TI AM654 SoC - "ti,am654-ospi", "cdns,qspi-nor". >>> + For Intel LGM SoC - "intel,lgm-qspi", "cdns,qspi-nor". >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - enum: >>> + - ti,k2g-qspi >>> + - const: cdns,qspi-nor >>> + >>> + - items: >>> + - enum: >>> + - ti,am654-ospi >>> + - const: cdns,qspi-nor >>> + >>> + - items: >>> + - enum: >>> + - intel,lgm-qspi >>> + - const: cdns,qspi-nor >>> + >>> + - items: >>> + - const: cdns,qspi-nor >>> + >>> + reg: >>> + maxItems: 2 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + cdns,fifo-depth: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + Size of the data FIFO in words. >> A 4GB fifo is valid? Add some constraints. > 128 is valid, will update. Nope, the width of this field is 8bits -> 256 bytes >>> + >>> + cdns,fifo-width: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + Bus width of the data FIFO in bytes. >> Add some constraints. > 4 is valid , will fix it. >>> + >>> + cdns,trigger-address: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + 32-bit indirect AHB trigger address. >>> + >>> + cdns,rclk-en: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: | >>> + Flag to indicate that QSPI return clock is used to latch the >>> read data >>> + rather than the QSPI clock. Make sure that QSPI return clock >>> is populated >>> + on the board before using this property. >>> + >>> +# subnode's properties >>> +patternProperties: >>> + "^.*@[0-9a-fA-F]+$": >>> + type: object >>> + description: >>> + flash device uses the subnodes below defined properties. >>> + >>> + cdns,read-delay: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + Delay for read capture logic, in clock cycles. >> 4 billion clock delay is valid? > based on the ref_clk , will add it. >>> + >>> + cdns,tshsl-ns: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >> You can drop this, anything with a standard unit suffix already has a >> type. > sure , will drop it. >>> + description: | >>> + Delay in nanoseconds for the length that the master mode chip >>> select >>> + outputs are de-asserted between transactions. >> Constraints...? > > 50ns, will add it. > This really depends on the input clk: Clock Delay for Chip Select Deassert: Delay in master reference clocks for the length that the master mode chip select outputs are de-asserted between transactions The minimum delay is always SCLK period to ensure the chip select is never re-asserted within an SCLK period. I don't see a easy way of verifying constraints in yaml. Same applies for below 3 properties as well. Regards Vignesh > Regards > Vadivel >>> + >>> + cdns,tsd2d-ns: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: | >>> + Delay in nanoseconds between one chip select being de-activated >>> + and the activation of another. >>> + >>> + cdns,tchsh-ns: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: | >>> + Delay in nanoseconds between last bit of current transaction and >>> + deasserting the device chip select (qspi_n_ss_out). >>> + >>> + cdns,tslch-ns: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: | >>> + Delay in nanoseconds between setting qspi_n_ss_out low and >>> + first bit transfer. >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + - clocks >>> + - cdns,fifo-depth >>> + - cdns,fifo-width >>> + - cdns,trigger-address >>> + >>> +examples: >>> + - | >>> + qspi: spi@ff705000 { >>> + compatible = "cdns,qspi-nor"; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + reg = <0xff705000 0x1000>, >>> + <0xffa00000 0x1000>; >>> + interrupts = <0 151 4>; >>> + clocks = <&qspi_clk>; >>> + cdns,fifo-depth = <128>; >>> + cdns,fifo-width = <4>; >>> + cdns,trigger-address = <0x00000000>; >>> + >>> + flash0: n25q00@0 { >>> + compatible = "jedec,spi-nor"; >>> + reg = <0x0>; >>> + cdns,read-delay = <4>; >>> + cdns,tshsl-ns = <50>; >>> + cdns,tsd2d-ns = <50>; >>> + cdns,tchsh-ns = <4>; >>> + cdns,tslch-ns = <4>; >>> + }; >>> + }; >>> + >>> -- >>> 2.11.0 >>> -- Regards Vignesh