mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mason <slash.tmp@free.fr>
To: Vinod Koul <vinod.koul@intel.com>
Cc: dmaengine@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Dan Williams <dan.j.williams@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Jon Mason <jdmason@kudzu.us>, Mark Brown <broonie@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Lee Jones <lee.jones@linaro.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Maxime Ripard <maxime.ripard@free-electrons.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Mans Rullgard <mans@mansr.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: Tearing down DMA transfer setup after DMA client has finished
Date: Fri, 25 Nov 2016 13:46:26 +0100	[thread overview]
Message-ID: <092f44ee-4560-be17-25f7-00948dba3cfa@free.fr> (raw)
In-Reply-To: <20161125045549.GC2698@localhost>

On 25/11/2016 05:55, Vinod Koul wrote:

> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
>
>> On my platform, setting up a DMA transfer is a two-step process:
>>
>> 1) configure the "switch box" to connect a device to a memory channel
>> 2) configure the transfer details (address, size, command)
>>
>> When the transfer is done, the sbox setup can be torn down,
>> and the DMA driver can start another transfer.
>>
>> The current software architecture for my NFC (NAND Flash controller)
>> driver is as follows (for one DMA transfer).
>>
>>   sg_init_one
>>   dma_map_sg
>>   dmaengine_prep_slave_sg
>>   dmaengine_submit
>>   dma_async_issue_pending
>>   configure_NFC_transfer
>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>   wait_for_NFC_idle
>>   dma_unmap_sg
> 
> Looking at thread and discussion now, first thinking would be to ensure
> the transaction is completed properly and then isr fired. You may need
> to talk to your HW designers to find a way for that. It is quite common
> that DMA controllers will fire and complete whereas the transaction is
> still in flight.

It seems there is a disconnect between what Linux expects - an IRQ
when the transfer is complete - and the quirks of this HW :-(

On this system, there are MBUS "agents" connected via a "switch box".
An agent fires an IRQ when it has dealt with its *half* of the transfer.

SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT

Here are the steps for a transfer, in the general case:

1) setup the sbox to connect SOURCE TO DEST
2) configure source to send N bytes
3) configure dest to receive N bytes

When SOURCE_AGENT has sent N bytes, it fires an IRQ
When DEST_AGENT has received N bytes, it fires an IRQ
The sbox connection can be torn down only when the destination
agent has received all bytes.
(And the twist is that some agents do not have an IRQ line.)

The system provides 3 RAM-to-sbox agents (read channels)
and 3 sbox-to-RAM agents (write channels).

The NAND Flash controller read and write agents do not have
IRQ lines.

So for a NAND-to-memory transfer (read from device)
- nothing happens when the NFC has finished sending N bytes to the sbox
- the write channel fires an IRQ when it has received N bytes

In that case, one IRQ fires when the transfer is complete,
like Linux expects.

For a memory-to-NAND transfer (write to device)
- the read channel fires an IRQ when it has sent N bytes
- the NFC driver is supposed to poll the NFC to determine
when the controller has finished writing N bytes

In that case, the IRQ does not indicate that the transfer
is complete, merely that the sending half has finished
its part.

For a memory-to-memory transfer (memcpy)
- the read channel fires an IRQ when it has sent N bytes
- the write channel fires an IRQ when it has received N bytes

So you actually get two IRQs in that case, which I don't
think Linux (or the current DMA driver) expects.

