From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vinod Subject: Re: [PATCH v3 4/7] dmaengine: stm32-dma: Add DMA/MDMA chaining support Date: Wed, 10 Oct 2018 09:33:43 +0530 Message-ID: <20181010040343.GO2372@vkoul-mobl> References: <1538139715-24406-1-git-send-email-pierre-yves.mordret@st.com> <1538139715-24406-5-git-send-email-pierre-yves.mordret@st.com> <20181007160030.GB2372@vkoul-mobl> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Pierre Yves MORDRET Cc: Rob Herring , Mark Rutland , Alexandre Torgue , Maxime Coquelin , Dan Williams , devicetree@vger.kernel.org, dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org List-Id: devicetree@vger.kernel.org On 09-10-18, 10:40, Pierre Yves MORDRET wrote: > > > On 10/07/2018 06:00 PM, Vinod wrote: > > On 28-09-18, 15:01, Pierre-Yves MORDRET wrote: > >> This patch adds support of DMA/MDMA chaining support. > >> It introduces an intermediate transfer between peripherals and STM32 DMA. > >> This intermediate transfer is triggered by SW for single M2D transfer and > >> by STM32 DMA IP for all other modes (sg, cyclic) and direction (D2M). > >> > >> A generic SRAM allocator is used for this intermediate buffer > >> Each DMA channel will be able to define its SRAM needs to achieve chaining > >> feature : (2 ^ order) * PAGE_SIZE. > >> For cyclic, SRAM buffer is derived from period length (rounded on > >> PAGE_SIZE). > > > > So IIUC, you chain two dma txns together and transfer data via an SRAM? > > Correct. one DMA is DMAv2 (stm32-dma) and the other is MDMA(stm32-mdma). > Intermediate transfer is between device and memory. > This intermediate transfer is using SDRAM. Ah so you use dma calls to setup mdma xtfers? I dont think that is a good idea. How do you know you should use mdma for subsequent transfer? > >> drivers/dma/stm32-dma.c | 879 ++++++++++++++++++++++++++++++++++++++++++------ > > > > that is a lot of change for a driver, consider splitting it up > > logically in smaller changes... > > > > This feature is rather monolithic. Difficult to split up. > All the code is required at once. It can be enabled at last but split up logically. Intrusive changes to a driver make it hard to review.. -- ~Vinod