Hello Dan,
On 6/16/2023 6:12 PM, Dan Carpenter wrote:
Hello Vijaya Krishna Nivarthi,
The patch b5762d95607e: "spi: spi-qcom-qspi: Add DMA mode support"
from Apr 24, 2023, leads to the following Smatch static checker
warning:
drivers/spi/spi-qcom-qspi.c:368 qcom_qspi_setup_dma_desc()
warn: sleeping in atomic context
drivers/spi/spi-qcom-qspi.c
336 static int qcom_qspi_setup_dma_desc(struct qcom_qspi *ctrl,
337 struct spi_transfer *xfer)
338 {
339 int ret;
340 struct sg_table *sgt;
341 dma_addr_t dma_ptr_sg;
342 unsigned int dma_len_sg;
343 int i;
344
345 if (ctrl->n_cmd_desc) {
346 dev_err(ctrl->dev, "Remnant dma buffers n_cmd_desc-%d\n", ctrl->n_cmd_desc);
347 return -EIO;
348 }
349
350 sgt = (ctrl->xfer.dir == QSPI_READ) ? &xfer->rx_sg : &xfer->tx_sg;
351 if (!sgt->nents || sgt->nents > QSPI_MAX_SG) {
352 dev_warn_once(ctrl->dev, "Cannot handle %d entries in scatter list\n", sgt->nents);
353 return -EAGAIN;
354 }
355
356 for (i = 0; i < sgt->nents; i++) {
357 dma_ptr_sg = sg_dma_address(sgt->sgl + i);
358 if (!IS_ALIGNED(dma_ptr_sg, QSPI_ALIGN_REQ)) {
359 dev_warn_once(ctrl->dev, "dma_address not aligned to %d\n", QSPI_ALIGN_REQ);
360 return -EAGAIN;
361 }
362 }
363
364 for (i = 0; i < sgt->nents; i++) {
365 dma_ptr_sg = sg_dma_address(sgt->sgl + i);
366 dma_len_sg = sg_dma_len(sgt->sgl + i);
367
--> 368 ret = qcom_qspi_alloc_desc(ctrl, dma_ptr_sg, dma_len_sg);
The qcom_qspi_alloc_desc() is a sleeping allocation, but the caller
qcom_qspi_transfer_one() is holding spin_lock_irqsave(&ctrl->lock, flags).
I think We should be able to move qcom_qspi_setup_dma_desc() , and thus
qcom_qspi_alloc_desc(), out of critical section.
Right now we are looking into some issues with DMA mode transfer that
have showed up recently and will likely have a Fixes: patch next week.
Can we include this change in same series?
Another option is to include flags __GFP_ATOMIC in call to
dma_pool_alloc() and upload a Fixes: patch right away.
Would that work?
Can you please advise?
Thank you,
Vijay/
369 if (ret)
370 goto cleanup;
371 }
372 return 0;
373
374 cleanup:
375 for (i = 0; i < ctrl->n_cmd_desc; i++)
376 dma_pool_free(ctrl->dma_cmd_pool, ctrl->virt_cmd_desc[i],
377 ctrl->dma_cmd_desc[i]);
378 ctrl->n_cmd_desc = 0;
379 return ret;
380 }
regards,
dan carpenter