All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Sinan Kaya <okaya@codeaurora.org>
Cc: Vinod Koul <vinod.koul@intel.com>,
	linux-arm-msm@vger.kernel.org, timur@codeaurora.org,
	linux-kernel@vger.kernel.org,
	Christopher Covington <cov@codeaurora.org>,
	dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] dmaengine: qcom_hidma: release the descriptor before the callback
Date: Thu, 4 Aug 2016 15:40:04 +0100	[thread overview]
Message-ID: <20160804144003.GV1041@n2100.armlinux.org.uk> (raw)
In-Reply-To: <71a15611-645f-7523-1c26-14b420aff667@codeaurora.org>

On Thu, Aug 04, 2016 at 10:17:24AM -0400, Sinan Kaya wrote:
> On 8/4/2016 8:55 AM, Vinod Koul wrote:
> > Dmaengine tells transaction is complete. It does not say if the txn is
> > success or failure. It can transfer data and not say if data was
> > correct. A successful transaction implies data integrity as well, which
> > dmaengine can't provide.
> 
> Thanks for describing this. I was confused about DMA_SUCCESS and DMA_COMPLETE.
> I now understand that tx_success API just returns information that the request
> was executed whether the result is error or not. This makes sense now.
> 
> However, if the txn is failure; then we should never call the client callback
> since DMA engine cannot provide such feedback to the client without Dave's patch.
> You are saying that the calling the callback is optional.
> 
> Then, the callback cannot be optional in the error case for old behavior.
> 
> How does the client know if memcpy executed or not? The client got its callback
> and tx_status is also DMA_COMPLETE.

If an error occurred, then dma_async_is_tx_complete() is supposed to
return DMA_ERROR.  It's up to the DMA engine driver to ensure that
this happens if it has error detection abilities.

Most of the helpers in drivers/dma/dmaengine.h are there to _assist_
the driver writer - they can't do magic.  dma_cookie_status() will
return from the point of view of the generic DMA code what the status
of a particular cookie is, and the cookie state.  It doesn't take
care of whether a particular transaction associated with a cookie
failed or not - that's up to the driver.

So, if dma_cookie_status() says that a cookie has DMA_COMPLETED
status, and the DMA engine is able to detect errors on individual
transfers, then the driver needs to do further status lookup to
determine whether the particular transaction referred to by the
cookie did fail, and modify the returned status appropriately.

If dma_cookie_status() says that the cookie is DMA_IN_PROGRESS,
then the driver is expected to calculate and report the residue
(the remaining number of bytes) of the referred to transaction.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

WARNING: multiple messages have this Message-ID (diff)
From: linux@armlinux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] dmaengine: qcom_hidma: release the descriptor before the callback
Date: Thu, 4 Aug 2016 15:40:04 +0100	[thread overview]
Message-ID: <20160804144003.GV1041@n2100.armlinux.org.uk> (raw)
In-Reply-To: <71a15611-645f-7523-1c26-14b420aff667@codeaurora.org>

On Thu, Aug 04, 2016 at 10:17:24AM -0400, Sinan Kaya wrote:
> On 8/4/2016 8:55 AM, Vinod Koul wrote:
> > Dmaengine tells transaction is complete. It does not say if the txn is
> > success or failure. It can transfer data and not say if data was
> > correct. A successful transaction implies data integrity as well, which
> > dmaengine can't provide.
> 
> Thanks for describing this. I was confused about DMA_SUCCESS and DMA_COMPLETE.
> I now understand that tx_success API just returns information that the request
> was executed whether the result is error or not. This makes sense now.
> 
> However, if the txn is failure; then we should never call the client callback
> since DMA engine cannot provide such feedback to the client without Dave's patch.
> You are saying that the calling the callback is optional.
> 
> Then, the callback cannot be optional in the error case for old behavior.
> 
> How does the client know if memcpy executed or not? The client got its callback
> and tx_status is also DMA_COMPLETE.

If an error occurred, then dma_async_is_tx_complete() is supposed to
return DMA_ERROR.  It's up to the DMA engine driver to ensure that
this happens if it has error detection abilities.

Most of the helpers in drivers/dma/dmaengine.h are there to _assist_
the driver writer - they can't do magic.  dma_cookie_status() will
return from the point of view of the generic DMA code what the status
of a particular cookie is, and the cookie state.  It doesn't take
care of whether a particular transaction associated with a cookie
failed or not - that's up to the driver.

So, if dma_cookie_status() says that a cookie has DMA_COMPLETED
status, and the DMA engine is able to detect errors on individual
transfers, then the driver needs to do further status lookup to
determine whether the particular transaction referred to by the
cookie did fail, and modify the returned status appropriately.

