[PATCH 0/6] Tegra QUAD SPI combined sequence mode

[PATCH 0/6] Tegra QUAD SPI combined sequence mode

[Krishna Yarlagadda]

Add ACPI support for Tegra210 QUAD SPI driver and support
new Tegra194 feature, combined sequence mode.

Krishna Yarlagadda (6):
  spi: tegra210-quad: use device_reset method
  dt-bindings: spi: Tegra234 QUAD SPI compatible
  spi: tegra210-quad: add new chips to compatible
  spi: tegra210-quad: add acpi support
  dt-bindings: spi: Tegra QUAD SPI combined sequence
  spi: tegra210-quad: combined sequence mode

 .../bindings/spi/nvidia,tegra210-quad.yaml         |   9 +
 drivers/spi/spi-tegra210-quad.c                    | 313 +++++++++++++++++++--
 2 files changed, 291 insertions(+), 31 deletions(-)

-- 
2.7.4

[PATCH 1/6] spi: tegra210-quad: use device_reset method

[Krishna Yarlagadda]

Use device_reset api to replace duplicate code in driver to call
reset_control_get api with reset handle.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index ce1bdb4..ef93e40 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -137,7 +137,6 @@ struct tegra_qspi {
 	spinlock_t				lock;
 
 	struct clk				*clk;
-	struct reset_control			*rst;
 	void __iomem				*base;
 	phys_addr_t				phys;
 	unsigned int				irq;
@@ -948,9 +947,7 @@ static void tegra_qspi_handle_error(struct tegra_qspi *tqspi)
 	dev_err(tqspi->dev, "error in transfer, fifo status 0x%08x\n", tqspi->status_reg);
 	tegra_qspi_dump_regs(tqspi);
 	tegra_qspi_flush_fifos(tqspi, true);
-	reset_control_assert(tqspi->rst);
-	udelay(2);
-	reset_control_deassert(tqspi->rst);
+	device_reset(tqspi->dev);
 }
 
 static void tegra_qspi_transfer_end(struct spi_device *spi)
@@ -1249,13 +1246,6 @@ static int tegra_qspi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	tqspi->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
-	if (IS_ERR(tqspi->rst)) {
-		ret = PTR_ERR(tqspi->rst);
-		dev_err(&pdev->dev, "failed to get reset control: %d\n", ret);
-		return ret;
-	}
-
 	tqspi->max_buf_size = QSPI_FIFO_DEPTH << 2;
 	tqspi->dma_buf_size = DEFAULT_QSPI_DMA_BUF_LEN;
 
@@ -1277,9 +1267,7 @@ static int tegra_qspi_probe(struct platform_device *pdev)
 		goto exit_pm_disable;
 	}
 
-	reset_control_assert(tqspi->rst);
-	udelay(2);
-	reset_control_deassert(tqspi->rst);
+	device_reset(tqspi->dev);
 
 	tqspi->def_command1_reg = QSPI_M_S | QSPI_CS_SW_HW |  QSPI_CS_SW_VAL;
 	tegra_qspi_writel(tqspi, tqspi->def_command1_reg, QSPI_COMMAND1);
-- 
2.7.4

[PATCH 2/6] dt-bindings: spi: Tegra234 QUAD SPI compatible

[Krishna Yarlagadda]

Add compatible string for Tegra234 for Tegra QUAD SPI

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
index 35a8045..6efea89 100644
--- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
+++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
@@ -19,6 +19,7 @@ properties:
       - nvidia,tegra210-qspi
       - nvidia,tegra186-qspi
       - nvidia,tegra194-qspi
+      - nvidia,tegra234-qspi
 
   reg:
     maxItems: 1
-- 
2.7.4

Re: [PATCH 2/6] dt-bindings: spi: Tegra234 QUAD SPI compatible

[Rob Herring]

On Fri, 04 Feb 2022 15:59:32 +0530, Krishna Yarlagadda wrote:
> Add compatible string for Tegra234 for Tegra QUAD SPI
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/6] spi: Tegra234 QUAD SPI compatible

[Mark Brown]

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

On Fri, Feb 04, 2022 at 03:59:32PM +0530, Krishna Yarlagadda wrote:
> Add compatible string for Tegra234 for Tegra QUAD SPI
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>

This breaks an x86_64 allmodconfig build:

