Message-ID: <yw1xlgw78y8k.fsf@unicorn.mansr.com>
Date:   Fri, 25 Nov 2016 13:50:35 +0000
From:   Måns Rullgård <mans@...sr.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Vinod Koul <vinod.koul@...el.com>, Mason <slash.tmp@...e.fr>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Dave Jiang <dave.jiang@...el.com>,
        Arnd Bergmann <arnd@...db.de>, Mark Brown <broonie@...aro.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        dmaengine@...r.kernel.org, Dan Williams <dan.j.williams@...el.com>,
        Jon Mason <jon.mason@...el.com>,
        Lee Jones <lee.jones@...aro.org>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: Tearing down DMA transfer setup after DMA client has finished

Russell King - ARM Linux <linux@...linux.org.uk> writes:

> On Fri, Nov 25, 2016 at 01:07:05PM +0000, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@...linux.org.uk> writes:
>> 
>> > On Fri, Nov 25, 2016 at 10:25:49AM +0530, Vinod Koul wrote:
>> >> 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.
>> >> 
>> >> If that is not doable, then since you claim this is custom part which
>> >> other vendors wont use (hope we are 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.
>> >> 
>> >> Only problem with that would be it wont be a generic solution and you
>> >> seem to be fine with that.
>> >
>> > Isn't this just the same problem as PL08x or any other system which
>> > has multiple requests from devices, but only a limited number of
>> > hardware channels - so you have to route the request signals to the
>> > appropriate hardware channels according to the requests queued up?
>> >
>> > If so, no new "custom" APIs are required, it's already able to be
>> > solved within the DMA engine drivers...
>> 
>> That isn't the problem.  The multiplexing of many devices on a limited
>> number of hardware channels is working fine.  The problem is that (some)
>> client devices need the routing to remain for some time after the dma
>> interrupt signals completion.  I'd characterise this hardware as broken,
>> but there's nothing we can do about that.
>> 
>> The fix has to provide some way for the dma driver to delay reusing a
>> hardware channel until the client device indicates completion.  If only
>> a short delay (a few bus cycles) is needed, it is probably acceptable to
>> rework the driver such that the descriptor completion callback can do
>> the necessary waiting (e.g. by busy-polling a device status register).
>> If the delay can be longer, some other method needs to be devised.
>
> What I understood from the original mail is:
>
> | The problem is that the DMA driver tears down the sbox setup
> | as soon as it receives the IRQ. However, when writing to the
> | device, the interrupt only means "I have pushed all data from
> | memory to the memory channel". These data have not reached
> | the device yet, and may still be "in flight". Thus the sbox
> | setup can only be torn down after the NFC is idle.
>
> The interrupt comes in after it's read the the last data from memory,
> but the data is still sitting in the engine's buffers and has not yet
> been passed to the device.
>
> It sounds like the DMA engine buffers the data on its way to the device,
> and it's not clear from the description whether that is done in
> response to a request from the device or whether the data is prefetched.
> IOW, what the lifetime of the data in the dma engine is.

It's not clear from the information I have exactly when the interrupt
fires, only that it appears to be somewhat too early.

> It seems odd that the DMA engine provides no way to know whether the
> channel still contains data that is in-flight to the device, whether
> by interrupt (from the descriptions the IRQ is way too early) or by
> polling some status register within the DMA engine itself.
>
> If the delay is predictable, why not use a delayed workqueue or a
> hrtimer to wait a period after the IRQ before completing the DMA
> transaction?  If it's not predictable and you haven't some status
> register in the DMA engine hardware that indicates whether there's
> remaining data, then the design really is screwed up, and I don't
> think there's a reasonable solution to the problem - anything
> would be a horrid hack that would be specific to this SoC.

This would hardly be the first screwed up hardware design.  There is a
completion indicator, just not in the dma engine.  For some idiotic
reason, the designers put this responsibility on the client devices and
their respective drivers.

> It would be unfair to augment the API and add the burden on everyone
> for the new API when 99.999% of the world doesn't require it.

I don't think making this particular dma driver wait for the descriptor
callback to return before reusing a channel quite amounts to a horrid
hack.  It certainly wouldn't burden anyone other than the poor drivers
for devices connected to it, all of which are specific to Sigma AFAIK.

-- 
Måns Rullgård

Powered by blists - more mailing lists