From patchwork Fri Sep 14 14:03:18 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Hunter, Jon" X-Patchwork-Id: 1459051 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (unknown [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id D3ADF402E1 for ; Fri, 14 Sep 2012 14:15:28 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TCWUn-0000yW-Ao; Fri, 14 Sep 2012 14:03:49 +0000 Received: from arroyo.ext.ti.com ([192.94.94.40]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TCWUZ-0000uN-OC for linux-arm-kernel@lists.infradead.org; Fri, 14 Sep 2012 14:03:38 +0000 Received: from dlelxv30.itg.ti.com ([172.17.2.17]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id q8EE3LRM010877; Fri, 14 Sep 2012 09:03:21 -0500 Received: from DLEE74.ent.ti.com (dlee74.ent.ti.com [157.170.170.8]) by dlelxv30.itg.ti.com (8.13.8/8.13.8) with ESMTP id q8EE3JQK015935; Fri, 14 Sep 2012 09:03:21 -0500 Received: from [172.24.1.27] (172.24.1.27) by DLEE74.ent.ti.com (157.170.170.8) with Microsoft SMTP Server id 14.1.323.3; Fri, 14 Sep 2012 09:03:18 -0500 Message-ID: <50533926.1070201@ti.com> Date: Fri, 14 Sep 2012 09:03:18 -0500 From: Jon Hunter User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0 MIME-Version: 1.0 To: Arnd Bergmann Subject: Re: [PATCH V4 1/2] of: Add generic device tree DMA helpers References: <1347573629-21299-1-git-send-email-jon-hunter@ti.com> <201209140943.49904.arnd@arndb.de> <505330B4.4070605@ti.com> <201209141332.42420.arnd@arndb.de> In-Reply-To: <201209141332.42420.arnd@arndb.de> X-Originating-IP: [172.24.1.27] X-Spam-Note: CRM114 invocation failed X-Spam-Score: -7.4 (-------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-7.4 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [192.94.94.40 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.5 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Stephen Warren , Benoit Cousson , Vinod Koul , device-tree , Nicolas Ferre , Rob Herring , Grant Likely , Dan Williams , Russell King , linux-omap , linux-arm X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 09/14/2012 08:32 AM, Arnd Bergmann wrote: > On Friday 14 September 2012, Jon Hunter wrote: >> On 09/14/2012 04:43 AM, Arnd Bergmann wrote: >>>> + >>>> +Client drivers should specify the DMA property using a phandle to the controller >>>> +followed by DMA controller specific data. >>>> + >>>> +Required property: >>>> +- dmas: List of one or more DMA specifiers, each consisting of >>>> + - A phandle pointing to DMA controller node >>>> + - A single integer cell containing DMA controller >>>> + specific information. This typically contains a dma >>>> + request line number or a channel number, but can >>>> + contain any data that is used required for configuring >>>> + a channel. >>>> +- dma-names: Contains one identifier string for each dma specifier in >>>> + the dmas property. The specific strings that can be used >>>> + are defined in the binding of the DMA client device. >>> >>> I think here we need to clarify that listing the same name multiple times implies >>> having multiple alternatives for the same channel. >> >> Ok, however, the way it works right now is that we will use the first >> specifier that matches the name. So if there are multiple with the same >> name that would imply that someone will need call the >> xxx_request_slave_channel() multiple times to extract these. Is that ok? > > I would expect a driver to only call the function once, and get something > back from the dmaengine layer that works. If there are two controllers > to choose from and one is busy, then it should definitely give a channel > from the non-busy one. > > It's not much of an issue if the code doesn't handle all corner cases at > first, but I would expect that the binding correctly describes how to write > a device tree that will work once the code implements it correctly. Gotcha, may be something like the following should work then ... diff --git a/drivers/of/dma.c b/drivers/of/dma.c index 4025f2f..de59611 100644 --- a/drivers/of/dma.c +++ b/drivers/of/dma.c @@ -127,13 +127,15 @@ static int of_dma_find_channel(struct device_node *np, char *name, return count; for (i = 0; i < count; i++) { - of_property_read_string_index(np, "dma-names", i, &s); + if (of_property_read_string_index(np, "dma-names", i, &s)) + continue; if (strcmp(name, s)) continue; - return of_parse_phandle_with_args(np, "dmas", "#dma-cells", - i, dma_spec); + if (!of_parse_phandle_with_args(np, "dmas", "#dma-cells", i, + dma_spec)) + return 0; } return -ENODEV; @@ -159,33 +161,34 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np, return NULL; } - r = of_dma_find_channel(np, name, &dma_spec); - - if (r) { - pr_err("%s: can't find DMA channel\n", np->full_name); - return NULL; - } + do { + r = of_dma_find_channel(np, name, &dma_spec); + if (r) { + pr_err("%s: can't find DMA channel\n", np->full_name); + return NULL; + } + + ofdma = of_dma_find_controller(dma_spec.np); + if (!ofdma) { + pr_debug("%s: can't find DMA controller %s\n", + np->full_name, dma_spec.np->full_name); + continue; + } - ofdma = of_dma_find_controller(dma_spec.np); - if (!ofdma) { - pr_err("%s: can't find DMA controller %s\n", np->full_name, - dma_spec.np->full_name); - return NULL; - } + if (dma_spec.args_count != ofdma->of_dma_nbcells) { + pr_debug("%s: wrong #dma-cells for %s\n", np->full_name, + dma_spec.np->full_name); + continue; + } - if (dma_spec.args_count != ofdma->of_dma_nbcells) { - pr_err("%s: wrong #dma-cells for %s\n", np->full_name, - dma_spec.np->full_name); - return NULL; - } + chan = ofdma->of_dma_xlate(&dma_spec, ofdma); - chan = ofdma->of_dma_xlate(&dma_spec, ofdma); + of_node_put(dma_spec.np); - of_node_put(dma_spec.np); + } while (!chan); return chan; } -EXPORT_SYMBOL_GPL(of_dma_request_slave_channel); Cheers Jon