/mnt/kernel/drivers/spi/spi-tegra210-quad.c: In function 'tegra_qspi_probe':
/mnt/kernel/drivers/spi/spi-tegra210-quad.c:1322:2: error: ignoring return value of 'device_reset' declared with attribute 'warn_unused_result' [-Werror=unused-result]
 1322 |  device_reset(tqspi->dev);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~
/mnt/kernel/drivers/spi/spi-tegra210-quad.c: In function 'tegra_qspi_handle_error':
/mnt/kernel/drivers/spi/spi-tegra210-quad.c:957:2: error: ignoring return value of 'device_reset' declared with attribute 'warn_unused_result' [-Werror=unused-result]
  957 |  device_reset(tqspi->dev);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~


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

[PATCH 3/6] spi: tegra210-quad: add new chips to compatible

[Krishna Yarlagadda]

Add support for Tegra234 and soc data to select capabilities.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index ef93e40..2e5f20c 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -125,6 +125,10 @@
 #define QSPI_DMA_TIMEOUT			(msecs_to_jiffies(1000))
 #define DEFAULT_QSPI_DMA_BUF_LEN		(64 * 1024)
 
+struct tegra_qspi_soc_data {
+	bool has_dma;
+};
+
 struct tegra_qspi_client_data {
 	int tx_clk_tap_delay;
 	int rx_clk_tap_delay;
@@ -184,6 +188,7 @@ struct tegra_qspi {
 	u32					*tx_dma_buf;
 	dma_addr_t				tx_dma_phys;
 	struct dma_async_tx_descriptor		*tx_dma_desc;
+	const struct tegra_qspi_soc_data	*soc_data;
 };
 
 static inline u32 tegra_qspi_readl(struct tegra_qspi *tqspi, unsigned long offset)
@@ -1190,10 +1195,32 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
 	return handle_dma_based_xfer(tqspi);
 }
 
+static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
+	.has_dma = true,
+};
+
+static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
+	.has_dma = true,
+};
+
+static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
+	.has_dma = false,
+};
+
 static const struct of_device_id tegra_qspi_of_match[] = {
-	{ .compatible = "nvidia,tegra210-qspi", },
-	{ .compatible = "nvidia,tegra186-qspi", },
-	{ .compatible = "nvidia,tegra194-qspi", },
+	{
+		.compatible = "nvidia,tegra210-qspi",
+		.data	    = &tegra210_qspi_soc_data,
+	}, {
+		.compatible = "nvidia,tegra186-qspi",
+		.data	    = &tegra186_qspi_soc_data,
+	}, {
+		.compatible = "nvidia,tegra194-qspi",
+		.data	    = &tegra186_qspi_soc_data,
+	}, {
+		.compatible = "nvidia,tegra234-qspi",
+		.data	    = &tegra234_qspi_soc_data,
+	},
 	{}
 };
 
-- 
2.7.4

[PATCH 4/6] spi: tegra210-quad: add acpi support

[Krishna Yarlagadda]

Add ACPI ID for Tegra QUAD SPI. Switch to common device property calls.
Skip clock calls that are not updated in ACPI boot.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 50 ++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 2e5f20c..c83701b 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -21,6 +21,8 @@
 #include <linux/of_device.h>
 #include <linux/reset.h>
 #include <linux/spi/spi.h>
+#include <linux/acpi.h>
+#include <linux/property.h>
 
 #define QSPI_COMMAND1				0x000
 #define QSPI_BIT_LENGTH(x)			(((x) & 0x1f) << 0)
