diff mbox series

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

Message ID 20200520123612.11797-1-vadivel.muruganx.ramuthevar@linux.intel.com
State Changes Requested
Headers show
Series [v2,1/1] dt-bindings: spi: Add schema for Cadence QSPI Controller driver | expand

Checks

Context Check Description
robh/dt-meta-schema success
robh/checkpatch warning total: 0 errors, 3 warnings, 133 lines checked

Commit Message

Ramuthevar, Vadivel MuruganX May 20, 2020, 12:36 p.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/mtd/cadence-quadspi.txt    |  67 -----------
 .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 133 +++++++++++++++++++++
 2 files changed, 133 insertions(+), 67 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
 create mode 100644 Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml

Comments

Mark Brown May 20, 2020, 12:43 p.m. UTC | #1
On Wed, May 20, 2020 at 08:36:12PM +0800, Ramuthevar,Vadivel MuruganX wrote:
> 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/mtd/cadence-quadspi.txt    |  67 -----------
>  .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 133 +++++++++++++++++++++

The changelog says this is adding a new binding but the actual change is
mostly a conversion to YAML.  Please split the additions out into a
separate change, ideally doing that before the conversion since there is
a backlog on review of YAML conversions.
Ramuthevar, Vadivel MuruganX May 21, 2020, 2:18 a.m. UTC | #2
Hi Mark,

  Thank you for the review comments...

On 20/5/2020 8:43 pm, Mark Brown wrote:
> On Wed, May 20, 2020 at 08:36:12PM +0800, Ramuthevar,Vadivel MuruganX wrote:
>> 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/mtd/cadence-quadspi.txt    |  67 -----------
>>   .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 133 +++++++++++++++++++++
> 
> The changelog says this is adding a new binding but the actual change is
> mostly a conversion to YAML.  Please split the additions out into a
> separate change, ideally doing that before the conversion since there is
> a backlog on review of YAML conversions.

Initially was sending the only YAML file alone, then reviewers suggest 
to me do this way so I did, next by split the patches like below...

1. remove the cadence-quadspi.txt (patch1)
2. convert txt to YAML (patch2)

Regards
Vadivel
>
Mark Brown May 21, 2020, 10:56 a.m. UTC | #3
On Thu, May 21, 2020 at 10:18:26AM +0800, Ramuthevar, Vadivel MuruganX wrote:
> On 20/5/2020 8:43 pm, Mark Brown wrote:
> > On Wed, May 20, 2020 at 08:36:12PM +0800, Ramuthevar,Vadivel MuruganX wrote:

> > >   .../devicetree/bindings/mtd/cadence-quadspi.txt    |  67 -----------
> > >   .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 133 +++++++++++++++++++++

> > The changelog says this is adding a new binding but the actual change is
> > mostly a conversion to YAML.  Please split the additions out into a
> > separate change, ideally doing that before the conversion since there is
> > a backlog on review of YAML conversions.

> Initially was sending the only YAML file alone, then reviewers suggest to me
> do this way so I did, next by split the patches like below...

> 1. remove the cadence-quadspi.txt (patch1)
> 2. convert txt to YAML (patch2)

That doesn't address either of the issues.  The removal of the old
bindings and addition of the YAML ones needs to be in a single patch
doing that conversion.  What I'm suggesting should be done separately is
whatever changes to the semantics of the bindings you are (according to
your changelog) doing.
Ramuthevar, Vadivel MuruganX May 21, 2020, 12:14 p.m. UTC | #4
Hi Mark,

