From patchwork Mon Nov 25 17:45:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Williams X-Patchwork-Id: 3233541 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 4964D9F3AE for ; Mon, 25 Nov 2013 17:46:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 16FB6201FE for ; Mon, 25 Nov 2013 17:46:03 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B5CAD201F7 for ; Mon, 25 Nov 2013 17:46:01 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Vl0EE-0002MQ-BY; Mon, 25 Nov 2013 17:45:46 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Vl0EB-0000it-UE; Mon, 25 Nov 2013 17:45:43 +0000 Received: from mail-qe0-x22e.google.com ([2607:f8b0:400d:c02::22e]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Vl0E8-0000iM-FX for linux-arm-kernel@lists.infradead.org; Mon, 25 Nov 2013 17:45:41 +0000 Received: by mail-qe0-f46.google.com with SMTP id a11so4310664qen.33 for ; Mon, 25 Nov 2013 09:45:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=Us907ujymc4KnTbQ4WdRLoveVudq3w+JuL44RT72gqE=; b=iWNzUDFujUMHBhdVR/ctu9hGwsajXFNvtZcJHYSL+TajQ6LZoUfex06wEoaB6jUaNr OMw5d7OXWkZ9pzJXY6LCHqj4jDtgoR+gNKw/Vc4yAohZ7BLJU33r7RNldBjRCkcN+2HF dorDFOMHi5HAvz9TBGgJ/LsDMj/9Ho2ZAVzYQjwG2Q9M2tC2V0dBWDiZY/W3JWRar+og FX7B/YDtisdtNSsysoppbooNRH4eoTXT272MhdnaxioGaGjEE4TEMERv4bmTZojxOqkK 2psC1+Zlw0ES0lJMFkioEXFKxxV3Qhg/rDrufco96OqdW5K/unCZOUIi469cbap+c/CV p7ag== MIME-Version: 1.0 X-Received: by 10.49.53.66 with SMTP id z2mr5707423qeo.45.1385401518833; Mon, 25 Nov 2013 09:45:18 -0800 (PST) Received: by 10.140.84.74 with HTTP; Mon, 25 Nov 2013 09:45:18 -0800 (PST) In-Reply-To: <5293883A.8050808@wwwdotorg.org> References: <528D0C06.9080006@wwwdotorg.org> <1384979000.2067.5.camel@dwillia2-mobl1.amr.corp.intel.com> <528D28A1.2050307@wwwdotorg.org> <528E4F55.9070204@wwwdotorg.org> <528F95A9.2050305@wwwdotorg.org> <528F9DF9.6080706@wwwdotorg.org> <20131123003442.GH16735@n2100.arm.linux.org.uk> <5293883A.8050808@wwwdotorg.org> Date: Mon, 25 Nov 2013 09:45:18 -0800 X-Google-Sender-Auth: GbOeoyIqzAkXbZpLMMCQsS6SRKk Message-ID: Subject: Re: [PATCH 11/31] dma: add channel request API that supports deferred probe From: Dan Williams To: Stephen Warren X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131125_124540_600198_60F57553 X-CRM114-Status: GOOD ( 19.69 ) X-Spam-Score: -1.9 (-) Cc: Andy Shevchenko , Stephen Warren , "Koul, Vinod" , "pdeschrijver@nvidia.com" , "dmaengine@vger.kernel.org" , "linux-tegra@vger.kernel.org" , Russell King - ARM Linux , "treding@nvidia.com" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Nov 25, 2013 at 9:26 AM, Stephen Warren wrote: > On 11/22/2013 05:34 PM, Russell King - ARM Linux wrote: > [... various discussions re: IS_ERR, IS_ERR_OR_NULL, etc.] > > Russell, so if we document dma_request_slave_channel() as follows: > >> /* >> * dma_request_slave_channel - try to allocate an exclusive slave channel >> ... >> * Returns pointer to appropriate DMA channel on success or an error pointer. >> * In order to ease compatibility with other DMA request APIs, this function >> * guarantees NEVER to return NULL. >> */ > > Are you then OK with clients doing either of e.g.: > > chan = dma_request_slave_channel(...); > if (IS_ERR(chan)) > // This returns NULL or a valid handle/pointer > chan = dma_request_channel() > if (!chan) > Here, chan is invalid; > return; > Here, chan is valid > If the driver falls back to dma_request_channel then this one is cleaner. > or: > > if (xxx) { > chan = dma_request_slave_channel(...); > // Convert to same error value as else{} block generates > if (IS_ERR(chan)) > chan = NULL > } else { > // This returns NULL or a valid handle/pointer > chan = dma_request_channel() > } > if (!chan) > Here, chan is invalid > return; > Here, chan is valid Regardless of whether the driver was dma_request_channel as a fallback, it will currently use NULL to indicate an allocation failure. We could go and audit the driver's usage of the result to add more IS_ERR() guards. In the absence of a later call to dma_request_channel() immediately converting to NULL is the least error prone conversion. Of course it's up to the driver, for example: goto err; ...but need to go back and see if this can be cleaned up to not invent the error code here. diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index aaa22867e656..c0f400c3c954 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -278,7 +278,7 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port * chan = dma_request_slave_channel(dev, "tx"); - if (!chan) { + if (IS_ERR(chan)) { /* We need platform data */ if (!plat || !plat->dma_filter) { dev_info(uap->port.dev, "no DMA platform data\n"); ...does not need to convert it to NULL. Nor this one... diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c index ff9d6de2b746..b71c5f138968 100644 --- a/drivers/usb/musb/musb_cppi41.c +++ b/drivers/usb/musb/musb_cppi41.c @@ -502,7 +502,7 @@ static int cppi41_dma_controller_start(struct cppi41_dma_controller *controller) musb_dma->max_len = SZ_4M; dc = dma_request_slave_channel(dev, str); - if (!dc) { + if (IS_ERR(dc)) { dev_err(dev, "Falied to request %s.\n", str); ret = -EPROBE_DEFER;