Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751427AbeACGvr (ORCPT + 1 other); Wed, 3 Jan 2018 01:51:47 -0500 Received: from esa4.microchip.iphmx.com ([68.232.154.123]:39250 "EHLO esa4.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751167AbeACGvp (ORCPT ); Wed, 3 Jan 2018 01:51:45 -0500 X-IronPort-AV: E=Sophos;i="5.45,500,1508828400"; d="scan'208";a="9857767" Date: Wed, 3 Jan 2018 07:51:29 +0100 From: "ludovic.desroches@microchip.com" To: Trent Piepho CC: "cyrille.pitchen@wedev4u.fr" , "ludovic.desroches@microchip.com" , "nicolas.ferre@microchip.com" , "linux-kernel@vger.kernel.org" , "linux-mtd@lists.infradead.org" , "radu.pirea@microchip.com" , "linux@armlinux.org.uk" , "devicetree@vger.kernel.org" , "broonie@kernel.org" , "linux-spi@vger.kernel.org" , "robh@kernel.org" , "dwmw2@infradead.org" , "computersforpeace@gmail.com" , "boris.brezillon@free-electrons.com" , "vigneshr@ti.com" , "marek.vasut@gmail.com" , "richard@nod.at" Subject: Re: [PATCH 2/3] dt-bindings: mtd: atmel-quadspi: add an optional property 'dmacap,memcpy' Message-ID: <20180103065129.GH2612@rfolt0960.corp.atmel.com> References: <143542c61ca674d53da4985bbabc142e8e6ebefc.1514087323.git.cyrille.pitchen@wedev4u.fr> <20171226232330.trljk2ofyvu4ly3n@rob-hp-laptop> <0149ba05-64b1-2739-e61f-78d5170775fb@wedev4u.fr> <20180102102234.GG2612@rfolt0960.corp.atmel.com> <1514920738.26695.171.camel@impinj.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1514920738.26695.171.camel@impinj.com> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, Jan 02, 2018 at 07:18:58PM +0000, Trent Piepho wrote: > On Tue, 2018-01-02 at 11:22 +0100, Ludovic Desroches wrote: > > On Wed, Dec 27, 2017 at 10:40:00PM +0100, Cyrille Pitchen wrote: > > > > > Or maybe no change at all is required at the at_xdmac.c driver side: we > > > just don't care about the provided flags in the "dmas" property, especially > > > the "peripheral id". They would be ignored anyway when the atmel-quadspi.c > > > driver later calls dmaengine_prep_dma_memcpy(). So I could simply set the > > > dma cells to 0 in the device-tree? > > > > > > Ludovic, what do you think about that ? > > > > It may work but I won't do this. Usually, channels requested through the xlate > > function have usually their capaiblities set to DMA_SLAVE and not DMA_MEMCPY. > > In the at_xdmac case, it won't be an issue but if you have a controller > > which has channels which can support only mem-to-mem or peripheral, it > > won't work. > > Maybe one could create an "AT91_XDMAC_DT_" macro to indicate a memcpy > channel. There are still unused bits for another flag. It also looks > like at_xdma uses peripheral id 0x3f for memcpy transfers (will that > work with memcpy DMA on multiple channels at the same time?). So > perhaps perid 0x3f could be the indication of wanting a memcpy channel, > rather than another flag bit. But however it's done, one writes: > > dmas = <&dma0 AT91_XDMAC_DT_MEMCPY>; dma-names = "rx-tx"; > If have no objection about doing that, my concerns are: - most (all ?) of the dma controllers used the xlate function to provide slave channel. Does it have to provide slave channel or can we use it for all kind of channel? From my point of view, we can do it, just need the confirmation. - this set of patches if focused on the atmel qspi controller but other ones may be interested in doing the same thing so they would have to update the behavior of the xlate function of the DMA controller they are using. So having the request of a DMA_MEMCPY channel inside the spi/qspi controller doesn't seem to be a wrong idea. Moreover, it may be confusing for the user who don't know the context: why do I have to use memcpy and not slave as usal? Honestly I have no opinion about the way to do it. Both have pros and cons. > I think one could have the quadspi driver automatically fill in the dma > cell in the dma specifier if it is not present in the device tree. So > one could write "dmas = <&dma0>" and the driver adds the > AT91_XDMAC_DT_MEMCPY cell before xlating. I'm not sure if that's a > good idea or not. I don't think so, there is enough black magic, let's try to not add more :p Regards Ludovic