[PATCH v3 0/2] add support for Cadence's XSPI controller

[PATCH v3 0/2] add support for Cadence's XSPI controller

[Parshuram Thombare]

This patch series adds support for Cadence's XSPI controller.
It supports 3 work modes.
1. ACMD (auto command) work mode
    ACMD name is because it uses auto command engine in the controller.
    It further has 2 modes PIO and CDMA (command DMA).
    The CDMA work mode is dedicated for high-performance application
    where very low software overhead is required. In this mode the
    Command Engine is programmed by the series of linked descriptors
    stored in system memory. These descriptors provide commands to execute
    and store status information for finished commands.
    The PIO mode work mode is dedicated for single operation where
    constructing a linked list of descriptors would require too
    much effort.
2. STIG (Software Triggered Instruction Generator) work mode
    In STIG mode, controller sends low-level instructions to memory.
    Each instruction is 128-bit width. There is special instruction
    DataSequence which carries information about data phase.
    Driver uses Slave DMA interface to transfer data as only this
    interface can be used in STIG work mode.
3. Direct work mode
    This work mode allows sending data without invoking any command through
    the slave interface.
Currently ACMD PIO mode is used for NOR flash read, program, erase
operations, all other operations are handled in STIG work mode.

Changes since v2:
1. Removed extra lock around exec_op.
2. Removed PHY parameters setting from the driver, those will be
   handled by bootstrap pins available in the controller.

Changes since v1:
1. Use ACMD PIO work mode for NOR read, program and erase operations,
   for everything else use STIG(Software Triggered Instruction
   Generator) work mode.
2. Changes suggested by Lukas.

Parshuram Thombare (2):
  spi: cadence: add dt-bindings documentation for Cadence XSPI
    controller
  spi: cadence: add support for Cadence XSPI controller

 .../devicetree/bindings/spi/cdns,xspi.yaml         |  66 ++
 drivers/spi/Kconfig                                |  11 +
 drivers/spi/Makefile                               |   1 +
 drivers/spi/spi-cadence-xspi.c                     | 837 +++++++++++++++++++++
 4 files changed, 915 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/cdns,xspi.yaml
 create mode 100644 drivers/spi/spi-cadence-xspi.c

-- 
2.7.4

[PATCH v3 1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

[Parshuram Thombare]

Add DT binding for Cadence's XSPI controller driver.

Signed-off-by: Konrad Kociolek <konrad@cadence.com>
Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 .../devicetree/bindings/spi/cdns,xspi.yaml         | 66 ++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/cdns,xspi.yaml

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
new file mode 100644
index 0000000..e52d6fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020-21 Cadence
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/spi/cdns,xspi.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Cadence XSPI Controller
+
+maintainers:
+  - Parshuram Thombare <pthombar@cadence.com>
+
+description: |
+  The XSPI controller allows SPI protocol communication in
+  single, dual, quad or octal wire transmission modes for
+  read/write access to slaves such as SPI-NOR flash.
+
+properties:
+  compatible:
+    const: cdns,xspi-nor
+
+  reg:
+    items:
+      - description: address and length of the controller register set
+      - description: address and length of the Slave DMA data port
+      - description: address and length of the auxiliary registers
+
+  reg-names:
+    items:
+      - const: xspi-iobase
+      - const: xspi-sdmabase
+      - const: xspi-auxbase
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    xspi: spi@a0010000 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "cdns,xspi-nor";
+        reg = <0x0 0xa0010000 0x0 0x10000>,
+              <0x0 0xb0000000 0x0 0x10000>,
+              <0x0 0xa0020000 0x0 0x10000>;
+        reg-names = "xspi-iobase", "xspi-sdmabase", "xspi-auxbase";
+        interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-parent = <&gic>;
+        mt35xu512@0 {
+            compatible = "spi-nor", "micron,mt35xu512";
+            spi-max-frequency = <75000000>;
+            reg = <0>;
+        };
+        mt35xu512@1 {
+            compatible = "spi-nor", "micron,mt35xu512";
+            spi-max-frequency = <75000000>;
+            reg = <1>;
+        };
+    };
-- 
2.7.4

