[PATCH 0/6] Configure Clocks, Add Native Dma support.

[PATCH 0/6] Configure Clocks, Add Native Dma support.

[Vishwaroop A]

This patch series configures qspi clocks, fixes combined sequence
programming and introduces native dma support. 

Vishwaroop A (6):
  arm64: tegra: Configure QSPI clocks and add DMA
  spi: tegra210-quad: Update dummy sequence configuration
  spi: tegra210-quad: Fix X1_X2_X4 encoding and support x4 transfers
  spi: tegra210-quad: remove redundant error handling code
  spi: tegra210-quad: modify chip select (CS) deactivation
  spi: tegra210-quad: Introduce native DMA support

 arch/arm64/boot/dts/nvidia/tegra234.dtsi |  14 ++
 drivers/spi/spi-tegra210-quad.c          | 273 +++++++++++++----------
 2 files changed, 174 insertions(+), 113 deletions(-)

-- 
2.17.1

[PATCH V1 1/6] arm64: tegra: Configure QSPI clocks and add DMA

[Vishwaroop A]

Set QSPI0_2X_PM to 199.99 MHz and QSPI0_PM to 99.99 MHz using
PLLC as the parent clock. These frequencies allow Quad IO DT
reads up to 99.99 MHz, which is the fastest that can be
achieved considering various PLL and clock divider constraints.

Populate the DMA and IOMMU properties for the Tegra234 QSPI devices to
enable DMA support.

Change-Id: I1dded904aa8e0f278c89998481e829f1ce474e8c
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 984c85eab41a..96d0f13390ae 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -2948,6 +2948,13 @@
 				 <&bpmp TEGRA234_CLK_QSPI0_PM>;
 			clock-names = "qspi", "qspi_out";
 			resets = <&bpmp TEGRA234_RESET_QSPI0>;
+			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI0_2X_PM>,
+					  <&bpmp TEGRA234_CLK_QSPI0_PM>;
+			assigned-clock-rates = <199999999 99999999>;
+			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
+			dma-names = "rx", "tx";
+			dma-coherent;
+			iommus = <&smmu_niso1 TEGRA234_SID_QSPI0>;
 			status = "disabled";
 		};
 
@@ -3031,6 +3038,13 @@
 				 <&bpmp TEGRA234_CLK_QSPI1_PM>;
 			clock-names = "qspi", "qspi_out";
 			resets = <&bpmp TEGRA234_RESET_QSPI1>;
+			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI1_2X_PM>,
+					  <&bpmp TEGRA234_CLK_QSPI1_PM>;
+			assigned-clock-rates = <199999999 99999999>;
+			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
+			dma-names = "rx", "tx";
+			dma-coherent;
+			iommus = <&smmu_niso1 TEGRA234_SID_QSPI1>;
 			status = "disabled";
 		};
 
-- 
2.17.1

[PATCH V1 2/6] spi: tegra210-quad: Update dummy sequence configuration

[Vishwaroop A]

Adding support for the dummy sequence configuration. The dummy sequence
introduces a delay between the command and the data phases of a
transfer. This delay, measured in clock cycles, allows the slave
device to prepare for data transmission, ensuring data integrity and
proper synchronization.

Change-Id: I4dc347a247830452754f83e88aa95a7d231722cd
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 08e49a876894..02478e8efc8f 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -22,6 +22,7 @@
 #include <linux/spi/spi.h>
 #include <linux/acpi.h>
 #include <linux/property.h>
+#include <linux/sizes.h>
 
 #define QSPI_COMMAND1				0x000
 #define QSPI_BIT_LENGTH(x)			(((x) & 0x1f) << 0)
@@ -156,10 +157,14 @@
 #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
+#define DEFAULT_QSPI_DMA_BUF_LEN		SZ_64K
+
+enum tegra_qspi_transfer_type {
+	CMD_TRANSFER   = 0,
+	ADDR_TRANSFER  = 1,
+	DUMMY_TRANSFER = 2,
+	DATA_TRANSFER  = 3
+};
 
 struct tegra_qspi_soc_data {
 	bool has_dma;
@@ -1089,6 +1094,13 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 							     xfer->len);
 			address_value = *((const u32 *)(xfer->tx_buf));
 			break;
+		case DUMMY_TRANSFER:
+			if (xfer->dummy_data) {
+				tqspi->dummy_cycles = xfer->len * 8 / xfer->tx_nbits;
+				break;
+			}
+			transfer_phase++;
+			fallthrough;
 		case DATA_TRANSFER:
 			/* Program Command, Address value in register */
 			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
@@ -1300,7 +1312,9 @@ static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		transfer_count++;
 	}
-	if (!tqspi->soc_data->cmb_xfer_capable || transfer_count != 3)
+	if (!tqspi->soc_data->cmb_xfer_capable)
+		return false;
+	if (transfer_count > 4 || transfer_count < 3)
 		return false;
 	xfer = list_first_entry(&msg->transfers, typeof(*xfer),
 				transfer_list);
@@ -1310,6 +1324,13 @@ static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
 	if (xfer->len > 4 || xfer->len < 3)
 		return false;
 	xfer = list_next_entry(xfer, transfer_list);
+	if (transfer_count == 4) {
+		if (xfer->dummy_data != 1)
+			return false;
+		if ((xfer->len * 8 / xfer->tx_nbits) > QSPI_DUMMY_CYCLES_MAX)
+			return false;
+		xfer = list_next_entry(xfer, transfer_list);
+	}
 	if (!tqspi->soc_data->has_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
 		return false;
 
-- 
2.17.1

[PATCH V1 3/6] spi: tegra210-quad: Fix X1_X2_X4 encoding and support x4 transfers

[Vishwaroop A]

This patch corrects the QSPI_COMMAND_X1_X2_X4 and QSPI_ADDRESS_X1_X2_X4
macros to properly encode the bus width for x1, x2, and x4 transfers.
Although these macros were previously incorrect, they were not being
used in the driver, so no functionality was affected.

The patch updates tegra_qspi_cmd_config() and tegra_qspi_addr_config()
function calls to use the actual bus width from the transfer, instead of
hardcoding it to 0 (which implied x1 mode). This change enables proper
support for x1, x2, and x4 data transfers by correctly configuring the
interface width for commands and addresses.

These modifications improve the QSPI driver's flexibility and prepare it
for future use cases that may require different bus widths for commands
and addresses.

Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")

Change-Id: Ic650c919535a99aa02f52c25e2a200d1b301381e
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 02478e8efc8f..43d6587fad09 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -135,7 +135,7 @@
 #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(x)		((((x) >> 1) & 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)
@@ -148,7 +148,7 @@
 #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(x)		((((x) >> 1) & 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)
@@ -1041,10 +1041,6 @@ 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
@@ -1084,13 +1080,13 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 		switch (transfer_phase) {
 		case CMD_TRANSFER:
 			/* X1 SDR mode */
-			cmd_config = tegra_qspi_cmd_config(false, 0,
+			cmd_config = tegra_qspi_cmd_config(false, xfer->tx_nbits,
 							   xfer->len);
 			cmd_value = *((const u8 *)(xfer->tx_buf));
 			break;
 		case ADDR_TRANSFER:
 			/* X1 SDR mode */
-			addr_config = tegra_qspi_addr_config(false, 0,
+			addr_config = tegra_qspi_addr_config(false, xfer->tx_nbits,
 							     xfer->len);
 			address_value = *((const u32 *)(xfer->tx_buf));
 			break;
-- 
2.17.1

[PATCH V1 4/6] spi: tegra210-quad: remove redundant error handling code

[Vishwaroop A]

Remove unnecessary error handling code that terminated transfers and
executed delay on errors. This code was redundant as error handling is
already done at a higher level in the SPI core.

Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")

Change-Id: I9e77732db64d7a1674b7e87048c7b59d8dbab645
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 43d6587fad09..2d7a2e3da337 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1187,10 +1187,6 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 
 exit:
 	msg->status = ret;
-	if (ret < 0) {
-		tegra_qspi_transfer_end(spi);
-		spi_transfer_delay_exec(xfer);
-	}
 
 	return ret;
 }
-- 
2.17.1

[PATCH V1 5/6] spi: tegra210-quad: modify chip select (CS) deactivation

[Vishwaroop A]

Modify the chip select (CS) deactivation and inter-transfer delay
execution to be performed only during the DATA_TRANSFER phase when
the cs_change flag is not set. This ensures proper CS handling and
timing between transfers while eliminating redundant operations.

Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")

Change-Id: I58670873bee222311e3c2251fad98d7f28cdca14
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 2d7a2e3da337..04f41e92c1e2 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1171,16 +1171,16 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 				ret = -EIO;
 				goto exit;
 			}
-			if (!xfer->cs_change) {
-				tegra_qspi_transfer_end(spi);
-				spi_transfer_delay_exec(xfer);
-			}
 			break;
 		default:
 			ret = -EINVAL;
 			goto exit;
 		}
 		msg->actual_length += xfer->len;
+		if (!xfer->cs_change && transfer_phase == DATA_TRANSFER) {
+			tegra_qspi_transfer_end(spi);
+			spi_transfer_delay_exec(xfer);
+		}
 		transfer_phase++;
 	}
 	ret = 0;
-- 
2.17.1

[PATCH V1 6/6] spi: tegra210-quad: Introduce native DMA support

[Vishwaroop A]

Previous generations of Tegra supported DMA operations by an external
DMA controller, but the QSPI on Tegra234 devices now have an internal
DMA controller.

Internal DMA: Uses the QSPI controller's built-in DMA engine, which is
limited in capabilities and tied directly to the QSPI module.

External DMA: Utilizes a separate, GPCDMA DMA controller that can
transfer data between QSPI and any memory location.

Native DMA Initialization: Introduce routines to initialize and
configure native DMA channels for both transmit and receive paths.
Set up DMA mapping functions to manage buffer addresses effectively.

Enhance Transfer Logic: Implement logic to choose between CPU-based
and DMA-based transfers based on data size.

Change-Id: Icf3ef4767947cef67821c092ecd9ea6bccb2a4e4
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 218 ++++++++++++++++++--------------
 1 file changed, 126 insertions(+), 92 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 04f41e92c1e2..066caee85c52 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -111,6 +111,9 @@
 #define QSPI_DMA_BLK				0x024
 #define QSPI_DMA_BLK_SET(x)			(((x) & 0xffff) << 0)
 
+#define QSPI_DMA_MEM_ADDRESS_REG		0x028
+#define QSPI_DMA_HI_ADDRESS_REG			0x02c
+
 #define QSPI_TX_FIFO				0x108
 #define QSPI_RX_FIFO				0x188
 
@@ -167,9 +170,9 @@ enum tegra_qspi_transfer_type {
 };
 
 struct tegra_qspi_soc_data {
-	bool has_dma;
 	bool cmb_xfer_capable;
 	bool supports_tpm;
+	bool has_ext_dma;
 	unsigned int cs_count;
 };
 
