From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753032AbbBSK3b (ORCPT ); Thu, 19 Feb 2015 05:29:31 -0500 Received: from svenfoo.org ([82.94.215.22]:52978 "EHLO mail.zonque.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752510AbbBSK33 (ORCPT ); Thu, 19 Feb 2015 05:29:29 -0500 Message-ID: <54E5BB06.6000106@zonque.org> Date: Thu, 19 Feb 2015 11:29:26 +0100 From: Daniel Mack User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Robert Jarzmik , Haojian Zhuang , Vinod Koul , Arnd Bergmann , Vasily Khoruzhick CC: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org Subject: Re: [PATCH v2 1/2] dma: mmp_dma: add support for legacy transition References: <1423954053-21760-1-git-send-email-robert.jarzmik@free.fr> <1424205547-15429-1-git-send-email-robert.jarzmik@free.fr> In-Reply-To: <1424205547-15429-1-git-send-email-robert.jarzmik@free.fr> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Robert, Thanks for pushing this topic :) On 02/17/2015 09:39 PM, Robert Jarzmik wrote: > In order to achieve smooth transition of pxa drivers from old legacy dma > handling to new dmaengine, introduce a function to "hide" dma physical > channels from dmaengine. > > This is temporary situation where pxa dma will be handled in 2 places : > - arch/arm/plat-pxa/dma.c > - drivers/dma/mmp_pdma.c > > The resources, ie. dma channels, will be controlled by mmp_dma. The > legacy code will request or release a channel with > mmp_pdma_toggle_reserved_channel(). > > This is not very pretty, but it ensures both legacy and dmaengine > consumers can live in the same kernel until the conversion is done. I like the approach. I actually thought the legacy DMA driver would claim the io port range, which would have made the two drivers mutually exclusive. But it doesn't, so this can in fact work, even though it's really not pretty. Also, it's limited to one single instance of the mmp-pdma driver, but that reflects reality, so I'm fine. One minor nit: > +int mmp_pdma_toggle_reserved_channel(int legacy_channel) > +{ > + if (legacy_unavailable & (1 << legacy_channel)) > + return -EBUSY; > + legacy_reserved ^= 1 << legacy_channel; > + return 0; > +} > +EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel); My concern is that if pxa_request_dma() is called more than once for whatever reason by a legacy implementation, the toggled bit mask might get out of sync. As we know exactly on the caller site what we want to achieve, let's make the API explicit with something like: int mmp_pdma_set_reserved_channel(int legacy_channel, bool reserved) Thanks, Daniel From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel@zonque.org (Daniel Mack) Date: Thu, 19 Feb 2015 11:29:26 +0100 Subject: [PATCH v2 1/2] dma: mmp_dma: add support for legacy transition In-Reply-To: <1424205547-15429-1-git-send-email-robert.jarzmik@free.fr> References: <1423954053-21760-1-git-send-email-robert.jarzmik@free.fr> <1424205547-15429-1-git-send-email-robert.jarzmik@free.fr> Message-ID: <54E5BB06.6000106@zonque.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Robert, Thanks for pushing this topic :) On 02/17/2015 09:39 PM, Robert Jarzmik wrote: > In order to achieve smooth transition of pxa drivers from old legacy dma > handling to new dmaengine, introduce a function to "hide" dma physical > channels from dmaengine. > > This is temporary situation where pxa dma will be handled in 2 places : > - arch/arm/plat-pxa/dma.c > - drivers/dma/mmp_pdma.c > > The resources, ie. dma channels, will be controlled by mmp_dma. The > legacy code will request or release a channel with > mmp_pdma_toggle_reserved_channel(). > > This is not very pretty, but it ensures both legacy and dmaengine > consumers can live in the same kernel until the conversion is done. I like the approach. I actually thought the legacy DMA driver would claim the io port range, which would have made the two drivers mutually exclusive. But it doesn't, so this can in fact work, even though it's really not pretty. Also, it's limited to one single instance of the mmp-pdma driver, but that reflects reality, so I'm fine. One minor nit: > +int mmp_pdma_toggle_reserved_channel(int legacy_channel) > +{ > + if (legacy_unavailable & (1 << legacy_channel)) > + return -EBUSY; > + legacy_reserved ^= 1 << legacy_channel; > + return 0; > +} > +EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel); My concern is that if pxa_request_dma() is called more than once for whatever reason by a legacy implementation, the toggled bit mask might get out of sync. As we know exactly on the caller site what we want to achieve, let's make the API explicit with something like: int mmp_pdma_set_reserved_channel(int legacy_channel, bool reserved) Thanks, Daniel