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
Use device_reset api to replace duplicate code in driver to call
reset_control_get api with reset handle.
Signed-off-by: Krishna Yarlagadda <[email protected]>
---
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
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 <[email protected]>
---
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
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?
Add support for Tegra234 and soc data to select capabilities.
Signed-off-by: Krishna Yarlagadda <[email protected]>
---
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
Add compatible string for Tegra234 for Tegra QUAD SPI
Signed-off-by: Krishna Yarlagadda <[email protected]>
---
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
> -----Original Message-----
> From: Mark Brown <[email protected]>
> Sent: 23 February 2022 00:12
> To: Krishna Yarlagadda <[email protected]>
> Cc: [email protected]; Jonathan Hunter
> <[email protected]>; [email protected]; linux-
> [email protected]; Sowjanya Komatineni
> <[email protected]>; Laxman Dewangan
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]
> 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
Regards
KY
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 23 February 2022 01:57
> To: Krishna Yarlagadda <[email protected]>
> Cc: [email protected]; [email protected]; Jonathan Hunter
> <[email protected]>; [email protected]; linux-
> [email protected]; Sowjanya Komatineni
> <[email protected]>; Laxman Dewangan
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> 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 <[email protected]>
> > ---
> > 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.
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 <[email protected]>
> ---
> 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.
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/[email protected]/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 <[email protected]>
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/[email protected]
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/[email protected]/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 <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
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/[email protected]
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