lkml.org 
[lkml]   [2011]   [Oct]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRE: [PATCHv4] DMAEngine: Define interleaved transfer request api
    Date
    From
    On Fri, Oct 14, 2011 at 2:37 PM, Jassi Brar <jaswinder.singh@linaro.org> wrote:
    >
    > On 14 October 2011 23:20, Bounine, Alexandre
    > <Alexandre.Bounine@idt.com> wrote:
    > >> On Fri, Oct 7, 2011 at 4:27 AM, Jassi Brar
    > > <jaswinder.singh@linaro.org>
    > >> wrote:
    > >> > On 7 October 2011 11:15, Vinod Koul <vinod.koul@intel.com> wrote:
    > >> >
    > >> >> Thru this patch Jassi gave a very good try at merging DMA_SLAVE
    > and
    > >> >> memcpy, but more we debate this, I am still not convinced about
    > >> merging
    > >> >> memcpy and DMA_SLAVE yet.
    > >> >>
    > >> > Nobody is merging memcpy and DMA_SLAVE right away.
    > >> > The api's primary purpose is to support interleave transfers.
    > >> > Possibility to merge other prepares into this is a side-effect.
    > >> >
    > >> >> I would still argue that if we split this on same lines as
    > current
    > >> >> mechanism, we have clean way to convey all details for both
    > cases.
    > >> >>
    > >> > Do you mean to have separate interleaved transfer apis for Slave
    > >> > and Mem->Mem ? Please clarify.
    > >> >
    > >>
    > >> This is a tangent, but it would be nice if this API extension also
    > >> covered the needs of the incoming RapidIO case which wants to
    > specify
    > >> new device context information per operation (and not once at
    > >> configuration time, like slave case).  Would it be enough if the
    > >> transfer template included a (struct device *context) member at the
    > >> end?  Most dma users could ignore it, but RapidIO could use it to do
    > >> something like:
    > >>
    > >>    struct rio_dev *rdev = container_of(context, typeof(*rdev),
    > > device);
    > >>
    > >> That might not be enough, but I'm concerned that making the context
    > a
    > >> (void *) is too flexible.  I'd rather have something like this than
    > >> acquiring a lock in rio_dma_prep_slave_sg() and holding it over
    > >> ->prep().  The alternative is to extend device_prep_slave_sg to take
    > >> an extra parameter, but that impacts all other slave implementations
    > >> with a dead parameter.
    > >>
    > >
    > > Having context limited to the device structure will not be enough for
    > > RapidIO because of 66-bit target address (dma_addr_t will not work
    > > here).
    > > Probably that range is out of practical use at this moment but it is
    > > defined by RIO specification and I would prefer to deal with it now
    > > instead of postponing it for future. Passing context using (void *)
    > will
    > > solve this.
    > >
    > OK so you need a void* to contain all info. Agreed.
    > But doesn't the info, pointed to by this (void *), remain same for
    > every
    > transfer to a particular target/remote device ?
    No. An address within the target may (and most likely will) be changed for
    every transfer. Target destination ID will be the same for given virtual channel.

    > If so, couldn't you stick this (void *) to the virtual channel's
    > 'private' ? 'private' :D

    This is what I am trying to do for physical channel ;).
    Virtual channel may bring the same challenge and I may need a channel locking
    if more than one requester try to read/write data to the same target RIO device.

    Currently, I am leaning towards adopting Dan's idea of having subsystem specific
    prep_sg() routine which will be associated with rio_mport device that provides DMA
    support but keep it registered as DMA_SLAVE. In this context I am happy to see
    that your patch removes BUG_ON check for DMA_SLAVE.

    This also gives RapidIO greater level of independence in dealing with
    RIO transfer details.

    I am sorry for my delayed replies - I was on vacation.

    Alex.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-10-14 21:19    [W:0.047 / U:92.540 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site