From 17aaf9ea07b656016316dc37716e987742b3e296 Mon Sep 17 00:00:00 2001 From: Vijaya Krishna Nivarthi Date: Tue, 25 Jul 2023 17:18:06 +0530 Subject: [PATCH 1/6] spi: spi-qcom-qspi: Ignore disabled interrupts' status in isr During FIFO/DMA modes dynamic switching, only corresponding interrupts are enabled. However its possible that FIFO related interrupt status registers get set during DMA mode. For example WR_FIFO_EMPTY bit is set during DMA TX. Ignore such status bits so that they don't trip unwanted operations. Suggested-by: Douglas Anderson Signed-off-by: Vijaya Krishna Nivarthi Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support") Reviewed-by: Douglas Anderson Tested-by: Douglas Anderson Link: https://lore.kernel.org/r/1690285689-30233-2-git-send-email-quic_vnivarth@quicinc.com Signed-off-by: Mark Brown --- drivers/spi/spi-qcom-qspi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index a8a683d6145c..21a13605d3b0 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -603,6 +603,9 @@ static irqreturn_t qcom_qspi_irq(int irq, void *dev_id) int_status = readl(ctrl->base + MSTR_INT_STATUS); writel(int_status, ctrl->base + MSTR_INT_STATUS); + /* Ignore disabled interrupts */ + int_status &= readl(ctrl->base + MSTR_INT_EN); + /* PIO mode handling */ if (ctrl->xfer.dir == QSPI_WRITE) { if (int_status & WR_FIFO_EMPTY) From f7ba36d399c4558f36886adff9400be591b245f6 Mon Sep 17 00:00:00 2001 From: Vijaya Krishna Nivarthi Date: Tue, 25 Jul 2023 17:18:07 +0530 Subject: [PATCH 2/6] spi: spi-qcom-qspi: Use GFP_ATOMIC flag while allocating for descriptor While allocating for DMA descriptor, GFP_KERNEL flag is being used and this allocation happens within critical section with spinlock acquired. This generates a static checker warning. Use GFP_ATOMIC to prevent sleeping; and since this increases chances of allocation failure, add handling accordingly. Reported-by: Dan Carpenter Closes: https://lore.kernel.org/all/abc223e8-44af-40bb-a0bd-9865b393f435@moroto.mountain/ Signed-off-by: Vijaya Krishna Nivarthi Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support") Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/1690285689-30233-3-git-send-email-quic_vnivarth@quicinc.com Signed-off-by: Mark Brown --- drivers/spi/spi-qcom-qspi.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index 21a13605d3b0..67c101792a74 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -308,9 +308,11 @@ static int qcom_qspi_alloc_desc(struct qcom_qspi *ctrl, dma_addr_t dma_ptr, dma_addr_t dma_cmd_desc; /* allocate for dma cmd descriptor */ - virt_cmd_desc = dma_pool_alloc(ctrl->dma_cmd_pool, GFP_KERNEL | __GFP_ZERO, &dma_cmd_desc); - if (!virt_cmd_desc) - return -ENOMEM; + virt_cmd_desc = dma_pool_alloc(ctrl->dma_cmd_pool, GFP_ATOMIC | __GFP_ZERO, &dma_cmd_desc); + if (!virt_cmd_desc) { + dev_warn_once(ctrl->dev, "Couldn't find memory for descriptor\n"); + return -EAGAIN; + } ctrl->virt_cmd_desc[ctrl->n_cmd_desc] = virt_cmd_desc; ctrl->dma_cmd_desc[ctrl->n_cmd_desc] = dma_cmd_desc; From cfb81f2243b25a0d79accc6510ad66c5c5ad99ba Mon Sep 17 00:00:00 2001 From: Vijaya Krishna Nivarthi Date: Tue, 25 Jul 2023 17:18:08 +0530 Subject: [PATCH 3/6] spi: spi-qcom-qspi: Call dma_wmb() after setting up descriptors After setting up dma descriptors and before initiaiting dma transfer, call dma_wmb() to ensure all writes go through. This doesn't fix any reported problem but is added for safety. Signed-off-by: Vijaya Krishna Nivarthi Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support") Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/1690285689-30233-4-git-send-email-quic_vnivarth@quicinc.com Signed-off-by: Mark Brown --- drivers/spi/spi-qcom-qspi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index 67c101792a74..75fcb9145d91 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -443,8 +443,10 @@ static int qcom_qspi_transfer_one(struct spi_master *master, ret = qcom_qspi_setup_dma_desc(ctrl, xfer); if (ret != -EAGAIN) { - if (!ret) + if (!ret) { + dma_wmb(); qcom_qspi_dma_xfer(ctrl); + } goto exit; } dev_warn_once(ctrl->dev, "DMA failure, falling back to PIO\n"); From 916a4edf3daed845b1e5d6cf0578a7e43c6f520e Mon Sep 17 00:00:00 2001 From: Vijaya Krishna Nivarthi Date: Tue, 25 Jul 2023 17:18:09 +0530 Subject: [PATCH 4/6] spi: spi-qcom-qspi: Add DMA_CHAIN_DONE to ALL_IRQS Add latest added DMA_CHAIN_DONE irq to QSPI_ALL_IRQS that encompasses all of the qspi IRQs. Signed-off-by: Vijaya Krishna Nivarthi Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/1690285689-30233-5-git-send-email-quic_vnivarth@quicinc.com Signed-off-by: Mark Brown --- drivers/spi/spi-qcom-qspi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index 75fcb9145d91..9b3092e22942 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -69,7 +69,7 @@ WR_FIFO_OVERRUN) #define QSPI_ALL_IRQS (QSPI_ERR_IRQS | RESP_FIFO_RDY | \ WR_FIFO_EMPTY | WR_FIFO_FULL | \ - TRANSACTION_DONE) + TRANSACTION_DONE | DMA_CHAIN_DONE) #define PIO_XFER_CTRL 0x0014 #define REQUEST_COUNT_MSK 0xffff From 138d73b627c71bf2b2f61502dc6c1137b9656434 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Tue, 25 Jul 2023 11:02:26 -0700 Subject: [PATCH 5/6] spi: spi-qcom-qspi: Fallback to PIO for xfers that aren't multiples of 4 bytes The Qualcomm QSPI driver appears to require that any reads using DMA are a mutliple of 4 bytes. If this isn't true then the controller will clobber any extra bytes in memory following the last word. Let's detect this and falback to PIO. This fixes problems reported by slub_debug=FZPUA, which would complain about "kmalloc Redzone overwritten". One such instance said: 0xffffff80c29d541a-0xffffff80c29d541b @offset=21530. First byte 0x0 instead of 0xcc Allocated in mtd_kmalloc_up_to+0x98/0xac age=36 cpu=3 pid=6658 Tracing through what was happening I saw that, while we often did DMA tranfers of 0x1000 bytes, sometimes we'd end up doing ones of 0x41a bytes. Those 0x41a byte transfers were the problem. NOTE: a future change will enable the SPI "mem ops" to help avoid this case, but it still seems good to add the extra check in the transfer. Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support") Signed-off-by: Douglas Anderson Reviewed-by: Vijaya Krishna Nivarthi Reviewed-by: Bjorn Andersson Link: https://lore.kernel.org/r/20230725110226.1.Ia2f980fc7cd0b831e633391f0bb1272914d8f381@changeid Signed-off-by: Mark Brown --- drivers/spi/spi-qcom-qspi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index 9b3092e22942..db1dd133762e 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -357,10 +357,22 @@ static int qcom_qspi_setup_dma_desc(struct qcom_qspi *ctrl, for (i = 0; i < sgt->nents; i++) { dma_ptr_sg = sg_dma_address(sgt->sgl + i); + dma_len_sg = sg_dma_len(sgt->sgl + i); if (!IS_ALIGNED(dma_ptr_sg, QSPI_ALIGN_REQ)) { dev_warn_once(ctrl->dev, "dma_address not aligned to %d\n", QSPI_ALIGN_REQ); return -EAGAIN; } + /* + * When reading with DMA the controller writes to memory 1 word + * at a time. If the length isn't a multiple of 4 bytes then + * the controller can clobber the things later in memory. + * Fallback to PIO to be safe. + */ + if (ctrl->xfer.dir == QSPI_READ && (dma_len_sg & 0x03)) { + dev_warn_once(ctrl->dev, "fallback to PIO for read of size %#010x\n", + dma_len_sg); + return -EAGAIN; + } } for (i = 0; i < sgt->nents; i++) { From cc71c42b3dc1085d3e72dfa5603e827b9eb59da1 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Tue, 25 Jul 2023 11:02:27 -0700 Subject: [PATCH 6/6] spi: spi-qcom-qspi: Add mem_ops to avoid PIO for badly sized reads In the patch ("spi: spi-qcom-qspi: Fallback to PIO for xfers that aren't multiples of 4 bytes") we detect reads that we can't handle properly and fallback to PIO mode. While that's correct behavior, we can do better by adding "spi_controller_mem_ops" for our controller. Once we do this then the caller will give us a transfer that's a multiple of 4-bytes so we can DMA. Fixes: b5762d95607e ("spi: spi-qcom-qspi: Add DMA mode support") Signed-off-by: Douglas Anderson Reviewed-by: Vijaya Krishna Nivarthi Link: https://lore.kernel.org/r/20230725110226.2.Id4a39804e01e4a06dae9b73fd2a5194c4c7ea453@changeid Signed-off-by: Mark Brown --- drivers/spi/spi-qcom-qspi.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index db1dd133762e..1954c39b3d08 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -666,6 +666,30 @@ static irqreturn_t qcom_qspi_irq(int irq, void *dev_id) return ret; } +static int qcom_qspi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op) +{ + /* + * If qcom_qspi_can_dma() is going to return false we don't need to + * adjust anything. + */ + if (op->data.nbytes <= QSPI_MAX_BYTES_FIFO) + return 0; + + /* + * When reading, the transfer needs to be a multiple of 4 bytes so + * shrink the transfer if that's not true. The caller will then do a + * second transfer to finish things up. + */ + if (op->data.dir == SPI_MEM_DATA_IN && (op->data.nbytes & 0x3)) + op->data.nbytes &= ~0x3; + + return 0; +} + +static const struct spi_controller_mem_ops qcom_qspi_mem_ops = { + .adjust_op_size = qcom_qspi_adjust_op_size, +}; + static int qcom_qspi_probe(struct platform_device *pdev) { int ret; @@ -750,6 +774,7 @@ static int qcom_qspi_probe(struct platform_device *pdev) if (of_property_read_bool(pdev->dev.of_node, "iommus")) master->can_dma = qcom_qspi_can_dma; master->auto_runtime_pm = true; + master->mem_ops = &qcom_qspi_mem_ops; ret = devm_pm_opp_set_clkname(&pdev->dev, "core"); if (ret)