From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <507794B6.2000107@st.com> Date: Fri, 12 Oct 2012 09:25:34 +0530 From: Vipin Kumar MIME-Version: 1.0 To: Linus Walleij , viresh kumar , Artem Bityutskiy Subject: Re: [PATCH 07/11] fsmc/nand: Provide contiguous buffers to dma References: <2b88c853b3691338fae037f569917fc300cd6032.1349778821.git.vipin.kumar@st.com> In-Reply-To: Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: Vinod Koul , "linux-mtd@lists.infradead.org" , spear-devel , "plagnioj@jcrosoft.com" , "linux-arm-kernel@lists.infradead.org" List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 10/11/2012 9:36 PM, Linus Walleij wrote: > On Thu, Oct 11, 2012 at 6:15 AM, viresh kumar wrote: >> On Wed, Oct 10, 2012 at 10:37 PM, Linus Walleij >> wrote: > >>> dma_sync_single_for_device() is translating the virtual >>> address to physical for every chunk BTW. >> >> I pray that i am wrong here, otherwise i would be thrown out from >> the maintainers list for this driver :) >> >> dma_sync_single_for_device() is not doing anything on the buffer, but >> on the LLI item. Actually it is flushing LLI struct so that DMA h/w can get >> the correct values. > > Sorry no, I'm the one who's wrong... > > So the DMA engine memcpy() is not mapping virt->phys > but expects physical addresses to be provided. > > So dma_map_single() needs to be called on the stuff > passed in to dev->device_prep_dma_memcpy(). > > And currently there is indeed a dma_map_single() in > dma_xfer() in fsmc_nand.c which should work just fine. > > dma_map_single() will only work if the buffer is > physically contiguous. > > And the block layer of the subsystem should take care > of only handing the driver buffers that are contiguous > I think? Not that I'm an expert here ... more some > guesswork :-/ The buffers provided to the driver are actually user buffers. The reason I say that is because the generic nand test modules eg drivers/mtd/nand/mtd_stresstest.c calls mtd->_read with a user buffer as an argument This same buffer directly trickles down to the driver Artem, should we clearly cast this buffer as a user pointer instead of just a 'uint8_t *'. Regards Vipin From mboxrd@z Thu Jan 1 00:00:00 1970 From: vipin.kumar@st.com (Vipin Kumar) Date: Fri, 12 Oct 2012 09:25:34 +0530 Subject: [PATCH 07/11] fsmc/nand: Provide contiguous buffers to dma In-Reply-To: References: <2b88c853b3691338fae037f569917fc300cd6032.1349778821.git.vipin.kumar@st.com> Message-ID: <507794B6.2000107@st.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 10/11/2012 9:36 PM, Linus Walleij wrote: > On Thu, Oct 11, 2012 at 6:15 AM, viresh kumar wrote: >> On Wed, Oct 10, 2012 at 10:37 PM, Linus Walleij >> wrote: > >>> dma_sync_single_for_device() is translating the virtual >>> address to physical for every chunk BTW. >> >> I pray that i am wrong here, otherwise i would be thrown out from >> the maintainers list for this driver :) >> >> dma_sync_single_for_device() is not doing anything on the buffer, but >> on the LLI item. Actually it is flushing LLI struct so that DMA h/w can get >> the correct values. > > Sorry no, I'm the one who's wrong... > > So the DMA engine memcpy() is not mapping virt->phys > but expects physical addresses to be provided. > > So dma_map_single() needs to be called on the stuff > passed in to dev->device_prep_dma_memcpy(). > > And currently there is indeed a dma_map_single() in > dma_xfer() in fsmc_nand.c which should work just fine. > > dma_map_single() will only work if the buffer is > physically contiguous. > > And the block layer of the subsystem should take care > of only handing the driver buffers that are contiguous > I think? Not that I'm an expert here ... more some > guesswork :-/ The buffers provided to the driver are actually user buffers. The reason I say that is because the generic nand test modules eg drivers/mtd/nand/mtd_stresstest.c calls mtd->_read with a user buffer as an argument This same buffer directly trickles down to the driver Artem, should we clearly cast this buffer as a user pointer instead of just a 'uint8_t *'. Regards Vipin