From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Subject: Re: [PATCH 11/31] dma: add channel request API that supports deferred probe Date: Wed, 20 Nov 2013 14:24:49 -0700 Message-ID: <528D28A1.2050307@wwwdotorg.org> References: <1384548866-13141-1-git-send-email-swarren@wwwdotorg.org> <1384548866-13141-12-git-send-email-swarren@wwwdotorg.org> <1384766276.14845.155.camel@smile> <528A5170.3090809@wwwdotorg.org> <1384862421.14845.222.camel@smile> <528B9CAE.3040600@wwwdotorg.org> <528BFDD0.3090503@wwwdotorg.org> <528CFE68.6060908@wwwdotorg.org> <528D0C06.9080006@wwwdotorg.org> <1384979000.2067.5.camel@dwillia2-mobl1.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1384979000.2067.5.camel-p8uTFz9XbKgxhm4521IUFVnYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Sender: linux-tegra-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Williams, Dan J" Cc: Andy Shevchenko , Stephen Warren , "Koul, Vinod" , "pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org" , "dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: linux-tegra@vger.kernel.org On 11/20/2013 01:23 PM, Williams, Dan J wrote: ... > Why do the drivers that call dma_request_channel need to convert it to > an ERR value? i.e. what's problematic about the below (not compile > tested)? ... > diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c ... > @@ -22,16 +22,20 @@ static unsigned samsung_dmadev_request(enum dma_ch dma_ch, > struct samsung_dma_req *param, > struct device *dev, char *ch_name) ... > + if (dev->of_node) { > + chan = dma_request_slave_channel(dev, ch_name); > + return IS_ERR(chan) ? (unsigned) NULL : (unsigned) chan; > + } else { > return (unsigned)dma_request_channel(mask, pl330_filter, > (void *)dma_ch); > + } The argument is that if a function returns errors encoded as an ERR pointer, then callers must assume that any non-IS_ERR value that the function returns is valid. NULL is one of those values. As such, callers can no longer check the value against NULL, but must use IS_ERR(). Converting any IS_ERR() returns to NULL theoretically is the act of converting one valid return value to some other completely random return value. The converse is true for functions that return errors encoded as NULL; callers must check those return results against NULL. There's no intersection between those two sets of legal tests. From mboxrd@z Thu Jan 1 00:00:00 1970 From: swarren@wwwdotorg.org (Stephen Warren) Date: Wed, 20 Nov 2013 14:24:49 -0700 Subject: [PATCH 11/31] dma: add channel request API that supports deferred probe In-Reply-To: <1384979000.2067.5.camel@dwillia2-mobl1.amr.corp.intel.com> References: <1384548866-13141-1-git-send-email-swarren@wwwdotorg.org> <1384548866-13141-12-git-send-email-swarren@wwwdotorg.org> <1384766276.14845.155.camel@smile> <528A5170.3090809@wwwdotorg.org> <1384862421.14845.222.camel@smile> <528B9CAE.3040600@wwwdotorg.org> <528BFDD0.3090503@wwwdotorg.org> <528CFE68.6060908@wwwdotorg.org> <528D0C06.9080006@wwwdotorg.org> <1384979000.2067.5.camel@dwillia2-mobl1.amr.corp.intel.com> Message-ID: <528D28A1.2050307@wwwdotorg.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 11/20/2013 01:23 PM, Williams, Dan J wrote: ... > Why do the drivers that call dma_request_channel need to convert it to > an ERR value? i.e. what's problematic about the below (not compile > tested)? ... > diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c ... > @@ -22,16 +22,20 @@ static unsigned samsung_dmadev_request(enum dma_ch dma_ch, > struct samsung_dma_req *param, > struct device *dev, char *ch_name) ... > + if (dev->of_node) { > + chan = dma_request_slave_channel(dev, ch_name); > + return IS_ERR(chan) ? (unsigned) NULL : (unsigned) chan; > + } else { > return (unsigned)dma_request_channel(mask, pl330_filter, > (void *)dma_ch); > + } The argument is that if a function returns errors encoded as an ERR pointer, then callers must assume that any non-IS_ERR value that the function returns is valid. NULL is one of those values. As such, callers can no longer check the value against NULL, but must use IS_ERR(). Converting any IS_ERR() returns to NULL theoretically is the act of converting one valid return value to some other completely random return value. The converse is true for functions that return errors encoded as NULL; callers must check those return results against NULL. There's no intersection between those two sets of legal tests.