I'm not sure how we're supposed to handle this kind of HW
in Linux? (That's why I started this thread.)


> If that is not doable, then since you claim this is custom part which
> other vendors won't use (hope we are wrong down the line),

I'm not sure how to interpret "you claim this is custom part".
Do you mean I may be wrong, that it is not custom?
I don't know if other vendors may have HW with the same
quirky behavior. What do you mean about being wrong down
the line?

> then we can have a custom api,
> 
> foo_sbox_configure(bool enable, ...);
> 
> This can be invoked from NFC driver when required for configuration and
> teardown. For very specific cases where people need some specific
> configuration we do allow custom APIs.

I don't think that would work. The fundamental issue is
that Linux expects a single IRQ to indicate "transfer
complete". And the driver (as written) starts a new
transfer as soon as the IRQ fires.

But the HW may generate 0, 1, or even 2 IRQs for a single
transfer. And when there is a single IRQ, it may not
indicate "transfer complete" (as seen above).

> Only problem with that would be it wont be a generic solution
> and you seem to be fine with that.

I think it is possible to have a generic solution:
Right now, the callback is called from tasklet context.
If we can have a new flag to have the callback invoked
directly from the ISR, then the driver for the client
device can do what is required.

For example, the NFC driver waits for the IRQ from the
memory agent, and then polls the controller itself.

I can whip up a proof-of-concept if it's better to
illustrate with a patch?

Regards.

  parent reply	other threads:[~2016-11-25 12:48 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-23 10:25 Mason
2016-11-23 12:13 ` Måns Rullgård
2016-11-23 12:41   ` Mason
2016-11-23 17:21     ` Måns Rullgård
2016-11-24 10:53       ` Mason
2016-11-24 14:17         ` Måns Rullgård
2016-11-24 15:20           ` Mason
2016-11-24 16:37             ` Måns Rullgård
2016-11-25  4:55 ` Vinod Koul
2016-11-25 11:57   ` Måns Rullgård
2016-11-25 14:05     ` Mason
2016-11-25 14:12       ` Måns Rullgård
2016-11-25 14:28         ` Mason
2016-11-25 14:42           ` Måns Rullgård
2016-11-25 12:45   ` Russell King - ARM Linux
2016-11-25 13:07     ` Måns Rullgård
2016-11-25 13:34       ` Russell King - ARM Linux
2016-11-25 13:50         ` Måns Rullgård
2016-11-25 13:58           ` Russell King - ARM Linux
2016-11-25 14:03             ` Måns Rullgård
2016-11-25 14:17               ` Russell King - ARM Linux
2016-11-25 14:40                 ` Måns Rullgård
2016-11-25 14:56                   ` Russell King - ARM Linux
2016-11-25 15:08                     ` Måns Rullgård
2016-11-25 15:02                 ` Mason
2016-11-25 15:12                   ` Måns Rullgård
2016-11-25 15:21                     ` Mason
2016-11-25 15:28                       ` Måns Rullgård
2016-11-25 12:46   ` Mason [this message]
2016-11-25 13:11     ` Måns Rullgård
2016-11-25 14:21       ` Mason
2016-11-25 14:37         ` Måns Rullgård
2016-11-25 15:35           ` Mason
2016-11-29 18:25     ` Mason
2016-12-06  5:12       ` Vinod Koul
2016-12-06 12:42         ` Mason
2016-12-06 13:14           ` Måns Rullgård
2016-12-06 15:24             ` Mason
2016-12-06 15:34               ` Måns Rullgård
2016-12-06 22:55                 ` Mason
2016-12-07 16:43             ` Vinod Koul
2016-12-07 16:45               ` Måns Rullgård
2016-12-08 10:39                 ` Vinod Koul
2016-12-08 10:54                   ` Mason
2016-12-08 11:18                     ` Geert Uytterhoeven
2016-12-08 11:47                       ` Måns Rullgård
2016-12-08 12:03                         ` Geert Uytterhoeven
2016-12-08 12:17                           ` Mason
2016-12-08 12:21                           ` Måns Rullgård
2016-12-08 16:37                     ` Vinod Koul
2016-12-08 16:48                       ` Måns Rullgård
2016-12-09  6:59                         ` Vinod Koul
2016-12-09 10:25                           ` Sebastian Frias
2016-12-09 11:34                             ` Måns Rullgård
2016-12-09 11:35                             ` 1Måns Rullgård
2016-12-09 17:17                             ` Vinod Koul
2016-12-09 17:28                               ` Måns Rullgård
2016-12-09 17:53                                 ` Vinod Koul
2016-12-09 17:34                               ` Mason
2016-12-09 17:56                                 ` Vinod Koul
2016-12-09 18:17                                   ` Vinod Koul
2016-12-09 18:23                                   ` Mason
2016-12-12  5:01                                     ` Vinod Koul
2016-12-15 11:17                                     ` Mark Brown
2016-12-08 11:44                   ` Måns Rullgård
2016-12-08 11:59                     ` Geert Uytterhoeven
2016-12-08 12:20                       ` Måns Rullgård
2016-12-08 12:31                         ` Geert Uytterhoeven
2016-12-08 12:41                         ` Mason
2016-12-08 12:44                           ` Måns Rullgård
2016-12-08 13:29                             ` Mason
2016-12-08 13:39                               ` Måns Rullgård
2016-12-08 15:50                         ` Vinod Koul
2016-12-08 16:36                           ` Måns Rullgård
2016-12-08 15:40                     ` Vinod Koul
2016-12-08 15:43                       ` Mason
2016-12-08 16:21                         ` Vinod Koul
2016-12-08 16:46                       ` Måns Rullgård
2016-12-07 16:41           ` Vinod Koul
2016-12-07 16:44             ` Måns Rullgård
2016-12-08 10:37               ` Vinod Koul
2016-12-08 11:44                 ` Måns Rullgård

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=092f44ee-4560-be17-25f7-00948dba3cfa@free.fr \
    --to=slash.tmp@free.fr \
    --cc=arnd@arndb.de \
    --cc=b.zolnierkie@samsung.com \
    --cc=broonie@kernel.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=jdmason@kudzu.us \
    --cc=lars@metafoo.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mans@mansr.com \
    --cc=maxime.ripard@free-electrons.com \
    --cc=peter.ujfalusi@ti.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®