* [bug report] spi: spi-qcom-qspi: Add DMA mode support
@ 2023-06-16 12:42 Dan Carpenter
2023-06-16 14:12 ` Vijaya Krishna Nivarthi
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2023-06-16 12:42 UTC (permalink / raw)
To: quic_vnivarth; +Cc: linux-spi
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).
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] spi: spi-qcom-qspi: Add DMA mode support
2023-06-16 12:42 [bug report] spi: spi-qcom-qspi: Add DMA mode support Dan Carpenter
@ 2023-06-16 14:12 ` Vijaya Krishna Nivarthi
2023-06-16 14:24 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Vijaya Krishna Nivarthi @ 2023-06-16 14:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-spi, Doug Anderson, Stephen Boyd, Mark Brown
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] spi: spi-qcom-qspi: Add DMA mode support
2023-06-16 14:12 ` Vijaya Krishna Nivarthi
@ 2023-06-16 14:24 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2023-06-16 14:24 UTC (permalink / raw)
To: Vijaya Krishna Nivarthi
Cc: linux-spi, Doug Anderson, Stephen Boyd, Mark Brown
On Fri, Jun 16, 2023 at 07:42:30PM +0530, Vijaya Krishna Nivarthi wrote:
> 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?
This is from static analysis not testing. It doesn't affect me at all,
so I don't have any opinion. I imagine that fixing it next week is
fine.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-16 14:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-16 12:42 [bug report] spi: spi-qcom-qspi: Add DMA mode support Dan Carpenter
2023-06-16 14:12 ` Vijaya Krishna Nivarthi
2023-06-16 14:24 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).