Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752675AbaAZN7z (ORCPT ); Sun, 26 Jan 2014 08:59:55 -0500 Received: from mga11.intel.com ([192.55.52.93]:2939 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751444AbaAZN7x (ORCPT ); Sun, 26 Jan 2014 08:59:53 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.95,724,1384329600"; d="scan'208";a="470987228" Date: Sun, 26 Jan 2014 19:29:33 +0530 From: Vinod Koul To: Lars-Peter Clausen Cc: Srikanth Thokala , dan.j.williams@intel.com, michal.simek@xilinx.com, Grant Likely , robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org, "linux-kernel@vger.kernel.org" , devicetree@vger.kernel.org, dmaengine@vger.kernel.org Subject: Re: [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support Message-ID: <20140126135933.GD10628@intel.com> References: <1390409565-4200-1-git-send-email-sthokal@xilinx.com> <1390409565-4200-2-git-send-email-sthokal@xilinx.com> <52E0FC22.8060903@metafoo.de> <52E2698B.6070001@metafoo.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52E2698B.6070001@metafoo.de> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 24, 2014 at 02:24:27PM +0100, Lars-Peter Clausen wrote: > On 01/24/2014 12:16 PM, Srikanth Thokala wrote: > > Hi Lars, > > > > On Thu, Jan 23, 2014 at 4:55 PM, Lars-Peter Clausen wrote: > >> On 01/22/2014 05:52 PM, Srikanth Thokala wrote: > >> [...] > >>> +/** > >>> + * xilinx_vdma_device_control - Configure DMA channel of the device > >>> + * @dchan: DMA Channel pointer > >>> + * @cmd: DMA control command > >>> + * @arg: Channel configuration > >>> + * > >>> + * Return: '0' on success and failure value on error > >>> + */ > >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan, > >>> + enum dma_ctrl_cmd cmd, unsigned long arg) > >>> +{ > >>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan); > >>> + > >>> + switch (cmd) { > >>> + case DMA_TERMINATE_ALL: > >>> + xilinx_vdma_terminate_all(chan); > >>> + return 0; > >>> + case DMA_SLAVE_CONFIG: > >>> + return xilinx_vdma_slave_config(chan, > >>> + (struct xilinx_vdma_config *)arg); > >> > >> You really shouldn't be overloading the generic API with your own semantics. > >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else. > > > > Ok. The driver needs few additional configuration from the slave > > device like Vertical > > Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you > > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START > > defined for Freescale drivers? > > In my opinion it is not a good idea to have driver implement a generic API, > but at the same time let the driver have custom semantics for those API > calls. It's a bit like having a gpio driver that expects 23 and 42 as the > values passed to gpio_set_value instead of 0 and 1. It completely defeats > the purpose of a generic API, namely that you are able to write generic code > that makes use of the API without having to know about which implementation > API it is talking to. The dmaengine framework provides the > dmaengine_prep_interleaved_dma() function to setup two dimensional > transfers, e.g. take a look at sirf-dma.c or imx-dma.c. The question here i think would be waht this device supports? Is the hardware capable of doing interleaved transfers, then would make sense. While we do try to get users use dma_slave_config, but there will always be someone who have specfic params. If we can generalize then we might want to add to the dma_slave_config as well -- ~Vinod -- 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/