On Fri, Dec 31, 2010 at 1:50 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
No, the dma slave usage model is undocumented, and the generic offload
case needs updating. I will write up the results of this thread in
Documentation/dmaengine.txt.
For the generic offload usage case dma drivers need to present
consistent behaviour because they are reused generically.
The slave usages are not generic. They grew up around wanting to
reuse dmaengine's channel allocation boilerplate, while maintaining
architecture-specific (dma driver / slave driver pairing) behaviour.
Certainly the ->device_prep* methods can be made to present a
consistent/documented interface. The ->device_control() method, on
the other hand, defines a common interface to pass control messages,
but the semantics are implementation specific (slave drivers take
liberties with the knowledge that they will only ever talk to a given
dma driver/channel).
None, if prep fails then no descriptor was allocated and no cleanup
should be required.
dma_submit_error() is something I should have removed after commit
a0587bcf "ioat1: move descriptor allocation from submit to prep" all
errors should be notified by prep failing to return a descriptor
handle. Negative dma_cookie_t values are only returned by the
dma_async_memcpy* calls which translate a prep failure into -ENOMEM.
This assumes that all submission and completion occurs in tasklet
context? I think something is wrong if the terminate_all
implementation is not taking the channel spinlock or otherwise
guaranteeing that it is not racing against new submissions and the
completion tasklet.
A single thread must not call any other dmaengine methods between prep
and submit. Some drivers lock in prep and unlock in submit as they
need to ensure in-order submission. The only reason they are separate
calls is to preclude needing to specify callback information to every
prep call as some usages will never need it.
Now for the case of ->prep() on cpu0 with an intervening call to
terminate_all on cpu1 before the submit. I would say that the
terminate_all operation should ignore the yet-to-be-submitted
descriptor, i.e. terminate_all stops all submitted operations.
Proper handling of that flag is only required by drivers that select
ASYNC_TX_ENABLE_CHANNEL_SWITCH. These are raid offload drivers where
the memory copy operation is implemented on a separate channel from
the xor operation. The flag allows cross channel dependency chains to
be established. In the slave case they are a no-op because we are
only ever performing one operation type, and a single client is never
using more than one channel at a time.
I asked Linus a similar question [1] wrt the lifetime rules of when
the transaction status could be retrieved, but we did not reach a
conclusion.
As per above, submit must fail if prep succeeds.
I should have caught the amba-pl08x excursions... my only concern with
this is specifying the locking which is driver specific. However,
maybe it would make things cleaner if the core could take a generic
channel lock that was also taken by the drivers.
Drivers are using -EBUSY to indicate descriptor is in the process of
being submitted.
Currently it is driver specific because the dma mapped addresses are
only saved in the physical descriptors. But looking at it now this is
really an implementation wart carried forward from when there was a
device_prep call for every source / destination address type
permutation (single vs page). If all clients were required to keep a
scatterlist around for the duration of the transaction that would
eliminate the need to recover dma addresses from the physical
descriptors. (It probably is also the only way to solve the "dma_map
twice" problem you point out later in this thread.)
Now that this upstart dma-slave usage model has grown to overtake the
original generic offload usage model we definitely need defined
semantics and a clear line drawn between the two usages.
--
Dan
[1]: http://marc.info/?l=linux-arm-kernel&m=126947350019600&w=2
--
