On Thu, Aug 04, 2016 at 11:27:46AM -0400, Sinan Kaya wrote:
> On 8/4/2016 10:40 AM, Russell King - ARM Linux wrote:
> > 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.
> >
>
> Well, that's not what I'm getting from Vinod and also from the current usage
> in most of the drivers that I looked.
Sorry but, you are not interpreting it correctly. Me and Russell are saying the
same thing!
>
> The current drivers implement tx_status as follows.
>
> static enum dma_status xyz_tx_status(struct dma_chan *chan,
> dma_cookie_t cookie, struct dma_tx_state *state)
> {
> ...
> ret = dma_cookie_status(&c->vc.chan, cookie, state);
> if (ret == DMA_COMPLETE)
> return ret;
> ...
> }
>
> What Vinod is telling me that I need to set the cookie to complete
> whether the transaction is successful or not if the request was accepted
> by HW. xyz_tx_status is just an indication that the transaction was accepted
> by HW. An error can happen as a result of transaction execution.
Nope, if the txn is completed you mark it complete. If you can detect error
(can you??) then you can report DMA_ERROR.
In that latter case do not use dma_async_is_complete() to check. You would
need to store and report that cookie 'x' failed which you report status in
.tx_statis()
>
> If I call dma_cookie_complete for all transactions regardless of transaction
> success or not, then the xyz_tx_status returns DMA_COMPLETE.
Again that is based on your implementation.
> This also matches what Vinod is saying. The transaction is complete but
> it may not be success. I'm saying that if we follow this scheme, then
> we should not call the callback.
That is not in driver's control. If the callback is set, you have to call
it. Client may choose to not set it.
> I also made the argument that the driver should not call dma_cookie_complete
> for failure cases and somehow return an error for transactions that failed.
> This is your suggestion.
>
> I don't think it matches Vinod's expectation.
It does!
> > 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.
> >
>
> This part is fine. I'm worried about transactions that are failing.
And you issue is complete orthogonal to this debate. I am not saying we
should not discuss this, but you fix would be entirely different here (going
by data you have provided till now)
--
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
