| Message ID | 146887860471.16107.12723305692901007453.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 7/18/2016 5:50 PM, Dave Jiang wrote: > list_for_each_entry_safe(mdesc, tmp, &list, node) { > struct dma_async_tx_descriptor *txd = &mdesc->desc; > - dma_async_tx_callback callback = mdesc->desc.callback; > - void *param = mdesc->desc.callback_param; > + struct dmaengine_desc_callback cb; > > + dmaengine_desc_get_callback(&mdesc->desc, &cb); > dma_descriptor_unmap(txd); > > - if (callback) > - callback(param); > + dmaengine_desc_callback_invoke(&cb, NULL); This could have been done as + struct dmaengine_desc_callback cb; + + dmaengine_desc_get_callback_invoke(desc, &cb, NULL); too.
> -----Original Message----- > From: Sinan Kaya [mailto:okaya@codeaurora.org] > Sent: Tuesday, July 19, 2016 11:18 AM > To: Jiang, Dave <dave.jiang@intel.com>; Koul, Vinod <vinod.koul@intel.com>; lars@metafoo.de > Cc: dmaengine@vger.kernel.org; Williams, Dan J <dan.j.williams@intel.com>; laurent.pinchart@ideasonboard.com > Subject: Re: [PATCH v3 25/41] dmaengine: qcom_hidma: convert callback to helper function > > On 7/18/2016 5:50 PM, Dave Jiang wrote: > > list_for_each_entry_safe(mdesc, tmp, &list, node) { > > struct dma_async_tx_descriptor *txd = &mdesc->desc; > > - dma_async_tx_callback callback = mdesc->desc.callback; > > - void *param = mdesc->desc.callback_param; > > + struct dmaengine_desc_callback cb; > > > > + dmaengine_desc_get_callback(&mdesc->desc, &cb); > > dma_descriptor_unmap(txd); > > > > - if (callback) > > - callback(param); > > + dmaengine_desc_callback_invoke(&cb, NULL); > > This could have been done as > > + struct dmaengine_desc_callback cb; > + > + dmaengine_desc_get_callback_invoke(desc, &cb, NULL); > > too. Do you want the unmap before or after the callback? I think they all should be before, but I've seen quite a few drivers doing it after and I'm wondering if they are wrong.... > > -- > Sinan Kaya > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. > Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
On 7/19/2016 2:22 PM, Jiang, Dave wrote:
> Do you want the unmap before or after the callback? I think they all should be before, but I've seen quite a few drivers doing it after and I'm wondering if they are wrong....
Before. Let's not change the current behavior.
diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c index 41b5c6d..9411f66 100644 --- a/drivers/dma/qcom/hidma.c +++ b/drivers/dma/qcom/hidma.c @@ -132,8 +132,11 @@ static void hidma_process_completed(struct hidma_chan *mchan) spin_unlock_irqrestore(&mchan->lock, irqflags); llstat = hidma_ll_status(mdma->lldev, mdesc->tre_ch); - if (desc->callback && (llstat == DMA_COMPLETE)) - desc->callback(desc->callback_param); + if (llstat == DMA_COMPLETE) { + struct dmaengine_desc_callback cb; + + dmaengine_desc_get_callback_invoke(desc, &cb, NULL); + } last_cookie = desc->cookie; dma_run_dependencies(desc); @@ -413,13 +416,12 @@ static int hidma_terminate_channel(struct dma_chan *chan) /* return all user requests */ list_for_each_entry_safe(mdesc, tmp, &list, node) { struct dma_async_tx_descriptor *txd = &mdesc->desc; - dma_async_tx_callback callback = mdesc->desc.callback; - void *param = mdesc->desc.callback_param; + struct dmaengine_desc_callback cb; + dmaengine_desc_get_callback(&mdesc->desc, &cb); dma_descriptor_unmap(txd); - if (callback) - callback(param); + dmaengine_desc_callback_invoke(&cb, NULL); dma_run_dependencies(txd);
Convert driver to use the new helper function for callback Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/dma/qcom/hidma.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html