@@ -771,7 +773,7 @@ static u32 tegra_qspi_setup_transfer_one(struct spi_device *spi, struct spi_tran
 	u32 tx_tap = 0, rx_tap = 0;
 	int req_mode;
 
-	if (speed != tqspi->cur_speed) {
+	if (!has_acpi_companion(tqspi->dev) && speed != tqspi->cur_speed) {
 		clk_set_rate(tqspi->clk, speed);
 		tqspi->cur_speed = speed;
 	}
@@ -879,16 +881,16 @@ static int tegra_qspi_start_transfer_one(struct spi_device *spi,
 static struct tegra_qspi_client_data *tegra_qspi_parse_cdata_dt(struct spi_device *spi)
 {
 	struct tegra_qspi_client_data *cdata;
-	struct device_node *slave_np = spi->dev.of_node;
 
 	cdata = devm_kzalloc(&spi->dev, sizeof(*cdata), GFP_KERNEL);
 	if (!cdata)
 		return NULL;
 
-	of_property_read_u32(slave_np, "nvidia,tx-clk-tap-delay",
-			     &cdata->tx_clk_tap_delay);
-	of_property_read_u32(slave_np, "nvidia,rx-clk-tap-delay",
-			     &cdata->rx_clk_tap_delay);
+	device_property_read_u32(&spi->dev, "nvidia,tx-clk-tap-delay",
+				 &cdata->tx_clk_tap_delay);
+	device_property_read_u32(&spi->dev, "nvidia,rx-clk-tap-delay",
+				 &cdata->rx_clk_tap_delay);
+
 	return cdata;
 }
 
@@ -1226,6 +1228,24 @@ static const struct of_device_id tegra_qspi_of_match[] = {
 
 MODULE_DEVICE_TABLE(of, tegra_qspi_of_match);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id tegra_qspi_acpi_match[] = {
+	{
+		.id = "NVDA1213",
+		.driver_data = (kernel_ulong_t)&tegra210_qspi_soc_data,
+	}, {
+		.id = "NVDA1313",
+		.driver_data = (kernel_ulong_t)&tegra186_qspi_soc_data,
+	}, {
+		.id = "NVDA1413",
+		.driver_data = (kernel_ulong_t)&tegra234_qspi_soc_data,
+	},
+	{}
+};
+
+MODULE_DEVICE_TABLE(acpi, tegra_qspi_acpi_match);
+#endif
+
 static int tegra_qspi_probe(struct platform_device *pdev)
 {
 	struct spi_master	*master;
@@ -1266,11 +1286,14 @@ static int tegra_qspi_probe(struct platform_device *pdev)
 	qspi_irq = platform_get_irq(pdev, 0);
 	tqspi->irq = qspi_irq;
 
-	tqspi->clk = devm_clk_get(&pdev->dev, "qspi");
-	if (IS_ERR(tqspi->clk)) {
-		ret = PTR_ERR(tqspi->clk);
-		dev_err(&pdev->dev, "failed to get clock: %d\n", ret);
-		return ret;
+	if (!has_acpi_companion(tqspi->dev)) {
+		tqspi->clk = devm_clk_get(&pdev->dev, "qspi");
+		if (IS_ERR(tqspi->clk)) {
+			ret = PTR_ERR(tqspi->clk);
+			dev_err(&pdev->dev, "failed to get clock: %d\n", ret);
+			return ret;
+		}
+
 	}
 
 	tqspi->max_buf_size = QSPI_FIFO_DEPTH << 2;
@@ -1373,6 +1396,8 @@ static int __maybe_unused tegra_qspi_runtime_suspend(struct device *dev)
 	struct spi_master *master = dev_get_drvdata(dev);
 	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
 
+	if (has_acpi_companion(tqspi->dev))
+		return 0;
 	/* flush all write which are in PPSB queue by reading back */
 	tegra_qspi_readl(tqspi, QSPI_COMMAND1);
 
@@ -1387,6 +1412,8 @@ static int __maybe_unused tegra_qspi_runtime_resume(struct device *dev)
 	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
 	int ret;
 
+	if (has_acpi_companion(tqspi->dev))
+		return 0;
 	ret = clk_prepare_enable(tqspi->clk);
 	if (ret < 0)
 		dev_err(tqspi->dev, "failed to enable clock: %d\n", ret);
@@ -1404,6 +1431,7 @@ static struct platform_driver tegra_qspi_driver = {
 		.name		= "tegra-qspi",
 		.pm		= &tegra_qspi_pm_ops,
 		.of_match_table	= tegra_qspi_of_match,
+		.acpi_match_table = ACPI_PTR(tegra_qspi_acpi_match),
 	},
 	.probe =	tegra_qspi_probe,
 	.remove =	tegra_qspi_remove,
-- 
2.7.4

[PATCH 5/6] dt-bindings: spi: Tegra QUAD SPI combined sequence

[Krishna Yarlagadda]

Tegra194 and later chips support combined sequence mode which result
in less interrupts and better perf. This flag helps enable it.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
index 6efea89..3767059 100644
--- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
+++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
@@ -46,6 +46,14 @@ properties:
       - const: rx
       - const: tx
 
+  nvidia,cmb-xfer:
+    description:
+      Enable combined sequence transfers for read and program sequence
+      if supported by hardware. Tegra194 and later chips support this
+      feature. Default is non combined sequence. SPI message should
+      contain CMD-ADDR-DATA transfers to combine and send to hardware.
+    type: boolean
+
 patternProperties:
   "@[0-9a-f]+":
     type: object
-- 
2.7.4

Re: [PATCH 5/6] dt-bindings: spi: Tegra QUAD SPI combined sequence

[Mark Brown]

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

On Fri, Feb 04, 2022 at 03:59:35PM +0530, Krishna Yarlagadda wrote:

> +  nvidia,cmb-xfer:
> +    description:
> +      Enable combined sequence transfers for read and program sequence
> +      if supported by hardware. Tegra194 and later chips support this
> +      feature. Default is non combined sequence. SPI message should
> +      contain CMD-ADDR-DATA transfers to combine and send to hardware.
> +    type: boolean

Why is this a DT property - why would systems not wish to use this
feature if it is available?

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

[PATCH 6/6] spi: tegra210-quad: combined sequence mode

[Krishna Yarlagadda]

Add combined sequence mode supported by Tegra QSPI controller.
For commands which contain cmd, addr, data parts to it,controller
can accept all 3 transfers at once and xfer avoiding interrupt for each
transfer. This would improve read & write performance.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 216 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 212 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index c83701b..1c6cec8 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -121,19 +121,45 @@
 #define QSPI_NUM_DUMMY_CYCLE(x)			(((x) & 0xff) << 0)
 #define QSPI_DUMMY_CYCLES_MAX			0xff
 
+#define QSPI_CMB_SEQ_CMD			0x19c
+#define QSPI_COMMAND_VALUE_SET(X)		(((x) & 0xFF) << 0)
+
+#define QSPI_CMB_SEQ_CMD_CFG			0x1a0
+#define QSPI_COMMAND_X1_X2_X4(x)		(((x) & 0x3) << 13)
+#define QSPI_COMMAND_X1_X2_X4_MASK		(0x03 << 13)
+#define QSPI_COMMAND_SDR_DDR			BIT(12)
+#define QSPI_COMMAND_SIZE_SET(x)		(((x) & 0xFF) << 0)
+
+#define QSPI_GLOBAL_CONFIG			0X1a4
+#define QSPI_CMB_SEQ_EN				BIT(0)
+
+#define QSPI_CMB_SEQ_ADDR			0x1a8
+#define QSPI_ADDRESS_VALUE_SET(X)		(((x) & 0xFFFF) << 0)
+
+#define QSPI_CMB_SEQ_ADDR_CFG			0x1ac
+#define QSPI_ADDRESS_X1_X2_X4(x)		(((x) & 0x3) << 13)
+#define QSPI_ADDRESS_X1_X2_X4_MASK		(0x03 << 13)
+#define QSPI_ADDRESS_SDR_DDR			BIT(12)
+#define QSPI_ADDRESS_SIZE_SET(x)		(((x) & 0xFF) << 0)
+
 #define DATA_DIR_TX				BIT(0)
 #define DATA_DIR_RX				BIT(1)
 
 #define QSPI_DMA_TIMEOUT			(msecs_to_jiffies(1000))
 #define DEFAULT_QSPI_DMA_BUF_LEN		(64 * 1024)
+#define CMD_TRANSFER				0
+#define ADDR_TRANSFER				1
+#define DATA_TRANSFER				2
 
 struct tegra_qspi_soc_data {
 	bool has_dma;
+	bool cmb_xfer_capable;
 };
 
 struct tegra_qspi_client_data {
 	int tx_clk_tap_delay;
 	int rx_clk_tap_delay;
+	bool is_cmb_xfer;
 };
 
 struct tegra_qspi {
@@ -880,6 +906,7 @@ static int tegra_qspi_start_transfer_one(struct spi_device *spi,
 
 static struct tegra_qspi_client_data *tegra_qspi_parse_cdata_dt(struct spi_device *spi)
 {
+	struct tegra_qspi *tqspi = spi_master_get_devdata(spi->master);
 	struct tegra_qspi_client_data *cdata;
 
 	cdata = devm_kzalloc(&spi->dev, sizeof(*cdata), GFP_KERNEL);
@@ -890,6 +917,12 @@ static struct tegra_qspi_client_data *tegra_qspi_parse_cdata_dt(struct spi_devic
 				 &cdata->tx_clk_tap_delay);
 	device_property_read_u32(&spi->dev, "nvidia,rx-clk-tap-delay",
 				 &cdata->rx_clk_tap_delay);
+	if (tqspi->soc_data->cmb_xfer_capable)
+		cdata->is_cmb_xfer = device_property_read_bool
+					(&spi->dev,
+					 "nvidia,cmb-xfer");
+	else
+		cdata->is_cmb_xfer = false;
 
 	return cdata;
 }
@@ -912,7 +945,6 @@ static int tegra_qspi_setup(struct spi_device *spi)
 		cdata = tegra_qspi_parse_cdata_dt(spi);
 		spi->controller_data = cdata;
 	}
-
 	spin_lock_irqsave(&tqspi->lock, flags);
 
 	/* keep default cs state to inactive */
@@ -970,9 +1002,160 @@ static void tegra_qspi_transfer_end(struct spi_device *spi)
 	tegra_qspi_writel(tqspi, tqspi->def_command1_reg, QSPI_COMMAND1);
 }
 
-static int tegra_qspi_transfer_one_message(struct spi_master *master, struct spi_message *msg)
+static u32 tegra_qspi_cmd_config(bool is_ddr, u8 bus_width, u8 len)
+{
+	u32 cmd_config = 0;
+
+	/* Extract Command configuration and value */
+	if (is_ddr)
+		cmd_config |= QSPI_COMMAND_SDR_DDR;
+	else
+		cmd_config &= ~QSPI_COMMAND_SDR_DDR;
+
+	cmd_config |= QSPI_COMMAND_X1_X2_X4(bus_width);
+	cmd_config |= QSPI_COMMAND_SIZE_SET((len * 8) - 1);
+
+	return cmd_config;
+}
+
+static u32 tegra_qspi_addr_config(bool is_ddr, u8 bus_width, u8 len)
+{
+	u32 addr_config = 0;
+
+	/* Extract Address configuration and value */
+	is_ddr = 0; //Only SDR mode supported
+	bus_width = 0; //X1 mode
+
+	if (is_ddr)
+		addr_config |= QSPI_ADDRESS_SDR_DDR;
+	else
+		addr_config &= ~QSPI_ADDRESS_SDR_DDR;
+
+	addr_config |= QSPI_ADDRESS_X1_X2_X4(bus_width);
+	addr_config |= QSPI_ADDRESS_SIZE_SET((len * 8) - 1);
+
+	return addr_config;
+}
+
+static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
+					struct spi_message *msg)
+{
+	bool is_first_msg = true;
+	int single_xfer;
+	struct spi_transfer *xfer;
+	struct spi_device *spi = msg->spi;
+	u8 transfer_phase = 0;
+	u32 cmd1 = 0, dma_ctl = 0;
+	int ret;
+	u32 address_value = 0;
+	u32 cmd_config = 0, addr_config = 0;
+	u8 cmd_value = 0, len = 0, val = 0;
+
+	/* Enable Combined sequence mode */
+	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
+	val |= QSPI_CMB_SEQ_EN;
+	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
+	single_xfer = list_is_singular(&msg->transfers);
+	/* Process individual transfer list */
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		if (transfer_phase == CMD_TRANSFER) {
+			/* X1 SDR mode */
+			cmd_config = tegra_qspi_cmd_config(false, 0,
+							   xfer->len);
+			cmd_value = *((const u8 *)(xfer->tx_buf));
+
+		} else if (transfer_phase == ADDR_TRANSFER) {
+			len = xfer->len;
+			/* X1 SDR mode */
+			addr_config = tegra_qspi_addr_config(false, 0,
+							     xfer->len);
+			address_value = *((const u32 *)(xfer->tx_buf));
+		} else {
+			/* Program Command, Address value in register */
+			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
+			tegra_qspi_writel(tqspi, address_value,
+					  QSPI_CMB_SEQ_ADDR);
+			/* Program Command and Address config in register */
+			tegra_qspi_writel(tqspi, cmd_config,
+					  QSPI_CMB_SEQ_CMD_CFG);
+			tegra_qspi_writel(tqspi, addr_config,
+					  QSPI_CMB_SEQ_ADDR_CFG);
+
+			reinit_completion(&tqspi->xfer_completion);
+			cmd1 = tegra_qspi_setup_transfer_one(spi, xfer,
+							     is_first_msg);
+			ret = tegra_qspi_start_transfer_one(spi, xfer,
+							    cmd1);
+
+			if (ret < 0) {
+				dev_err(tqspi->dev, "Failed to start transfer-one: %d\n",
+					ret);
+				return ret;
+			}
+
+			is_first_msg = false;
+			ret = wait_for_completion_timeout
+					(&tqspi->xfer_completion,
+					QSPI_DMA_TIMEOUT);
+
+			if (WARN_ON(ret == 0)) {
+				dev_err(tqspi->dev, "QSPI Transfer failed with timeout: %d\n",
+					ret);
+				if (tqspi->is_curr_dma_xfer &&
+				    (tqspi->cur_direction & DATA_DIR_TX))
+					dmaengine_terminate_all
+						(tqspi->tx_dma_chan);
+
+				if (tqspi->is_curr_dma_xfer &&
+				    (tqspi->cur_direction & DATA_DIR_RX))
+					dmaengine_terminate_all
+						(tqspi->rx_dma_chan);
+
+				/* Abort transfer by resetting pio/dma bit */
+				if (!tqspi->is_curr_dma_xfer) {
+					cmd1 = tegra_qspi_readl
+							(tqspi,
+							 QSPI_COMMAND1);
+					cmd1 &= ~QSPI_PIO;
+					tegra_qspi_writel
+							(tqspi, cmd1,
+							 QSPI_COMMAND1);
+				} else {
+					dma_ctl = tegra_qspi_readl
+							(tqspi,
+							 QSPI_DMA_CTL);
+					dma_ctl &= ~QSPI_DMA_EN;
+					tegra_qspi_writel(tqspi, dma_ctl,
+							  QSPI_DMA_CTL);
+				}
+
+				/* Reset controller if timeout happens */
+				device_reset(tqspi->dev);
+				ret = -EIO;
+				goto exit;
+			}
+
+			if (tqspi->tx_status ||  tqspi->rx_status) {
+				dev_err(tqspi->dev, "QSPI Transfer failed\n");
+				tqspi->tx_status = 0;
+				tqspi->rx_status = 0;
+				ret = -EIO;
+				goto exit;
+			}
+		}
+		msg->actual_length += xfer->len;
+		transfer_phase++;
+	}
+
+exit:
+	msg->status = ret;
+
+	return ret;
+}
+
+static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
+					    struct spi_message *msg)
 {
-	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
 	struct spi_device *spi = msg->spi;
 	struct spi_transfer *transfer;
 	bool is_first_msg = true;
@@ -1020,7 +1203,6 @@ static int tegra_qspi_transfer_one_message(struct spi_master *master, struct spi
 			goto complete_xfer;
 		}
 
-		is_first_msg = false;
 		ret = wait_for_completion_timeout(&tqspi->xfer_completion,
 						  QSPI_DMA_TIMEOUT);
 		if (WARN_ON(ret == 0)) {
@@ -1065,7 +1247,29 @@ static int tegra_qspi_transfer_one_message(struct spi_master *master, struct spi
 	ret = 0;
 exit:
 	msg->status = ret;
+
+	return ret;
+}
+
+static int tegra_qspi_transfer_one_message(struct spi_master *master,
+					   struct spi_message *msg)
+{
+	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
+	struct tegra_qspi_client_data *cdata = msg->spi->controller_data;
+	int ret;
+	int transfer_count = 0;
+	struct spi_transfer *transfer;
+
+	list_for_each_entry(transfer, &msg->transfers, transfer_list) {
+		transfer_count++;
+	}
+	if (cdata->is_cmb_xfer && transfer_count == 3)
+		ret = tegra_qspi_combined_seq_xfer(tqspi, msg);
+	else
+		ret = tegra_qspi_non_combined_seq_xfer(tqspi, msg);
+
 	spi_finalize_current_message(master);
+
 	return ret;
 }
 
@@ -1199,14 +1403,17 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
 
 static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
 	.has_dma = true,
+	.cmb_xfer_capable = false,
 };
 
 static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
 	.has_dma = true,
+	.cmb_xfer_capable = true,
 };
 
 static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
 	.has_dma = false,
+	.cmb_xfer_capable = true,
 };
 
 static const struct of_device_id tegra_qspi_of_match[] = {
@@ -1277,6 +1484,7 @@ static int tegra_qspi_probe(struct platform_device *pdev)
 	tqspi->dev = &pdev->dev;
 	spin_lock_init(&tqspi->lock);
 
+	tqspi->soc_data = device_get_match_data(&pdev->dev);
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	tqspi->base = devm_ioremap_resource(&pdev->dev, r);
 	if (IS_ERR(tqspi->base))
-- 
2.7.4

Re: [PATCH 6/6] spi: tegra210-quad: combined sequence mode

[Mark Brown]

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

On Fri, Feb 04, 2022 at 03:59:36PM +0530, Krishna Yarlagadda wrote:

> +	/* Process individual transfer list */
> +	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> +		if (transfer_phase == CMD_TRANSFER) {

> +		} else if (transfer_phase == ADDR_TRANSFER) {

> +		} else {

Looks like you're writing a switch statement here...

> +			/* X1 SDR mode */
> +			cmd_config = tegra_qspi_cmd_config(false, 0,
> +							   xfer->len);
> +			cmd_value = *((const u8 *)(xfer->tx_buf));
> +
> +			len = xfer->len;

> +			/* X1 SDR mode */
> +			addr_config = tegra_qspi_addr_config(false, 0,
> +							     xfer->len);
> +			address_value = *((const u32 *)(xfer->tx_buf));

> +			/* Program Command, Address value in register */
> +			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
> +			tegra_qspi_writel(tqspi, address_value,
> +					  QSPI_CMB_SEQ_ADDR);
> +			/* Program Command and Address config in register */
> +			tegra_qspi_writel(tqspi, cmd_config,
> +					  QSPI_CMB_SEQ_CMD_CFG);
> +			tegra_qspi_writel(tqspi, addr_config,
> +					  QSPI_CMB_SEQ_ADDR_CFG);

It looks like the command and address have to be specific lengths?  If
that's the case then

> +	if (cdata->is_cmb_xfer && transfer_count == 3)
> +		ret = tegra_qspi_combined_seq_xfer(tqspi, msg);
> +	else
> +		ret = tegra_qspi_non_combined_seq_xfer(tqspi, msg);

This check needs to be more specific.  But like I said in reply to the
binding patch I don't see why we can't just pattern match on the data
without requiring a property here, we'd need to check that the message
is suitable no matter what.

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

RE: [PATCH 6/6] spi: tegra210-quad: combined sequence mode

[Krishna Yarlagadda]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 04 February 2022 19:39
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> linux-spi@vger.kernel.org; linux-tegra@vger.kernel.org; Sowjanya Komatineni
> <skomatineni@nvidia.com>; Laxman Dewangan <ldewangan@nvidia.com>;
> robh+dt@kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; p.zabel@pengutronix.de
> Subject: Re: [PATCH 6/6] spi: tegra210-quad: combined sequence mode
> 
> On Fri, Feb 04, 2022 at 03:59:36PM +0530, Krishna Yarlagadda wrote:
> 
> > +	/* Process individual transfer list */
> > +	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> > +		if (transfer_phase == CMD_TRANSFER) {
> 
> > +		} else if (transfer_phase == ADDR_TRANSFER) {
> 
> > +		} else {
> 
> Looks like you're writing a switch statement here...
Yes. This can be switch statement.
> 
> > +			/* X1 SDR mode */
> > +			cmd_config = tegra_qspi_cmd_config(false, 0,
> > +							   xfer->len);
> > +			cmd_value = *((const u8 *)(xfer->tx_buf));
> > +
> > +			len = xfer->len;
> 
> > +			/* X1 SDR mode */
> > +			addr_config = tegra_qspi_addr_config(false, 0,
> > +							     xfer->len);
> > +			address_value = *((const u32 *)(xfer->tx_buf));
> 
> > +			/* Program Command, Address value in register */
> > +			tegra_qspi_writel(tqspi, cmd_value,
> QSPI_CMB_SEQ_CMD);
> > +			tegra_qspi_writel(tqspi, address_value,
> > +					  QSPI_CMB_SEQ_ADDR);
> > +			/* Program Command and Address config in register
> */
> > +			tegra_qspi_writel(tqspi, cmd_config,
> > +					  QSPI_CMB_SEQ_CMD_CFG);
> > +			tegra_qspi_writel(tqspi, addr_config,
> > +					  QSPI_CMB_SEQ_ADDR_CFG);
> 
> It looks like the command and address have to be specific lengths?  If that's the
> case then
Cmd  and address are configurable to a limit. Will add min and max check.
> 
> > +	if (cdata->is_cmb_xfer && transfer_count == 3)
> > +		ret = tegra_qspi_combined_seq_xfer(tqspi, msg);
> > +	else
> > +		ret = tegra_qspi_non_combined_seq_xfer(tqspi, msg);
> 
> This check needs to be more specific.  But like I said in reply to the binding
> patch I don't see why we can't just pattern match on the data without requiring
> a property here, we'd need to check that the message is suitable no matter
> what.
There is no real-world use case we encountered so far preventing us stick to pattern.
But this was to avoid any corner case where there could be 3 different transfers sent in single msg.

Re: [PATCH 6/6] spi: tegra210-quad: combined sequence mode

[Mark Brown]

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

On Mon, Feb 07, 2022 at 02:54:00PM +0000, Krishna Yarlagadda wrote:

> > > +	if (cdata->is_cmb_xfer && transfer_count == 3)
> > > +		ret = tegra_qspi_combined_seq_xfer(tqspi, msg);
> > > +	else
> > > +		ret = tegra_qspi_non_combined_seq_xfer(tqspi, msg);

> > This check needs to be more specific.  But like I said in reply to the binding
> > patch I don't see why we can't just pattern match on the data without requiring
> > a property here, we'd need to check that the message is suitable no matter
> > what.

> There is no real-world use case we encountered so far preventing us stick to pattern.
> But this was to avoid any corner case where there could be 3 different transfers sent in single msg.

So you'll remove the property and just pattern match on the message?

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

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

RE: [PATCH 6/6] spi: tegra210-quad: combined sequence mode

[Krishna Yarlagadda]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 07 February 2022 20:36
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> linux-spi@vger.kernel.org; linux-tegra@vger.kernel.org; Sowjanya
> Komatineni <skomatineni@nvidia.com>; Laxman Dewangan
> <ldewangan@nvidia.com>; robh+dt@kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> p.zabel@pengutronix.de
> Subject: Re: [PATCH 6/6] spi: tegra210-quad: combined sequence mode
> 
> On Mon, Feb 07, 2022 at 02:54:00PM +0000, Krishna Yarlagadda wrote:
> 
> > > > +	if (cdata->is_cmb_xfer && transfer_count == 3)
> > > > +		ret = tegra_qspi_combined_seq_xfer(tqspi, msg);
> > > > +	else
> > > > +		ret = tegra_qspi_non_combined_seq_xfer(tqspi, msg);
> 
> > > This check needs to be more specific.  But like I said in reply to
> > > the binding patch I don't see why we can't just pattern match on the
> > > data without requiring a property here, we'd need to check that the
> > > message is suitable no matter what.
> 
> > There is no real-world use case we encountered so far preventing us stick
> to pattern.
> > But this was to avoid any corner case where there could be 3 different
> transfers sent in single msg.
> 
> So you'll remove the property and just pattern match on the message?
Yes. I will send out V2 without property.
> 
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much
> easier to read and reply to.
Sorry. Fixed mail client now.

Re: (subset) [PATCH 0/6] Tegra QUAD SPI combined sequence mode

[Mark Brown]

On Fri, 4 Feb 2022 15:59:30 +0530, Krishna Yarlagadda wrote:
> Add ACPI support for Tegra210 QUAD SPI driver and support
> new Tegra194 feature, combined sequence mode.
> 
> Krishna Yarlagadda (6):
>   spi: tegra210-quad: use device_reset method
>   dt-bindings: spi: Tegra234 QUAD SPI compatible
>   spi: tegra210-quad: add new chips to compatible
>   spi: tegra210-quad: add acpi support
>   dt-bindings: spi: Tegra QUAD SPI combined sequence
>   spi: tegra210-quad: combined sequence mode
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/6] spi: tegra210-quad: use device_reset method
      commit: ac982578e7d340dc4f4fd243f4a4b24787d28c3f
[2/6] dt-bindings: spi: Tegra234 QUAD SPI compatible
      (no commit info)
[3/6] spi: tegra210-quad: add new chips to compatible
      commit: ea23f0e148b82e5bcbc6c814926f53133552f0f3
[4/6] spi: tegra210-quad: add acpi support
      (no commit info)

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark