From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932331Ab1JNTPz (ORCPT ); Fri, 14 Oct 2011 15:15:55 -0400 Received: from mxout1.idt.com ([157.165.5.25]:43088 "EHLO mxout1.idt.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753599Ab1JNTPx convert rfc822-to-8bit (ORCPT ); Fri, 14 Oct 2011 15:15:53 -0400 x-mimeole: Produced By Microsoft Exchange V6.5 Content-Class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8BIT Subject: RE: [PATCHv4] DMAEngine: Define interleaved transfer request api Date: Fri, 14 Oct 2011 12:15:30 -0700 Message-ID: <0CE8B6BE3C4AD74AB97D9D29BD24E55202321C11@CORPEXCH1.na.ads.idt.com> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCHv4] DMAEngine: Define interleaved transfer request api Thread-Index: AcyKoD//xIZx/ieYR+mvDPRGb/z9OQAAGPGg References: <1317191992-3635-1-git-send-email-jaswinder.singh@linaro.org><1317200618.1573.1765.camel@vkoul-udesk3><1317295068.1573.1780.camel@vkoul-udesk3><20111003161349.GB28287@flint.arm.linux.org.uk><1317966346.1573.2252.camel@vkoul-udesk3><0CE8B6 BE3C4AD74AB9 7D9D29BD24E55202321B8F@CORPEXCH1.na.ads.idt.com> From: "Bounine, Alexandre" To: Jassi Brar CC: "Williams, Dan J" , Vinod Koul , Russell King , Barry Song <21cnbao@gmail.com>, , DL-SHA-WorkGroupLinux , Dave Jiang Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 14, 2011 at 2:37 PM, Jassi Brar wrote: > > On 14 October 2011 23:20, Bounine, Alexandre > wrote: > >> On Fri, Oct 7, 2011 at 4:27 AM, Jassi Brar > > > >> wrote: > >> > On 7 October 2011 11:15, Vinod Koul 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.