@@ -605,17 +608,21 @@ static void tegra_qspi_dma_unmap_xfer(struct tegra_qspi *tqspi, struct spi_trans
 
 	len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
 
-	dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
-	dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
+	if (t->tx_buf)
+		dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
+	if (t->rx_buf)
+		dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
 }
 
 static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
 {
 	struct dma_slave_config dma_sconfig = { 0 };
+	dma_addr_t rx_dma_phys, tx_dma_phys;
 	unsigned int len;
 	u8 dma_burst;
 	int ret = 0;
 	u32 val;
+	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
 
 	if (tqspi->is_packed) {
 		ret = tegra_qspi_dma_map_xfer(tqspi, t);
@@ -634,60 +641,85 @@ static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct
 		len = tqspi->curr_dma_words * 4;
 
 	/* set attention level based on length of transfer */
-	val = 0;
-	if (len & 0xf) {
-		val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
-		dma_burst = 1;
-	} else if (((len) >> 4) & 0x1) {
-		val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
-		dma_burst = 4;
-	} else {
-		val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
-		dma_burst = 8;
+	if (has_ext_dma) {
+		val = 0;
+		if (len & 0xf) {
+			val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
+			dma_burst = 1;
+		} else if (((len) >> 4) & 0x1) {
+			val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
+			dma_burst = 4;
+		} else {
+			val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
+			dma_burst = 8;
+		}
+
+		tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
 	}
 
-	tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
 	tqspi->dma_control_reg = val;
 
 	dma_sconfig.device_fc = true;
-	if (tqspi->cur_direction & DATA_DIR_TX) {
-		dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
-		dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-		dma_sconfig.dst_maxburst = dma_burst;
-		ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
-			return ret;
-		}
 
-		tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
-		ret = tegra_qspi_start_tx_dma(tqspi, t, len);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
-			return ret;
+	if ((tqspi->cur_direction & DATA_DIR_TX)) {
+		if (has_ext_dma) {
+			dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
+			dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+			dma_sconfig.dst_maxburst = dma_burst;
+			ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
+				return ret;
+			}
+
+			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
+			ret = tegra_qspi_start_tx_dma(tqspi, t, len);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
+				return ret;
+			}
+		} else {
+			if (tqspi->is_packed)
+				tx_dma_phys = t->tx_dma;
+			else
+				tx_dma_phys = tqspi->tx_dma_phys;
+			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
+			tegra_qspi_writel(tqspi, lower_32_bits(tx_dma_phys),
+					  QSPI_DMA_MEM_ADDRESS_REG);
+			tegra_qspi_writel(tqspi, (upper_32_bits(tx_dma_phys) & 0xff),
+					  QSPI_DMA_HI_ADDRESS_REG);
 		}
 	}
 
 	if (tqspi->cur_direction & DATA_DIR_RX) {
-		dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
-		dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-		dma_sconfig.src_maxburst = dma_burst;
-		ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
-			return ret;
-		}
-
-		dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
-					   tqspi->dma_buf_size,
-					   DMA_FROM_DEVICE);
-
-		ret = tegra_qspi_start_rx_dma(tqspi, t, len);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
-			if (tqspi->cur_direction & DATA_DIR_TX)
-				dmaengine_terminate_all(tqspi->tx_dma_chan);
-			return ret;
+		if (has_ext_dma) {
+			dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
+			dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+			dma_sconfig.src_maxburst = dma_burst;
+			ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
+				return ret;
+			}
+			dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
+						   tqspi->dma_buf_size, DMA_FROM_DEVICE);
+			ret = tegra_qspi_start_rx_dma(tqspi, t, len);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
+				if (tqspi->cur_direction & DATA_DIR_TX)
+					dmaengine_terminate_all(tqspi->tx_dma_chan);
+				return ret;
+			}
+		} else {
+			if (tqspi->is_packed)
+				rx_dma_phys = t->rx_dma;
+			else
+				rx_dma_phys = tqspi->rx_dma_phys;
+
+			tegra_qspi_writel(tqspi, (rx_dma_phys & 0xffffffff),
+					  QSPI_DMA_MEM_ADDRESS_REG);
+			tegra_qspi_writel(tqspi, ((rx_dma_phys >> 32) & 0xff),
+					  QSPI_DMA_HI_ADDRESS_REG);
 		}
 	}
 
@@ -726,9 +758,6 @@ static int tegra_qspi_start_cpu_based_transfer(struct tegra_qspi *qspi, struct s
 
 static void tegra_qspi_deinit_dma(struct tegra_qspi *tqspi)
 {
-	if (!tqspi->soc_data->has_dma)
-		return;
-
 	if (tqspi->tx_dma_buf) {
 		dma_free_coherent(tqspi->dev, tqspi->dma_buf_size,
 				  tqspi->tx_dma_buf, tqspi->tx_dma_phys);
@@ -759,16 +788,26 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
 	u32 *dma_buf;
 	int err;
 
-	if (!tqspi->soc_data->has_dma)
-		return 0;
+	if (tqspi->soc_data->has_ext_dma) {
+		dma_chan = dma_request_chan(tqspi->dev, "rx");
+		if (IS_ERR(dma_chan)) {
+			err = PTR_ERR(dma_chan);
+			goto err_out;
+		}
 
-	dma_chan = dma_request_chan(tqspi->dev, "rx");
-	if (IS_ERR(dma_chan)) {
-		err = PTR_ERR(dma_chan);
-		goto err_out;
-	}
+		tqspi->rx_dma_chan = dma_chan;
 
-	tqspi->rx_dma_chan = dma_chan;
+		dma_chan = dma_request_chan(tqspi->dev, "tx");
+		if (IS_ERR(dma_chan)) {
+			err = PTR_ERR(dma_chan);
+			goto err_out;
+		}
+
+		tqspi->tx_dma_chan = dma_chan;
+	} else {
+		tqspi->rx_dma_chan = NULL;
+		tqspi->tx_dma_chan = NULL;
+	}
 
 	dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
 	if (!dma_buf) {
@@ -779,14 +818,6 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
 	tqspi->rx_dma_buf = dma_buf;
 	tqspi->rx_dma_phys = dma_phys;
 
-	dma_chan = dma_request_chan(tqspi->dev, "tx");
-	if (IS_ERR(dma_chan)) {
-		err = PTR_ERR(dma_chan);
-		goto err_out;
-	}
-
-	tqspi->tx_dma_chan = dma_chan;
-
 	dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
 	if (!dma_buf) {
 		err = -ENOMEM;
@@ -1056,6 +1087,7 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 					struct spi_message *msg)
 {
 	bool is_first_msg = true;
+	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
 	struct spi_transfer *xfer;
 	struct spi_device *spi = msg->spi;
 	u8 transfer_phase = 0;
@@ -1128,15 +1160,12 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 			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);
+				if (tqspi->is_curr_dma_xfer && has_ext_dma) {
+					if (tqspi->cur_direction & DATA_DIR_TX)
+						dmaengine_terminate_all(tqspi->tx_dma_chan);
+					if (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) {
@@ -1197,6 +1226,7 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
 	struct spi_device *spi = msg->spi;
 	struct spi_transfer *transfer;
 	bool is_first_msg = true;
+	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
 	int ret = 0, val = 0;
 
 	msg->status = 0;
@@ -1251,10 +1281,12 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
 						  QSPI_DMA_TIMEOUT);
 		if (WARN_ON(ret == 0)) {
 			dev_err(tqspi->dev, "transfer timeout\n");
-			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);
+			if (tqspi->is_curr_dma_xfer && has_ext_dma) {
+				if (tqspi->cur_direction & DATA_DIR_TX)
+					dmaengine_terminate_all(tqspi->tx_dma_chan);
+				if (tqspi->cur_direction & DATA_DIR_RX)
+					dmaengine_terminate_all(tqspi->rx_dma_chan);
+			}
 			tegra_qspi_handle_error(tqspi);
 			ret = -EIO;
 			goto complete_xfer;
@@ -1323,7 +1355,7 @@ static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
 			return false;
 		xfer = list_next_entry(xfer, transfer_list);
 	}
-	if (!tqspi->soc_data->has_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
+	if (!tqspi->soc_data->has_ext_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
 		return false;
 
 	return true;
@@ -1388,30 +1420,32 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
 
 	if (tqspi->cur_direction & DATA_DIR_TX) {
 		if (tqspi->tx_status) {
-			dmaengine_terminate_all(tqspi->tx_dma_chan);
-			err += 1;
-		} else {
+			if (tqspi->tx_dma_chan)
+				dmaengine_terminate_all(tqspi->tx_dma_chan);
+			err++;
+		} else if (tqspi->tx_dma_chan) {
 			wait_status = wait_for_completion_interruptible_timeout(
 				&tqspi->tx_dma_complete, QSPI_DMA_TIMEOUT);
 			if (wait_status <= 0) {
 				dmaengine_terminate_all(tqspi->tx_dma_chan);
 				dev_err(tqspi->dev, "failed TX DMA transfer\n");
-				err += 1;
+				err++;
 			}
 		}
 	}
 
 	if (tqspi->cur_direction & DATA_DIR_RX) {
 		if (tqspi->rx_status) {
-			dmaengine_terminate_all(tqspi->rx_dma_chan);
-			err += 2;
-		} else {
+			if (tqspi->rx_dma_chan)
+				dmaengine_terminate_all(tqspi->rx_dma_chan);
+			err++;
+		} else if (tqspi->rx_dma_chan) {
 			wait_status = wait_for_completion_interruptible_timeout(
 				&tqspi->rx_dma_complete, QSPI_DMA_TIMEOUT);
 			if (wait_status <= 0) {
 				dmaengine_terminate_all(tqspi->rx_dma_chan);
 				dev_err(tqspi->dev, "failed RX DMA transfer\n");
-				err += 2;
+				err++;
 			}
 		}
 	}
@@ -1474,28 +1508,28 @@ 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,
+	.has_ext_dma = true,
 	.cmb_xfer_capable = false,
 	.supports_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
-	.has_dma = true,
+	.has_ext_dma = true,
 	.cmb_xfer_capable = true,
 	.supports_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
-	.has_dma = false,
+	.has_ext_dma = false,
 	.cmb_xfer_capable = true,
 	.supports_tpm = true,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
-	.has_dma = false,
+	.has_ext_dma = true,
 	.cmb_xfer_capable = true,
 	.supports_tpm = true,
 	.cs_count = 4,
-- 
2.17.1

Re: [PATCH V1 6/6] spi: tegra210-quad: Introduce native DMA support

[kernel test robot]

Hi Vishwaroop,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on robh/for-next broonie-sound/for-next linus/master v6.13-rc5 next-20241220]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vishwaroop-A/arm64-tegra-Configure-QSPI-clocks-and-add-DMA/20250103-141217
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20250103060407.1064107-7-va%40nvidia.com
patch subject: [PATCH V1 6/6] spi: tegra210-quad: Introduce native DMA support
config: sparc-randconfig-002-20250103 (https://download.01.org/0day-ci/archive/20250103/202501032202.m6t4wlQB-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250103/202501032202.m6t4wlQB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501032202.m6t4wlQB-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/spi/spi-tegra210-quad.c: In function 'tegra_qspi_start_dma_based_transfer':
>> drivers/spi/spi-tegra210-quad.c:721:64: warning: right shift count >= width of type [-Wshift-count-overflow]
     721 |                         tegra_qspi_writel(tqspi, ((rx_dma_phys >> 32) & 0xff),
         |                                                                ^~


vim +721 drivers/spi/spi-tegra210-quad.c

   616	
   617	static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
   618	{
   619		struct dma_slave_config dma_sconfig = { 0 };
   620		dma_addr_t rx_dma_phys, tx_dma_phys;
   621		unsigned int len;
   622		u8 dma_burst;
   623		int ret = 0;
   624		u32 val;
   625		bool has_ext_dma = tqspi->soc_data->has_ext_dma;
   626	
   627		if (tqspi->is_packed) {
   628			ret = tegra_qspi_dma_map_xfer(tqspi, t);
   629			if (ret < 0)
   630				return ret;
   631		}
   632	
   633		val = QSPI_DMA_BLK_SET(tqspi->curr_dma_words - 1);
   634		tegra_qspi_writel(tqspi, val, QSPI_DMA_BLK);
   635	
   636		tegra_qspi_unmask_irq(tqspi);
   637	
   638		if (tqspi->is_packed)
   639			len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
   640		else
   641			len = tqspi->curr_dma_words * 4;
   642	
   643		/* set attention level based on length of transfer */
   644		if (has_ext_dma) {
   645			val = 0;
   646			if (len & 0xf) {
   647				val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
   648				dma_burst = 1;
   649			} else if (((len) >> 4) & 0x1) {
   650				val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
   651				dma_burst = 4;
   652			} else {
   653				val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
   654				dma_burst = 8;
   655			}
   656	
   657			tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
   658		}
   659	
   660		tqspi->dma_control_reg = val;
   661	
   662		dma_sconfig.device_fc = true;
   663	
   664		if ((tqspi->cur_direction & DATA_DIR_TX)) {
   665			if (has_ext_dma) {
   666				dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
   667				dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
   668				dma_sconfig.dst_maxburst = dma_burst;
   669				ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
   670				if (ret < 0) {
   671					dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
   672					return ret;
   673				}
   674	
   675				tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
   676				ret = tegra_qspi_start_tx_dma(tqspi, t, len);
   677				if (ret < 0) {
   678					dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
   679					return ret;
   680				}
   681			} else {
   682				if (tqspi->is_packed)
   683					tx_dma_phys = t->tx_dma;
   684				else
   685					tx_dma_phys = tqspi->tx_dma_phys;
   686				tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
   687				tegra_qspi_writel(tqspi, lower_32_bits(tx_dma_phys),
   688						  QSPI_DMA_MEM_ADDRESS_REG);
   689				tegra_qspi_writel(tqspi, (upper_32_bits(tx_dma_phys) & 0xff),
   690						  QSPI_DMA_HI_ADDRESS_REG);
   691			}
   692		}
   693	
   694		if (tqspi->cur_direction & DATA_DIR_RX) {
   695			if (has_ext_dma) {
   696				dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
   697				dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
   698				dma_sconfig.src_maxburst = dma_burst;
   699				ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
   700				if (ret < 0) {
   701					dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
   702					return ret;
   703				}
   704				dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
   705							   tqspi->dma_buf_size, DMA_FROM_DEVICE);
   706				ret = tegra_qspi_start_rx_dma(tqspi, t, len);
   707				if (ret < 0) {
   708					dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
   709					if (tqspi->cur_direction & DATA_DIR_TX)
   710						dmaengine_terminate_all(tqspi->tx_dma_chan);
   711					return ret;
   712				}
   713			} else {
   714				if (tqspi->is_packed)
   715					rx_dma_phys = t->rx_dma;
   716				else
   717					rx_dma_phys = tqspi->rx_dma_phys;
   718	
   719				tegra_qspi_writel(tqspi, (rx_dma_phys & 0xffffffff),
   720						  QSPI_DMA_MEM_ADDRESS_REG);
 > 721				tegra_qspi_writel(tqspi, ((rx_dma_phys >> 32) & 0xff),
   722						  QSPI_DMA_HI_ADDRESS_REG);
   723			}
   724		}
   725	
   726		tegra_qspi_writel(tqspi, tqspi->command1_reg, QSPI_COMMAND1);
   727	
   728		tqspi->is_curr_dma_xfer = true;
   729		tqspi->dma_control_reg = val;
   730		val |= QSPI_DMA_EN;
   731		tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
   732	
   733		return ret;
   734	}
   735	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH V1 6/6] spi: tegra210-quad: Introduce native DMA support

[kernel test robot]

Hi Vishwaroop,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on robh/for-next broonie-sound/for-next linus/master v6.13-rc5 next-20241220]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vishwaroop-A/arm64-tegra-Configure-QSPI-clocks-and-add-DMA/20250103-141217
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20250103060407.1064107-7-va%40nvidia.com
patch subject: [PATCH V1 6/6] spi: tegra210-quad: Introduce native DMA support
config: arm-randconfig-001-20250104 (https://download.01.org/0day-ci/archive/20250104/202501040605.Ndat3QJw-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 096551537b2a747a3387726ca618ceeb3950e9bc)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250104/202501040605.Ndat3QJw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501040605.Ndat3QJw-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/spi/spi-tegra210-quad.c:8:
   In file included from include/linux/dmaengine.h:12:
   In file included from include/linux/scatterlist.h:8:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/spi/spi-tegra210-quad.c:721:43: warning: shift count >= width of type [-Wshift-count-overflow]
     721 |                         tegra_qspi_writel(tqspi, ((rx_dma_phys >> 32) & 0xff),
         |                                                                ^  ~~
   2 warnings generated.


vim +721 drivers/spi/spi-tegra210-quad.c

   616	
   617	static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
   618	{
   619		struct dma_slave_config dma_sconfig = { 0 };
   620		dma_addr_t rx_dma_phys, tx_dma_phys;
   621		unsigned int len;
   622		u8 dma_burst;
   623		int ret = 0;
   624		u32 val;
   625		bool has_ext_dma = tqspi->soc_data->has_ext_dma;
   626	
   627		if (tqspi->is_packed) {
   628			ret = tegra_qspi_dma_map_xfer(tqspi, t);
   629			if (ret < 0)
   630				return ret;
   631		}
   632	
   633		val = QSPI_DMA_BLK_SET(tqspi->curr_dma_words - 1);
   634		tegra_qspi_writel(tqspi, val, QSPI_DMA_BLK);
   635	
   636		tegra_qspi_unmask_irq(tqspi);
   637	
   638		if (tqspi->is_packed)
   639			len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
   640		else
   641			len = tqspi->curr_dma_words * 4;
   642	
   643		/* set attention level based on length of transfer */
   644		if (has_ext_dma) {
   645			val = 0;
   646			if (len & 0xf) {
   647				val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
   648				dma_burst = 1;
   649			} else if (((len) >> 4) & 0x1) {
   650				val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
   651				dma_burst = 4;
   652			} else {
   653				val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
   654				dma_burst = 8;
   655			}
   656	
   657			tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
   658		}
   659	
   660		tqspi->dma_control_reg = val;
   661	
   662		dma_sconfig.device_fc = true;
   663	
   664		if ((tqspi->cur_direction & DATA_DIR_TX)) {
   665			if (has_ext_dma) {
   666				dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
   667				dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
   668				dma_sconfig.dst_maxburst = dma_burst;
   669				ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
   670				if (ret < 0) {
   671					dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
   672					return ret;
   673				}
   674	
   675				tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
   676				ret = tegra_qspi_start_tx_dma(tqspi, t, len);
   677				if (ret < 0) {
   678					dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
   679					return ret;
   680				}
   681			} else {
   682				if (tqspi->is_packed)
   683					tx_dma_phys = t->tx_dma;
   684				else
   685					tx_dma_phys = tqspi->tx_dma_phys;
   686				tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
   687				tegra_qspi_writel(tqspi, lower_32_bits(tx_dma_phys),
   688						  QSPI_DMA_MEM_ADDRESS_REG);
   689				tegra_qspi_writel(tqspi, (upper_32_bits(tx_dma_phys) & 0xff),
   690						  QSPI_DMA_HI_ADDRESS_REG);
   691			}
   692		}
   693	
   694		if (tqspi->cur_direction & DATA_DIR_RX) {
   695			if (has_ext_dma) {
   696				dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
   697				dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
   698				dma_sconfig.src_maxburst = dma_burst;
   699				ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
   700				if (ret < 0) {
   701					dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
   702					return ret;
   703				}
   704				dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
   705							   tqspi->dma_buf_size, DMA_FROM_DEVICE);
   706				ret = tegra_qspi_start_rx_dma(tqspi, t, len);
   707				if (ret < 0) {
   708					dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
   709					if (tqspi->cur_direction & DATA_DIR_TX)
   710						dmaengine_terminate_all(tqspi->tx_dma_chan);
   711					return ret;
   712				}
   713			} else {
   714				if (tqspi->is_packed)
   715					rx_dma_phys = t->rx_dma;
   716				else
   717					rx_dma_phys = tqspi->rx_dma_phys;
   718	
   719				tegra_qspi_writel(tqspi, (rx_dma_phys & 0xffffffff),
   720						  QSPI_DMA_MEM_ADDRESS_REG);
 > 721				tegra_qspi_writel(tqspi, ((rx_dma_phys >> 32) & 0xff),
   722						  QSPI_DMA_HI_ADDRESS_REG);
   723			}
   724		}
   725	
   726		tegra_qspi_writel(tqspi, tqspi->command1_reg, QSPI_COMMAND1);
   727	
   728		tqspi->is_curr_dma_xfer = true;
   729		tqspi->dma_control_reg = val;
   730		val |= QSPI_DMA_EN;
   731		tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
   732	
   733		return ret;
   734	}
   735	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH V1 6/6] spi: tegra210-quad: Introduce native DMA support

[Mark Brown]

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

On Fri, Jan 03, 2025 at 06:04:07AM +0000, Vishwaroop A wrote:

> Change-Id: Icf3ef4767947cef67821c092ecd9ea6bccb2a4e4

Please don't include internal only things like change IDs in upstream
submissions.

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

Re: [PATCH V1 1/6] arm64: tegra: Configure QSPI clocks and add DMA

[Thierry Reding]

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

On Fri, Jan 03, 2025 at 06:04:02AM +0000, Vishwaroop A wrote:
> Set QSPI0_2X_PM to 199.99 MHz and QSPI0_PM to 99.99 MHz using
> PLLC as the parent clock. These frequencies allow Quad IO DT
> reads up to 99.99 MHz, which is the fastest that can be
> achieved considering various PLL and clock divider constraints.
> 
> Populate the DMA and IOMMU properties for the Tegra234 QSPI devices to
> enable DMA support.
> 
> Change-Id: I1dded904aa8e0f278c89998481e829f1ce474e8c
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 984c85eab41a..96d0f13390ae 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -2948,6 +2948,13 @@
>  				 <&bpmp TEGRA234_CLK_QSPI0_PM>;
>  			clock-names = "qspi", "qspi_out";
>  			resets = <&bpmp TEGRA234_RESET_QSPI0>;
> +			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI0_2X_PM>,
> +					  <&bpmp TEGRA234_CLK_QSPI0_PM>;
> +			assigned-clock-rates = <199999999 99999999>;
> +			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
> +			dma-names = "rx", "tx";
> +			dma-coherent;
> +			iommus = <&smmu_niso1 TEGRA234_SID_QSPI0>;
>  			status = "disabled";
>  		};
>  
> @@ -3031,6 +3038,13 @@
>  				 <&bpmp TEGRA234_CLK_QSPI1_PM>;
>  			clock-names = "qspi", "qspi_out";
>  			resets = <&bpmp TEGRA234_RESET_QSPI1>;
> +			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI1_2X_PM>,
> +					  <&bpmp TEGRA234_CLK_QSPI1_PM>;
> +			assigned-clock-rates = <199999999 99999999>;
> +			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
> +			dma-names = "rx", "tx";
> +			dma-coherent;
> +			iommus = <&smmu_niso1 TEGRA234_SID_QSPI1>;
>  			status = "disabled";
>  		};
>  

It looks like these are missing the "dmas" properties that go along with
"dma-names".

Thierry

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

Re: [PATCH V1 1/6] arm64: tegra: Configure QSPI clocks and add DMA

[Vishwaroop A]

________________________________________
From: Thierry Reding
Sent: Thursday, January 09, 2025 16:10
To: Vishwaroop A
Cc: Jon Hunter; Sowjanya Komatineni; Laxman Dewangan; broonie@kernel.org; linux-spi@vger.kernel.org; linux-tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Krishna Yarlagadda; Suresh Mangipudi
Subject: Re: [PATCH V1 1/6] arm64: tegra: Configure QSPI clocks and add DMA

On Fri, Jan 03, 2025 at 06:04:02AM +0000, Vishwaroop A wrote:
> Set QSPI0_2X_PM to 199.99 MHz and QSPI0_PM to 99.99 MHz using
> PLLC as the parent clock. These frequencies allow Quad IO DT
> reads up to 99.99 MHz, which is the fastest that can be
> achieved considering various PLL and clock divider constraints.
>
> Populate the DMA and IOMMU properties for the Tegra234 QSPI devices to
> enable DMA support.
>
> Change-Id: I1dded904aa8e0f278c89998481e829f1ce474e8c
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 984c85eab41a..96d0f13390ae 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -2948,6 +2948,13 @@
>                                <&bpmp TEGRA234_CLK_QSPI0_PM>;
>                       clock-names = "qspi", "qspi_out";
>                       resets = <&bpmp TEGRA234_RESET_QSPI0>;
> +                     assigned-clocks = <&bpmp TEGRA234_CLK_QSPI0_2X_PM>,
> +                                       <&bpmp TEGRA234_CLK_QSPI0_PM>;
> +                     assigned-clock-rates = <199999999 99999999>;
> +                     assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
> +                     dma-names = "rx", "tx";
> +                     dma-coherent;
> +                     iommus = <&smmu_niso1 TEGRA234_SID_QSPI0>;
>                       status = "disabled";
>               };
>
> @@ -3031,6 +3038,13 @@
>                                <&bpmp TEGRA234_CLK_QSPI1_PM>;
>                       clock-names = "qspi", "qspi_out";
>                       resets = <&bpmp TEGRA234_RESET_QSPI1>;
> +                     assigned-clocks = <&bpmp TEGRA234_CLK_QSPI1_2X_PM>,
> +                                       <&bpmp TEGRA234_CLK_QSPI1_PM>;
> +                     assigned-clock-rates = <199999999 99999999>;
> +                     assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
> +                     dma-names = "rx", "tx";
> +                     dma-coherent;
> +                     iommus = <&smmu_niso1 TEGRA234_SID_QSPI1>;
>                       status = "disabled";
>               };
>

>> It looks like these are missing the "dmas" properties that go along with "dma-names".
[Vishwaroop A] dmas property is not required as QSPI uses native dma engine. dmas property is used for assigning  the dma channels. In case of QSPI it has own native DMA engine.  

Thanks,
Vishwaroop

Thierry

[PATCH v2 0/6] Configure Clocks, Add Native Dma support.

[Vishwaroop A]

This patch series configures qspi clocks, fixes combined sequence
programming and introduces native dma support.

Vishwaroop A (6):
  arm64: tegra: Configure QSPI clocks and add DMA
  spi: tegra210-quad: Update dummy sequence configuration
  spi: tegra210-quad: Fix X1_X2_X4 encoding and support x4 transfers
  spi: tegra210-quad: remove redundant error handling code
  spi: tegra210-quad: modify chip select (CS) deactivation
  spi: tegra210-quad: Introduce native DMA support

 arch/arm64/boot/dts/nvidia/tegra234.dtsi |  14 ++
 drivers/spi/spi-tegra210-quad.c          | 273 +++++++++++++----------
 2 files changed, 174 insertions(+), 113 deletions(-)

---
v1 -> v2:
	* Removed Change-IDs from the patches.
        * Addressed kernel test bot warnings.
---
-- 
2.17.1

[PATCH v2 1/6] arm64: tegra: Configure QSPI clocks and add DMA

[Vishwaroop A]

Set QSPI0_2X_PM to 199.99 MHz and QSPI0_PM to 99.99 MHz using
PLLC as the parent clock. These frequencies allow Quad IO DT
reads up to 99.99 MHz, which is the fastest that can be
achieved considering various PLL and clock divider constraints.

Populate the DMA and IOMMU properties for the Tegra234 QSPI devices to
enable DMA support.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 2601b43b2d8c..0ac2d3aba930 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -2948,6 +2948,13 @@
 				 <&bpmp TEGRA234_CLK_QSPI0_PM>;
 			clock-names = "qspi", "qspi_out";
 			resets = <&bpmp TEGRA234_RESET_QSPI0>;
+			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI0_2X_PM>,
+					  <&bpmp TEGRA234_CLK_QSPI0_PM>;
+			assigned-clock-rates = <199999999 99999999>;
+			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
+			dma-names = "rx", "tx";
+			dma-coherent;
+			iommus = <&smmu_niso1 TEGRA234_SID_QSPI0>;
 			status = "disabled";
 		};
 
@@ -3031,6 +3038,13 @@
 				 <&bpmp TEGRA234_CLK_QSPI1_PM>;
 			clock-names = "qspi", "qspi_out";
 			resets = <&bpmp TEGRA234_RESET_QSPI1>;
+			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI1_2X_PM>,
+					  <&bpmp TEGRA234_CLK_QSPI1_PM>;
+			assigned-clock-rates = <199999999 99999999>;
+			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
+			dma-names = "rx", "tx";
+			dma-coherent;
+			iommus = <&smmu_niso1 TEGRA234_SID_QSPI1>;
 			status = "disabled";
 		};
 
-- 
2.17.1

[PATCH v2 2/6] spi: tegra210-quad: Update dummy sequence configuration

[Vishwaroop A]

Adding support for the dummy sequence configuration. The dummy sequence
introduces a delay between the command and the data phases of a
transfer. This delay, measured in clock cycles, allows the slave
device to prepare for data transmission, ensuring data integrity and
proper synchronization.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 08e49a876894..02478e8efc8f 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -22,6 +22,7 @@
 #include <linux/spi/spi.h>
 #include <linux/acpi.h>
 #include <linux/property.h>
+#include <linux/sizes.h>
 
 #define QSPI_COMMAND1				0x000
 #define QSPI_BIT_LENGTH(x)			(((x) & 0x1f) << 0)
@@ -156,10 +157,14 @@
 #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
+#define DEFAULT_QSPI_DMA_BUF_LEN		SZ_64K
+
+enum tegra_qspi_transfer_type {
+	CMD_TRANSFER   = 0,
+	ADDR_TRANSFER  = 1,
+	DUMMY_TRANSFER = 2,
+	DATA_TRANSFER  = 3
+};
 
 struct tegra_qspi_soc_data {
 	bool has_dma;
@@ -1089,6 +1094,13 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 							     xfer->len);
 			address_value = *((const u32 *)(xfer->tx_buf));
 			break;
+		case DUMMY_TRANSFER:
+			if (xfer->dummy_data) {
+				tqspi->dummy_cycles = xfer->len * 8 / xfer->tx_nbits;
+				break;
+			}
+			transfer_phase++;
+			fallthrough;
 		case DATA_TRANSFER:
 			/* Program Command, Address value in register */
 			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
@@ -1300,7 +1312,9 @@ static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		transfer_count++;
 	}
-	if (!tqspi->soc_data->cmb_xfer_capable || transfer_count != 3)
+	if (!tqspi->soc_data->cmb_xfer_capable)
+		return false;
+	if (transfer_count > 4 || transfer_count < 3)
 		return false;
 	xfer = list_first_entry(&msg->transfers, typeof(*xfer),
 				transfer_list);
@@ -1310,6 +1324,13 @@ static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
 	if (xfer->len > 4 || xfer->len < 3)
 		return false;
 	xfer = list_next_entry(xfer, transfer_list);
+	if (transfer_count == 4) {
+		if (xfer->dummy_data != 1)
+			return false;
+		if ((xfer->len * 8 / xfer->tx_nbits) > QSPI_DUMMY_CYCLES_MAX)
+			return false;
+		xfer = list_next_entry(xfer, transfer_list);
+	}
 	if (!tqspi->soc_data->has_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
 		return false;
 
-- 
2.17.1

[PATCH v2 3/6] spi: tegra210-quad: Fix X1_X2_X4 encoding and support x4 transfers

[Vishwaroop A]

This patch corrects the QSPI_COMMAND_X1_X2_X4 and QSPI_ADDRESS_X1_X2_X4
macros to properly encode the bus width for x1, x2, and x4 transfers.
Although these macros were previously incorrect, they were not being
used in the driver, so no functionality was affected.

The patch updates tegra_qspi_cmd_config() and tegra_qspi_addr_config()
function calls to use the actual bus width from the transfer, instead of
hardcoding it to 0 (which implied x1 mode). This change enables proper
support for x1, x2, and x4 data transfers by correctly configuring the
interface width for commands and addresses.

These modifications improve the QSPI driver's flexibility and prepare it
for future use cases that may require different bus widths for commands
and addresses.

Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 02478e8efc8f..43d6587fad09 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -135,7 +135,7 @@
 #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(x)		((((x) >> 1) & 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)
@@ -148,7 +148,7 @@
 #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(x)		((((x) >> 1) & 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)
@@ -1041,10 +1041,6 @@ 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
@@ -1084,13 +1080,13 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 		switch (transfer_phase) {
 		case CMD_TRANSFER:
 			/* X1 SDR mode */
-			cmd_config = tegra_qspi_cmd_config(false, 0,
+			cmd_config = tegra_qspi_cmd_config(false, xfer->tx_nbits,
 							   xfer->len);
 			cmd_value = *((const u8 *)(xfer->tx_buf));
 			break;
 		case ADDR_TRANSFER:
 			/* X1 SDR mode */
-			addr_config = tegra_qspi_addr_config(false, 0,
+			addr_config = tegra_qspi_addr_config(false, xfer->tx_nbits,
 							     xfer->len);
 			address_value = *((const u32 *)(xfer->tx_buf));
 			break;
-- 
2.17.1

[PATCH v2 4/6] spi: tegra210-quad: remove redundant error handling code

[Vishwaroop A]

Remove unnecessary error handling code that terminated transfers and
executed delay on errors. This code was redundant as error handling is
already done at a higher level in the SPI core.

Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 43d6587fad09..2d7a2e3da337 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1187,10 +1187,6 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 
 exit:
 	msg->status = ret;
-	if (ret < 0) {
-		tegra_qspi_transfer_end(spi);
-		spi_transfer_delay_exec(xfer);
-	}
 
 	return ret;
 }
-- 
2.17.1

[PATCH v2 5/6] spi: tegra210-quad: modify chip select (CS) deactivation

[Vishwaroop A]

Modify the chip select (CS) deactivation and inter-transfer delay
execution to be performed only during the DATA_TRANSFER phase when
the cs_change flag is not set. This ensures proper CS handling and
timing between transfers while eliminating redundant operations.

Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 2d7a2e3da337..04f41e92c1e2 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1171,16 +1171,16 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 				ret = -EIO;
 				goto exit;
 			}
-			if (!xfer->cs_change) {
-				tegra_qspi_transfer_end(spi);
-				spi_transfer_delay_exec(xfer);
-			}
 			break;
 		default:
 			ret = -EINVAL;
 			goto exit;
 		}
 		msg->actual_length += xfer->len;
+		if (!xfer->cs_change && transfer_phase == DATA_TRANSFER) {
+			tegra_qspi_transfer_end(spi);
+			spi_transfer_delay_exec(xfer);
+		}
 		transfer_phase++;
 	}
 	ret = 0;
-- 
2.17.1

[PATCH v2 6/6] spi: tegra210-quad: Introduce native DMA support

[Vishwaroop A]

Previous generations of Tegra supported DMA operations by an external
DMA controller, but the QSPI on Tegra234 devices now have an internal
DMA controller.

Internal DMA: Uses the QSPI controller's built-in DMA engine, which is
limited in capabilities and tied directly to the QSPI module.

External DMA: Utilizes a separate, GPCDMA DMA controller that can
transfer data between QSPI and any memory location.

Native DMA Initialization: Introduce routines to initialize and
configure native DMA channels for both transmit and receive paths.
Set up DMA mapping functions to manage buffer addresses effectively.

Enhance Transfer Logic: Implement logic to choose between CPU-based
and DMA-based transfers based on data size.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 218 ++++++++++++++++++--------------
 1 file changed, 126 insertions(+), 92 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 04f41e92c1e2..7463b00b1ffb 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -111,6 +111,9 @@
 #define QSPI_DMA_BLK				0x024
 #define QSPI_DMA_BLK_SET(x)			(((x) & 0xffff) << 0)
 
+#define QSPI_DMA_MEM_ADDRESS_REG		0x028
+#define QSPI_DMA_HI_ADDRESS_REG			0x02c
+
 #define QSPI_TX_FIFO				0x108
 #define QSPI_RX_FIFO				0x188
 
@@ -167,9 +170,9 @@ enum tegra_qspi_transfer_type {
 };
 
 struct tegra_qspi_soc_data {
-	bool has_dma;
 	bool cmb_xfer_capable;
 	bool supports_tpm;
+	bool has_ext_dma;
 	unsigned int cs_count;
 };
 
@@ -605,17 +608,21 @@ static void tegra_qspi_dma_unmap_xfer(struct tegra_qspi *tqspi, struct spi_trans
 
 	len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
 
-	dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
-	dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
+	if (t->tx_buf)
+		dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
+	if (t->rx_buf)
+		dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
 }
 
 static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
 {
 	struct dma_slave_config dma_sconfig = { 0 };
+	dma_addr_t rx_dma_phys, tx_dma_phys;
 	unsigned int len;
 	u8 dma_burst;
 	int ret = 0;
 	u32 val;
+	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
 
 	if (tqspi->is_packed) {
 		ret = tegra_qspi_dma_map_xfer(tqspi, t);
@@ -634,60 +641,85 @@ static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct
 		len = tqspi->curr_dma_words * 4;
 
 	/* set attention level based on length of transfer */
-	val = 0;
-	if (len & 0xf) {
-		val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
-		dma_burst = 1;
-	} else if (((len) >> 4) & 0x1) {
-		val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
-		dma_burst = 4;
-	} else {
-		val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
-		dma_burst = 8;
+	if (has_ext_dma) {
+		val = 0;
+		if (len & 0xf) {
+			val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
+			dma_burst = 1;
+		} else if (((len) >> 4) & 0x1) {
+			val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
+			dma_burst = 4;
+		} else {
+			val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
+			dma_burst = 8;
+		}
+
+		tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
 	}
 
-	tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
 	tqspi->dma_control_reg = val;
 
 	dma_sconfig.device_fc = true;
-	if (tqspi->cur_direction & DATA_DIR_TX) {
-		dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
-		dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-		dma_sconfig.dst_maxburst = dma_burst;
-		ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
-			return ret;
-		}
 
-		tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
-		ret = tegra_qspi_start_tx_dma(tqspi, t, len);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
-			return ret;
+	if ((tqspi->cur_direction & DATA_DIR_TX)) {
+		if (has_ext_dma) {
+			dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
+			dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+			dma_sconfig.dst_maxburst = dma_burst;
+			ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
+				return ret;
+			}
+
+			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
+			ret = tegra_qspi_start_tx_dma(tqspi, t, len);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
+				return ret;
+			}
+		} else {
+			if (tqspi->is_packed)
+				tx_dma_phys = t->tx_dma;
+			else
+				tx_dma_phys = tqspi->tx_dma_phys;
+			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
+			tegra_qspi_writel(tqspi, lower_32_bits(tx_dma_phys),
+					  QSPI_DMA_MEM_ADDRESS_REG);
+			tegra_qspi_writel(tqspi, (upper_32_bits(tx_dma_phys) & 0xff),
+					  QSPI_DMA_HI_ADDRESS_REG);
 		}
 	}
 
 	if (tqspi->cur_direction & DATA_DIR_RX) {
-		dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
-		dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-		dma_sconfig.src_maxburst = dma_burst;
-		ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
-			return ret;
-		}
-
-		dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
-					   tqspi->dma_buf_size,
-					   DMA_FROM_DEVICE);
-
-		ret = tegra_qspi_start_rx_dma(tqspi, t, len);
-		if (ret < 0) {
-			dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
-			if (tqspi->cur_direction & DATA_DIR_TX)
-				dmaengine_terminate_all(tqspi->tx_dma_chan);
-			return ret;
+		if (has_ext_dma) {
+			dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
+			dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+			dma_sconfig.src_maxburst = dma_burst;
+			ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
+				return ret;
+			}
+			dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
+						   tqspi->dma_buf_size, DMA_FROM_DEVICE);
+			ret = tegra_qspi_start_rx_dma(tqspi, t, len);
+			if (ret < 0) {
+				dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
+				if (tqspi->cur_direction & DATA_DIR_TX)
+					dmaengine_terminate_all(tqspi->tx_dma_chan);
+				return ret;
+			}
+		} else {
+			if (tqspi->is_packed)
+				rx_dma_phys = t->rx_dma;
+			else
+				rx_dma_phys = tqspi->rx_dma_phys;
+
+			tegra_qspi_writel(tqspi, lower_32_bits(rx_dma_phys),
+					  QSPI_DMA_MEM_ADDRESS_REG);
+			tegra_qspi_writel(tqspi, (upper_32_bits(rx_dma_phys) & 0xff),
+					  QSPI_DMA_HI_ADDRESS_REG);
 		}
 	}
 
@@ -726,9 +758,6 @@ static int tegra_qspi_start_cpu_based_transfer(struct tegra_qspi *qspi, struct s
 
 static void tegra_qspi_deinit_dma(struct tegra_qspi *tqspi)
 {
-	if (!tqspi->soc_data->has_dma)
-		return;
-
 	if (tqspi->tx_dma_buf) {
 		dma_free_coherent(tqspi->dev, tqspi->dma_buf_size,
 				  tqspi->tx_dma_buf, tqspi->tx_dma_phys);
@@ -759,16 +788,26 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
 	u32 *dma_buf;
 	int err;
 
-	if (!tqspi->soc_data->has_dma)
-		return 0;
+	if (tqspi->soc_data->has_ext_dma) {
+		dma_chan = dma_request_chan(tqspi->dev, "rx");
+		if (IS_ERR(dma_chan)) {
+			err = PTR_ERR(dma_chan);
+			goto err_out;
+		}
 
-	dma_chan = dma_request_chan(tqspi->dev, "rx");
-	if (IS_ERR(dma_chan)) {
-		err = PTR_ERR(dma_chan);
-		goto err_out;
-	}
+		tqspi->rx_dma_chan = dma_chan;
 
-	tqspi->rx_dma_chan = dma_chan;
+		dma_chan = dma_request_chan(tqspi->dev, "tx");
+		if (IS_ERR(dma_chan)) {
+			err = PTR_ERR(dma_chan);
+			goto err_out;
+		}
+
+		tqspi->tx_dma_chan = dma_chan;
+	} else {
+		tqspi->rx_dma_chan = NULL;
+		tqspi->tx_dma_chan = NULL;
+	}
 
 	dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
 	if (!dma_buf) {
@@ -779,14 +818,6 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
 	tqspi->rx_dma_buf = dma_buf;
 	tqspi->rx_dma_phys = dma_phys;
 
-	dma_chan = dma_request_chan(tqspi->dev, "tx");
-	if (IS_ERR(dma_chan)) {
-		err = PTR_ERR(dma_chan);
-		goto err_out;
-	}
-
-	tqspi->tx_dma_chan = dma_chan;
-
 	dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
 	if (!dma_buf) {
 		err = -ENOMEM;
@@ -1056,6 +1087,7 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 					struct spi_message *msg)
 {
 	bool is_first_msg = true;
+	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
 	struct spi_transfer *xfer;
 	struct spi_device *spi = msg->spi;
 	u8 transfer_phase = 0;
@@ -1128,15 +1160,12 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 			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);
+				if (tqspi->is_curr_dma_xfer && has_ext_dma) {
+					if (tqspi->cur_direction & DATA_DIR_TX)
+						dmaengine_terminate_all(tqspi->tx_dma_chan);
+					if (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) {
@@ -1197,6 +1226,7 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
 	struct spi_device *spi = msg->spi;
 	struct spi_transfer *transfer;
 	bool is_first_msg = true;
+	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
 	int ret = 0, val = 0;
 
 	msg->status = 0;
@@ -1251,10 +1281,12 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
 						  QSPI_DMA_TIMEOUT);
 		if (WARN_ON(ret == 0)) {
 			dev_err(tqspi->dev, "transfer timeout\n");
-			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);
+			if (tqspi->is_curr_dma_xfer && has_ext_dma) {
+				if (tqspi->cur_direction & DATA_DIR_TX)
+					dmaengine_terminate_all(tqspi->tx_dma_chan);
+				if (tqspi->cur_direction & DATA_DIR_RX)
+					dmaengine_terminate_all(tqspi->rx_dma_chan);
+			}
 			tegra_qspi_handle_error(tqspi);
 			ret = -EIO;
 			goto complete_xfer;
@@ -1323,7 +1355,7 @@ static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
 			return false;
 		xfer = list_next_entry(xfer, transfer_list);
 	}
-	if (!tqspi->soc_data->has_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
+	if (!tqspi->soc_data->has_ext_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
 		return false;
 
 	return true;
@@ -1388,30 +1420,32 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
 
 	if (tqspi->cur_direction & DATA_DIR_TX) {
 		if (tqspi->tx_status) {
-			dmaengine_terminate_all(tqspi->tx_dma_chan);
-			err += 1;
-		} else {
+			if (tqspi->tx_dma_chan)
+				dmaengine_terminate_all(tqspi->tx_dma_chan);
+			err++;
+		} else if (tqspi->tx_dma_chan) {
 			wait_status = wait_for_completion_interruptible_timeout(
 				&tqspi->tx_dma_complete, QSPI_DMA_TIMEOUT);
 			if (wait_status <= 0) {
 				dmaengine_terminate_all(tqspi->tx_dma_chan);
 				dev_err(tqspi->dev, "failed TX DMA transfer\n");
-				err += 1;
+				err++;
 			}
 		}
 	}
 
 	if (tqspi->cur_direction & DATA_DIR_RX) {
 		if (tqspi->rx_status) {
-			dmaengine_terminate_all(tqspi->rx_dma_chan);
-			err += 2;
-		} else {
+			if (tqspi->rx_dma_chan)
+				dmaengine_terminate_all(tqspi->rx_dma_chan);
+			err++;
+		} else if (tqspi->rx_dma_chan) {
 			wait_status = wait_for_completion_interruptible_timeout(
 				&tqspi->rx_dma_complete, QSPI_DMA_TIMEOUT);
 			if (wait_status <= 0) {
 				dmaengine_terminate_all(tqspi->rx_dma_chan);
 				dev_err(tqspi->dev, "failed RX DMA transfer\n");
-				err += 2;
+				err++;
 			}
 		}
 	}
@@ -1474,28 +1508,28 @@ 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,
+	.has_ext_dma = true,
 	.cmb_xfer_capable = false,
 	.supports_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
-	.has_dma = true,
+	.has_ext_dma = true,
 	.cmb_xfer_capable = true,
 	.supports_tpm = false,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
-	.has_dma = false,
+	.has_ext_dma = false,
 	.cmb_xfer_capable = true,
 	.supports_tpm = true,
 	.cs_count = 1,
 };
 
 static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
-	.has_dma = false,
+	.has_ext_dma = true,
 	.cmb_xfer_capable = true,
 	.supports_tpm = true,
 	.cs_count = 4,
-- 
2.17.1

Re: [PATCH v2 0/6] Configure Clocks, Add Native Dma support.

[Rob Herring (Arm)]

On Wed, 12 Feb 2025 14:46:45 +0000, Vishwaroop A wrote:
> This patch series configures qspi clocks, fixes combined sequence
> programming and introduces native dma support.
> 
> Vishwaroop A (6):
>   arm64: tegra: Configure QSPI clocks and add DMA
>   spi: tegra210-quad: Update dummy sequence configuration
>   spi: tegra210-quad: Fix X1_X2_X4 encoding and support x4 transfers
>   spi: tegra210-quad: remove redundant error handling code
>   spi: tegra210-quad: modify chip select (CS) deactivation
>   spi: tegra210-quad: Introduce native DMA support
> 
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi |  14 ++
>  drivers/spi/spi-tegra210-quad.c          | 273 +++++++++++++----------
>  2 files changed, 174 insertions(+), 113 deletions(-)
> 
> ---
> v1 -> v2:
> 	* Removed Change-IDs from the patches.
>         * Addressed kernel test bot warnings.
> ---
> --
> 2.17.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/nvidia/' for 20250212144651.2433086-1-va@nvidia.com:

arch/arm64/boot/dts/nvidia/tegra234-sim-vdk.dtb: spi@3270000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0000.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0000.dtb: spi@3270000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0008.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0008.dtb: spi@3270000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-sim-vdk.dtb: spi@3300000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0005.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dtb: spi@3270000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0005.dtb: spi@3270000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3740-0002+p3701-0008.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0000.dtb: spi@3300000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3740-0002+p3701-0008.dtb: spi@3270000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0008.dtb: spi@3300000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dtb: spi@3300000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0005.dtb: spi@3300000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#
arch/arm64/boot/dts/nvidia/tegra234-p3740-0002+p3701-0008.dtb: spi@3300000: 'dmas' is a dependency of 'dma-names'
	from schema $id: http://devicetree.org/schemas/dma/dma.yaml#

Re: [PATCH v2 1/6] arm64: tegra: Configure QSPI clocks and add DMA

[Thierry Reding]

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

On Wed, Feb 12, 2025 at 02:46:46PM +0000, Vishwaroop A wrote:
> Set QSPI0_2X_PM to 199.99 MHz and QSPI0_PM to 99.99 MHz using
> PLLC as the parent clock. These frequencies allow Quad IO DT
> reads up to 99.99 MHz, which is the fastest that can be
> achieved considering various PLL and clock divider constraints.
> 
> Populate the DMA and IOMMU properties for the Tegra234 QSPI devices to
> enable DMA support.
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 2601b43b2d8c..0ac2d3aba930 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -2948,6 +2948,13 @@
>  				 <&bpmp TEGRA234_CLK_QSPI0_PM>;
>  			clock-names = "qspi", "qspi_out";
>  			resets = <&bpmp TEGRA234_RESET_QSPI0>;
> +			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI0_2X_PM>,
> +					  <&bpmp TEGRA234_CLK_QSPI0_PM>;
> +			assigned-clock-rates = <199999999 99999999>;
> +			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
> +			dma-names = "rx", "tx";

Other than the dma-names property which was already discussed, this
looks fine.

Thierry

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

Re: [PATCH v2 2/6] spi: tegra210-quad: Update dummy sequence configuration

[Thierry Reding]

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

On Wed, Feb 12, 2025 at 02:46:47PM +0000, Vishwaroop A wrote:
> Adding support for the dummy sequence configuration. The dummy sequence
> introduces a delay between the command and the data phases of a
> transfer. This delay, measured in clock cycles, allows the slave
> device to prepare for data transmission, ensuring data integrity and
> proper synchronization.
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>  drivers/spi/spi-tegra210-quad.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

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

Re: [PATCH v2 3/6] spi: tegra210-quad: Fix X1_X2_X4 encoding and support x4 transfers

[Thierry Reding]

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

On Wed, Feb 12, 2025 at 02:46:48PM +0000, Vishwaroop A wrote:
> This patch corrects the QSPI_COMMAND_X1_X2_X4 and QSPI_ADDRESS_X1_X2_X4
> macros to properly encode the bus width for x1, x2, and x4 transfers.
> Although these macros were previously incorrect, they were not being
> used in the driver, so no functionality was affected.
> 
> The patch updates tegra_qspi_cmd_config() and tegra_qspi_addr_config()
> function calls to use the actual bus width from the transfer, instead of
> hardcoding it to 0 (which implied x1 mode). This change enables proper
> support for x1, x2, and x4 data transfers by correctly configuring the
> interface width for commands and addresses.
> 
> These modifications improve the QSPI driver's flexibility and prepare it
> for future use cases that may require different bus widths for commands
> and addresses.
> 
> Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>

With that blank line between Fixes: and S-o-b: dropped, this is:

Acked-by: Thierry Reding <treding@nvidia.com>

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

Re: [PATCH v2 4/6] spi: tegra210-quad: remove redundant error handling code

[Thierry Reding]

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

On Wed, Feb 12, 2025 at 02:46:49PM +0000, Vishwaroop A wrote:
> Remove unnecessary error handling code that terminated transfers and
> executed delay on errors. This code was redundant as error handling is
> already done at a higher level in the SPI core.
> 
> Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>

Again, the blank line can be dropped. Otherwise:

Acked-by: Thierry Reding <treding@nvidia.com>

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

Re: [PATCH v2 5/6] spi: tegra210-quad: modify chip select (CS) deactivation

[Thierry Reding]

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

On Wed, Feb 12, 2025 at 02:46:50PM +0000, Vishwaroop A wrote:
> Modify the chip select (CS) deactivation and inter-transfer delay
> execution to be performed only during the DATA_TRANSFER phase when
> the cs_change flag is not set. This ensures proper CS handling and
> timing between transfers while eliminating redundant operations.
> 
> Fixes: 1b8342cc4a38 ("spi: tegra210-quad: combined sequence mode")
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>  drivers/spi/spi-tegra210-quad.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
> index 2d7a2e3da337..04f41e92c1e2 100644
> --- a/drivers/spi/spi-tegra210-quad.c
> +++ b/drivers/spi/spi-tegra210-quad.c
> @@ -1171,16 +1171,16 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
>  				ret = -EIO;
>  				goto exit;
>  			}
> -			if (!xfer->cs_change) {
> -				tegra_qspi_transfer_end(spi);
> -				spi_transfer_delay_exec(xfer);
> -			}
>  			break;
>  		default:
>  			ret = -EINVAL;
>  			goto exit;
>  		}
>  		msg->actual_length += xfer->len;
> +		if (!xfer->cs_change && transfer_phase == DATA_TRANSFER) {
> +			tegra_qspi_transfer_end(spi);
> +			spi_transfer_delay_exec(xfer);
> +		}
>  		transfer_phase++;

I'd prefer blank lines around the "if" block to make this less
cluttered, but since this was always like this, either way:

Acked-by: Thierry Reding <treding@nvidia.com>

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

Re: [PATCH v2 0/6] Configure Clocks, Add Native Dma support.

[Jon Hunter]

On 12/02/2025 14:46, Vishwaroop A wrote:
> This patch series configures qspi clocks, fixes combined sequence
> programming and introduces native dma support.
> 
> Vishwaroop A (6):
>    arm64: tegra: Configure QSPI clocks and add DMA
>    spi: tegra210-quad: Update dummy sequence configuration
>    spi: tegra210-quad: Fix X1_X2_X4 encoding and support x4 transfers
>    spi: tegra210-quad: remove redundant error handling code
>    spi: tegra210-quad: modify chip select (CS) deactivation
>    spi: tegra210-quad: Introduce native DMA support

Can you re-order these so that all fixes are first and then new features 
are 2nd?

Thanks
Jon

-- 
nvpublic

Re: [PATCH v2 1/6] arm64: tegra: Configure QSPI clocks and add DMA

[Jon Hunter]

On 12/02/2025 14:46, Vishwaroop A wrote:
> Set QSPI0_2X_PM to 199.99 MHz and QSPI0_PM to 99.99 MHz using
> PLLC as the parent clock. These frequencies allow Quad IO DT
> reads up to 99.99 MHz, which is the fastest that can be
> achieved considering various PLL and clock divider constraints.
> 
> Populate the DMA and IOMMU properties for the Tegra234 QSPI devices to
> enable DMA support.
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 2601b43b2d8c..0ac2d3aba930 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -2948,6 +2948,13 @@
>   				 <&bpmp TEGRA234_CLK_QSPI0_PM>;
>   			clock-names = "qspi", "qspi_out";
>   			resets = <&bpmp TEGRA234_RESET_QSPI0>;
> +			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI0_2X_PM>,
> +					  <&bpmp TEGRA234_CLK_QSPI0_PM>;
> +			assigned-clock-rates = <199999999 99999999>;
> +			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
> +			dma-names = "rx", "tx";
> +			dma-coherent;
> +			iommus = <&smmu_niso1 TEGRA234_SID_QSPI0>;
>   			status = "disabled";
>   		};
>   
> @@ -3031,6 +3038,13 @@
>   				 <&bpmp TEGRA234_CLK_QSPI1_PM>;
>   			clock-names = "qspi", "qspi_out";
>   			resets = <&bpmp TEGRA234_RESET_QSPI1>;
> +			assigned-clocks = <&bpmp TEGRA234_CLK_QSPI1_2X_PM>,
> +					  <&bpmp TEGRA234_CLK_QSPI1_PM>;
> +			assigned-clock-rates = <199999999 99999999>;
> +			assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
> +			dma-names = "rx", "tx";
> +			dma-coherent;
> +			iommus = <&smmu_niso1 TEGRA234_SID_QSPI1>;
>   			status = "disabled";
>   		};
>   


Make sure you CC the DT mailing list on this.

With this change I am seeing the following warnings ...

arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
   DTC [C] arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0008.dtb
arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0008.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
   DTC [C] arch/arm64/boot/dts/nvidia/tegra234-p3740-0002+p3701-0008.dtb
arch/arm64/boot/dts/nvidia/tegra234-p3740-0002+p3701-0008.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
   DTC [C] arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0000.dtb
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0000.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#
   DTC [C] arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0005.dtb
arch/arm64/boot/dts/nvidia/tegra234-p3768-0000+p3767-0005.dtb: spi@3270000: Unevaluated properties are not allowed ('dma-coherent', 'iommus' were unexpected)
	from schema $id: http://devicetree.org/schemas/spi/nvidia,tegra210-quad.yaml#

Jon

-- 
nvpublic

Re: [PATCH v2 6/6] spi: tegra210-quad: Introduce native DMA support

[Thierry Reding]

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

On Wed, Feb 12, 2025 at 02:46:51PM +0000, Vishwaroop A wrote:
> Previous generations of Tegra supported DMA operations by an external
> DMA controller, but the QSPI on Tegra234 devices now have an internal
> DMA controller.
> 
> Internal DMA: Uses the QSPI controller's built-in DMA engine, which is
> limited in capabilities and tied directly to the QSPI module.
> 
> External DMA: Utilizes a separate, GPCDMA DMA controller that can
> transfer data between QSPI and any memory location.
> 
> Native DMA Initialization: Introduce routines to initialize and
> configure native DMA channels for both transmit and receive paths.
> Set up DMA mapping functions to manage buffer addresses effectively.
> 
> Enhance Transfer Logic: Implement logic to choose between CPU-based
> and DMA-based transfers based on data size.
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>  drivers/spi/spi-tegra210-quad.c | 218 ++++++++++++++++++--------------
>  1 file changed, 126 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
> index 04f41e92c1e2..7463b00b1ffb 100644
> --- a/drivers/spi/spi-tegra210-quad.c
> +++ b/drivers/spi/spi-tegra210-quad.c
> @@ -111,6 +111,9 @@
>  #define QSPI_DMA_BLK				0x024
>  #define QSPI_DMA_BLK_SET(x)			(((x) & 0xffff) << 0)
>  
> +#define QSPI_DMA_MEM_ADDRESS_REG		0x028
> +#define QSPI_DMA_HI_ADDRESS_REG			0x02c

I'd drop the _REG suffix since we don't use it on any of the other
register definitions.

> +
>  #define QSPI_TX_FIFO				0x108
>  #define QSPI_RX_FIFO				0x188
>  
> @@ -167,9 +170,9 @@ enum tegra_qspi_transfer_type {
>  };
>  
>  struct tegra_qspi_soc_data {
> -	bool has_dma;
>  	bool cmb_xfer_capable;
>  	bool supports_tpm;
> +	bool has_ext_dma;
>  	unsigned int cs_count;
>  };
>  
> @@ -605,17 +608,21 @@ static void tegra_qspi_dma_unmap_xfer(struct tegra_qspi *tqspi, struct spi_trans
>  
>  	len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
>  
> -	dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
> -	dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
> +	if (t->tx_buf)
> +		dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
> +	if (t->rx_buf)
> +		dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
>  }
>  
>  static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
>  {
>  	struct dma_slave_config dma_sconfig = { 0 };
> +	dma_addr_t rx_dma_phys, tx_dma_phys;
>  	unsigned int len;
>  	u8 dma_burst;
>  	int ret = 0;
>  	u32 val;
> +	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
>  
>  	if (tqspi->is_packed) {
>  		ret = tegra_qspi_dma_map_xfer(tqspi, t);
> @@ -634,60 +641,85 @@ static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct
>  		len = tqspi->curr_dma_words * 4;
>  
>  	/* set attention level based on length of transfer */
> -	val = 0;
> -	if (len & 0xf) {
> -		val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
> -		dma_burst = 1;
> -	} else if (((len) >> 4) & 0x1) {
> -		val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
> -		dma_burst = 4;
> -	} else {
> -		val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
> -		dma_burst = 8;
> +	if (has_ext_dma) {
> +		val = 0;
> +		if (len & 0xf) {
> +			val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
> +			dma_burst = 1;
> +		} else if (((len) >> 4) & 0x1) {
> +			val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
> +			dma_burst = 4;
> +		} else {
> +			val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
> +			dma_burst = 8;
> +		}
> +
> +		tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
>  	}
>  
> -	tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
>  	tqspi->dma_control_reg = val;
>  
>  	dma_sconfig.device_fc = true;
> -	if (tqspi->cur_direction & DATA_DIR_TX) {
> -		dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
> -		dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -		dma_sconfig.dst_maxburst = dma_burst;
> -		ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> -			return ret;
> -		}
>  
> -		tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
> -		ret = tegra_qspi_start_tx_dma(tqspi, t, len);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
> -			return ret;
> +	if ((tqspi->cur_direction & DATA_DIR_TX)) {
> +		if (has_ext_dma) {
> +			dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
> +			dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			dma_sconfig.dst_maxburst = dma_burst;
> +			ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> +				return ret;
> +			}
> +
> +			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
> +			ret = tegra_qspi_start_tx_dma(tqspi, t, len);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
> +				return ret;
> +			}
> +		} else {
> +			if (tqspi->is_packed)
> +				tx_dma_phys = t->tx_dma;
> +			else
> +				tx_dma_phys = tqspi->tx_dma_phys;
> +			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
> +			tegra_qspi_writel(tqspi, lower_32_bits(tx_dma_phys),
> +					  QSPI_DMA_MEM_ADDRESS_REG);
> +			tegra_qspi_writel(tqspi, (upper_32_bits(tx_dma_phys) & 0xff),
> +					  QSPI_DMA_HI_ADDRESS_REG);
>  		}
>  	}
>  
>  	if (tqspi->cur_direction & DATA_DIR_RX) {
> -		dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
> -		dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -		dma_sconfig.src_maxburst = dma_burst;
> -		ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> -			return ret;
> -		}
> -
> -		dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
> -					   tqspi->dma_buf_size,
> -					   DMA_FROM_DEVICE);
> -
> -		ret = tegra_qspi_start_rx_dma(tqspi, t, len);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
> -			if (tqspi->cur_direction & DATA_DIR_TX)
> -				dmaengine_terminate_all(tqspi->tx_dma_chan);
> -			return ret;
> +		if (has_ext_dma) {
> +			dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
> +			dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			dma_sconfig.src_maxburst = dma_burst;
> +			ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> +				return ret;
> +			}
> +			dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
> +						   tqspi->dma_buf_size, DMA_FROM_DEVICE);
> +			ret = tegra_qspi_start_rx_dma(tqspi, t, len);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
> +				if (tqspi->cur_direction & DATA_DIR_TX)
> +					dmaengine_terminate_all(tqspi->tx_dma_chan);
> +				return ret;
> +			}

Please keep the whitespace that was there before (maybe even add a few
blank lines) to make this less cluttered.

> +		} else {
> +			if (tqspi->is_packed)
> +				rx_dma_phys = t->rx_dma;
> +			else
> +				rx_dma_phys = tqspi->rx_dma_phys;
> +
> +			tegra_qspi_writel(tqspi, lower_32_bits(rx_dma_phys),
> +					  QSPI_DMA_MEM_ADDRESS_REG);
> +			tegra_qspi_writel(tqspi, (upper_32_bits(rx_dma_phys) & 0xff),
> +					  QSPI_DMA_HI_ADDRESS_REG);
>  		}

This doesn't look right. You're passing a memory buffer to hardware
here, so this needs DMA sync operations, too.

[...]
> @@ -1388,30 +1420,32 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
>  
>  	if (tqspi->cur_direction & DATA_DIR_TX) {
>  		if (tqspi->tx_status) {
> -			dmaengine_terminate_all(tqspi->tx_dma_chan);
> -			err += 1;
> -		} else {
> +			if (tqspi->tx_dma_chan)
> +				dmaengine_terminate_all(tqspi->tx_dma_chan);
> +			err++;
> +		} else if (tqspi->tx_dma_chan) {
>  			wait_status = wait_for_completion_interruptible_timeout(
>  				&tqspi->tx_dma_complete, QSPI_DMA_TIMEOUT);
>  			if (wait_status <= 0) {
>  				dmaengine_terminate_all(tqspi->tx_dma_chan);
>  				dev_err(tqspi->dev, "failed TX DMA transfer\n");
> -				err += 1;
> +				err++;
>  			}
>  		}
>  	}
>  
>  	if (tqspi->cur_direction & DATA_DIR_RX) {
>  		if (tqspi->rx_status) {
> -			dmaengine_terminate_all(tqspi->rx_dma_chan);
> -			err += 2;
> -		} else {
> +			if (tqspi->rx_dma_chan)
> +				dmaengine_terminate_all(tqspi->rx_dma_chan);
> +			err++;
> +		} else if (tqspi->rx_dma_chan) {
>  			wait_status = wait_for_completion_interruptible_timeout(
>  				&tqspi->rx_dma_complete, QSPI_DMA_TIMEOUT);
>  			if (wait_status <= 0) {
>  				dmaengine_terminate_all(tqspi->rx_dma_chan);
>  				dev_err(tqspi->dev, "failed RX DMA transfer\n");
> -				err += 2;
> +				err++;

Maybe we should change the "err" variable to something like "errors" or
"num_errors" to make it clear what this does. As it is, it's easily
mistaken to be a negative error code, in which case ++ wouldn't make
sense.

Thierry

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

Re: [PATCH v2 6/6] spi: tegra210-quad: Introduce native DMA support

[Jon Hunter]

On 12/02/2025 14:46, Vishwaroop A wrote:
> Previous generations of Tegra supported DMA operations by an external
> DMA controller, but the QSPI on Tegra234 devices now have an internal
> DMA controller.
> 
> Internal DMA: Uses the QSPI controller's built-in DMA engine, which is
> limited in capabilities and tied directly to the QSPI module.
> 
> External DMA: Utilizes a separate, GPCDMA DMA controller that can
> transfer data between QSPI and any memory location.
> 
> Native DMA Initialization: Introduce routines to initialize and
> configure native DMA channels for both transmit and receive paths.
> Set up DMA mapping functions to manage buffer addresses effectively.
> 
> Enhance Transfer Logic: Implement logic to choose between CPU-based
> and DMA-based transfers based on data size.
> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>   drivers/spi/spi-tegra210-quad.c | 218 ++++++++++++++++++--------------
>   1 file changed, 126 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
> index 04f41e92c1e2..7463b00b1ffb 100644
> --- a/drivers/spi/spi-tegra210-quad.c
> +++ b/drivers/spi/spi-tegra210-quad.c
> @@ -111,6 +111,9 @@
>   #define QSPI_DMA_BLK				0x024
>   #define QSPI_DMA_BLK_SET(x)			(((x) & 0xffff) << 0)
>   
> +#define QSPI_DMA_MEM_ADDRESS_REG		0x028
> +#define QSPI_DMA_HI_ADDRESS_REG			0x02c
> +
>   #define QSPI_TX_FIFO				0x108
>   #define QSPI_RX_FIFO				0x188
>   
> @@ -167,9 +170,9 @@ enum tegra_qspi_transfer_type {
>   };
>   
>   struct tegra_qspi_soc_data {
> -	bool has_dma;
>   	bool cmb_xfer_capable;
>   	bool supports_tpm;
> +	bool has_ext_dma;
>   	unsigned int cs_count;
>   };
>   
> @@ -605,17 +608,21 @@ static void tegra_qspi_dma_unmap_xfer(struct tegra_qspi *tqspi, struct spi_trans
>   
>   	len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
>   
> -	dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
> -	dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
> +	if (t->tx_buf)
> +		dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
> +	if (t->rx_buf)
> +		dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
>   }
>   
>   static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
>   {
>   	struct dma_slave_config dma_sconfig = { 0 };
> +	dma_addr_t rx_dma_phys, tx_dma_phys;
>   	unsigned int len;
>   	u8 dma_burst;
>   	int ret = 0;
>   	u32 val;
> +	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
>   
>   	if (tqspi->is_packed) {
>   		ret = tegra_qspi_dma_map_xfer(tqspi, t);
> @@ -634,60 +641,85 @@ static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct
>   		len = tqspi->curr_dma_words * 4;
>   
>   	/* set attention level based on length of transfer */
> -	val = 0;
> -	if (len & 0xf) {
> -		val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
> -		dma_burst = 1;
> -	} else if (((len) >> 4) & 0x1) {
> -		val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
> -		dma_burst = 4;
> -	} else {
> -		val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
> -		dma_burst = 8;
> +	if (has_ext_dma) {
> +		val = 0;
> +		if (len & 0xf) {
> +			val |= QSPI_TX_TRIG_1 | QSPI_RX_TRIG_1;
> +			dma_burst = 1;
> +		} else if (((len) >> 4) & 0x1) {
> +			val |= QSPI_TX_TRIG_4 | QSPI_RX_TRIG_4;
> +			dma_burst = 4;
> +		} else {
> +			val |= QSPI_TX_TRIG_8 | QSPI_RX_TRIG_8;
> +			dma_burst = 8;
> +		}
> +
> +		tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
>   	}
>   
> -	tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
>   	tqspi->dma_control_reg = val;
>   
>   	dma_sconfig.device_fc = true;
> -	if (tqspi->cur_direction & DATA_DIR_TX) {
> -		dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
> -		dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -		dma_sconfig.dst_maxburst = dma_burst;
> -		ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> -			return ret;
> -		}
>   
> -		tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
> -		ret = tegra_qspi_start_tx_dma(tqspi, t, len);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
> -			return ret;
> +	if ((tqspi->cur_direction & DATA_DIR_TX)) {
> +		if (has_ext_dma) {

For consistency with 'handle_dma_based_xfer' I think it is better to 
use 'tqspi->tx_dma_chan' here.

> +			dma_sconfig.dst_addr = tqspi->phys + QSPI_TX_FIFO;
> +			dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			dma_sconfig.dst_maxburst = dma_burst;
> +			ret = dmaengine_slave_config(tqspi->tx_dma_chan, &dma_sconfig);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> +				return ret;
> +			}
> +
> +			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
> +			ret = tegra_qspi_start_tx_dma(tqspi, t, len);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed to starting TX DMA: %d\n", ret);
> +				return ret;
> +			}
> +		} else {
> +			if (tqspi->is_packed)
> +				tx_dma_phys = t->tx_dma;
> +			else
> +				tx_dma_phys = tqspi->tx_dma_phys;
> +			tegra_qspi_copy_client_txbuf_to_qspi_txbuf(tqspi, t);
> +			tegra_qspi_writel(tqspi, lower_32_bits(tx_dma_phys),
> +					  QSPI_DMA_MEM_ADDRESS_REG);
> +			tegra_qspi_writel(tqspi, (upper_32_bits(tx_dma_phys) & 0xff),
> +					  QSPI_DMA_HI_ADDRESS_REG);
>   		}
>   	}
>   
>   	if (tqspi->cur_direction & DATA_DIR_RX) {
> -		dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
> -		dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> -		dma_sconfig.src_maxburst = dma_burst;
> -		ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> -			return ret;
> -		}
> -
> -		dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
> -					   tqspi->dma_buf_size,
> -					   DMA_FROM_DEVICE);
> -
> -		ret = tegra_qspi_start_rx_dma(tqspi, t, len);
> -		if (ret < 0) {
> -			dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
> -			if (tqspi->cur_direction & DATA_DIR_TX)
> -				dmaengine_terminate_all(tqspi->tx_dma_chan);
> -			return ret;
> +		if (has_ext_dma) {

Same here.

> +			dma_sconfig.src_addr = tqspi->phys + QSPI_RX_FIFO;
> +			dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +			dma_sconfig.src_maxburst = dma_burst;
> +			ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
> +				return ret;
> +			}
> +			dma_sync_single_for_device(tqspi->dev, tqspi->rx_dma_phys,
> +						   tqspi->dma_buf_size, DMA_FROM_DEVICE);
> +			ret = tegra_qspi_start_rx_dma(tqspi, t, len);
> +			if (ret < 0) {
> +				dev_err(tqspi->dev, "failed to start RX DMA: %d\n", ret);
> +				if (tqspi->cur_direction & DATA_DIR_TX)
> +					dmaengine_terminate_all(tqspi->tx_dma_chan);
> +				return ret;
> +			}
> +		} else {
> +			if (tqspi->is_packed)
> +				rx_dma_phys = t->rx_dma;
> +			else
> +				rx_dma_phys = tqspi->rx_dma_phys;
> +
> +			tegra_qspi_writel(tqspi, lower_32_bits(rx_dma_phys),
> +					  QSPI_DMA_MEM_ADDRESS_REG);
> +			tegra_qspi_writel(tqspi, (upper_32_bits(rx_dma_phys) & 0xff),
> +					  QSPI_DMA_HI_ADDRESS_REG);
>   		}
>   	}
>   
> @@ -726,9 +758,6 @@ static int tegra_qspi_start_cpu_based_transfer(struct tegra_qspi *qspi, struct s
>   
>   static void tegra_qspi_deinit_dma(struct tegra_qspi *tqspi)
>   {
> -	if (!tqspi->soc_data->has_dma)
> -		return;
> -
>   	if (tqspi->tx_dma_buf) {
>   		dma_free_coherent(tqspi->dev, tqspi->dma_buf_size,
>   				  tqspi->tx_dma_buf, tqspi->tx_dma_phys);
> @@ -759,16 +788,26 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
>   	u32 *dma_buf;
>   	int err;
>   
> -	if (!tqspi->soc_data->has_dma)
> -		return 0;
> +	if (tqspi->soc_data->has_ext_dma) {
> +		dma_chan = dma_request_chan(tqspi->dev, "rx");
> +		if (IS_ERR(dma_chan)) {
> +			err = PTR_ERR(dma_chan);
> +			goto err_out;
> +		}
>   
> -	dma_chan = dma_request_chan(tqspi->dev, "rx");
> -	if (IS_ERR(dma_chan)) {
> -		err = PTR_ERR(dma_chan);
> -		goto err_out;
> -	}
> +		tqspi->rx_dma_chan = dma_chan;
>   
> -	tqspi->rx_dma_chan = dma_chan;
> +		dma_chan = dma_request_chan(tqspi->dev, "tx");
> +		if (IS_ERR(dma_chan)) {
> +			err = PTR_ERR(dma_chan);
> +			goto err_out;
> +		}
> +
> +		tqspi->tx_dma_chan = dma_chan;
> +	} else {
> +		tqspi->rx_dma_chan = NULL;
> +		tqspi->tx_dma_chan = NULL;
> +	}
>   
>   	dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
>   	if (!dma_buf) {
> @@ -779,14 +818,6 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
>   	tqspi->rx_dma_buf = dma_buf;
>   	tqspi->rx_dma_phys = dma_phys;
>   
> -	dma_chan = dma_request_chan(tqspi->dev, "tx");
> -	if (IS_ERR(dma_chan)) {
> -		err = PTR_ERR(dma_chan);
> -		goto err_out;
> -	}
> -
> -	tqspi->tx_dma_chan = dma_chan;
> -
>   	dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
>   	if (!dma_buf) {
>   		err = -ENOMEM;
> @@ -1056,6 +1087,7 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
>   					struct spi_message *msg)
>   {
>   	bool is_first_msg = true;
> +	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
>   	struct spi_transfer *xfer;
>   	struct spi_device *spi = msg->spi;
>   	u8 transfer_phase = 0;
> @@ -1128,15 +1160,12 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
>   			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);
> +				if (tqspi->is_curr_dma_xfer && has_ext_dma) {
> +					if (tqspi->cur_direction & DATA_DIR_TX)
> +						dmaengine_terminate_all(tqspi->tx_dma_chan);
> +					if (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) {
> @@ -1197,6 +1226,7 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
>   	struct spi_device *spi = msg->spi;
>   	struct spi_transfer *transfer;
>   	bool is_first_msg = true;
> +	bool has_ext_dma = tqspi->soc_data->has_ext_dma;
>   	int ret = 0, val = 0;
>   
>   	msg->status = 0;
> @@ -1251,10 +1281,12 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
>   						  QSPI_DMA_TIMEOUT);
>   		if (WARN_ON(ret == 0)) {
>   			dev_err(tqspi->dev, "transfer timeout\n");
> -			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);
> +			if (tqspi->is_curr_dma_xfer && has_ext_dma) {

And here.

> +				if (tqspi->cur_direction & DATA_DIR_TX)
> +					dmaengine_terminate_all(tqspi->tx_dma_chan);
> +				if (tqspi->cur_direction & DATA_DIR_RX)
> +					dmaengine_terminate_all(tqspi->rx_dma_chan);
> +			}
>   			tegra_qspi_handle_error(tqspi);
>   			ret = -EIO;
>   			goto complete_xfer;
> @@ -1323,7 +1355,7 @@ static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
>   			return false;
>   		xfer = list_next_entry(xfer, transfer_list);
>   	}
> -	if (!tqspi->soc_data->has_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
> +	if (!tqspi->soc_data->has_ext_dma && xfer->len > (QSPI_FIFO_DEPTH << 2))
>   		return false;
>   
>   	return true;
> @@ -1388,30 +1420,32 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
>   
>   	if (tqspi->cur_direction & DATA_DIR_TX) {
>   		if (tqspi->tx_status) {
> -			dmaengine_terminate_all(tqspi->tx_dma_chan);
> -			err += 1;
> -		} else {
> +			if (tqspi->tx_dma_chan)
> +				dmaengine_terminate_all(tqspi->tx_dma_chan);
> +			err++;
> +		} else if (tqspi->tx_dma_chan) {
>   			wait_status = wait_for_completion_interruptible_timeout(
>   				&tqspi->tx_dma_complete, QSPI_DMA_TIMEOUT);
>   			if (wait_status <= 0) {
>   				dmaengine_terminate_all(tqspi->tx_dma_chan);
>   				dev_err(tqspi->dev, "failed TX DMA transfer\n");
> -				err += 1;
> +				err++;
>   			}
>   		}
>   	}
>   
>   	if (tqspi->cur_direction & DATA_DIR_RX) {
>   		if (tqspi->rx_status) {
> -			dmaengine_terminate_all(tqspi->rx_dma_chan);
> -			err += 2;
> -		} else {
> +			if (tqspi->rx_dma_chan)
> +				dmaengine_terminate_all(tqspi->rx_dma_chan);
> +			err++;
> +		} else if (tqspi->rx_dma_chan) {
>   			wait_status = wait_for_completion_interruptible_timeout(
>   				&tqspi->rx_dma_complete, QSPI_DMA_TIMEOUT);
>   			if (wait_status <= 0) {
>   				dmaengine_terminate_all(tqspi->rx_dma_chan);
>   				dev_err(tqspi->dev, "failed RX DMA transfer\n");
> -				err += 2;
> +				err++;
>   			}
>   		}
>   	}
> @@ -1474,28 +1508,28 @@ 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,
> +	.has_ext_dma = true,
>   	.cmb_xfer_capable = false,
>   	.supports_tpm = false,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
> -	.has_dma = true,
> +	.has_ext_dma = true,
>   	.cmb_xfer_capable = true,
>   	.supports_tpm = false,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
> -	.has_dma = false,
> +	.has_ext_dma = false,
>   	.cmb_xfer_capable = true,
>   	.supports_tpm = true,
>   	.cs_count = 1,
>   };
>   
>   static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
> -	.has_dma = false,
> +	.has_ext_dma = true,
>   	.cmb_xfer_capable = true,
>   	.supports_tpm = true,
>   	.cs_count = 4,

-- 
nvpublic

Re: [PATCH V1 1/6] arm64: tegra: Configure QSPI clocks and add DMA

[Jon Hunter]

On 12/02/2025 14:39, Vishwaroop A wrote:
> 
> 
> ________________________________________
> From: Thierry Reding
> Sent: Thursday, January 09, 2025 16:10
> To: Vishwaroop A
> Cc: Jon Hunter; Sowjanya Komatineni; Laxman Dewangan; broonie@kernel.org; linux-spi@vger.kernel.org; linux-tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Krishna Yarlagadda; Suresh Mangipudi
> Subject: Re: [PATCH V1 1/6] arm64: tegra: Configure QSPI clocks and add DMA
> 
> On Fri, Jan 03, 2025 at 06:04:02AM +0000, Vishwaroop A wrote:
>> Set QSPI0_2X_PM to 199.99 MHz and QSPI0_PM to 99.99 MHz using
>> PLLC as the parent clock. These frequencies allow Quad IO DT
>> reads up to 99.99 MHz, which is the fastest that can be
>> achieved considering various PLL and clock divider constraints.
>>
>> Populate the DMA and IOMMU properties for the Tegra234 QSPI devices to
>> enable DMA support.
>>
>> Change-Id: I1dded904aa8e0f278c89998481e829f1ce474e8c
>> Signed-off-by: Vishwaroop A <va@nvidia.com>
>> ---
>>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>> index 984c85eab41a..96d0f13390ae 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>> @@ -2948,6 +2948,13 @@
>>                                 <&bpmp TEGRA234_CLK_QSPI0_PM>;
>>                        clock-names = "qspi", "qspi_out";
>>                        resets = <&bpmp TEGRA234_RESET_QSPI0>;
>> +                     assigned-clocks = <&bpmp TEGRA234_CLK_QSPI0_2X_PM>,
>> +                                       <&bpmp TEGRA234_CLK_QSPI0_PM>;
>> +                     assigned-clock-rates = <199999999 99999999>;
>> +                     assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
>> +                     dma-names = "rx", "tx";
>> +                     dma-coherent;
>> +                     iommus = <&smmu_niso1 TEGRA234_SID_QSPI0>;
>>                        status = "disabled";
>>                };
>>
>> @@ -3031,6 +3038,13 @@
>>                                 <&bpmp TEGRA234_CLK_QSPI1_PM>;
>>                        clock-names = "qspi", "qspi_out";
>>                        resets = <&bpmp TEGRA234_RESET_QSPI1>;
>> +                     assigned-clocks = <&bpmp TEGRA234_CLK_QSPI1_2X_PM>,
>> +                                       <&bpmp TEGRA234_CLK_QSPI1_PM>;
>> +                     assigned-clock-rates = <199999999 99999999>;
>> +                     assigned-clock-parents = <&bpmp TEGRA234_CLK_PLLC>;
>> +                     dma-names = "rx", "tx";
>> +                     dma-coherent;
>> +                     iommus = <&smmu_niso1 TEGRA234_SID_QSPI1>;
>>                        status = "disabled";
>>                };
>>
> 
>>> It looks like these are missing the "dmas" properties that go along with "dma-names".
> [Vishwaroop A] dmas property is not required as QSPI uses native dma engine. dmas property is used for assigning  the dma channels. In case of QSPI it has own native DMA engine.


OK so that means we don't need the 'dma-names' here either. Please 
remove this.

Jon

-- 
nvpublic