Re: [PATCH v3 1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

[Rob Herring]

On Wed, 01 Sep 2021 14:37:09 +0200, Parshuram Thombare wrote:
> Add DT binding for Cadence's XSPI controller driver.
> 
> Signed-off-by: Konrad Kociolek <konrad@cadence.com>
> Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  .../devicetree/bindings/spi/cdns,xspi.yaml         | 66 ++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.yaml: 'additionalProperties' is a required property
	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.yaml: ignoring, error in schema: 
warning: no schema found in file: ./Documentation/devicetree/bindings/spi/cdns,xspi.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml: example-0: spi@a0010000:reg:0: [0, 2684420096, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml: example-0: spi@a0010000:reg:1: [0, 2952790016, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml: example-0: spi@a0010000:reg:2: [0, 2684485632, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000: failed to match any schema with compatible: ['cdns,xspi-nor']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@0: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@0: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@1: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@1: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1523137

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

RE: [PATCH v3 1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

[Parshuram Raju Thombare]

Hi Rob,

>See
>https://urldefense.com/v3/__https://patchwork.ozlabs.org/patch/1523137__;!!E
>HscmS1ygiU1lA!V71KMbkd1YPERLm96tbPDX_W05cj0TCcttcwVXU9H3lDADvSBnc
>5GbQunqxwGIA$
>
>This check can fail if there are any dependencies. The base for a patch
>series is generally the most recent rc1.
>
>If you already ran 'make dt_binding_check' and didn't see the above
>error(s), then make sure 'yamllint' is installed and dt-schema is up to
>date:
>
>pip3 install dtschema --upgrade
>
>Please check and re-submit.

Thanks for your reply. I will fix this issue in next version of patchset.

Regards,
Parshuram Thombare

Re: [PATCH v3 1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

[Pratyush Yadav]

On 01/09/21 02:37PM, Parshuram Thombare wrote:
> Add DT binding for Cadence's XSPI controller driver.
> 
> Signed-off-by: Konrad Kociolek <konrad@cadence.com>
> Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  .../devicetree/bindings/spi/cdns,xspi.yaml         | 66 ++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> new file mode 100644
> index 0000000..e52d6fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020-21 Cadence
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/spi/cdns,xspi.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Cadence XSPI Controller
> +
> +maintainers:
> +  - Parshuram Thombare <pthombar@cadence.com>
> +
> +description: |
> +  The XSPI controller allows SPI protocol communication in
> +  single, dual, quad or octal wire transmission modes for
> +  read/write access to slaves such as SPI-NOR flash.

This needs to be a "subclass" of the spi-controller.yaml binding.

allOf:
  - $ref: spi-controller.yaml#

> +
> +properties:
> +  compatible:
> +    const: cdns,xspi-nor
> +
> +  reg:
> +    items:
> +      - description: address and length of the controller register set
> +      - description: address and length of the Slave DMA data port
> +      - description: address and length of the auxiliary registers
> +
> +  reg-names:
> +    items:
> +      - const: xspi-iobase
> +      - const: xspi-sdmabase
> +      - const: xspi-auxbase
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    xspi: spi@a0010000 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        compatible = "cdns,xspi-nor";
> +        reg = <0x0 0xa0010000 0x0 0x10000>,
> +              <0x0 0xb0000000 0x0 0x10000>,
> +              <0x0 0xa0020000 0x0 0x10000>;
> +        reg-names = "xspi-iobase", "xspi-sdmabase", "xspi-auxbase";
> +        interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-parent = <&gic>;
> +        mt35xu512@0 {

Node name should be flash@0.

> +            compatible = "spi-nor", "micron,mt35xu512";

These compatibles are arbitrary and undocumented. You probably just need 
"jedec,spi-nor". If you need anything else, you need to justify why.

Please run dt_binding_check, it should point this out. See [0].

> +            spi-max-frequency = <75000000>;
> +            reg = <0>;
> +        };
> +        mt35xu512@1 {
> +            compatible = "spi-nor", "micron,mt35xu512";

Same as above.

> +            spi-max-frequency = <75000000>;
> +            reg = <1>;
> +        };
> +    };
> -- 
> 2.7.4
> 

[0] https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-schema.html#testing

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

RE: [PATCH v3 1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

[Parshuram Raju Thombare]

>This needs to be a "subclass" of the spi-controller.yaml binding.
>
>allOf:
>  - $ref: spi-controller.yaml#

Isn't stating that validation need against spi-controller.yaml as well as
this schema sufficient ? Can you please point an example how to make
controller binding a "subclass" of spi-controller.yaml binding ?

>Node name should be flash@0.

I think spi-controller.yaml uses wildcard for the name of a device node,
so anything in string@hexvalue: should work.

>> +            compatible = "spi-nor", "micron,mt35xu512";
>
>These compatibles are arbitrary and undocumented. You probably just need
>"jedec,spi-nor". If you need anything else, you need to justify why.

Although just "spi-nor" also works, I agree to use "jedec, spi-nor" and drop
device name.

Regards,
Parshuram Thombare

Re: [PATCH v3 1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

[Pratyush Yadav]

On 08/09/21 06:52AM, Parshuram Raju Thombare wrote:
> >This needs to be a "subclass" of the spi-controller.yaml binding.
> >
> >allOf:
> >  - $ref: spi-controller.yaml#
> 
> Isn't stating that validation need against spi-controller.yaml as well as
> this schema sufficient ? Can you please point an example how to make
> controller binding a "subclass" of spi-controller.yaml binding ?

I just showed you. You need to add the below lines:

allOf:
  - $ref: spi-controller.yaml#

See cdns,qspi-nor.yaml or nvidia,tegra210-quad.yaml or any of the 
multiple controller bindings we already have.

By "subclass" I did not mean a programming construct, I just meant it 
should logically be a subclass of the spi-controller.yaml binding, which 
can be done by the allOf.

> 
> >Node name should be flash@0.
> 
> I think spi-controller.yaml uses wildcard for the name of a device node,
> so anything in string@hexvalue: should work.

Sure, but mtd.yaml (which the SPI NOR binding depends on) requires it.

> 
> >> +            compatible = "spi-nor", "micron,mt35xu512";
> >
> >These compatibles are arbitrary and undocumented. You probably just need
> >"jedec,spi-nor". If you need anything else, you need to justify why.
> 
> Although just "spi-nor" also works, I agree to use "jedec, spi-nor" and drop
> device name.

Does it? I see "jedec,spi-nor" compatible documented, but not just 
"spi-nor". And I don't see "micron,mt35xu512" compatible documented 
anywhere either.

Anyway, please just use "jedec,spi-nor".

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

RE: [PATCH v3 1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

[Parshuram Raju Thombare]

>> Isn't stating that validation need against spi-controller.yaml as well as
>> this schema sufficient ? Can you please point an example how to make
>> controller binding a "subclass" of spi-controller.yaml binding ?
>
>I just showed you. You need to add the below lines:

Oh, my bad. I already added that dependency in my local patches for next version,
I thought you are suggesting something else. 

>> I think spi-controller.yaml uses wildcard for the name of a device node,
>> so anything in string@hexvalue: should work.
>
>Sure, but mtd.yaml (which the SPI NOR binding depends on) requires it.

Ok, using "jedec,spi-nor" creates dependency on spi-nor schema, otherwise
I was not seeing any issue in " make dt_binding_check" with just "spi-nor".

This will be taken care in next version of patch set.

Regards,
Parshuram Thombare

[PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Parshuram Thombare]

This patch adds driver for Cadence's XSPI controller.
It supports 3 work modes.
1. ACMD (auto command) work mode
    ACMD name is because it uses auto command engine in the controller.
    It further has 2 modes PIO and CDMA (command DMA).
    The CDMA work mode is dedicated for high-performance application
    where very low software overhead is required. In this mode the
    Command Engine is programmed by the series of linked descriptors
    stored in system memory. These descriptors provide commands to execute
    and store status information for finished commands.
    The PIO mode work mode is dedicated for single operation where
    constructing a linked list of descriptors would require too
    much effort.
2. STIG (Software Triggered Instruction Generator) work mode
    In STIG mode, controller sends low-level instructions to memory.
    Each instruction is 128-bit width. There is special instruction
    DataSequence which carries information about data phase.
    Driver uses Slave DMA interface to transfer data as only this
    interface can be used in STIG work mode.
3. Direct work mode
    This work mode allows sending data without invoking any command through
    the slave interface.
Currently ACMD PIO mode is used for NOR flash read, program, erase
operations, all other operations are handled in STIG work mode.

Signed-off-by: Konrad Kociolek <konrad@cadence.com>
Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/spi/Kconfig            |  11 +
 drivers/spi/Makefile           |   1 +
 drivers/spi/spi-cadence-xspi.c | 837 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 849 insertions(+)
 create mode 100644 drivers/spi/spi-cadence-xspi.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index e71a4c5..874f7aa 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -228,6 +228,17 @@ config SPI_CADENCE_QUADSPI
 	  device with a Cadence QSPI controller and want to access the
 	  Flash as an MTD device.
 
+config SPI_CADENCE_XSPI
+	tristate "Cadence XSPI controller"
+	depends on (OF || COMPILE_TEST) && HAS_IOMEM
+	help
+	  Enable support for the Cadence XSPI Flash controller.
+
+	  Cadence XSPI is a specialized controller for connecting an SPI
+	  Flash over upto 8bit wide bus. Enable this option if you have a
+	  device with a Cadence XSPI controller and want to access the
+	  Flash as an MTD device.
+
 config SPI_CLPS711X
 	tristate "CLPS711X host SPI controller"
 	depends on ARCH_CLPS711X || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 13e54c4..93229a8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_SPI_AR934X)		+= spi-ar934x.o
 obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
 obj-$(CONFIG_SPI_ATMEL_QUADSPI)		+= atmel-quadspi.o
+obj-$(CONFIG_SPI_CADENCE_XSPI)		+= spi-cadence-xspi.o
 obj-$(CONFIG_SPI_AT91_USART)		+= spi-at91-usart.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
new file mode 100644
index 0000000..c2b33e2
--- /dev/null
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -0,0 +1,837 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Cadence XSPI flash controller driver
+ *
+ * Copyright (C) 2020-21 Cadence
+ *
+ */
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/bitfield.h>
+#include <linux/limits.h>
+#include <linux/log2.h>
+
+#define CDNS_XSPI_MAGIC_NUM_VALUE	0x6522
+#define CDNS_XSPI_MAX_BANKS		8
+#define CDNS_XSPI_NAME			"cadence-xspi"
+
+/*
+ * Note: below are additional auxiliary registers to
+ * configure XSPI controller pin-strap settings
+ */
+
+/* PHY DQ timing register */
+#define CDNS_XSPI_CCP_PHY_DQ_TIMING		0x0000
+
+/* PHY DQS timing register */
+#define CDNS_XSPI_CCP_PHY_DQS_TIMING		0x0004
+
+/* PHY gate loopback control register */
+#define CDNS_XSPI_CCP_PHY_GATE_LPBCK_CTRL	0x0008
+
+/* PHY DLL slave control register */
+#define CDNS_XSPI_CCP_PHY_DLL_SLAVE_CTRL	0x0010
+
+/* DLL PHY control register */
+#define CDNS_XSPI_DLL_PHY_CTRL			0x1034
+
+/* Command registers */
+#define CDNS_XSPI_CMD_REG_0			0x0000
+#define CDNS_XSPI_CMD_REG_1			0x0004
+#define CDNS_XSPI_CMD_REG_2			0x0008
+#define CDNS_XSPI_CMD_REG_3			0x000C
+#define CDNS_XSPI_CMD_REG_4			0x0010
+#define CDNS_XSPI_CMD_REG_5			0x0014
+
+/* Command status registers */
+#define CDNS_XSPI_CMD_STATUS_REG		0x0044
+#define CDNS_XSPI_CMD_COMPLETED			BIT(15)
+#define CDNS_XSPI_CMD_FAILED			BIT(14)
+
+#define CDNS_XSPI_CMD_STATUS_PTR_REG		0x0040
+
+#define CDNS_XSPI_DEV_ERR			BIT(4)
+#define CDNS_XSPI_DQS_ERR			BIT(3)
+#define CDNS_XSPI_CRC_ERR			BIT(2)
+#define CDNS_XSPI_BUS_ERR			BIT(1)
+#define CDNS_XSPI_CMD_ERR			BIT(0)
+
+#define CDNS_XSPI_ACMD_CMD_TYPE			GENMASK(15, 0)
+#define CDNS_XSPI_ACMD_READ_MB_XIP_EN		BIT(16)
+#define CDNS_XSPI_ACMD_READ_MB_XIP_DIS		BIT(17)
+#define CDNS_XSPI_ACMD_INT			BIT(18)
+#define CDNS_XSPI_ACMD_RW_DMA_SEL		BIT(19)
+#define CDNS_XSPI_ACMD_BANK			GENMASK(22, 20)
+#define CDNS_XSPI_ACMD_TRD_NUM			GENMASK(26, 24)
+#define CDNS_XSPI_ACMD_MODE			GENMASK(31, 30)
+
+#define CDNS_XSPI_ACMD_RW_DMA_MASTER		1
+#define CDNS_XSPI_ACMD_RW_DMA_SLAVE		0
+
+#define CDNS_XSPI_ACMD_MODE_PIO			1
+
+/* Controller status register */
+#define CDNS_XSPI_CTRL_STATUS_REG		0x0100
+#define CDNS_XSPI_INIT_COMPLETED		BIT(16)
+#define CDNS_XSPI_INIT_LEGACY			BIT(9)
+#define CDNS_XSPI_INIT_FAIL			BIT(8)
+#define CDNS_XSPI_CTRL_BUSY			BIT(7)
+
+/* Controller interrupt status register */
+#define CDNS_XSPI_INTR_STATUS_REG		0x0110
+#define CDNS_XSPI_STIG_DONE			BIT(23)
+#define CDNS_XSPI_SDMA_ERROR			BIT(22)
+#define CDNS_XSPI_SDMA_TRIGGER			BIT(21)
+#define CDNS_XSPI_CMD_IGNRD_EN			BIT(20)
+#define CDNS_XSPI_DDMA_TERR_EN			BIT(18)
+#define CDNS_XSPI_CDMA_TREE_EN			BIT(17)
+#define CDNS_XSPI_CTRL_IDLE_EN			BIT(16)
+
+#define CDNS_XSPI_TRD_COMP_INTR_STATUS		0x0120
+#define CDNS_XSPI_TRD_ERR_INTR_STATUS		0x0130
+#define CDNS_XSPI_TRD_ERR_INTR_EN		0x0134
+
+/* Controller interrupt enable register */
+#define CDNS_XSPI_INTR_ENABLE_REG		0x0114
+#define CDNS_XSPI_INTR_EN			BIT(31)
+#define CDNS_XSPI_STIG_DONE_EN			BIT(23)
+#define CDNS_XSPI_SDMA_ERROR_EN			BIT(22)
+#define CDNS_XSPI_SDMA_TRIGGER_EN		BIT(21)
+
+#define CDNS_XSPI_INTR_MASK (CDNS_XSPI_INTR_EN | \
+	CDNS_XSPI_STIG_DONE_EN  | \
+	CDNS_XSPI_SDMA_ERROR_EN | \
+	CDNS_XSPI_SDMA_TRIGGER_EN)
+
+/* Controller config register */
+#define CDNS_XSPI_CTRL_CONFIG_REG		0x0230
+#define CDNS_XSPI_CTRL_WORK_MODE		GENMASK(6, 5)
+
+#define CDNS_XSPI_WORK_MODE_DIRECT		0
+#define CDNS_XSPI_WORK_MODE_STIG		1
+#define CDNS_XSPI_WORK_MODE_ACMD		3
+
+/* SDMA trigger transaction registers */
+#define CDNS_XSPI_SDMA_SIZE_REG			0x0240
+#define CDNS_XSPI_SDMA_TRD_INFO_REG		0x0244
+#define CDNS_XSPI_SDMA_DIR			BIT(8)
+
+/* Controller features register */
+#define CDNS_XSPI_CTRL_FEATURES_REG		0x0F04
+#define CDNS_XSPI_NUM_BANKS			GENMASK(25, 24)
+#define CDNS_XSPI_DMA_DATA_WIDTH		BIT(21)
+#define CDNS_XSPI_NUM_THREADS			GENMASK(3, 0)
+
+/* Controller version register */
+#define CDNS_XSPI_CTRL_VERSION_REG		0x0F00
+#define CDNS_XSPI_MAGIC_NUM			GENMASK(31, 16)
+#define CDNS_XSPI_CTRL_REV			GENMASK(7, 0)
+
+/* STIG Profile 1.0 instruction fields (split into registers) */
+#define CDNS_XSPI_CMD_INSTR_TYPE		GENMASK(6, 0)
+#define CDNS_XSPI_CMD_P1_R1_ADDR0		GENMASK(31, 24)
+#define CDNS_XSPI_CMD_P1_R2_ADDR1		GENMASK(7, 0)
+#define CDNS_XSPI_CMD_P1_R2_ADDR2		GENMASK(15, 8)
+#define CDNS_XSPI_CMD_P1_R2_ADDR3		GENMASK(23, 16)
+#define CDNS_XSPI_CMD_P1_R2_ADDR4		GENMASK(31, 24)
+#define CDNS_XSPI_CMD_P1_R3_ADDR5		GENMASK(7, 0)
+#define CDNS_XSPI_CMD_P1_R3_CMD			GENMASK(23, 16)
+#define CDNS_XSPI_CMD_P1_R3_NUM_ADDR_BYTES	GENMASK(30, 28)
+#define CDNS_XSPI_CMD_P1_R4_ADDR_IOS		GENMASK(1, 0)
+#define CDNS_XSPI_CMD_P1_R4_CMD_IOS		GENMASK(9, 8)
+#define CDNS_XSPI_CMD_P1_R4_BANK		GENMASK(14, 12)
+
+/* STIG data sequence instruction fields (split into registers) */
+#define CDNS_XSPI_CMD_DSEQ_R2_DCNT_L		GENMASK(31, 16)
+#define CDNS_XSPI_CMD_DSEQ_R3_DCNT_H		GENMASK(15, 0)
+#define CDNS_XSPI_CMD_DSEQ_R3_NUM_OF_DUMMY	GENMASK(25, 20)
+#define CDNS_XSPI_CMD_DSEQ_R4_BANK		GENMASK(14, 12)
+#define CDNS_XSPI_CMD_DSEQ_R4_DATA_IOS		GENMASK(9, 8)
+#define CDNS_XSPI_CMD_DSEQ_R4_DIR		BIT(4)
+
+/* STIG command status fields */
+#define CDNS_XSPI_CMD_STATUS_COMPLETED		BIT(15)
+#define CDNS_XSPI_CMD_STATUS_FAILED		BIT(14)
+#define CDNS_XSPI_CMD_STATUS_DQS_ERROR		BIT(3)
+#define CDNS_XSPI_CMD_STATUS_CRC_ERROR		BIT(2)
+#define CDNS_XSPI_CMD_STATUS_BUS_ERROR		BIT(1)
+#define CDNS_XSPI_CMD_STATUS_INV_SEQ_ERROR	BIT(0)
+
+#define CDNS_XSPI_STIG_DONE_FLAG		BIT(0)
+#define CDNS_XSPI_TRD_STATUS			0x0104
+
+/* Helper macros for filling command registers */
+#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_1(op, data_phase) ( \
+	FIELD_PREP(CDNS_XSPI_CMD_INSTR_TYPE, (data_phase) ? \
+		CDNS_XSPI_STIG_INSTR_TYPE_1 : CDNS_XSPI_STIG_INSTR_TYPE_0) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R1_ADDR0, (op)->addr.val & 0xff))
+
+#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_2(op) ( \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR1, ((op)->addr.val >> 8)  & 0xFF) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR2, ((op)->addr.val >> 16) & 0xFF) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR3, ((op)->addr.val >> 24) & 0xFF) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR4, ((op)->addr.val >> 32) & 0xFF))
+
+#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op) ( \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R3_ADDR5, ((op)->addr.val >> 40) & 0xFF) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R3_CMD, (op)->cmd.opcode) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R3_NUM_ADDR_BYTES, (op)->addr.nbytes))
+
+#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_4(op, chipsel) ( \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R4_ADDR_IOS, ilog2((op)->addr.buswidth)) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R4_CMD_IOS, ilog2((op)->cmd.buswidth)) | \
+	FIELD_PREP(CDNS_XSPI_CMD_P1_R4_BANK, chipsel))
+
+#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_1(op) \
+	FIELD_PREP(CDNS_XSPI_CMD_INSTR_TYPE, CDNS_XSPI_STIG_INSTR_TYPE_DATA_SEQ)
+
+#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_2(op) \
+	FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R2_DCNT_L, (op)->data.nbytes & 0xFFFF)
+
+#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op) ( \
+	FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R3_DCNT_H, \
+		((op)->data.nbytes >> 16) & 0xffff) | \
+	FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R3_NUM_OF_DUMMY, (op)->dummy.nbytes * 8))
+
+#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_4(op, chipsel) ( \
+	FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R4_BANK, chipsel) | \
+	FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R4_DATA_IOS, ilog2((op)->data.buswidth)) | \
+	FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R4_DIR, \
+		((op)->data.dir == SPI_MEM_DATA_IN) ? \
+		CDNS_XSPI_STIG_CMD_DIR_READ : CDNS_XSPI_STIG_CMD_DIR_WRITE))
+
+enum cdns_xspi_stig_instr_type {
+	CDNS_XSPI_STIG_INSTR_TYPE_0,
+	CDNS_XSPI_STIG_INSTR_TYPE_1,
+	CDNS_XSPI_STIG_INSTR_TYPE_DATA_SEQ = 127,
+};
+
+enum cdns_xspi_sdma_dir {
+	CDNS_XSPI_SDMA_DIR_READ,
+	CDNS_XSPI_SDMA_DIR_WRITE,
+};
+
+enum cdns_xspi_stig_cmd_dir {
+	CDNS_XSPI_STIG_CMD_DIR_READ,
+	CDNS_XSPI_STIG_CMD_DIR_WRITE,
+};
+
+enum cdns_xspi_acmd_cmd_type {
+	CDNS_XSPI_ACMD_CMD_RESET = 0x1100,
+	CDNS_XSPI_ACMD_CMD_SECT_ERASE = 0x1000,
+	CDNS_XSPI_ACMD_CMD_CHIP_ERASE = 0x1001,
+	CDNS_XSPI_ACMD_CMD_READ = 0x2200,
+	CDNS_XSPI_ACMD_CMD_PROGRAM = 0x2100,
+};
+
+struct cdns_xspi_dev {
+	struct platform_device *pdev;
+	struct device *dev;
+
+	void __iomem *iobase;
+	void __iomem *auxbase;
+	void __iomem *sdmabase;
+
+	int irq;
+	int cur_cs;
+
+	struct completion cmd_complete;
+	struct completion auto_cmd_complete;
+	struct completion sdma_complete;
+	bool sdma_error;
+
+	void *in_buffer;
+	const void *out_buffer;
+
+	u8 hw_num_banks;
+};
+
+static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
+{
+	u32 ctrl_stat;
+
+	return readl_relaxed_poll_timeout(cdns_xspi->iobase +
+					  CDNS_XSPI_CTRL_STATUS_REG,
+					  ctrl_stat,
+					  ((ctrl_stat &
+					    CDNS_XSPI_CTRL_BUSY) == 0),
+					  100, 1000);
+}
+
+static int cdns_xspi_wait_for_thread_idle(struct cdns_xspi_dev *cdns_xspi,
+					  int thread_num)
+{
+	u32 trd_stat;
+
+	return readl_relaxed_poll_timeout(cdns_xspi->iobase +
+					  CDNS_XSPI_TRD_STATUS,
+					  trd_stat,
+					  ((trd_stat & BIT(thread_num)) == 0),
+					  100, 1000);
+}
+
+static void cdns_xspi_trigger_command(struct cdns_xspi_dev *cdns_xspi,
+				      u32 cmd_regs[5])
+{
+	writel(cmd_regs[5], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_5);
+	writel(cmd_regs[4], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_4);
+	writel(cmd_regs[3], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_3);
+	writel(cmd_regs[2], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_2);
+	writel(cmd_regs[1], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_1);
+	writel(cmd_regs[0], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_0);
+}
+
+static int cdns_xspi_check_command_status(struct cdns_xspi_dev *cdns_xspi)
+{
+	int ret = 0;
+	u32 cmd_status = readl(cdns_xspi->iobase + CDNS_XSPI_CMD_STATUS_REG);
+
+	if (cmd_status & CDNS_XSPI_CMD_STATUS_COMPLETED) {
+		if ((cmd_status & CDNS_XSPI_CMD_STATUS_FAILED) != 0) {
+			if (cmd_status & CDNS_XSPI_CMD_STATUS_DQS_ERROR) {
+				dev_err(cdns_xspi->dev,
+					"Incorrect DQS pulses detected\n");
+				ret = -EPROTO;
+			}
+			if (cmd_status & CDNS_XSPI_CMD_STATUS_CRC_ERROR) {
+				dev_err(cdns_xspi->dev,
+					"CRC error received\n");
+				ret = -EPROTO;
+			}
+			if (cmd_status & CDNS_XSPI_CMD_STATUS_BUS_ERROR) {
+				dev_err(cdns_xspi->dev,
+					"Error resp on system DMA interface\n");
+				ret = -EPROTO;
+			}
+			if (cmd_status & CDNS_XSPI_CMD_STATUS_INV_SEQ_ERROR) {
+				dev_err(cdns_xspi->dev,
+					"Invalid command sequence detected\n");
+				ret = -EPROTO;
+			}
+		}
+	} else {
+		dev_err(cdns_xspi->dev, "Fatal err - command not completed\n");
+		ret = -EPROTO;
+	}
+
+	return ret;
+}
+
+static void cdns_xspi_set_interrupts(struct cdns_xspi_dev *cdns_xspi,
+				     bool enabled)
+{
+	u32 intr_enable;
+
+	intr_enable = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_ENABLE_REG);
+	if (enabled)
+		intr_enable |= CDNS_XSPI_INTR_MASK;
+	else
+		intr_enable &= ~CDNS_XSPI_INTR_MASK;
+	writel(intr_enable, cdns_xspi->iobase + CDNS_XSPI_INTR_ENABLE_REG);
+}
+
+static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
+{
+	u32 ctrl_ver;
+	u32 ctrl_features;
+	u16 hw_magic_num;
+
+	ctrl_ver = readl(cdns_xspi->iobase + CDNS_XSPI_CTRL_VERSION_REG);
+	hw_magic_num = FIELD_GET(CDNS_XSPI_MAGIC_NUM, ctrl_ver);
+	if (hw_magic_num != CDNS_XSPI_MAGIC_NUM_VALUE) {
+		dev_err(cdns_xspi->dev,
+			"Incorrect XSPI magic nunber: %x, expected: %x\n",
+			hw_magic_num, CDNS_XSPI_MAGIC_NUM_VALUE);
+		return -EIO;
+	}
+
+	ctrl_features = readl(cdns_xspi->iobase + CDNS_XSPI_CTRL_FEATURES_REG);
+	cdns_xspi->hw_num_banks = FIELD_GET(CDNS_XSPI_NUM_BANKS, ctrl_features);
+	cdns_xspi_set_interrupts(cdns_xspi, false);
+
+	return 0;
+}
+
+static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
+{
+	u32 sdma_size, sdma_trd_info;
+	u8 sdma_dir;
+
+	sdma_size = readl(cdns_xspi->iobase + CDNS_XSPI_SDMA_SIZE_REG);
+	sdma_trd_info = readl(cdns_xspi->iobase + CDNS_XSPI_SDMA_TRD_INFO_REG);
+	sdma_dir = FIELD_GET(CDNS_XSPI_SDMA_DIR, sdma_trd_info);
+
+	switch (sdma_dir) {
+	case CDNS_XSPI_SDMA_DIR_READ:
+		ioread8_rep(cdns_xspi->sdmabase,
+			    cdns_xspi->in_buffer, sdma_size);
+		break;
+
+	case CDNS_XSPI_SDMA_DIR_WRITE:
+		iowrite8_rep(cdns_xspi->sdmabase,
+			     cdns_xspi->out_buffer, sdma_size);
+		break;
+	}
+}
+
+static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
+				       const struct spi_mem_op *op,
+				       bool data_phase)
+{
+	u32 cmd_regs[5];
+	u32 cmd_status;
+	int ret;
+
+	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
+	if (ret < 0)
+		return -EIO;
+
+	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
+	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+
+	cdns_xspi_set_interrupts(cdns_xspi, true);
+	cdns_xspi->sdma_error = false;
+
+	memset(cmd_regs, 0, sizeof(cmd_regs));
+	cmd_regs[1] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_1(op, data_phase);
+	cmd_regs[2] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_2(op);
+	cmd_regs[3] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op);
+	cmd_regs[4] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_4(op,
+						       cdns_xspi->cur_cs);
+
+	cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
+
+	if (data_phase) {
+		cmd_regs[0] = CDNS_XSPI_STIG_DONE_FLAG;
+		cmd_regs[1] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_1(op);
+		cmd_regs[2] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_2(op);
+		cmd_regs[3] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op);
+		cmd_regs[4] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_4(op,
+							   cdns_xspi->cur_cs);
+
+		cdns_xspi->in_buffer = op->data.buf.in;
+		cdns_xspi->out_buffer = op->data.buf.out;
+
+		cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
+
+		wait_for_completion(&cdns_xspi->sdma_complete);
+		if (cdns_xspi->sdma_error) {
+			cdns_xspi_set_interrupts(cdns_xspi, false);
+			return -EIO;
+		}
+		cdns_xspi_sdma_handle(cdns_xspi);
+	}
+
+	wait_for_completion(&cdns_xspi->cmd_complete);
+	cdns_xspi_set_interrupts(cdns_xspi, false);
+
+	cmd_status = cdns_xspi_check_command_status(cdns_xspi);
+	if (cmd_status)
+		return -EPROTO;
+
+	return 0;
+}
+
+static ssize_t cdns_xspi_nor_read(struct cdns_xspi_dev *cdns_xspi, loff_t from,
+				  size_t len, u8 *read_buf)
+{
+	u32 cmd_status;
+	int ret;
+
+	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
+	if (ret < 0)
+		return -EIO;
+
+	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_ACMD),
+	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+
+	ret = cdns_xspi_wait_for_thread_idle(cdns_xspi, 0);
+	if (ret < 0)
+		return -EIO;
+
+	cdns_xspi_set_interrupts(cdns_xspi, true);
+	cdns_xspi->sdma_error = false;
+	cdns_xspi->in_buffer = read_buf;
+
+	writel(from, cdns_xspi->iobase + CDNS_XSPI_CMD_REG_1);
+	writel(len - 1, cdns_xspi->iobase + CDNS_XSPI_CMD_REG_4);
+
+	writel(FIELD_PREP(CDNS_XSPI_ACMD_CMD_TYPE, CDNS_XSPI_ACMD_CMD_READ) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_MODE, CDNS_XSPI_ACMD_MODE_PIO) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_INT, 1) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_BANK, cdns_xspi->cur_cs) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_RW_DMA_SEL,
+			  CDNS_XSPI_ACMD_RW_DMA_SLAVE),
+	       cdns_xspi->iobase + CDNS_XSPI_CMD_REG_0);
+
+	wait_for_completion(&cdns_xspi->sdma_complete);
+	if (cdns_xspi->sdma_error) {
+		cdns_xspi_set_interrupts(cdns_xspi, false);
+		return -EIO;
+	}
+	cdns_xspi_sdma_handle(cdns_xspi);
+
+	wait_for_completion(&cdns_xspi->auto_cmd_complete);
+	cdns_xspi_set_interrupts(cdns_xspi, false);
+
+	cmd_status = cdns_xspi_check_command_status(cdns_xspi);
+	if (cmd_status)
+		return -EPROTO;
+
+	return 0;
+}
+
+static ssize_t cdns_xspi_nor_write(struct cdns_xspi_dev *cdns_xspi, loff_t to,
+				   size_t len, const u8 *write_buf)
+{
+	u32 cmd_status;
+	int ret;
+
+	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
+	if (ret < 0)
+		return -EIO;
+
+	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_ACMD),
+	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+
+	ret = cdns_xspi_wait_for_thread_idle(cdns_xspi, 0);
+	if (ret < 0)
+		return -EIO;
+
+	cdns_xspi_set_interrupts(cdns_xspi, true);
+	cdns_xspi->sdma_error = false;
+	cdns_xspi->out_buffer = write_buf;
+
+	writel(to, cdns_xspi->iobase + CDNS_XSPI_CMD_REG_1);
+	writel(len - 1, cdns_xspi->iobase + CDNS_XSPI_CMD_REG_4);
+
+	writel(FIELD_PREP(CDNS_XSPI_ACMD_CMD_TYPE, CDNS_XSPI_ACMD_CMD_PROGRAM) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_MODE, CDNS_XSPI_ACMD_MODE_PIO) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_INT, 1) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_BANK, cdns_xspi->cur_cs) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_RW_DMA_SEL,
+			  CDNS_XSPI_ACMD_RW_DMA_SLAVE),
+	       cdns_xspi->iobase + CDNS_XSPI_CMD_REG_0);
+
+	wait_for_completion(&cdns_xspi->sdma_complete);
+	if (cdns_xspi->sdma_error) {
+		cdns_xspi_set_interrupts(cdns_xspi, false);
+		return -EIO;
+	}
+	cdns_xspi_sdma_handle(cdns_xspi);
+
+	wait_for_completion(&cdns_xspi->auto_cmd_complete);
+	cdns_xspi_set_interrupts(cdns_xspi, false);
+
+	cmd_status = cdns_xspi_check_command_status(cdns_xspi);
+	if (cmd_status)
+		return -EPROTO;
+
+	return 0;
+}
+
+static int cdns_xspi_nor_erase(struct cdns_xspi_dev *cdns_xspi, loff_t offs)
+{
+	u32 cmd_status;
+	int ret;
+
+	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
+	if (ret < 0)
+		return -EIO;
+
+	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_ACMD),
+	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+
+	ret = cdns_xspi_wait_for_thread_idle(cdns_xspi, 0);
+	if (ret < 0)
+		return -EIO;
+
+	cdns_xspi_set_interrupts(cdns_xspi, true);
+
+	writel(offs, cdns_xspi->iobase + CDNS_XSPI_CMD_REG_1);
+	writel(1, cdns_xspi->iobase + CDNS_XSPI_CMD_REG_4);
+
+	writel(FIELD_PREP(CDNS_XSPI_ACMD_CMD_TYPE,
+			  CDNS_XSPI_ACMD_CMD_SECT_ERASE) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_INT, 1) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_BANK, cdns_xspi->cur_cs) |
+	       FIELD_PREP(CDNS_XSPI_ACMD_MODE, CDNS_XSPI_ACMD_MODE_PIO),
+	       cdns_xspi->iobase + CDNS_XSPI_CMD_REG_0);
+
+	wait_for_completion(&cdns_xspi->auto_cmd_complete);
+	cdns_xspi_set_interrupts(cdns_xspi, false);
+
+	cmd_status = cdns_xspi_check_command_status(cdns_xspi);
+	if (cmd_status)
+		return -EPROTO;
+
+	return 0;
+}
+
+static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
+			    struct spi_mem *mem,
+			    const struct spi_mem_op *op)
+{
+	struct spi_driver *spidrv = to_spi_driver(mem->spi->dev.driver);
+	enum spi_mem_data_dir dir = op->data.dir;
+
+	if (cdns_xspi->cur_cs != mem->spi->chip_select)
+		cdns_xspi->cur_cs = mem->spi->chip_select;
+
+	if (!strstr(spidrv->driver.name, "nor") ||
+	    (!op->addr.buswidth && !op->addr.nbytes && !op->addr.val)) {
+		return cdns_xspi_send_stig_command(cdns_xspi, op,
+						   (dir != SPI_MEM_NO_DATA));
+	} else {
+		if (dir == SPI_MEM_DATA_IN)
+			return cdns_xspi_nor_read(cdns_xspi, op->addr.val,
+						  op->data.nbytes,
+						  op->data.buf.in);
+		else if (dir == SPI_MEM_DATA_OUT)
+			return cdns_xspi_nor_write(cdns_xspi, op->addr.val,
+						   op->data.nbytes,
+						   op->data.buf.out);
+		else
+			return cdns_xspi_nor_erase(cdns_xspi, op->addr.val);
+	}
+}
+
+static int cdns_xspi_mem_op_execute(struct spi_mem *mem,
+				    const struct spi_mem_op *op)
+{
+	struct cdns_xspi_dev *cdns_xspi =
+		spi_master_get_devdata(mem->spi->master);
+	int ret = 0;
+
+	ret = cdns_xspi_mem_op(cdns_xspi, mem, op);
+
+	return ret;
+}
+
+static const struct spi_controller_mem_ops cadence_xspi_mem_ops = {
+	.exec_op = cdns_xspi_mem_op_execute,
+};
+
+static int cdns_xspi_setup(struct spi_device *spi_dev)
+{
+	if (spi_dev->chip_select > spi_dev->master->num_chipselect) {
+		dev_err(&spi_dev->dev,
+			"%d chip-select is out of range\n",
+			spi_dev->chip_select);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static irqreturn_t cdns_xspi_irq_handler(int this_irq, void *dev)
+{
+	struct cdns_xspi_dev *cdns_xspi = dev;
+	u32 irq_status;
+	irqreturn_t result = IRQ_NONE;
+
+	irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
+	if (irq_status) {
+		writel(irq_status,
+		       cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
+
+		if (irq_status & CDNS_XSPI_SDMA_ERROR) {
+			dev_err(cdns_xspi->dev,
+				"Slave DMA transaction error\n");
+			cdns_xspi->sdma_error = true;
+			complete(&cdns_xspi->sdma_complete);
+		}
+
+		if (irq_status & CDNS_XSPI_SDMA_TRIGGER)
+			complete(&cdns_xspi->sdma_complete);
+
+		if (irq_status & CDNS_XSPI_STIG_DONE)
+			complete(&cdns_xspi->cmd_complete);
+
+		result = IRQ_HANDLED;
+	}
+
+	irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_TRD_COMP_INTR_STATUS);
+	if (irq_status) {
+		writel(irq_status,
+		       cdns_xspi->iobase + CDNS_XSPI_TRD_COMP_INTR_STATUS);
+
+		complete(&cdns_xspi->auto_cmd_complete);
+
+		result = IRQ_HANDLED;
+	}
+
+	return result;
+}
+
+static int cdns_xspi_of_get_plat_data(struct platform_device *pdev)
+{
+	struct device_node *node_prop = pdev->dev.of_node;
+	struct device_node *node_child;
+	unsigned int cs;
+
+	for_each_child_of_node(node_prop, node_child) {
+		if (!of_device_is_available(node_child))
+			continue;
+
+		if (of_property_read_u32(node_child, "reg", &cs)) {
+			dev_err(&pdev->dev, "Couldn't get memory chip select\n");
+			return -ENXIO;
+		} else if (cs >= CDNS_XSPI_MAX_BANKS) {
+			dev_err(&pdev->dev, "reg (cs) parameter value too large\n");
+			return -ENXIO;
+		}
+	}
+
+	return 0;
+}
+
+static void cdns_xspi_print_phy_config(struct cdns_xspi_dev *cdns_xspi)
+{
+	struct device *dev = cdns_xspi->dev;
+
+	dev_info(dev, "PHY configuration\n");
+	dev_info(dev, "   * xspi_dll_phy_ctrl: %08x\n",
+		 readl(cdns_xspi->iobase + CDNS_XSPI_DLL_PHY_CTRL));
+	dev_info(dev, "   * phy_dq_timing: %08x\n",
+		 readl(cdns_xspi->auxbase + CDNS_XSPI_CCP_PHY_DQ_TIMING));
+	dev_info(dev, "   * phy_dqs_timing: %08x\n",
+		 readl(cdns_xspi->auxbase + CDNS_XSPI_CCP_PHY_DQS_TIMING));
+	dev_info(dev, "   * phy_gate_loopback_ctrl: %08x\n",
+		 readl(cdns_xspi->auxbase + CDNS_XSPI_CCP_PHY_GATE_LPBCK_CTRL));
+	dev_info(dev, "   * phy_dll_slave_ctrl: %08x\n",
+		 readl(cdns_xspi->auxbase + CDNS_XSPI_CCP_PHY_DLL_SLAVE_CTRL));
+}
+
+static int cdns_xspi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct spi_master *master = NULL;
+	struct cdns_xspi_dev *cdns_xspi = NULL;
+	int ret;
+
+	master = devm_spi_alloc_master(dev, sizeof(*cdns_xspi));
+	if (!master)
+		return -ENOMEM;
+
+	master->mode_bits = SPI_3WIRE | SPI_TX_DUAL  | SPI_TX_QUAD  |
+		SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_OCTAL | SPI_RX_OCTAL |
+		SPI_MODE_0  | SPI_MODE_3;
+
+	master->mem_ops = &cadence_xspi_mem_ops;
+	master->setup = cdns_xspi_setup;
+	master->dev.of_node = pdev->dev.of_node;
+	master->bus_num = -1;
+
+	platform_set_drvdata(pdev, master);
+
+	cdns_xspi = spi_master_get_devdata(master);
+	cdns_xspi->pdev = pdev;
+	cdns_xspi->dev = &pdev->dev;
+	cdns_xspi->cur_cs = 0;
+
+	init_completion(&cdns_xspi->cmd_complete);
+	init_completion(&cdns_xspi->auto_cmd_complete);
+	init_completion(&cdns_xspi->sdma_complete);
+
+	ret = cdns_xspi_of_get_plat_data(pdev);
+	if (ret)
+		return -ENODEV;
+
+	cdns_xspi->iobase = devm_platform_ioremap_resource_byname(pdev,
+								  "xspi-iobase");
+	if (IS_ERR(cdns_xspi->iobase)) {
+		dev_err(dev, "Failed to remap controller base address\n");
+		return PTR_ERR(cdns_xspi->iobase);
+	}
+
+	cdns_xspi->sdmabase = devm_platform_ioremap_resource_byname(pdev,
+								    "xspi-sdmabase");
+	if (IS_ERR(cdns_xspi->sdmabase)) {
+		dev_err(dev, "Failed to remap SDMA address\n");
+		return PTR_ERR(cdns_xspi->sdmabase);
+	}
+
+	cdns_xspi->auxbase = devm_platform_ioremap_resource_byname(pdev,
+								   "xspi-auxbase");
+	if (IS_ERR(cdns_xspi->auxbase)) {
+		dev_err(dev, "Failed to remap AUX address\n");
+		return PTR_ERR(cdns_xspi->auxbase);
+	}
+
+	cdns_xspi->irq = platform_get_irq(pdev, 0);
+	if (cdns_xspi->irq < 0) {
+		dev_err(dev, "Failed to get IRQ\n");
+		return -ENXIO;
+	}
+
+	ret = devm_request_irq(dev, cdns_xspi->irq, cdns_xspi_irq_handler,
+			       IRQF_SHARED, pdev->name, cdns_xspi);
+	if (ret) {
+		dev_err(dev, "Failed to request IRQ: %d\n", cdns_xspi->irq);
+		return ret;
+	}
+
+	cdns_xspi_print_phy_config(cdns_xspi);
+
+	ret = cdns_xspi_controller_init(cdns_xspi);
+	if (ret) {
+		dev_err(dev, "Failed to initialize controller\n");
+		return ret;
+	}
+
+	master->num_chipselect = 1 << cdns_xspi->hw_num_banks;
+
+	ret = devm_spi_register_master(dev, master);
+	if (ret) {
+		dev_err(dev, "Failed to register SPI master\n");
+		return ret;
+	}
+
+	dev_info(dev, "Successfully registered SPI master\n");
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id cdns_xspi_of_match[] = {
+	{
+		.compatible = "cdns,xspi-nor",
+	},
+	{ /* end of table */}
+};
+MODULE_DEVICE_TABLE(of, cdns_xspi_of_match);
+#else
+#define cdns_xspi_of_match NULL
+#endif /* CONFIG_OF */
+
+static struct platform_driver cdns_xspi_platform_driver = {
+	.probe          = cdns_xspi_probe,
+	.remove         = NULL,
+	.driver = {
+		.name = CDNS_XSPI_NAME,
+		.of_match_table = cdns_xspi_of_match,
+	},
+};
+
+module_platform_driver(cdns_xspi_platform_driver);
+
+MODULE_DESCRIPTION("Cadence XSPI Controller Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" CDNS_XSPI_NAME);
+MODULE_AUTHOR("Konrad Kociolek <konrad@cadence.com>");
+MODULE_AUTHOR("Jayshri Pawar <jpawar@cadence.com>");
+MODULE_AUTHOR("Parshuram Thombare <pthombar@cadence.com>");
-- 
2.7.4

Re: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]

On Wed, Sep 01, 2021 at 02:37:38PM +0200, Parshuram Thombare wrote:

> +++ b/drivers/spi/spi-cadence-xspi.c
> @@ -0,0 +1,837 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Cadence XSPI flash controller driver

Please make the entire comment a C++ so things look more intentional.

> +static int cdns_xspi_setup(struct spi_device *spi_dev)
> +{
> +	if (spi_dev->chip_select > spi_dev->master->num_chipselect) {
> +		dev_err(&spi_dev->dev,
> +			"%d chip-select is out of range\n",
> +			spi_dev->chip_select);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

The core already validates this, are you seeing it happen?  If so we
should fix the core and either way just remove setup() entirely.

> +	if (irq_status) {
> +		writel(irq_status,
> +		       cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
> +
> +		if (irq_status & CDNS_XSPI_SDMA_ERROR) {
> +			dev_err(cdns_xspi->dev,
> +				"Slave DMA transaction error\n");
> +			cdns_xspi->sdma_error = true;
> +			complete(&cdns_xspi->sdma_complete);
> +		}
> +
> +		if (irq_status & CDNS_XSPI_SDMA_TRIGGER)
> +			complete(&cdns_xspi->sdma_complete);
> +
> +		if (irq_status & CDNS_XSPI_STIG_DONE)
> +			complete(&cdns_xspi->cmd_complete);
> +
> +		result = IRQ_HANDLED;
> +	}

We will just silently ignore any unknown interrupts here.  It would be
better to either only ack known interrupts (so genirq can notice if
there's a problem with other interrupts) or at least log that we're
seeing unexpected interrupts.  The current code will cause trouble if
this is deployed in a system with the interrupt line shared (which the
driver claims to support), or if something goes wrong and the IP starts
asserting some interrupt that isn't expected.

> +	master->mode_bits = SPI_3WIRE | SPI_TX_DUAL  | SPI_TX_QUAD  |
> +		SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_OCTAL | SPI_RX_OCTAL |
> +		SPI_MODE_0  | SPI_MODE_3;

I don't see any handling of these in the code?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Parshuram Raju Thombare]

Hi Mark,

Thanks for your reply.

>Please make the entire comment a C++ so things look more intentional.

Sure

>The core already validates this, are you seeing it happen?  If so we
>should fix the core and either way just remove setup() entirely.

Oh, right. spi_add_device() seems to be already doing that check.
I will remove it from the driver.

>We will just silently ignore any unknown interrupts here.  It would be
>better to either only ack known interrupts (so genirq can notice if
>there's a problem with other interrupts) or at least log that we're
>seeing unexpected interrupts.  The current code will cause trouble if
>this is deployed in a system with the interrupt line shared (which the
>driver claims to support), or if something goes wrong and the IP starts
>asserting some interrupt that isn't expected.

Ok, I will modify this to return IRQ_HANDLED only for enabled interrupts.

>> +	master->mode_bits = SPI_3WIRE | SPI_TX_DUAL  | SPI_TX_QUAD  |
>> +		SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_OCTAL | SPI_RX_OCTAL
>|
>> +		SPI_MODE_0  | SPI_MODE_3;
>
>I don't see any handling of these in the code?

This is just to declare controller's capability, so that spi_setup() can modify
spi->mode according to the capability of attached device.

Regards,
Parshuram Thombare

Re: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

On Fri, Sep 03, 2021 at 08:10:38AM +0000, Parshuram Raju Thombare wrote:

> >> +	master->mode_bits = SPI_3WIRE | SPI_TX_DUAL  | SPI_TX_QUAD  |
> >> +		SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_OCTAL | SPI_RX_OCTAL
> >|
> >> +		SPI_MODE_0  | SPI_MODE_3;

> >I don't see any handling of these in the code?

> This is just to declare controller's capability, so that spi_setup() can modify
> spi->mode according to the capability of attached device.

In order for this to work I would expect there to be code in the driver
which configures the controller into the specified mode.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Parshuram Raju Thombare]

>On Fri, Sep 03, 2021 at 08:10:38AM +0000, Parshuram Raju Thombare wrote:
>
>> >> +	master->mode_bits = SPI_3WIRE | SPI_TX_DUAL  | SPI_TX_QUAD  |
>> >> +		SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_OCTAL | SPI_RX_OCTAL
>> >|
>> >> +		SPI_MODE_0  | SPI_MODE_3;
>
>> >I don't see any handling of these in the code?
>
>> This is just to declare controller's capability, so that spi_setup() can modify
>> spi->mode according to the capability of attached device.
>
>In order for this to work I would expect there to be code in the driver
>which configures the controller into the specified mode.

Oh, ok.  That is done at power on reset by the controller in 2 ways.
1. Using device discovery module, controller try to auto detect the valid protocol
    mode by trying to read SFDP signature in various modes.
2. Particular protocol mode can be selected using bootstrap signals.

I think mode_bits need to include protocol mode which is auto detected or
set using bootstrap signals. I will make that change in next version. 

Regards,
Parshuram Thombare
     
    

Re: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

On Fri, Sep 03, 2021 at 10:47:50AM +0000, Parshuram Raju Thombare wrote:

> Oh, ok.  That is done at power on reset by the controller in 2 ways.
> 1. Using device discovery module, controller try to auto detect the valid protocol
>     mode by trying to read SFDP signature in various modes.
> 2. Particular protocol mode can be selected using bootstrap signals.

> I think mode_bits need to include protocol mode which is auto detected or
> set using bootstrap signals. I will make that change in next version. 

Yes, that sounds better.  I guess at least in the case of autodetection
the mode can be overridden by software later, but that can always be
added later if someone actually needs the feature.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Pratyush Yadav]

+ Tudor, Vignesh,

On 01/09/21 02:37PM, Parshuram Thombare wrote:
> This patch adds driver for Cadence's XSPI controller.
> It supports 3 work modes.
> 1. ACMD (auto command) work mode
>     ACMD name is because it uses auto command engine in the controller.
>     It further has 2 modes PIO and CDMA (command DMA).
>     The CDMA work mode is dedicated for high-performance application
>     where very low software overhead is required. In this mode the
>     Command Engine is programmed by the series of linked descriptors
>     stored in system memory. These descriptors provide commands to execute
>     and store status information for finished commands.
>     The PIO mode work mode is dedicated for single operation where
>     constructing a linked list of descriptors would require too
>     much effort.
> 2. STIG (Software Triggered Instruction Generator) work mode
>     In STIG mode, controller sends low-level instructions to memory.
>     Each instruction is 128-bit width. There is special instruction
>     DataSequence which carries information about data phase.
>     Driver uses Slave DMA interface to transfer data as only this
>     interface can be used in STIG work mode.
> 3. Direct work mode
>     This work mode allows sending data without invoking any command through
>     the slave interface.
> Currently ACMD PIO mode is used for NOR flash read, program, erase
> operations, all other operations are handled in STIG work mode.

Thanks. The commit message is much nicer now!

> 
> Signed-off-by: Konrad Kociolek <konrad@cadence.com>
> Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  drivers/spi/Kconfig            |  11 +
>  drivers/spi/Makefile           |   1 +
>  drivers/spi/spi-cadence-xspi.c | 837 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 849 insertions(+)
>  create mode 100644 drivers/spi/spi-cadence-xspi.c
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index e71a4c5..874f7aa 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -228,6 +228,17 @@ config SPI_CADENCE_QUADSPI
>  	  device with a Cadence QSPI controller and want to access the
>  	  Flash as an MTD device.
>  
> +config SPI_CADENCE_XSPI
> +	tristate "Cadence XSPI controller"
> +	depends on (OF || COMPILE_TEST) && HAS_IOMEM

Depends on SPI_MEM as well.

> +	help
> +	  Enable support for the Cadence XSPI Flash controller.
> +
> +	  Cadence XSPI is a specialized controller for connecting an SPI
> +	  Flash over upto 8bit wide bus. Enable this option if you have a
> +	  device with a Cadence XSPI controller and want to access the
> +	  Flash as an MTD device.
> +
>  config SPI_CLPS711X
>  	tristate "CLPS711X host SPI controller"
>  	depends on ARCH_CLPS711X || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 13e54c4..93229a8 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_SPI_AR934X)		+= spi-ar934x.o
>  obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
>  obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
>  obj-$(CONFIG_SPI_ATMEL_QUADSPI)		+= atmel-quadspi.o
> +obj-$(CONFIG_SPI_CADENCE_XSPI)		+= spi-cadence-xspi.o
>  obj-$(CONFIG_SPI_AT91_USART)		+= spi-at91-usart.o
>  obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
>  obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> new file mode 100644
> index 0000000..c2b33e2
> --- /dev/null
> +++ b/drivers/spi/spi-cadence-xspi.c
> @@ -0,0 +1,837 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Cadence XSPI flash controller driver
> + *
> + * Copyright (C) 2020-21 Cadence
> + *
> + */
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/spi/spi.h>
> +#include <linux/mtd/spi-nor.h>

Umm, this seems wrong to me. SPI MEM based drivers should generally not 
need to know anything about the subsystem driving them. Why do you need 
to include this?

More on this below.

> +#include <linux/bitfield.h>
> +#include <linux/limits.h>
> +#include <linux/log2.h>
> +
[...]
> +static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
> +			    struct spi_mem *mem,
> +			    const struct spi_mem_op *op)
> +{
> +	struct spi_driver *spidrv = to_spi_driver(mem->spi->dev.driver);
> +	enum spi_mem_data_dir dir = op->data.dir;
> +
> +	if (cdns_xspi->cur_cs != mem->spi->chip_select)
> +		cdns_xspi->cur_cs = mem->spi->chip_select;
> +
> +	if (!strstr(spidrv->driver.name, "nor") ||

I commented on this last time around as well. This does not look right 
at all. A SPI MEM based driver should *not* need to know anything about 
the subsystem driving it. That is the entire point of the API.

The controller seems to be able to extract the read and write opcodes 
from the SFDP on its own since you don't pass in that information to 
cdns_xspi_nor_read(). It looks like it is tied very heavily to a NOR 
flash, and I am not sure if it can really be used with a NAND flash, or 
something else entirely.

Which makes me wonder how we should handle controllers like these. I 
don't think they fit in very well with the SPI MEM model, since they 
can't execute arbitrary SPI MEM commands very well. At the same time we 
are trying to get rid of mtd/spi-nor/controllers. Dunno...

Mark, Tudor, Vignesh, any ideas?

> +	    (!op->addr.buswidth && !op->addr.nbytes && !op->addr.val)) {
> +		return cdns_xspi_send_stig_command(cdns_xspi, op,
> +						   (dir != SPI_MEM_NO_DATA));
> +	} else {
> +		if (dir == SPI_MEM_DATA_IN)
> +			return cdns_xspi_nor_read(cdns_xspi, op->addr.val,
> +						  op->data.nbytes,
> +						  op->data.buf.in);
> +		else if (dir == SPI_MEM_DATA_OUT)
> +			return cdns_xspi_nor_write(cdns_xspi, op->addr.val,
> +						   op->data.nbytes,
> +						   op->data.buf.out);
> +		else
> +			return cdns_xspi_nor_erase(cdns_xspi, op->addr.val);
> +	}
> +}
> +
> +static int cdns_xspi_mem_op_execute(struct spi_mem *mem,
> +				    const struct spi_mem_op *op)
> +{
> +	struct cdns_xspi_dev *cdns_xspi =
> +		spi_master_get_devdata(mem->spi->master);
> +	int ret = 0;
> +
> +	ret = cdns_xspi_mem_op(cdns_xspi, mem, op);
> +
> +	return ret;
> +}
> +
> +static const struct spi_controller_mem_ops cadence_xspi_mem_ops = {
> +	.exec_op = cdns_xspi_mem_op_execute,
> +};
> +

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

RE: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Parshuram Raju Thombare]

>Depends on SPI_MEM as well.

Ok

>I commented on this last time around as well. This does not look right
>at all. A SPI MEM based driver should *not* need to know anything about
>the subsystem driving it. That is the entire point of the API.
>
>The controller seems to be able to extract the read and write opcodes
>from the SFDP on its own since you don't pass in that information to
>cdns_xspi_nor_read(). It looks like it is tied very heavily to a NOR
>flash, and I am not sure if it can really be used with a NAND flash, or
>something else entirely.
>
>Which makes me wonder how we should handle controllers like these. I
>don't think they fit in very well with the SPI MEM model, since they
>can't execute arbitrary SPI MEM commands very well. At the same time we
>are trying to get rid of mtd/spi-nor/controllers. Dunno...
>
>Mark, Tudor, Vignesh, any ideas?

Ok, then for now I will drop ACMD PIO mode and use only STIG mode.
In STIG mode driver configures bus width and clock edge mode for
command, address and data for each operation. 

Regards,
Parshuram Thombare

Re: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Pratyush Yadav]

