lkml.org 
[lkml]   [2012]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRE: [PATCH 01/11] dmaengine: add context parameter toprep_slave_sg and prep_dma_cyclic
    Date
    From
    On Thu, Feb 02, 2012 at 4:44 PM, Russell King wrote:
    >
    > On Thu, Feb 02, 2012 at 04:32:11PM -0500, Alexandre Bounine wrote:
    > > Add context parameter to device_prep_slave_sg() and
    device_prep_dma_cyclic()
    > > interfaces to allow passing client/target specific information
    associated
    > > with the data transfer.
    >
    > I'm slightly concerned about having this as a vague 'void *' pointer.
    > What that means is that the data being passed through it is entirely
    > unspecified, and quite specific to the DMA engine.
    >
    > That's rather worrying when we (on ARM) are moving towards a model
    > where the same peripheral IP can be backed by multiple different DMA
    > engines. We already have peripheral drivers (clients of DMA engine)
    > which can be attached to completely different DMA engine drivers.
    >
    > If DMA engines continue to operate conventionally with this parameter
    > NULL then that's fine for current stuff. I would suggest that it's
    > made to be something a little better defined though, as the 'void *'
    > approach encourages each driver writer to invent their own way to
    > specify, eg, an interleaved transfer. We'll then end up with N
    > different ways to specify the same thing.
    >
    > Not only that, but peripheral drivers won't know what kind of data to
    > pass there (they would have to have additional knowledge of the DMA
    > engine which they're attached to.)
    >
    > Basically, allowing a DMA engine specific blob of data to be passed
    > destroys the idea of having a consistent API for peripheral drivers
    > to use, because they can then no longer be independent of their DMA
    > engine.
    >
    I agree with your concern about "void *" being quite wide-open
    definition.
    But it may be practically impossible to come up with better definition
    that may be equally suitable for different systems or HW combinations
    (e.g. definition that is good for ARM peripherals may not fit into the
    RapidIO model).

    Some level of protection should be expected from filter callbacks
    invoked
    by dma_request_channel(). This way, for example, RapidIO client will not
    acquire DMA channel which does not support RapidIO data transfers.

    After saying all above to defend the changes, I have to admit that your
    comment made me to look harder for an alternative way to pass
    per-transfer
    parameter. As a result, I am considering a new idea when per-transfer
    configuration data will be passed as the first element of the scatter
    list.
    Something like a packet header data but it will be used to setup
    buffer descriptors instead of being part of data stream.

    This new approach will preserve existing DMA engine interface and be
    easier
    for backporting of RapidIO drivers. Yes, there is a system dependency in
    scatter
    list format but it is completely protected by channel filtering
    mechanism.

    Because of that it may be a good idea to consider this set of patches
    being "on hold" unless there are other parties interested in proposed
    interface changes.

    AB.



    \
     
     \ /
      Last update: 2012-02-03 16:35    [W:0.025 / U:59.964 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site