All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vinod Koul <vinod.koul@linaro.org>
To: Frank Mori Hess <fmh6jj@gmail.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dan Williams <dan.j.williams@intel.com>,
	r.baldyga@hackerion.com, Krzysztof Kozlowski <krzk@kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Linux Samsung SOC <linux-samsung-soc@vger.kernel.org>
Subject: Revert "dmaengine: pl330: add DMA_PAUSE feature"
Date: Mon, 21 May 2018 14:46:26 +0530	[thread overview]
Message-ID: <20180521091626.GB14654@vkoul-mobl> (raw)

On 18-05-18, 14:56, Frank Mori Hess wrote:
> On Fri, May 18, 2018 at 12:03 AM, Vinod <vkoul@kernel.org> wrote:
> >
> > You are simply mixing things up!
> 
> It certainly feels like I'm mixed up.  If I have to resolve this, I'd
> like to be a little less mixed up before I submit more patches which
> are going to inevitably result in subtly broken code suddenly becoming
> prominently and unignorably broken code.  Unfortunately I get the
> impression I'm exhausting your patience to answer my questions, and
> I've failed to fully communicate what the question is.
> 
> 
> > On Pause we don't expect data loss, as user can
> > resume the transfer. This means as you rightly guessed, the DMA HW should not drop
> > any data, nor should SW.
> >
> > Now if you want to read residue at this point it is perfectly valid. But if you
> > decide to terminate the channel (yes it is terminate_all API), we abort and don't
> > have context to report back!
> 
> I understand the residue can't be read after terminate, that's why
> reading the residue is step 2 in pause/residue/terminate.  My question
> was whether the entire sequence pause/residue/terminate taken as a
> whole can or cannot lose data.  Saying that individual steps can or
> can't lose data is not enough, context is required.  The key point is
> whether pause flushes in-flight data to its destination or not.  If it
> does, and our residue is accurate, the terminate cannot cause data
> loss.  If pause doesn't flush, an additional step of flush_sync as
> Lars suggested is required.  So pause/flush_sync/residue/terminate
> would be the safe sequence that cannot lose data.

I wouldn't use cannot, shouldn't would be better here as it depends on HW and
where all data has been buffered and if it can be flushed or not.

Have you checked if pl330 supports such flushing?

> > As Lars rightly pointed out, residue calculation are very tricky, DMA fifo may
> > have data, some data may be in device FIFO, so residue is always from DMA point
> > of view and may differ from device view (more or less depending upon direction)
> >
> > Now if you require to add more features for your usecase, please do feel free to
> > send a patch. The framework can always be improved, we haven't solved world
> > hunger yet!
> 
> World hunger?  I don't see how asking questions about a dma engine's
> data integrity guarantees is either unreasonable or out of scope.

No they are not out of scope, and dmaengine APIs support requested usages.  Have
we solved all cases, hell no. Can we add more to support different scenarios,
absolutely!

WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vinod.koul@linaro.org>
To: Frank Mori Hess <fmh6jj@gmail.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dan Williams <dan.j.williams@intel.com>,
	r.baldyga@hackerion.com, Krzysztof Kozlowski <krzk@kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Linux Samsung SOC <linux-samsung-soc@vger.kernel.org>
Subject: Re: Revert "dmaengine: pl330: add DMA_PAUSE feature"
Date: Mon, 21 May 2018 14:46:26 +0530	[thread overview]
Message-ID: <20180521091626.GB14654@vkoul-mobl> (raw)
In-Reply-To: <CAJz5OpfSanwNS7Z=3FapR97XZzjWF_J23HqVouxVnPn2HwkKLw@mail.gmail.com>

On 18-05-18, 14:56, Frank Mori Hess wrote:
> On Fri, May 18, 2018 at 12:03 AM, Vinod <vkoul@kernel.org> wrote:
> >
> > You are simply mixing things up!
> 
> It certainly feels like I'm mixed up.  If I have to resolve this, I'd
> like to be a little less mixed up before I submit more patches which
> are going to inevitably result in subtly broken code suddenly becoming
> prominently and unignorably broken code.  Unfortunately I get the
> impression I'm exhausting your patience to answer my questions, and
> I've failed to fully communicate what the question is.
> 
> 
> > On Pause we don't expect data loss, as user can
> > resume the transfer. This means as you rightly guessed, the DMA HW should not drop
> > any data, nor should SW.
> >
> > Now if you want to read residue at this point it is perfectly valid. But if you
> > decide to terminate the channel (yes it is terminate_all API), we abort and don't
> > have context to report back!
> 
> I understand the residue can't be read after terminate, that's why
> reading the residue is step 2 in pause/residue/terminate.  My question
> was whether the entire sequence pause/residue/terminate taken as a
> whole can or cannot lose data.  Saying that individual steps can or
> can't lose data is not enough, context is required.  The key point is
> whether pause flushes in-flight data to its destination or not.  If it
> does, and our residue is accurate, the terminate cannot cause data
> loss.  If pause doesn't flush, an additional step of flush_sync as
> Lars suggested is required.  So pause/flush_sync/residue/terminate
> would be the safe sequence that cannot lose data.

