[PATCH v2 0/5] Tegra QUAD SPI combined sequence mode

[PATCH v2 0/5] Tegra QUAD SPI combined sequence mode

[Krishna Yarlagadda]

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

v2 changes:
- use combined sequence mode as default
- remove property to switch transfer modes
- fix compilation warnings

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

 .../bindings/spi/nvidia,tegra210-quad.yaml    |   1 +
 drivers/spi/spi-tegra210-quad.c               | 330 ++++++++++++++++--
 2 files changed, 300 insertions(+), 31 deletions(-)

-- 
2.17.1

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

[Krishna Yarlagadda]

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

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

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

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

[Krishna Yarlagadda]

Add compatible string for Tegra234 for Tegra QUAD SPI

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

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

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

[Krishna Yarlagadda]

Add support for Tegra234 and soc data to select capabilities.

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

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

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

[Krishna Yarlagadda]

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

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

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

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

[Krishna Yarlagadda]

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

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

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 0dbcb5fffc03..0899dca52c5a 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -121,14 +121,39 @@
 #define QSPI_NUM_DUMMY_CYCLE(x)			(((x) & 0xff) << 0)
 #define QSPI_DUMMY_CYCLES_MAX			0xff
 
+#define QSPI_CMB_SEQ_CMD			0x19c
+#define QSPI_COMMAND_VALUE_SET(X)		(((x) & 0xFF) << 0)
+
+#define QSPI_CMB_SEQ_CMD_CFG			0x1a0
+#define QSPI_COMMAND_X1_X2_X4(x)		(((x) & 0x3) << 13)
+#define QSPI_COMMAND_X1_X2_X4_MASK		(0x03 << 13)
+#define QSPI_COMMAND_SDR_DDR			BIT(12)
+#define QSPI_COMMAND_SIZE_SET(x)		(((x) & 0xFF) << 0)
+
+#define QSPI_GLOBAL_CONFIG			0X1a4
+#define QSPI_CMB_SEQ_EN				BIT(0)
+
+#define QSPI_CMB_SEQ_ADDR			0x1a8
+#define QSPI_ADDRESS_VALUE_SET(X)		(((x) & 0xFFFF) << 0)
+
+#define QSPI_CMB_SEQ_ADDR_CFG			0x1ac
+#define QSPI_ADDRESS_X1_X2_X4(x)		(((x) & 0x3) << 13)
+#define QSPI_ADDRESS_X1_X2_X4_MASK		(0x03 << 13)
+#define QSPI_ADDRESS_SDR_DDR			BIT(12)
+#define QSPI_ADDRESS_SIZE_SET(x)		(((x) & 0xFF) << 0)
+
 #define DATA_DIR_TX				BIT(0)
 #define DATA_DIR_RX				BIT(1)
 
 #define QSPI_DMA_TIMEOUT			(msecs_to_jiffies(1000))
 #define DEFAULT_QSPI_DMA_BUF_LEN		(64 * 1024)
+#define CMD_TRANSFER				0
+#define ADDR_TRANSFER				1
+#define DATA_TRANSFER				2
 
 struct tegra_qspi_soc_data {
 	bool has_dma;
+	bool cmb_xfer_capable;
 };
 
 struct tegra_qspi_client_data {
@@ -912,7 +937,6 @@ static int tegra_qspi_setup(struct spi_device *spi)
 		cdata = tegra_qspi_parse_cdata_dt(spi);
 		spi->controller_data = cdata;
 	}
-
 	spin_lock_irqsave(&tqspi->lock, flags);
 
 	/* keep default cs state to inactive */
@@ -971,9 +995,164 @@ static void tegra_qspi_transfer_end(struct spi_device *spi)
 	tegra_qspi_writel(tqspi, tqspi->def_command1_reg, QSPI_COMMAND1);
 }
 
-static int tegra_qspi_transfer_one_message(struct spi_master *master, struct spi_message *msg)
+static u32 tegra_qspi_cmd_config(bool is_ddr, u8 bus_width, u8 len)
+{
+	u32 cmd_config = 0;
+
+	/* Extract Command configuration and value */
+	if (is_ddr)
+		cmd_config |= QSPI_COMMAND_SDR_DDR;
+	else
+		cmd_config &= ~QSPI_COMMAND_SDR_DDR;
+
+	cmd_config |= QSPI_COMMAND_X1_X2_X4(bus_width);
+	cmd_config |= QSPI_COMMAND_SIZE_SET((len * 8) - 1);
+
+	return cmd_config;
+}
+
+static u32 tegra_qspi_addr_config(bool is_ddr, u8 bus_width, u8 len)
+{
+	u32 addr_config = 0;
+
+	/* Extract Address configuration and value */
+	is_ddr = 0; //Only SDR mode supported
+	bus_width = 0; //X1 mode
+
+	if (is_ddr)
+		addr_config |= QSPI_ADDRESS_SDR_DDR;
+	else
+		addr_config &= ~QSPI_ADDRESS_SDR_DDR;
+
+	addr_config |= QSPI_ADDRESS_X1_X2_X4(bus_width);
+	addr_config |= QSPI_ADDRESS_SIZE_SET((len * 8) - 1);
+
+	return addr_config;
+}
+
+static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
+					struct spi_message *msg)
+{
+	bool is_first_msg = true;
+	struct spi_transfer *xfer;
+	struct spi_device *spi = msg->spi;
+	u8 transfer_phase = 0;
+	u32 cmd1 = 0, dma_ctl = 0;
+	int ret;
+	u32 address_value = 0;
+	u32 cmd_config = 0, addr_config = 0;
+	u8 cmd_value = 0, len = 0, val = 0;
+
+	/* Enable Combined sequence mode */
+	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
+	val |= QSPI_CMB_SEQ_EN;
+	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
+	/* Process individual transfer list */
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		switch (transfer_phase) {
+		case CMD_TRANSFER:
+			/* X1 SDR mode */
+			cmd_config = tegra_qspi_cmd_config(false, 0,
+							   xfer->len);
+			cmd_value = *((const u8 *)(xfer->tx_buf));
+			break;
+		case ADDR_TRANSFER:
+			len = xfer->len;
+			/* X1 SDR mode */
+			addr_config = tegra_qspi_addr_config(false, 0,
+							     xfer->len);
+			address_value = *((const u32 *)(xfer->tx_buf));
+			break;
+		case DATA_TRANSFER:
+			/* Program Command, Address value in register */
+			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
+			tegra_qspi_writel(tqspi, address_value,
+					  QSPI_CMB_SEQ_ADDR);
+			/* Program Command and Address config in register */
+			tegra_qspi_writel(tqspi, cmd_config,
+					  QSPI_CMB_SEQ_CMD_CFG);
+			tegra_qspi_writel(tqspi, addr_config,
+					  QSPI_CMB_SEQ_ADDR_CFG);
+
+			reinit_completion(&tqspi->xfer_completion);
+			cmd1 = tegra_qspi_setup_transfer_one(spi, xfer,
+							     is_first_msg);
+			ret = tegra_qspi_start_transfer_one(spi, xfer,
+							    cmd1);
+
+			if (ret < 0) {
+				dev_err(tqspi->dev, "Failed to start transfer-one: %d\n",
+					ret);
+				return ret;
+			}
+
+			is_first_msg = false;
+			ret = wait_for_completion_timeout
+					(&tqspi->xfer_completion,
+					QSPI_DMA_TIMEOUT);
+
+			if (WARN_ON(ret == 0)) {
+				dev_err(tqspi->dev, "QSPI Transfer failed with timeout: %d\n",
+					ret);
+				if (tqspi->is_curr_dma_xfer &&
+				    (tqspi->cur_direction & DATA_DIR_TX))
+					dmaengine_terminate_all
+						(tqspi->tx_dma_chan);
+
+				if (tqspi->is_curr_dma_xfer &&
+				    (tqspi->cur_direction & DATA_DIR_RX))
+					dmaengine_terminate_all
+						(tqspi->rx_dma_chan);
+
+				/* Abort transfer by resetting pio/dma bit */
+				if (!tqspi->is_curr_dma_xfer) {
+					cmd1 = tegra_qspi_readl
+							(tqspi,
+							 QSPI_COMMAND1);
+					cmd1 &= ~QSPI_PIO;
+					tegra_qspi_writel
+							(tqspi, cmd1,
+							 QSPI_COMMAND1);
+				} else {
+					dma_ctl = tegra_qspi_readl
+							(tqspi,
+							 QSPI_DMA_CTL);
+					dma_ctl &= ~QSPI_DMA_EN;
+					tegra_qspi_writel(tqspi, dma_ctl,
+							  QSPI_DMA_CTL);
+				}
+
+				/* Reset controller if timeout happens */
+				if (device_reset(tqspi->dev) < 0)
+					dev_warn_once(tqspi->dev,
+						      "device reset failed\n");
+				ret = -EIO;
+				goto exit;
+			}
+
+			if (tqspi->tx_status ||  tqspi->rx_status) {
+				dev_err(tqspi->dev, "QSPI Transfer failed\n");
+				tqspi->tx_status = 0;
+				tqspi->rx_status = 0;
+				ret = -EIO;
+				goto exit;
+			}
+		default:
+			goto exit;
+		}
+		msg->actual_length += xfer->len;
+		transfer_phase++;
+	}
+
+exit:
+	msg->status = ret;
+
+	return ret;
+}
+
+static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
+					    struct spi_message *msg)
 {
-	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
 	struct spi_device *spi = msg->spi;
 	struct spi_transfer *transfer;
 	bool is_first_msg = true;
@@ -1021,7 +1200,6 @@ static int tegra_qspi_transfer_one_message(struct spi_master *master, struct spi
 			goto complete_xfer;
 		}
 
-		is_first_msg = false;
 		ret = wait_for_completion_timeout(&tqspi->xfer_completion,
 						  QSPI_DMA_TIMEOUT);
 		if (WARN_ON(ret == 0)) {
@@ -1066,7 +1244,48 @@ static int tegra_qspi_transfer_one_message(struct spi_master *master, struct spi
 	ret = 0;
 exit:
 	msg->status = ret;
+
+	return ret;
+}
+
+static bool tegra_qspi_validate_cmb_seq(struct tegra_qspi *tqspi,
+					struct spi_message *msg)
+{
+	int transfer_count = 0;
+	struct spi_transfer *xfer;
+
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		transfer_count++;
+	}
+	if (!tqspi->soc_data->cmb_xfer_capable || transfer_count != 3)
+		return false;
+	xfer = list_first_entry(&msg->transfers, typeof(*xfer),
+				transfer_list);
+	if (xfer->len > 2)
+		return false;
+	xfer = list_next_entry(xfer, transfer_list);
+	if (xfer->len > 4 || xfer->len < 3)
+		return false;
+	xfer = list_next_entry(xfer, transfer_list);
+	if (!tqspi->soc_data->has_dma || xfer->len > (QSPI_FIFO_DEPTH << 2))
+		return false;
+
+	return true;
+}
+
+static int tegra_qspi_transfer_one_message(struct spi_master *master,
+					   struct spi_message *msg)
+{
+	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
+	int ret;
+
+	if (tegra_qspi_validate_cmb_seq(tqspi, msg))
+		ret = tegra_qspi_combined_seq_xfer(tqspi, msg);
+	else
+		ret = tegra_qspi_non_combined_seq_xfer(tqspi, msg);
+
 	spi_finalize_current_message(master);
+
 	return ret;
 }
 
@@ -1200,14 +1419,17 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
 
 static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
 	.has_dma = true,
+	.cmb_xfer_capable = false,
 };
 
 static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
 	.has_dma = true,
+	.cmb_xfer_capable = true,
 };
 
 static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
 	.has_dma = false,
+	.cmb_xfer_capable = true,
 };
 
 static const struct of_device_id tegra_qspi_of_match[] = {
@@ -1278,6 +1500,7 @@ static int tegra_qspi_probe(struct platform_device *pdev)
 	tqspi->dev = &pdev->dev;
 	spin_lock_init(&tqspi->lock);
 
+	tqspi->soc_data = device_get_match_data(&pdev->dev);
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	tqspi->base = devm_ioremap_resource(&pdev->dev, r);
 	if (IS_ERR(tqspi->base))
-- 
2.17.1

Re: [PATCH v2 4/5] spi: tegra210-quad: add acpi support

[Mark Brown]

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

On Tue, Feb 22, 2022 at 11:26:10PM +0530, Krishna Yarlagadda wrote:

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

> @@ -1377,6 +1400,8 @@ static int __maybe_unused tegra_qspi_runtime_suspend(struct device *dev)
>  	struct spi_master *master = dev_get_drvdata(dev);
>  	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
>  
> +	if (has_acpi_companion(tqspi->dev))
> +		return 0;
>  	/* flush all write which are in PPSB queue by reading back */
>  	tegra_qspi_readl(tqspi, QSPI_COMMAND1);

As well as clock stuff this is also skipping flushing of pending writes
- is that intentional?  It's not called out in the changelog and seems
like something that could cause issues if someone runs on a system where
the firmware does implement runtime suspend.

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

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

[Mark Brown]

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

On Tue, Feb 22, 2022 at 11:26:11PM +0530, Krishna Yarlagadda wrote:

> +	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
> +	val |= QSPI_CMB_SEQ_EN;
> +	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);

I notice that nothing seems to clear QSPI_CMB_SEQ_EN - is that self
clearing or something?

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

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

[Rob Herring]

On Tue, Feb 22, 2022 at 11:26:08PM +0530, Krishna Yarlagadda wrote:
> Add compatible string for Tegra234 for Tegra QUAD SPI
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml | 1 +
>  1 file changed, 1 insertion(+)

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

RE: [PATCH v2 2/5] dt-bindings: spi: Tegra234 QUAD SPI compatible

[Krishna Yarlagadda]

Regards
KY

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 23 February 2022 01:57
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: broonie@kernel.org; thierry.reding@gmail.com; Jonathan Hunter
> <jonathanh@nvidia.com>; linux-spi@vger.kernel.org; linux-
> tegra@vger.kernel.org; Sowjanya Komatineni
> <skomatineni@nvidia.com>; Laxman Dewangan
> <ldewangan@nvidia.com>; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; p.zabel@pengutronix.de
> Subject: Re: [PATCH v2 2/5] dt-bindings: spi: Tegra234 QUAD SPI
> compatible
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Feb 22, 2022 at 11:26:08PM +0530, Krishna Yarlagadda wrote:
> > Add compatible string for Tegra234 for Tegra QUAD SPI
> >
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
> | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Please add Acked-by/Reviewed-by tags when posting new versions.
Sure. Will do that next time.
> However, there's no need to repost patches *only* to add the tags.
> The upstream maintainer will do that for acks received on the version
> they apply.
> 
I resent the patch as it was not clear to me if this patch can be applied.
I received warning and assumed this cannot be applied without resend.
-- This breaks an x86_64 allmodconfig build
> If a tag was not added on purpose, please state why and what
> changed.

RE: [PATCH v2 4/5] spi: tegra210-quad: add acpi support

[Krishna Yarlagadda]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 23 February 2022 00:07
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: thierry.reding@gmail.com; Jonathan Hunter
> <jonathanh@nvidia.com>; linux-spi@vger.kernel.org; linux-
> tegra@vger.kernel.org; Sowjanya Komatineni
> <skomatineni@nvidia.com>; Laxman Dewangan
> <ldewangan@nvidia.com>; robh+dt@kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> p.zabel@pengutronix.de
> Subject: Re: [PATCH v2 4/5] spi: tegra210-quad: add acpi support
> 
> On Tue, Feb 22, 2022 at 11:26:10PM +0530, Krishna Yarlagadda wrote:
> 
> > Add ACPI ID for Tegra QUAD SPI. Switch to common device property
> calls.
> > Skip clock calls that are not updated in ACPI boot.
> 
> > @@ -1377,6 +1400,8 @@ static int __maybe_unused
> tegra_qspi_runtime_suspend(struct device *dev)
> >  	struct spi_master *master = dev_get_drvdata(dev);
> >  	struct tegra_qspi *tqspi = spi_master_get_devdata(master);
> >
> > +	if (has_acpi_companion(tqspi->dev))
> > +		return 0;
> >  	/* flush all write which are in PPSB queue by reading back */
> >  	tegra_qspi_readl(tqspi, QSPI_COMMAND1);
> 
> As well as clock stuff this is also skipping flushing of pending writes
> - is that intentional?  It's not called out in the changelog and seems like
> something that could cause issues if someone runs on a system where
> the firmware does implement runtime suspend.
Runtime suspend is not enabled with ACPI firmware. Converted compiler flag in v1 to runtime check.
We must add more changes like setting DPM flags for runtime pm support with ACPI.
Can take this as part of a different series.

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

[Krishna Yarlagadda]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 23 February 2022 00:12
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: thierry.reding@gmail.com; Jonathan Hunter
> <jonathanh@nvidia.com>; linux-spi@vger.kernel.org; linux-
> tegra@vger.kernel.org; Sowjanya Komatineni
> <skomatineni@nvidia.com>; Laxman Dewangan
> <ldewangan@nvidia.com>; robh+dt@kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> p.zabel@pengutronix.de
> Subject: Re: [PATCH v2 5/5] spi: tegra210-quad: combined sequence
> mode
> 
> On Tue, Feb 22, 2022 at 11:26:11PM +0530, Krishna Yarlagadda wrote:
> 
> > +	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
> > +	val |= QSPI_CMB_SEQ_EN;
> > +	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
> 
> I notice that nothing seems to clear QSPI_CMB_SEQ_EN - is that self
> clearing or something?
Need a change in non-combined sequence code to reset this.
Will add the change in v3

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

[kernel test robot]

Hi Krishna,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on v5.17-rc5 next-20220222]
[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]

url:    https://github.com/0day-ci/linux/commits/Krishna-Yarlagadda/Tegra-QUAD-SPI-combined-sequence-mode/20220223-015906
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: riscv-randconfig-r042-20220221 (https://download.01.org/0day-ci/archive/20220223/202202231856.T049n8gm-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/3b852b330b0b8332a67fd7b183ae798031c7a207
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Krishna-Yarlagadda/Tegra-QUAD-SPI-combined-sequence-mode/20220223-015906
        git checkout 3b852b330b0b8332a67fd7b183ae798031c7a207
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/spi/spi-tegra210-quad.c:1044:20: warning: variable 'len' set but not used [-Wunused-but-set-variable]
           u8 cmd_value = 0, len = 0, val = 0;
                             ^
>> drivers/spi/spi-tegra210-quad.c:1140:3: warning: variable 'ret' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
                   default:
                   ^~~~~~~
   drivers/spi/spi-tegra210-quad.c:1148:16: note: uninitialized use occurs here
           msg->status = ret;
                         ^~~
>> drivers/spi/spi-tegra210-quad.c:1051:2: warning: variable 'ret' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
           list_for_each_entry(xfer, &msg->transfers, transfer_list) {
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/list.h:639:7: note: expanded from macro 'list_for_each_entry'
                !list_entry_is_head(pos, head, member);                    \
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/spi/spi-tegra210-quad.c:1148:16: note: uninitialized use occurs here
           msg->status = ret;
                         ^~~
   drivers/spi/spi-tegra210-quad.c:1051:2: note: remove the condition if it is always true
           list_for_each_entry(xfer, &msg->transfers, transfer_list) {
           ^
   include/linux/list.h:639:7: note: expanded from macro 'list_for_each_entry'
                !list_entry_is_head(pos, head, member);                    \
                ^
   drivers/spi/spi-tegra210-quad.c:1041:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
>> drivers/spi/spi-tegra210-quad.c:1140:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
                   default:
                   ^
   drivers/spi/spi-tegra210-quad.c:1140:3: note: insert '__attribute__((fallthrough));' to silence this warning
                   default:
                   ^
                   __attribute__((fallthrough)); 
   drivers/spi/spi-tegra210-quad.c:1140:3: note: insert 'break;' to avoid fall-through
                   default:
                   ^
                   break; 
   4 warnings generated.


vim +/len +1044 drivers/spi/spi-tegra210-quad.c

  1032	
  1033	static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
  1034						struct spi_message *msg)
  1035	{
  1036		bool is_first_msg = true;
  1037		struct spi_transfer *xfer;
  1038		struct spi_device *spi = msg->spi;
  1039		u8 transfer_phase = 0;
  1040		u32 cmd1 = 0, dma_ctl = 0;
> 1041		int ret;
  1042		u32 address_value = 0;
  1043		u32 cmd_config = 0, addr_config = 0;
> 1044		u8 cmd_value = 0, len = 0, val = 0;
  1045	
  1046		/* Enable Combined sequence mode */
  1047		val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
  1048		val |= QSPI_CMB_SEQ_EN;
  1049		tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
  1050		/* Process individual transfer list */
> 1051		list_for_each_entry(xfer, &msg->transfers, transfer_list) {
  1052			switch (transfer_phase) {
  1053			case CMD_TRANSFER:
  1054				/* X1 SDR mode */
  1055				cmd_config = tegra_qspi_cmd_config(false, 0,
  1056								   xfer->len);
  1057				cmd_value = *((const u8 *)(xfer->tx_buf));
  1058				break;
  1059			case ADDR_TRANSFER:
  1060				len = xfer->len;
  1061				/* X1 SDR mode */
  1062				addr_config = tegra_qspi_addr_config(false, 0,
  1063								     xfer->len);
  1064				address_value = *((const u32 *)(xfer->tx_buf));
  1065				break;
  1066			case DATA_TRANSFER:
  1067				/* Program Command, Address value in register */
  1068				tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
  1069				tegra_qspi_writel(tqspi, address_value,
  1070						  QSPI_CMB_SEQ_ADDR);
  1071				/* Program Command and Address config in register */
  1072				tegra_qspi_writel(tqspi, cmd_config,
  1073						  QSPI_CMB_SEQ_CMD_CFG);
  1074				tegra_qspi_writel(tqspi, addr_config,
  1075						  QSPI_CMB_SEQ_ADDR_CFG);
  1076	
  1077				reinit_completion(&tqspi->xfer_completion);
  1078				cmd1 = tegra_qspi_setup_transfer_one(spi, xfer,
  1079								     is_first_msg);
  1080				ret = tegra_qspi_start_transfer_one(spi, xfer,
  1081								    cmd1);
  1082	
  1083				if (ret < 0) {
  1084					dev_err(tqspi->dev, "Failed to start transfer-one: %d\n",
  1085						ret);
  1086					return ret;
  1087				}
  1088	
  1089				is_first_msg = false;
  1090				ret = wait_for_completion_timeout
  1091						(&tqspi->xfer_completion,
  1092						QSPI_DMA_TIMEOUT);
  1093	
  1094				if (WARN_ON(ret == 0)) {
  1095					dev_err(tqspi->dev, "QSPI Transfer failed with timeout: %d\n",
  1096						ret);
  1097					if (tqspi->is_curr_dma_xfer &&
  1098					    (tqspi->cur_direction & DATA_DIR_TX))
  1099						dmaengine_terminate_all
  1100							(tqspi->tx_dma_chan);
  1101	
  1102					if (tqspi->is_curr_dma_xfer &&
  1103					    (tqspi->cur_direction & DATA_DIR_RX))
  1104						dmaengine_terminate_all
  1105							(tqspi->rx_dma_chan);
  1106	
  1107					/* Abort transfer by resetting pio/dma bit */
  1108					if (!tqspi->is_curr_dma_xfer) {
  1109						cmd1 = tegra_qspi_readl
  1110								(tqspi,
  1111								 QSPI_COMMAND1);
  1112						cmd1 &= ~QSPI_PIO;
  1113						tegra_qspi_writel
  1114								(tqspi, cmd1,
  1115								 QSPI_COMMAND1);
  1116					} else {
  1117						dma_ctl = tegra_qspi_readl
  1118								(tqspi,
  1119								 QSPI_DMA_CTL);
  1120						dma_ctl &= ~QSPI_DMA_EN;
  1121						tegra_qspi_writel(tqspi, dma_ctl,
  1122								  QSPI_DMA_CTL);
  1123					}
  1124	
  1125					/* Reset controller if timeout happens */
  1126					if (device_reset(tqspi->dev) < 0)
  1127						dev_warn_once(tqspi->dev,
  1128							      "device reset failed\n");
  1129					ret = -EIO;
  1130					goto exit;
  1131				}
  1132	
  1133				if (tqspi->tx_status ||  tqspi->rx_status) {
  1134					dev_err(tqspi->dev, "QSPI Transfer failed\n");
  1135					tqspi->tx_status = 0;
  1136					tqspi->rx_status = 0;
  1137					ret = -EIO;
  1138					goto exit;
  1139				}
> 1140			default:
  1141				goto exit;
  1142			}
  1143			msg->actual_length += xfer->len;
  1144			transfer_phase++;
  1145		}
  1146	
  1147	exit:
  1148		msg->status = ret;
  1149	
  1150		return ret;
  1151	}
  1152	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

[kernel test robot]

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

CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220222175611.58051-6-kyarlagadda@nvidia.com>
References: <20220222175611.58051-6-kyarlagadda@nvidia.com>
TO: Krishna Yarlagadda <kyarlagadda@nvidia.com>
TO: broonie(a)kernel.org
TO: thierry.reding(a)gmail.com
TO: jonathanh(a)nvidia.com
TO: linux-spi(a)vger.kernel.org
TO: linux-tegra(a)vger.kernel.org
CC: skomatineni(a)nvidia.com
CC: ldewangan(a)nvidia.com
CC: robh+dt(a)kernel.org
CC: devicetree(a)vger.kernel.org
CC: linux-kernel(a)vger.kernel.org
CC: p.zabel(a)pengutronix.de
CC: Krishna Yarlagadda <kyarlagadda@nvidia.com>

Hi Krishna,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on v5.17-rc5 next-20220222]
[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]

url:    https://github.com/0day-ci/linux/commits/Krishna-Yarlagadda/Tegra-QUAD-SPI-combined-sequence-mode/20220223-015906
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
:::::: branch date: 34 hours ago
:::::: commit date: 34 hours ago
config: riscv-randconfig-m031-20220222 (https://download.01.org/0day-ci/archive/20220224/202202241115.MFFaDVz5-lkp(a)intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/spi/spi-tegra210-quad.c:1148 tegra_qspi_combined_seq_xfer() error: uninitialized symbol 'ret'.

Old smatch warnings:
arch/riscv/include/asm/atomic.h:204 arch_atomic_fetch_add_unless() warn: inconsistent indenting

vim +/ret +1148 drivers/spi/spi-tegra210-quad.c

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1032  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1033  static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1034  					struct spi_message *msg)
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1035  {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1036  	bool is_first_msg = true;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1037  	struct spi_transfer *xfer;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1038  	struct spi_device *spi = msg->spi;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1039  	u8 transfer_phase = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1040  	u32 cmd1 = 0, dma_ctl = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1041  	int ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1042  	u32 address_value = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1043  	u32 cmd_config = 0, addr_config = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1044  	u8 cmd_value = 0, len = 0, val = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1045  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1046  	/* Enable Combined sequence mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1047  	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1048  	val |= QSPI_CMB_SEQ_EN;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1049  	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1050  	/* Process individual transfer list */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1051  	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1052  		switch (transfer_phase) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1053  		case CMD_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1054  			/* X1 SDR mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1055  			cmd_config = tegra_qspi_cmd_config(false, 0,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1056  							   xfer->len);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1057  			cmd_value = *((const u8 *)(xfer->tx_buf));
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1058  			break;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1059  		case ADDR_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1060  			len = xfer->len;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1061  			/* X1 SDR mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1062  			addr_config = tegra_qspi_addr_config(false, 0,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1063  							     xfer->len);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1064  			address_value = *((const u32 *)(xfer->tx_buf));
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1065  			break;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1066  		case DATA_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1067  			/* Program Command, Address value in register */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1068  			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1069  			tegra_qspi_writel(tqspi, address_value,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1070  					  QSPI_CMB_SEQ_ADDR);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1071  			/* Program Command and Address config in register */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1072  			tegra_qspi_writel(tqspi, cmd_config,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1073  					  QSPI_CMB_SEQ_CMD_CFG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1074  			tegra_qspi_writel(tqspi, addr_config,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1075  					  QSPI_CMB_SEQ_ADDR_CFG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1076  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1077  			reinit_completion(&tqspi->xfer_completion);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1078  			cmd1 = tegra_qspi_setup_transfer_one(spi, xfer,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1079  							     is_first_msg);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1080  			ret = tegra_qspi_start_transfer_one(spi, xfer,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1081  							    cmd1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1082  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1083  			if (ret < 0) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1084  				dev_err(tqspi->dev, "Failed to start transfer-one: %d\n",
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1085  					ret);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1086  				return ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1087  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1088  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1089  			is_first_msg = false;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1090  			ret = wait_for_completion_timeout
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1091  					(&tqspi->xfer_completion,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1092  					QSPI_DMA_TIMEOUT);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1093  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1094  			if (WARN_ON(ret == 0)) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1095  				dev_err(tqspi->dev, "QSPI Transfer failed with timeout: %d\n",
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1096  					ret);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1097  				if (tqspi->is_curr_dma_xfer &&
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1098  				    (tqspi->cur_direction & DATA_DIR_TX))
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1099  					dmaengine_terminate_all
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1100  						(tqspi->tx_dma_chan);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1101  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1102  				if (tqspi->is_curr_dma_xfer &&
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1103  				    (tqspi->cur_direction & DATA_DIR_RX))
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1104  					dmaengine_terminate_all
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1105  						(tqspi->rx_dma_chan);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1106  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1107  				/* Abort transfer by resetting pio/dma bit */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1108  				if (!tqspi->is_curr_dma_xfer) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1109  					cmd1 = tegra_qspi_readl
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1110  							(tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1111  							 QSPI_COMMAND1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1112  					cmd1 &= ~QSPI_PIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1113  					tegra_qspi_writel
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1114  							(tqspi, cmd1,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1115  							 QSPI_COMMAND1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1116  				} else {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1117  					dma_ctl = tegra_qspi_readl
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1118  							(tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1119  							 QSPI_DMA_CTL);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1120  					dma_ctl &= ~QSPI_DMA_EN;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1121  					tegra_qspi_writel(tqspi, dma_ctl,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1122  							  QSPI_DMA_CTL);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1123  				}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1124  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1125  				/* Reset controller if timeout happens */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1126  				if (device_reset(tqspi->dev) < 0)
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1127  					dev_warn_once(tqspi->dev,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1128  						      "device reset failed\n");
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1129  				ret = -EIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1130  				goto exit;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1131  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1132  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1133  			if (tqspi->tx_status ||  tqspi->rx_status) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1134  				dev_err(tqspi->dev, "QSPI Transfer failed\n");
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1135  				tqspi->tx_status = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1136  				tqspi->rx_status = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1137  				ret = -EIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1138  				goto exit;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1139  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1140  		default:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1141  			goto exit;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1142  		}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1143  		msg->actual_length += xfer->len;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1144  		transfer_phase++;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1145  	}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1146  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1147  exit:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22 @1148  	msg->status = ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1149  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1150  	return ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1151  }
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1152  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

[Dan Carpenter]

Hi Krishna,

url:    https://github.com/0day-ci/linux/commits/Krishna-Yarlagadda/Tegra-QUAD-SPI-combined-sequence-mode/20220223-015906
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: riscv-randconfig-m031-20220222 (https://download.01.org/0day-ci/archive/20220224/202202241115.MFFaDVz5-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/spi/spi-tegra210-quad.c:1148 tegra_qspi_combined_seq_xfer() error: uninitialized symbol 'ret'.

vim +/ret +1148 drivers/spi/spi-tegra210-quad.c

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1032  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1033  static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1034  					struct spi_message *msg)
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1035  {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1036  	bool is_first_msg = true;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1037  	struct spi_transfer *xfer;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1038  	struct spi_device *spi = msg->spi;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1039  	u8 transfer_phase = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1040  	u32 cmd1 = 0, dma_ctl = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1041  	int ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1042  	u32 address_value = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1043  	u32 cmd_config = 0, addr_config = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1044  	u8 cmd_value = 0, len = 0, val = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1045  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1046  	/* Enable Combined sequence mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1047  	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1048  	val |= QSPI_CMB_SEQ_EN;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1049  	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1050  	/* Process individual transfer list */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1051  	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1052  		switch (transfer_phase) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1053  		case CMD_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1054  			/* X1 SDR mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1055  			cmd_config = tegra_qspi_cmd_config(false, 0,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1056  							   xfer->len);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1057  			cmd_value = *((const u8 *)(xfer->tx_buf));
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1058  			break;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1059  		case ADDR_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1060  			len = xfer->len;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1061  			/* X1 SDR mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1062  			addr_config = tegra_qspi_addr_config(false, 0,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1063  							     xfer->len);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1064  			address_value = *((const u32 *)(xfer->tx_buf));
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1065  			break;

It's easy to imagine paths like this where ret is not set.

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1066  		case DATA_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1067  			/* Program Command, Address value in register */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1068  			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1069  			tegra_qspi_writel(tqspi, address_value,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1070  					  QSPI_CMB_SEQ_ADDR);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1071  			/* Program Command and Address config in register */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1072  			tegra_qspi_writel(tqspi, cmd_config,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1073  					  QSPI_CMB_SEQ_CMD_CFG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1074  			tegra_qspi_writel(tqspi, addr_config,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1075  					  QSPI_CMB_SEQ_ADDR_CFG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1076  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1077  			reinit_completion(&tqspi->xfer_completion);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1078  			cmd1 = tegra_qspi_setup_transfer_one(spi, xfer,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1079  							     is_first_msg);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1080  			ret = tegra_qspi_start_transfer_one(spi, xfer,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1081  							    cmd1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1082  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1083  			if (ret < 0) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1084  				dev_err(tqspi->dev, "Failed to start transfer-one: %d\n",
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1085  					ret);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1086  				return ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1087  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1088  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1089  			is_first_msg = false;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1090  			ret = wait_for_completion_timeout
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1091  					(&tqspi->xfer_completion,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1092  					QSPI_DMA_TIMEOUT);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1093  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1094  			if (WARN_ON(ret == 0)) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1095  				dev_err(tqspi->dev, "QSPI Transfer failed with timeout: %d\n",
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1096  					ret);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1097  				if (tqspi->is_curr_dma_xfer &&
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1098  				    (tqspi->cur_direction & DATA_DIR_TX))
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1099  					dmaengine_terminate_all
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1100  						(tqspi->tx_dma_chan);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1101  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1102  				if (tqspi->is_curr_dma_xfer &&
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1103  				    (tqspi->cur_direction & DATA_DIR_RX))
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1104  					dmaengine_terminate_all
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1105  						(tqspi->rx_dma_chan);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1106  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1107  				/* Abort transfer by resetting pio/dma bit */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1108  				if (!tqspi->is_curr_dma_xfer) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1109  					cmd1 = tegra_qspi_readl
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1110  							(tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1111  							 QSPI_COMMAND1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1112  					cmd1 &= ~QSPI_PIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1113  					tegra_qspi_writel
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1114  							(tqspi, cmd1,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1115  							 QSPI_COMMAND1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1116  				} else {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1117  					dma_ctl = tegra_qspi_readl
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1118  							(tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1119  							 QSPI_DMA_CTL);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1120  					dma_ctl &= ~QSPI_DMA_EN;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1121  					tegra_qspi_writel(tqspi, dma_ctl,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1122  							  QSPI_DMA_CTL);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1123  				}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1124  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1125  				/* Reset controller if timeout happens */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1126  				if (device_reset(tqspi->dev) < 0)
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1127  					dev_warn_once(tqspi->dev,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1128  						      "device reset failed\n");
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1129  				ret = -EIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1130  				goto exit;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1131  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1132  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1133  			if (tqspi->tx_status ||  tqspi->rx_status) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1134  				dev_err(tqspi->dev, "QSPI Transfer failed\n");
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1135  				tqspi->tx_status = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1136  				tqspi->rx_status = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1137  				ret = -EIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1138  				goto exit;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1139  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1140  		default:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1141  			goto exit;

Or here

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1142  		}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1143  		msg->actual_length += xfer->len;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1144  		transfer_phase++;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1145  	}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1146  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1147  exit:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22 @1148  	msg->status = ret;
                                                                      ^^^

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1149  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1150  	return ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1151  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

[Dan Carpenter]

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

Hi Krishna,

url:    https://github.com/0day-ci/linux/commits/Krishna-Yarlagadda/Tegra-QUAD-SPI-combined-sequence-mode/20220223-015906
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: riscv-randconfig-m031-20220222 (https://download.01.org/0day-ci/archive/20220224/202202241115.MFFaDVz5-lkp(a)intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/spi/spi-tegra210-quad.c:1148 tegra_qspi_combined_seq_xfer() error: uninitialized symbol 'ret'.

vim +/ret +1148 drivers/spi/spi-tegra210-quad.c

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1032  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1033  static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1034  					struct spi_message *msg)
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1035  {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1036  	bool is_first_msg = true;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1037  	struct spi_transfer *xfer;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1038  	struct spi_device *spi = msg->spi;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1039  	u8 transfer_phase = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1040  	u32 cmd1 = 0, dma_ctl = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1041  	int ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1042  	u32 address_value = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1043  	u32 cmd_config = 0, addr_config = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1044  	u8 cmd_value = 0, len = 0, val = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1045  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1046  	/* Enable Combined sequence mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1047  	val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1048  	val |= QSPI_CMB_SEQ_EN;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1049  	tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1050  	/* Process individual transfer list */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1051  	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1052  		switch (transfer_phase) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1053  		case CMD_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1054  			/* X1 SDR mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1055  			cmd_config = tegra_qspi_cmd_config(false, 0,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1056  							   xfer->len);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1057  			cmd_value = *((const u8 *)(xfer->tx_buf));
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1058  			break;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1059  		case ADDR_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1060  			len = xfer->len;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1061  			/* X1 SDR mode */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1062  			addr_config = tegra_qspi_addr_config(false, 0,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1063  							     xfer->len);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1064  			address_value = *((const u32 *)(xfer->tx_buf));
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1065  			break;

It's easy to imagine paths like this where ret is not set.

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1066  		case DATA_TRANSFER:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1067  			/* Program Command, Address value in register */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1068  			tegra_qspi_writel(tqspi, cmd_value, QSPI_CMB_SEQ_CMD);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1069  			tegra_qspi_writel(tqspi, address_value,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1070  					  QSPI_CMB_SEQ_ADDR);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1071  			/* Program Command and Address config in register */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1072  			tegra_qspi_writel(tqspi, cmd_config,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1073  					  QSPI_CMB_SEQ_CMD_CFG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1074  			tegra_qspi_writel(tqspi, addr_config,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1075  					  QSPI_CMB_SEQ_ADDR_CFG);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1076  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1077  			reinit_completion(&tqspi->xfer_completion);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1078  			cmd1 = tegra_qspi_setup_transfer_one(spi, xfer,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1079  							     is_first_msg);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1080  			ret = tegra_qspi_start_transfer_one(spi, xfer,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1081  							    cmd1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1082  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1083  			if (ret < 0) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1084  				dev_err(tqspi->dev, "Failed to start transfer-one: %d\n",
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1085  					ret);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1086  				return ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1087  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1088  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1089  			is_first_msg = false;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1090  			ret = wait_for_completion_timeout
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1091  					(&tqspi->xfer_completion,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1092  					QSPI_DMA_TIMEOUT);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1093  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1094  			if (WARN_ON(ret == 0)) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1095  				dev_err(tqspi->dev, "QSPI Transfer failed with timeout: %d\n",
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1096  					ret);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1097  				if (tqspi->is_curr_dma_xfer &&
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1098  				    (tqspi->cur_direction & DATA_DIR_TX))
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1099  					dmaengine_terminate_all
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1100  						(tqspi->tx_dma_chan);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1101  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1102  				if (tqspi->is_curr_dma_xfer &&
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1103  				    (tqspi->cur_direction & DATA_DIR_RX))
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1104  					dmaengine_terminate_all
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1105  						(tqspi->rx_dma_chan);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1106  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1107  				/* Abort transfer by resetting pio/dma bit */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1108  				if (!tqspi->is_curr_dma_xfer) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1109  					cmd1 = tegra_qspi_readl
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1110  							(tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1111  							 QSPI_COMMAND1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1112  					cmd1 &= ~QSPI_PIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1113  					tegra_qspi_writel
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1114  							(tqspi, cmd1,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1115  							 QSPI_COMMAND1);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1116  				} else {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1117  					dma_ctl = tegra_qspi_readl
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1118  							(tqspi,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1119  							 QSPI_DMA_CTL);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1120  					dma_ctl &= ~QSPI_DMA_EN;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1121  					tegra_qspi_writel(tqspi, dma_ctl,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1122  							  QSPI_DMA_CTL);
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1123  				}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1124  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1125  				/* Reset controller if timeout happens */
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1126  				if (device_reset(tqspi->dev) < 0)
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1127  					dev_warn_once(tqspi->dev,
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1128  						      "device reset failed\n");
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1129  				ret = -EIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1130  				goto exit;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1131  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1132  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1133  			if (tqspi->tx_status ||  tqspi->rx_status) {
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1134  				dev_err(tqspi->dev, "QSPI Transfer failed\n");
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1135  				tqspi->tx_status = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1136  				tqspi->rx_status = 0;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1137  				ret = -EIO;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1138  				goto exit;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1139  			}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1140  		default:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1141  			goto exit;

Or here

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1142  		}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1143  		msg->actual_length += xfer->len;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1144  		transfer_phase++;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1145  	}
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1146  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1147  exit:
3b852b330b0b83 Krishna Yarlagadda 2022-02-22 @1148  	msg->status = ret;
                                                                      ^^^

3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1149  
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1150  	return ret;
3b852b330b0b83 Krishna Yarlagadda 2022-02-22  1151  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH v2 4/5] spi: tegra210-quad: add acpi support

[Mark Brown]

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

On Wed, Feb 23, 2022 at 06:32:56AM +0000, Krishna Yarlagadda wrote:

> > > +	if (has_acpi_companion(tqspi->dev))
> > > +		return 0;
> > >  	/* flush all write which are in PPSB queue by reading back */
> > >  	tegra_qspi_readl(tqspi, QSPI_COMMAND1);

> > As well as clock stuff this is also skipping flushing of pending writes
> > - is that intentional?  It's not called out in the changelog and seems like
> > something that could cause issues if someone runs on a system where
> > the firmware does implement runtime suspend.

> Runtime suspend is not enabled with ACPI firmware. Converted compiler flag in v1 to runtime check.
> We must add more changes like setting DPM flags for runtime pm support with ACPI.
> Can take this as part of a different series.

It at least needs to be clearer what's going on here, the changelog
doesn't match the code and it's not obvious from the code that ACPI
won't kick in and power manage the device as things stand.

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

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

[Mark Brown]

On Tue, 22 Feb 2022 23:26:06 +0530, Krishna Yarlagadda wrote:
> Add ACPI support for Tegra210 QUAD SPI driver and support
> new Tegra194 feature, combined sequence mode.
> 
> v2 changes:
> - use combined sequence mode as default
> - remove property to switch transfer modes
> - fix compilation warnings
> 
> [...]

Applied to

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

Thanks!

[1/5] spi: tegra210-quad: use device_reset method
      commit: ac982578e7d340dc4f4fd243f4a4b24787d28c3f
[2/5] dt-bindings: spi: Tegra234 QUAD SPI compatible
      commit: de2f678b11bdcbabb6d804c543f9a3325c0e83bf
[3/5] spi: tegra210-quad: add new chips to compatible
      commit: ea23f0e148b82e5bcbc6c814926f53133552f0f3

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

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

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

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

Thanks,
Mark

RE: [PATCH v2 4/5] spi: tegra210-quad: add acpi support

[Krishna Yarlagadda]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 24 February 2022 23:52
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>; linux-spi@vger.kernel.org; linux-tegra@vger.kernel.org;
> Sowjanya Komatineni <skomatineni@nvidia.com>; Laxman Dewangan <ldewangan@nvidia.com>; robh+dt@kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; p.zabel@pengutronix.de
> Subject: Re: [PATCH v2 4/5] spi: tegra210-quad: add acpi support
> 
> On Wed, Feb 23, 2022 at 06:32:56AM +0000, Krishna Yarlagadda wrote:
> 
> > > > +	if (has_acpi_companion(tqspi->dev))
> > > > +		return 0;
> > > >  	/* flush all write which are in PPSB queue by reading back */
> > > >  	tegra_qspi_readl(tqspi, QSPI_COMMAND1);
> 
> > > As well as clock stuff this is also skipping flushing of pending writes
> > > - is that intentional?  It's not called out in the changelog and seems like
> > > something that could cause issues if someone runs on a system where
> > > the firmware does implement runtime suspend.
> 
> > Runtime suspend is not enabled with ACPI firmware. Converted compiler flag in v1 to runtime check.
> > We must add more changes like setting DPM flags for runtime pm support with ACPI.
> > Can take this as part of a different series.
> 
> It at least needs to be clearer what's going on here, the changelog
> doesn't match the code and it's not obvious from the code that ACPI
> won't kick in and power manage the device as things stand.
Understood Mark. I will add comments to make it clear.
Also update change log indicating runtime suspend does not work
with ACPI.