All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vinod Koul <vinod.koul@intel.com>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>,
	Sinan Kaya <okaya@codeaurora.org>,
	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: Mon, 8 Aug 2016 14:38:52 +0530	[thread overview]
Message-ID: <20160808090852.GZ9681@localhost> (raw)
In-Reply-To: <32339445-5a1c-5a3e-cd07-d64e35dce2c4@metafoo.de>

On Thu, Aug 04, 2016 at 05:59:30PM +0200, Lars-Peter Clausen wrote:
> On 08/04/2016 05:38 PM, Russell King - ARM Linux wrote:
> [...]
> > What you instead need to do is to find some way to record in your
> > driver that transaction 2 failed, and when dma_cookie_status() says
> > that a transaction has DMA_COMPLETE status, you need to look up to
> > see whether it failed.
> 
> In my opinion this is where the current API is broken by design. For each
> transfer that fails you need to store the cookie associated with that
> transfer in some kind of lookup table. Since there is no lifetime associated
> with a cookie entries in this table would need to be retained forever and it
> will grow unbound.

And how many drivers can report errors? And how many drivers can guarantee
DMA_COMPLETE implies transaction was succesful.

> Ideally we'd mark error reporting through this interface as deprecated and
> discourage new users of the interface. As far as I can see most of the few
> drivers that do return DMA_ERROR get it wrong anyway, e.g. return it
> unconditionally for all cookies when an error occurred for any of them.

Error reporting is quite tricky as detection is a problem. So yes if you
can do so, it is highly encouraged to report using new interface which is
better than client checking after callback.

Btw what is the behaviour after error? I would think that client will see an
error and report to upper layer while initiaite closure of transaction. So
does driver need to keep the state for a longer time :-)

-- 
~Vinod

WARNING: multiple messages have this Message-ID (diff)
From: vinod.koul@intel.com (Vinod Koul)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] dmaengine: qcom_hidma: release the descriptor before the callback
Date: Mon, 8 Aug 2016 14:38:52 +0530	[thread overview]
Message-ID: <20160808090852.GZ9681@localhost> (raw)
In-Reply-To: <32339445-5a1c-5a3e-cd07-d64e35dce2c4@metafoo.de>

On Thu, Aug 04, 2016 at 05:59:30PM +0200, Lars-Peter Clausen wrote:
> On 08/04/2016 05:38 PM, Russell King - ARM Linux wrote:
> [...]
> > What you instead need to do is to find some way to record in your
> > driver that transaction 2 failed, and when dma_cookie_status() says
> > that a transaction has DMA_COMPLETE status, you need to look up to
> > see whether it failed.
> 
> In my opinion this is where the current API is broken by design. For each
> transfer that fails you need to store the cookie associated with that
> transfer in some kind of lookup table. Since there is no lifetime associated
> with a cookie entries in this table would need to be retained forever and it
> will grow unbound.

And how many drivers can report errors? And how many drivers can guarantee
DMA_COMPLETE implies transaction was succesful.

> Ideally we'd mark error reporting through this interface as deprecated and
> discourage new users of the interface. As far as I can see most of the few
> drivers that do return DMA_ERROR get it wrong anyway, e.g. return it
> unconditionally for all cookies when an error occurred for any of them.

Error reporting is quite tricky as detection is a problem. So yes if you
can do so, it is highly encouraged to report using new interface which is
better than client checking after callback.

Btw what is the behaviour after error? I would think that client will see an
error and report to upper layer while initiaite closure of transaction. So
does driver need to keep the state for a longer time :-)

-- 
~Vinod

  reply	other threads:[~2016-08-08  9:01 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
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 [this message]
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=20160808090852.GZ9681@localhost \
    --to=vinod.koul@intel.com \
    --cc=cov@codeaurora.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=okaya@codeaurora.org \
    --cc=timur@codeaurora.org \
    /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.