On 21/5/2020 6:56 pm, Mark Brown wrote:
> On Thu, May 21, 2020 at 10:18:26AM +0800, Ramuthevar, Vadivel MuruganX wrote:
>> On 20/5/2020 8:43 pm, Mark Brown wrote:
>>> On Wed, May 20, 2020 at 08:36:12PM +0800, Ramuthevar,Vadivel MuruganX wrote:
> 
>>>>    .../devicetree/bindings/mtd/cadence-quadspi.txt    |  67 -----------
>>>>    .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 133 +++++++++++++++++++++
> 
>>> The changelog says this is adding a new binding but the actual change is
>>> mostly a conversion to YAML.  Please split the additions out into a
>>> separate change, ideally doing that before the conversion since there is
>>> a backlog on review of YAML conversions.
> 
>> Initially was sending the only YAML file alone, then reviewers suggest to me
>> do this way so I did, next by split the patches like below...
> 
>> 1. remove the cadence-quadspi.txt (patch1)
>> 2. convert txt to YAML (patch2)
> 
> That doesn't address either of the issues.  The removal of the old
> bindings and addition of the YAML ones needs to be in a single patch
> doing that conversion.  What I'm suggesting should be done separately is
> whatever changes to the semantics of the bindings you are (according to
> your changelog) doing.
You mean semantics of the binding as a single patch you are suggesting 
me, right? , Thanks!

Regards
Vadivel

>
Mark Brown May 21, 2020, 12:20 p.m. UTC | #5
On Thu, May 21, 2020 at 08:14:04PM +0800, Ramuthevar, Vadivel MuruganX wrote:
> On 21/5/2020 6:56 pm, Mark Brown wrote:

> > That doesn't address either of the issues.  The removal of the old
> > bindings and addition of the YAML ones needs to be in a single patch
> > doing that conversion.  What I'm suggesting should be done separately is
> > whatever changes to the semantics of the bindings you are (according to
> > your changelog) doing.

> You mean semantics of the binding as a single patch you are suggesting me,
> right? , Thanks!

I mean that any changes to the bindings ought to be split out into
separate patches, if there's multiple changes it may make sense for
there to be multiple patches.
Ramuthevar, Vadivel MuruganX May 21, 2020, 12:34 p.m. UTC | #6
Hi Mark,

On 21/5/2020 8:20 pm, Mark Brown wrote:
> On Thu, May 21, 2020 at 08:14:04PM +0800, Ramuthevar, Vadivel MuruganX wrote:
>> On 21/5/2020 6:56 pm, Mark Brown wrote:
> 
>>> That doesn't address either of the issues.  The removal of the old
>>> bindings and addition of the YAML ones needs to be in a single patch
>>> doing that conversion.  What I'm suggesting should be done separately is
>>> whatever changes to the semantics of the bindings you are (according to
>>> your changelog) doing.
> 
>> You mean semantics of the binding as a single patch you are suggesting me,
>> right? , Thanks!
> 
> I mean that any changes to the bindings ought to be split out into
> separate patches, if there's multiple changes it may make sense for
> there to be multiple patches.
Got it, we do not have multiple changes since it is new YAML file.
in case if we feel anything to be added , we add as separate patches.
Thanks!

Regards
Vadivel

>
Mark Brown May 21, 2020, 12:37 p.m. UTC | #7
On Thu, May 21, 2020 at 08:34:43PM +0800, Ramuthevar, Vadivel MuruganX wrote:
> On 21/5/2020 8:20 pm, Mark Brown wrote:

> > I mean that any changes to the bindings ought to be split out into
> > separate patches, if there's multiple changes it may make sense for
> > there to be multiple patches.

> Got it, we do not have multiple changes since it is new YAML file.
> in case if we feel anything to be added , we add as separate patches.

If this is just a conversion to YAML then your changelog is wildly
inaccurate and needs to be improved, your changelog says you are adding
new things.
Ramuthevar, Vadivel MuruganX May 21, 2020, 12:43 p.m. UTC | #8
Hi Mark,

On 21/5/2020 8:37 pm, Mark Brown wrote:
> On Thu, May 21, 2020 at 08:34:43PM +0800, Ramuthevar, Vadivel MuruganX wrote:
>> On 21/5/2020 8:20 pm, Mark Brown wrote:
> 
>>> I mean that any changes to the bindings ought to be split out into
>>> separate patches, if there's multiple changes it may make sense for
>>> there to be multiple patches.
> 
>> Got it, we do not have multiple changes since it is new YAML file.
>> in case if we feel anything to be added , we add as separate patches.
> 
> If this is just a conversion to YAML then your changelog is wildly
> inaccurate and needs to be improved, your changelog says you are adding
> new things.
Yes , You are right, just conversion only, over that adding/modifying 
few of the properties like compatible and node name in  the example..etc
Sure I will make a multiple patches , Thanks!