If dma_cookie_status() says that the cookie is DMA_IN_PROGRESS,
then the driver is expected to calculate and report the residue
(the remaining number of bytes) of the referred to transaction.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

  reply	other threads:[~2016-08-04 14:40 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-14  2:57 [PATCH] dmaengine: qcom_hidma: release the descriptor before the callback Sinan Kaya
2016-07-14  2:57 ` Sinan Kaya
2016-07-16  1:00 ` Sinan Kaya
2016-07-16  1:00   ` Sinan Kaya
2016-07-24  6:24   ` Vinod Koul
2016-07-24  6:24     ` Vinod Koul
2016-07-25 14:19     ` Sinan Kaya
2016-07-25 14:19       ` Sinan Kaya
2016-08-04 12:55       ` Vinod Koul
2016-08-04 12:55         ` Vinod Koul
2016-08-04 12:55         ` Vinod Koul
2016-08-04 14:17         ` Sinan Kaya
2016-08-04 14:17           ` Sinan Kaya
2016-08-04 14:40           ` Russell King - ARM Linux [this message]
2016-08-04 14:40             ` Russell King - ARM Linux
2016-08-04 15:27             ` Sinan Kaya
2016-08-04 15:27               ` Sinan Kaya
2016-08-04 15:38               ` Russell King - ARM Linux
2016-08-04 15:38                 ` Russell King - ARM Linux
2016-08-04 15:59                 ` Lars-Peter Clausen
2016-08-04 15:59                   ` Lars-Peter Clausen
2016-08-08  9:08                   ` Vinod Koul
2016-08-08  9:08                     ` Vinod Koul
2016-08-08 12:25                     ` Lars-Peter Clausen
2016-08-08 12:25                       ` Lars-Peter Clausen
2016-08-10 17:23                       ` Vinod Koul
2016-08-10 17:23                         ` Vinod Koul
2016-08-10 17:23                         ` Vinod Koul
2016-08-04 16:08                 ` Sinan Kaya
2016-08-04 16:08                   ` Sinan Kaya
2016-08-04 16:15                   ` Lars-Peter Clausen
2016-08-04 16:15                     ` Lars-Peter Clausen
2016-08-05  6:32                     ` Robert Jarzmik
2016-08-05  6:32                       ` Robert Jarzmik
2016-08-05  8:34                       ` Lars-Peter Clausen
2016-08-05  8:34                         ` Lars-Peter Clausen
2016-08-05  8:34                         ` Lars-Peter Clausen
2016-08-05 15:17                         ` Sinan Kaya
2016-08-05 15:17                           ` Sinan Kaya
2016-08-08  9:02               ` Vinod Koul
2016-08-08  9:02                 ` Vinod Koul
2016-08-08 14:45                 ` Sinan Kaya
2016-08-08 14:45                   ` Sinan Kaya
2016-08-10 17:28                   ` Vinod Koul
2016-08-10 17:28                     ` Vinod Koul
2016-08-10 17:28                     ` Vinod Koul
2016-08-10 17:31                     ` Sinan Kaya
2016-08-10 17:31                       ` Sinan Kaya
2016-08-10 17:31                       ` Sinan Kaya
2016-08-19  2:48                       ` Vinod Koul
2016-08-19  2:48                         ` Vinod Koul
2016-08-19  3:26                         ` Sinan Kaya
2016-08-19  3:26                           ` Sinan Kaya
2016-08-19  3:26                           ` Sinan Kaya
2016-08-19  3:42                           ` Vinod Koul
2016-08-19  3:42                             ` Vinod Koul
2016-08-19  3:48                             ` Sinan Kaya
2016-08-19  3:48                               ` Sinan Kaya
2016-08-19  5:52                               ` Vinod Koul
2016-08-19  5:52                                 ` Vinod Koul
2016-08-19 11:13                                 ` okaya
2016-08-19 11:13                                   ` okaya at codeaurora.org
2016-08-19 11:13                                   ` okaya
2016-08-19 17:02                                   ` Vinod Koul
2016-08-19 17:02                                     ` Vinod Koul
2016-08-19 17:02                                     ` Vinod Koul
2016-08-19 17:21                                     ` Sinan Kaya
2016-08-19 17:21                                       ` Sinan Kaya
2016-08-19 17:21                                       ` Sinan Kaya
2016-08-22  6:08                                       ` Vinod Koul
2016-08-22  6:08                                         ` Vinod Koul
2016-08-22  6:08                                         ` Vinod Koul
2016-08-22 13:27                                         ` Sinan Kaya
2016-08-22 13:27                                           ` Sinan Kaya
2016-08-22 17:00                                           ` Vinod Koul
2016-08-22 17:00                                             ` Vinod Koul
2016-08-08  8:51           ` Vinod Koul
2016-08-08  8:51             ` Vinod Koul
2016-08-08 12:10             ` okaya
2016-08-08 12:10               ` okaya at codeaurora.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160804144003.GV1041@n2100.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=cov@codeaurora.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=okaya@codeaurora.org \
    --cc=timur@codeaurora.org \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.