On 01-02-22, 13:38, Ben Walker wrote:
> These are only used by a single driver (pxa_camera) which knows exactly
> which DMA engine it is dealing with and therefore knows the behavior
> of the cookie values. It can grab the value it needs directly from
> the dma_chan instead. No other DMA client assumes anything about
> the behavior of the cookie values, so the cookie becomes an opaque
> handle after this patch.
>
> Signed-off-by: Ben Walker <benjamin.walker@xxxxxxxxx>
> ---
> Documentation/driver-api/dmaengine/client.rst | 17 +++--------------
> .../driver-api/dmaengine/provider.rst | 6 +++---
> drivers/crypto/stm32/stm32-hash.c | 3 +--
> drivers/dma/dmaengine.c | 2 +-
> drivers/dma/dmatest.c | 3 +--
> drivers/media/platform/omap/omap_vout_vrfb.c | 2 +-
> drivers/media/platform/pxa_camera.c | 13 +++++++++++--
> drivers/rapidio/devices/rio_mport_cdev.c | 3 +--
> include/linux/dmaengine.h | 18 +++---------------
This needs to be split per subsysytem and cc respective maintainers so
that they can review and ack it...
> 9 files changed, 25 insertions(+), 42 deletions(-)
>
> diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
> index 85ecec2c40005..6e43c9c428e68 100644
> --- a/Documentation/driver-api/dmaengine/client.rst
> +++ b/Documentation/driver-api/dmaengine/client.rst
> @@ -259,8 +259,8 @@ The details of these operations are:
>
> dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc)
>
> - This returns a cookie can be used to check the progress of DMA engine
> - activity via other DMA engine calls not covered in this document.
> + This returns a cookie that can be used to check the progress of a transaction
> + via dma_async_is_tx_complete().
>
> dmaengine_submit() will not start the DMA operation, it merely adds
> it to the pending queue. For this, see step 5, dma_async_issue_pending.
> @@ -340,22 +340,11 @@ Further APIs
> .. code-block:: c
>
> enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
> - dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
> -
> - This can be used to check the status of the channel. Please see
> - the documentation in include/linux/dmaengine.h for a more complete
> - description of this API.
> + dma_cookie_t cookie)
>
> This can be used with the cookie returned from dmaengine_submit()
> to check for completion of a specific DMA transaction.
>
> - .. note::
> -
> - Not all DMA engine drivers can return reliable information for
> - a running DMA channel. It is recommended that DMA engine users
> - pause or stop (via dmaengine_terminate_all()) the channel before
> - using this API.
> -
> 5. Synchronize termination API
>
> .. code-block:: c
> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
> index 0072c9c7efd34..e9e9de18d6b02 100644
> --- a/Documentation/driver-api/dmaengine/provider.rst
> +++ b/Documentation/driver-api/dmaengine/provider.rst
> @@ -543,10 +543,10 @@ where to put them)
>
> dma_cookie_t
>
> -- it's a DMA transaction ID that will increment over time.
> +- it's a DMA transaction ID.
why drop this?
>
> -- Not really relevant any more since the introduction of ``virt-dma``
> - that abstracts it away.
> +- The value can be chosen by the provider, or use the helper APIs
> + such as dma_cookie_assign() and dma_cookie_complete().
>
> DMA_CTRL_ACK
>
> diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
> index d33006d43f761..9c3b6526e39f8 100644
> --- a/drivers/crypto/stm32/stm32-hash.c
> +++ b/drivers/crypto/stm32/stm32-hash.c
> @@ -453,8 +453,7 @@ static int stm32_hash_xmit_dma(struct stm32_hash_dev *hdev,
> msecs_to_jiffies(100)))
> err = -ETIMEDOUT;
>
> - if (dma_async_is_tx_complete(hdev->dma_lch, cookie,
> - NULL, NULL) != DMA_COMPLETE)
> + if (dma_async_is_tx_complete(hdev->dma_lch, cookie) != DMA_COMPLETE)
> err = -ETIMEDOUT;
>
> if (err) {
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 2cfa8458b51be..590f238a0671a 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -523,7 +523,7 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
>
> dma_async_issue_pending(chan);
> do {
> - status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
> + status = dma_async_is_tx_complete(chan, cookie);
> if (time_after_eq(jiffies, dma_sync_wait_timeout)) {
> dev_err(chan->device->dev, "%s: timeout!\n", __func__);
> return DMA_ERROR;
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index f696246f57fdb..0574090a80c8c 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -832,8 +832,7 @@ static int dmatest_func(void *data)
> done->done,
> msecs_to_jiffies(params->timeout));
>
> - status = dma_async_is_tx_complete(chan, cookie, NULL,
> - NULL);
> + status = dma_async_is_tx_complete(chan, cookie);
> }
>
> if (!done->done) {
> diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c b/drivers/media/platform/omap/omap_vout_vrfb.c
> index 0cfa0169875f0..d762939ffa5de 100644
> --- a/drivers/media/platform/omap/omap_vout_vrfb.c
> +++ b/drivers/media/platform/omap/omap_vout_vrfb.c
> @@ -289,7 +289,7 @@ int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
> vout->vrfb_dma_tx.tx_status == 1,
> VRFB_TX_TIMEOUT);
>
> - status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
> + status = dma_async_is_tx_complete(chan, cookie);
>
> if (vout->vrfb_dma_tx.tx_status == 0) {
> pr_err("%s: Timeout while waiting for DMA\n", __func__);
> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> index 3ba00b0f93200..6d5c36ea6622a 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -1049,8 +1049,17 @@ static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
> last_buf = list_entry(pcdev->capture.prev,
> struct pxa_buffer, queue);
> last_status = dma_async_is_tx_complete(pcdev->dma_chans[chan],
> - last_buf->cookie[chan],
> - NULL, &last_issued);
> + last_buf->cookie[chan]);
> + /*
> + * Peek into the channel and read the last cookie that was issued.
> + * This is a layering violation - the dmaengine API does not officially
> + * provide this information. Since this camera driver is tightly coupled
> + * with a specific DMA device we know exactly how this cookie value will
> + * behave. Otherwise, this wouldn't be safe.
> + */
> + last_issued = pcdev->dma_chans[chan]->cookie;
> + barrier();
> +
> if (camera_status & overrun &&
> last_status != DMA_COMPLETE) {
> dev_dbg(pcdev_to_dev(pcdev), "FIFO overrun! CISR: %x\n",
> diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
> index 7df466e222820..790e0c7a3306c 100644
> --- a/drivers/rapidio/devices/rio_mport_cdev.c
> +++ b/drivers/rapidio/devices/rio_mport_cdev.c
> @@ -597,8 +597,7 @@ static void dma_xfer_callback(void *param)
> struct mport_dma_req *req = (struct mport_dma_req *)param;
> struct mport_cdev_priv *priv = req->priv;
>
> - req->status = dma_async_is_tx_complete(priv->dmach, req->cookie,
> - NULL, NULL);
> + req->status = dma_async_is_tx_complete(priv->dmach, req->cookie);
> complete(&req->req_comp);
> kref_put(&req->refcount, dma_req_free);
> }
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 8c4934bc038ec..5f884fffe74cc 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1436,25 +1436,13 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
> * dma_async_is_tx_complete - poll for transaction completion
> * @chan: DMA channel
> * @cookie: transaction identifier to check status of
> - * @last: returns last completed cookie, can be NULL
> - * @used: returns last issued cookie, can be NULL
> - *
> - * If @last and @used are passed in, upon return they reflect the most
> - * recently submitted (used) cookie and the most recently completed
> - * cookie.
> */
> static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
> - dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
> + dma_cookie_t cookie)
With this I think we should take this opprtunity to rename the api to
dmaengine_async_is_tx_complete()
--
~Vinod
