On Thu, Jun 13, 2024 at 05:37:58PM +0300, Dan Carpenter wrote:
> Hello Peng Ma,
>
> Commit b092529e0aa0 ("dmaengine: fsl-qdma: Add qDMA controller driver
> for Layerscape SoCs") from Oct 30, 2018 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/dma/fsl-qdma.c:331 fsl_qdma_free_chan_resources()
> error: we previously assumed 'fsl_queue->comp_pool' could be null (see line 324)
Thank you for report.
This will never happen. fsl_qdma_alloc_chan_resource() will return failure
if one of comp_pool and desc_pool is NULL. then fsl_qdma_free_chan_resource()
will never be called.
I think it is false alarm by static checker tools. This check is not
necessary at all.
Frank
>
> drivers/dma/fsl-qdma.c
> 309 static void fsl_qdma_free_chan_resources(struct dma_chan *chan)
> 310 {
> 311 struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
> 312 struct fsl_qdma_queue *fsl_queue = fsl_chan->queue;
> 313 struct fsl_qdma_engine *fsl_qdma = fsl_chan->qdma;
> 314 struct fsl_qdma_comp *comp_temp, *_comp_temp;
> 315 unsigned long flags;
> 316 LIST_HEAD(head);
> 317
> 318 spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> 319 vchan_get_all_descriptors(&fsl_chan->vchan, &head);
> 320 spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> 321
> 322 vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
> 323
> 324 if (!fsl_queue->comp_pool && !fsl_queue->desc_pool)
>
> This should probably be || instead of &&.
>
> 325 return;
> 326
> 327 list_for_each_entry_safe(comp_temp, _comp_temp,
> 328 &fsl_queue->comp_used, list) {
> 329 dma_pool_free(fsl_queue->comp_pool,
> ^^^^^^^^^^^^^^^^^^^^
> If only one is free but not the other then it leads to an Oops.
>
> 330 comp_temp->virt_addr,
> --> 331 comp_temp->bus_addr);
> 332 dma_pool_free(fsl_queue->desc_pool,
> ^^^^^^^^^^^^^^^^^^^^
>
> 333 comp_temp->desc_virt_addr,
> 334 comp_temp->desc_bus_addr);
> 335 list_del(&comp_temp->list);
> 336 kfree(comp_temp);
> 337 }
>
> regards,
> dan carpenter
