diff mbox series

[v10,1/2] dt-bindings: spi: Add schema for Cadence QSPI Controller driver

Message ID 20200219022852.28065-2-vadivel.muruganx.ramuthevar@linux.intel.com (mailing list archive)
State Rejected
Headers show
Series spi: cadence-quadpsi: Add support for the Cadence QSPI controller | expand

Commit Message

Ramuthevar,Vadivel MuruganX Feb. 19, 2020, 2:28 a.m. UTC
From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

Add dt-bindings documentation for Cadence-QSPI controller to support
spi based flash memories.

Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
---
 .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 147 +++++++++++++++++++++
 1 file changed, 147 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml

Comments

Rob Herring Feb. 24, 2020, 3:54 p.m. UTC | #1
On Tue, Feb 18, 2020 at 8:29 PM Ramuthevar,Vadivel MuruganX
<vadivel.muruganx.ramuthevar@linux.intel.com> wrote:
>
> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

Cc the DT list if you want this reviewed.

>
> Add dt-bindings documentation for Cadence-QSPI controller to support
> spi based flash memories.
>
> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> ---
>  .../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@linux.intel.com>
> +
> +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.

> +
> +  cdns,fifo-width:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Bus width of the data FIFO in bytes.

Add some constraints.

> +
> +  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?

> +
> +  cdns,tshsl-ns:
> +    $ref: /schemas/types.yaml#/definitions/uint32

You can drop this, anything with a standard unit suffix already has a type.

> +    description: |
> +      Delay in nanoseconds for the length that the master mode chip select
> +      outputs are de-asserted between transactions.

Constraints...?

> +
> +  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
>
Ramuthevar,Vadivel MuruganX Feb. 25, 2020, 6:24 a.m. UTC | #2
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@linux.intel.com> wrote:
>> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> 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@linux.intel.com>
>> ---
>>   .../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@linux.intel.com>
>> +
>> +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.
>> +
>> +  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.

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
>>
Vignesh Raghavendra Feb. 25, 2020, 6:41 a.m. UTC | #3
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@linux.intel.com> wrote:
>>> From: Ramuthevar Vadivel Murugan
>>> <vadivel.muruganx.ramuthevar@linux.intel.com>
>> 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@linux.intel.com>
>>> ---
>>>   .../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@linux.intel.com>
>>> +
>>> +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
>>>
Ramuthevar,Vadivel MuruganX Feb. 25, 2020, 7:38 a.m. UTC | #4
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
>>> <vadivel.muruganx.ramuthevar@linux.intel.com> wrote:
>>>> From: Ramuthevar Vadivel Murugan
>>>> <vadivel.muruganx.ramuthevar@linux.intel.com>
>>> 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@linux.intel.com>
>>>> ---
>>>>    .../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@linux.intel.com>
>>>> +
>>>> +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
>>>>
Vignesh Raghavendra Feb. 25, 2020, 11 a.m. UTC | #5
On 25/02/20 1:08 pm, Ramuthevar, Vadivel MuruganX wrote:
>>>>> +
>>>>> +  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) .

This has nothing to do with quad-mode. Its about how much SRAM amount of
SRAM is present to buffer INDAC mode data. For TI platforms this is 256
bytes.
See CQSPI_REG_SRAMPARTITION definition in your datasheet.
Ramuthevar,Vadivel MuruganX Feb. 26, 2020, 1:32 a.m. UTC | #6
Hi,

On 25/2/2020 7:00 PM, Vignesh Raghavendra wrote:
>
> On 25/02/20 1:08 pm, Ramuthevar, Vadivel MuruganX wrote:
>>>>>> +
>>>>>> +  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) .
> This has nothing to do with quad-mode. Its about how much SRAM amount of
> SRAM is present to buffer INDAC mode data. For TI platforms this is 256
> bytes.
> See CQSPI_REG_SRAMPARTITION definition in your datasheet.
Agreed, Thanks!
Yes , I have gone through it , Intel and Altera SoC's SRAM(act as 
FIFO)size is 128 bytes and TI has 256 .
BTW old legacy DT binding mentioned size is 128, as per your earlier 
suggestion you have mention that
keep the contents from old dt bindings as it is, so shall I keep 128/256?

Regards
Vadivel
Vignesh Raghavendra Feb. 27, 2020, 5:23 a.m. UTC | #7
On 26/02/20 7:02 am, Ramuthevar, Vadivel MuruganX wrote:
> Hi,
> 
> On 25/2/2020 7:00 PM, Vignesh Raghavendra wrote:
>>
>> On 25/02/20 1:08 pm, Ramuthevar, Vadivel MuruganX wrote:
>>>>>>> +
>>>>>>> +  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) .
>> This has nothing to do with quad-mode. Its about how much SRAM amount of
>> SRAM is present to buffer INDAC mode data. For TI platforms this is 256
>> bytes.
>> See CQSPI_REG_SRAMPARTITION definition in your datasheet.
> Agreed, Thanks!
> Yes , I have gone through it , Intel and Altera SoC's SRAM(act as
> FIFO)size is 128 bytes and TI has 256 .
> BTW old legacy DT binding mentioned size is 128, as per your earlier
> suggestion you have mention that
> keep the contents from old dt bindings as it is, so shall I keep 128/256?

Old bindings does not impose a restriction that this needs to be 128
bytes always (Its just the example that shows this property to be set to
128)

What Rob is asking for is to add range of values that is valid for this
field and not single value. So, both 128 and 256 bytes should be allowed
as valid values for this property.
Ramuthevar,Vadivel MuruganX Feb. 27, 2020, 5:59 a.m. UTC | #8
Hi,

On 27/2/2020 1:23 PM, Vignesh Raghavendra wrote:
>
> On 26/02/20 7:02 am, Ramuthevar, Vadivel MuruganX wrote:
>> Hi,
>>
>> On 25/2/2020 7:00 PM, Vignesh Raghavendra wrote:
>>> On 25/02/20 1:08 pm, Ramuthevar, Vadivel MuruganX wrote:
>>>>>>>> +
>>>>>>>> +  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) .
>>> This has nothing to do with quad-mode. Its about how much SRAM amount of
>>> SRAM is present to buffer INDAC mode data. For TI platforms this is 256
>>> bytes.
>>> See CQSPI_REG_SRAMPARTITION definition in your datasheet.
>> Agreed, Thanks!
>> Yes , I have gone through it , Intel and Altera SoC's SRAM(act as
>> FIFO)size is 128 bytes and TI has 256 .
>> BTW old legacy DT binding mentioned size is 128, as per your earlier
>> suggestion you have mention that
>> keep the contents from old dt bindings as it is, so shall I keep 128/256?
> Old bindings does not impose a restriction that this needs to be 128
> bytes always (Its just the example that shows this property to be set to
> 128)
>
> What Rob is asking for is to add range of values that is valid for this
> field and not single value. So, both 128 and 256 bytes should be allowed
> as valid values for this property.

Thank you Vignesh, will add both.

Regards
Vadivel
>
diff mbox series

Patch

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@linux.intel.com>
+
+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.
+
+  cdns,fifo-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Bus width of the data FIFO in bytes.
+
+  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.
+
+  cdns,tshsl-ns:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Delay in nanoseconds for the length that the master mode chip select
+      outputs are de-asserted between transactions.
+
+  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>;
+          };
+    };
+