I wouldn't use cannot, shouldn't would be better here as it depends on HW and
where all data has been buffered and if it can be flushed or not.

Have you checked if pl330 supports such flushing?

> > As Lars rightly pointed out, residue calculation are very tricky, DMA fifo may
> > have data, some data may be in device FIFO, so residue is always from DMA point
> > of view and may differ from device view (more or less depending upon direction)
> >
> > Now if you require to add more features for your usecase, please do feel free to
> > send a patch. The framework can always be improved, we haven't solved world
> > hunger yet!
> 
> World hunger?  I don't see how asking questions about a dma engine's
> data integrity guarantees is either unreasonable or out of scope.

No they are not out of scope, and dmaengine APIs support requested usages.  Have
we solved all cases, hell no. Can we add more to support different scenarios,
absolutely!

-- 
~Vinod

             reply	other threads:[~2018-05-21  9:16 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-21  9:16 Vinod Koul [this message]
2018-05-21  9:16 ` Revert "dmaengine: pl330: add DMA_PAUSE feature" Vinod Koul
  -- strict thread matches above, loose matches on Subject: below --
2018-05-29  7:09 Vinod Koul
2018-05-29  7:09 ` Vinod
2018-05-29  5:17 Marek Szyprowski
2018-05-29  5:17 ` Marek Szyprowski
2018-05-23  5:39 Vinod Koul
2018-05-23  5:39 ` Vinod
2018-05-22 14:27 Frank Mori Hess
2018-05-22 14:27 ` Frank Mori Hess
2018-05-22  3:37 Vinod Koul
2018-05-22  3:37 ` Vinod Koul
2018-05-22  0:56 Frank Mori Hess
2018-05-22  0:56 ` Frank Mori Hess
2018-05-18 19:01 Frank Mori Hess
2018-05-18 19:01 ` Frank Mori Hess
2018-05-18 18:56 Frank Mori Hess
2018-05-18 18:56 ` Frank Mori Hess
2018-05-18  7:21 Vinod Koul
2018-05-18  7:21 ` Vinod
2018-05-18  6:28 Marek Szyprowski
2018-05-18  6:28 ` Marek Szyprowski
2018-05-18  4:03 Vinod Koul
2018-05-18  4:03 ` Vinod
2018-05-17 17:22 Lars-Peter Clausen
2018-05-17 17:22 ` Lars-Peter Clausen
2018-05-17 16:20 Frank Mori Hess
2018-05-17 16:20 ` Frank Mori Hess
2018-05-17  4:19 Vinod Koul
2018-05-17  4:19 ` Vinod Koul
2018-05-15 15:50 Frank Mori Hess
2018-05-15 15:50 ` Frank Mori Hess
2018-05-15 13:45 Vinod Koul
2018-05-15 13:45 ` Vinod
2018-05-15 12:24 Marek Szyprowski
2018-05-15 12:24 ` Marek Szyprowski
2018-05-15  6:21 Vinod Koul
2018-05-15  6:21 ` Vinod
2018-05-11 15:57 Frank Mori Hess
2018-05-11 15:57 ` Frank Mori Hess
2018-05-11 12:57 Marek Szyprowski
2018-05-11 12:57 ` Marek Szyprowski
2018-05-10 16:04 Frank Mori Hess
2018-05-10 16:04 ` Frank Mori Hess
2018-05-10  8:31 Marek Szyprowski
2018-05-10  8:31 ` Marek Szyprowski
2018-05-09 17:48 Frank Mori Hess
2018-05-09 17:48 ` Frank Mori Hess
2018-05-09 13:19 Marek Szyprowski
2018-05-09 13:19 ` Marek Szyprowski
2018-05-09  6:59 Vinod Koul
2018-05-09  6:59 ` Vinod Koul
2018-05-08 14:36 Frank Mori Hess
2018-05-08 14:36 ` Frank Mori Hess
2018-05-08  9:04 Marek Szyprowski
2018-05-08  9:04 ` Marek Szyprowski
2018-05-03 16:35 Vinod Koul
2018-05-03 16:35 ` [PATCH] " Vinod Koul
2018-05-03  9:01 Vinod Koul
2018-05-03  9:01 ` [PATCH] " Vinod Koul
2018-05-02 14:37 Frank Mori Hess
2018-05-02 14:37 ` [PATCH] " Frank Mori Hess
2018-05-02  4:32 Vinod Koul
2018-05-02  4:32 ` [PATCH] " Vinod Koul
2018-04-28 21:50 Frank Mori Hess
2018-04-28 21:50 ` [PATCH] " Frank Mori Hess

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=20180521091626.GB14654@vkoul-mobl \
    --to=vinod.koul@linaro.org \
    --cc=b.zolnierkie@samsung.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=fmh6jj@gmail.com \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=r.baldyga@hackerion.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.