From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ramuthevar, Vadivel MuruganX" Subject: Re: [PATCH v10 1/2] dt-bindings: spi: Add schema for Cadence QSPI Controller driver Date: Tue, 25 Feb 2020 15:38:12 +0800 Message-ID: References: <20200219022852.28065-1-vadivel.muruganx.ramuthevar@linux.intel.com> <20200219022852.28065-2-vadivel.muruganx.ramuthevar@linux.intel.com> <64b7ab12-0c11-df25-95e7-ee62227ec7ec@linux.intel.com> <85178128-4906-8b1a-e3f1-ab7a36ff8c23@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , linux-spi , Mark Brown , simon.k.r.goldschmidt-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Dinh Nguyen , tien.fong.chee-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, =?UTF-8?Q?Marek_Va=c5=a1ut?= , cheol.yong.kim-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, qi-ming.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org To: Vignesh Raghavendra , Rob Herring Return-path: In-Reply-To: <85178128-4906-8b1a-e3f1-ab7a36ff8c23-l0cyMroinI0@public.gmane.org> Content-Language: en-US Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Hi, On 25/2/2020 2:41 PM, Vignesh Raghavendra wrote: > > 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 >>> wrote: >>>> From: Ramuthevar Vadivel Murugan >>>> >>> 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 >>>> >>>> --- >>>>   .../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 >>>> >>>> + >>>> +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 correct me if I am wrong, the width of this field is 4bits -> 128 bytes (based on QUAD mode) . >>>> + >>>> +  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. Thank you vignesh for  the detailed explanation. Regards Vadivel > 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 >>>>