On 08/09/21 07:27AM, Parshuram Raju Thombare wrote:
> >Depends on SPI_MEM as well.
> 
> Ok
> 
> >I commented on this last time around as well. This does not look right
> >at all. A SPI MEM based driver should *not* need to know anything about
> >the subsystem driving it. That is the entire point of the API.
> >
> >The controller seems to be able to extract the read and write opcodes
> >from the SFDP on its own since you don't pass in that information to
> >cdns_xspi_nor_read(). It looks like it is tied very heavily to a NOR
> >flash, and I am not sure if it can really be used with a NAND flash, or
> >something else entirely.
> >
> >Which makes me wonder how we should handle controllers like these. I
> >don't think they fit in very well with the SPI MEM model, since they
> >can't execute arbitrary SPI MEM commands very well. At the same time we
> >are trying to get rid of mtd/spi-nor/controllers. Dunno...
> >
> >Mark, Tudor, Vignesh, any ideas?
> 
> Ok, then for now I will drop ACMD PIO mode and use only STIG mode.
> In STIG mode driver configures bus width and clock edge mode for
> command, address and data for each operation. 

But it would reduce performance by a lot, no? I think we should try to 
figure out how we can accomodate controllers like this before resorting 
to using the slower modes.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

RE: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Parshuram Raju Thombare]