Regards
Vadivel

>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
deleted file mode 100644
index 945be7d5b236..000000000000
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ /dev/null
@@ -1,67 +0,0 @@ 
-* Cadence Quad SPI controller
-
-Required properties:
-- 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".
-- reg : Contains two entries, each of which is a tuple consisting of a
-	physical address and length. The first entry is the address and
-	length of the controller register set. The second entry is the
-	address and length of the QSPI Controller data area.
-- interrupts : Unit interrupt specifier for the controller interrupt.
-- clocks : phandle to the Quad SPI clock.
-- cdns,fifo-depth : Size of the data FIFO in words.
-- cdns,fifo-width : Bus width of the data FIFO in bytes.
-- cdns,trigger-address : 32-bit indirect AHB trigger address.
-
-Optional properties:
-- cdns,is-decoded-cs : Flag to indicate whether decoder is used or not.
-- cdns,rclk-en : 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.
-
-Optional subnodes:
-Subnodes of the Cadence Quad SPI controller are spi slave nodes with additional
-custom properties:
-- cdns,read-delay : Delay for read capture logic, in clock cycles
-- cdns,tshsl-ns : Delay in nanoseconds for the length that the master
-                  mode chip select outputs are de-asserted between
-		  transactions.
-- cdns,tsd2d-ns : Delay in nanoseconds between one chip select being
-                  de-activated and the activation of another.
-- cdns,tchsh-ns : Delay in nanoseconds between last bit of current
-                  transaction and deasserting the device chip select
-		  (qspi_n_ss_out).
-- cdns,tslch-ns : Delay in nanoseconds between setting qspi_n_ss_out low
-                  and first bit transfer.
-- resets	: Must contain an entry for each entry in reset-names.
-		  See ../reset/reset.txt for details.
-- reset-names	: Must include either "qspi" and/or "qspi-ocp".
-
-Example:
-
-	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,is-decoded-cs;
-		cdns,fifo-depth = <128>;
-		cdns,fifo-width = <4>;
-		cdns,trigger-address = <0x00000000>;
-		resets = <&rst QSPI_RESET>, <&rst QSPI_OCP_RESET>;
-		reset-names = "qspi", "qspi-ocp";
-
-		flash0: n25q00@0 {
-			...
-			cdns,read-delay = <4>;
-			cdns,tshsl-ns = <50>;
-			cdns,tsd2d-ns = <50>;
-			cdns,tchsh-ns = <4>;
-			cdns,tslch-ns = <4>;
-		};
-	};
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..1c15acc184b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -0,0 +1,133 @@ 
+# 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.
+
+properties:
+  compatible:
+     enum:
+       - cdns,qspi-nor
+       - ti,k2g-qspi
+       - ti,am654-ospi
+       - intel,lgm-qspi
+
+  reg:
+    items:
+      - description:
+          The first entry is the address and length of
+          the controller register set.
+      - description:
+          The second entry is the address and length of
+          the QSPI Controller data area.
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  cdns,fifo-depth:
+    description:
+     Depth of hardware FIFO in words.
+    allOf:
+      - $ref: "/schemas/types.yaml#/definitions/uint32"
+      - enum: [ 128, 256 ]
+      - default: 128
+
+  cdns,fifo-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    multipleOf: 4
+    description:
+      4 byte 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:
+    type: boolean
+    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-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:
+    description: |
+      Delay in nanoseconds for the length that the master mode chip select
+      outputs are de-asserted between transactions.
+
+  cdns,tsd2d-ns:
+    description: |
+      Delay in nanoseconds between one chip select being de-activated
+      and the activation of another.
+
+  cdns,tchsh-ns:
+    description: |
+      Delay in nanoseconds between last bit of current transaction and
+      deasserting the device chip select (qspi_n_ss_out).
+
+  cdns,tslch-ns:
+    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:
+  - |
+    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>;
+
+          flash@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>;
+          };
+    };
+