>But it would reduce performance by a lot, no? I think we should try to
>figure out how we can accomodate controllers like this before resorting
>to using the slower modes.[>] 

I am not saying that functionality will be restricted to STIG mode.
But we can merge base driver with STIG mode for now and add other
functionalities/modes incrementally.
 
Regards,
Parshuram Thombare

Re: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Wed, Sep 08, 2021 at 04:51:15PM +0530, Pratyush Yadav wrote:

> But it would reduce performance by a lot, no? I think we should try to 
> figure out how we can accomodate controllers like this before resorting 
> to using the slower modes.

OTOH if it's going to be hard to figure out perhaps merging something
slower that works while we do so would be good - lets people use their
systems while things are figured out.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

[Pratyush Yadav]

On 08/09/21 01:24PM, Mark Brown wrote:
> On Wed, Sep 08, 2021 at 04:51:15PM +0530, Pratyush Yadav wrote:
> 
> > But it would reduce performance by a lot, no? I think we should try to 
> > figure out how we can accomodate controllers like this before resorting 
> > to using the slower modes.
> 
> OTOH if it's going to be hard to figure out perhaps merging something
> slower that works while we do so would be good - lets people use their
> systems while things are figured out.

I would prefer to solve this problem rather than leaving it for later, 
but either way is fine.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.