Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754479AbbK3Omh (ORCPT ); Mon, 30 Nov 2015 09:42:37 -0500 Received: from mail-yk0-f169.google.com ([209.85.160.169]:33579 "EHLO mail-yk0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754384AbbK3Ome (ORCPT ); Mon, 30 Nov 2015 09:42:34 -0500 MIME-Version: 1.0 In-Reply-To: <1448891145-10766-3-git-send-email-peter.ujfalusi@ti.com> References: <1448891145-10766-1-git-send-email-peter.ujfalusi@ti.com> <1448891145-10766-3-git-send-email-peter.ujfalusi@ti.com> Date: Mon, 30 Nov 2015 16:42:33 +0200 Message-ID: Subject: Re: [RFC v02 02/15] dmaengine: core: Move and merge the code paths using private_candidate From: Andy Shevchenko To: Peter Ujfalusi Cc: Vinod Koul , Arnd Bergmann , "linux-kernel@vger.kernel.org" , dmaengine , Linux OMAP Mailing List , linux-arm Mailing List , "linux-mmc@vger.kernel.org" , Sekhar Nori , linux-spi Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6180 Lines: 152 On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi wrote: > Channel matching with private_candidate() is used in two paths, the error > checking is slightly different in them and they are duplicating code also. > Move the code under dma_get_channel() to provide consistent execution and > going to allow us to reuse this mode of channel lookup later. > > Signed-off-by: Peter Ujfalusi > --- > drivers/dma/dmaengine.c | 81 +++++++++++++++++++++++++------------------------ > 1 file changed, 42 insertions(+), 39 deletions(-) > > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c > index 52c3eee48e2e..1249165fb4b2 100644 > --- a/drivers/dma/dmaengine.c > +++ b/drivers/dma/dmaengine.c > @@ -549,6 +549,42 @@ static struct dma_chan *private_candidate(const dma_cap_mask_t *mask, > return NULL; > } > > +static struct dma_chan *dma_get_channel(struct dma_device *device, Naming scheme inside dmaengine.c looks like a mess. Since it's static function that utilizes private_candidate() may I propose the name like find_candidate() ? > + const dma_cap_mask_t *mask, > + dma_filter_fn fn, void *fn_param) > +{ > + struct dma_chan *chan = private_candidate(mask, device, fn, fn_param); > + int err; > + > + if (chan) { > + /* Found a suitable channel, try to grab, prep, and return it. > + * We first set DMA_PRIVATE to disable balance_ref_count as this > + * channel will not be published in the general-purpose > + * allocator > + */ > + dma_cap_set(DMA_PRIVATE, device->cap_mask); > + device->privatecnt++; > + err = dma_chan_get(chan); > + > + if (err) { > + if (err == -ENODEV) { > + pr_debug("%s: %s module removed\n", __func__, > + dma_chan_name(chan)); > + list_del_rcu(&device->global_node); > + } else > + pr_debug("%s: failed to get %s: (%d)\n", > + __func__, dma_chan_name(chan), err); > + > + if (--device->privatecnt == 0) > + dma_cap_clear(DMA_PRIVATE, device->cap_mask); > + > + chan = ERR_PTR(err); > + } > + } > + > + return chan ? chan : ERR_PTR(-EPROBE_DEFER); > +} > + > /** > * dma_get_slave_channel - try to get specific channel exclusively > * @chan: target channel > @@ -587,7 +623,6 @@ struct dma_chan *dma_get_any_slave_channel(struct dma_device *device) > { > dma_cap_mask_t mask; > struct dma_chan *chan; > - int err; > > dma_cap_zero(mask); > dma_cap_set(DMA_SLAVE, mask); > @@ -595,23 +630,11 @@ struct dma_chan *dma_get_any_slave_channel(struct dma_device *device) > /* lock against __dma_request_channel */ > mutex_lock(&dma_list_mutex); > > - chan = private_candidate(&mask, device, NULL, NULL); > - if (chan) { > - dma_cap_set(DMA_PRIVATE, device->cap_mask); > - device->privatecnt++; > - err = dma_chan_get(chan); > - if (err) { > - pr_debug("%s: failed to get %s: (%d)\n", > - __func__, dma_chan_name(chan), err); > - chan = NULL; > - if (--device->privatecnt == 0) > - dma_cap_clear(DMA_PRIVATE, device->cap_mask); > - } > - } > + chan = dma_get_channel(device, &mask, NULL, NULL); > > mutex_unlock(&dma_list_mutex); > > - return chan; > + return IS_ERR(chan) ? NULL : chan; > } > EXPORT_SYMBOL_GPL(dma_get_any_slave_channel); > > @@ -628,35 +651,15 @@ struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask, > { > struct dma_device *device, *_d; > struct dma_chan *chan = NULL; > - int err; > > /* Find a channel */ > mutex_lock(&dma_list_mutex); > list_for_each_entry_safe(device, _d, &dma_device_list, global_node) { > - chan = private_candidate(mask, device, fn, fn_param); > - if (chan) { > - /* Found a suitable channel, try to grab, prep, and > - * return it. We first set DMA_PRIVATE to disable > - * balance_ref_count as this channel will not be > - * published in the general-purpose allocator > - */ > - dma_cap_set(DMA_PRIVATE, device->cap_mask); > - device->privatecnt++; > - err = dma_chan_get(chan); > + chan = dma_get_channel(device, mask, fn, fn_param); > + if (!IS_ERR(chan)) > + break; > > - if (err == -ENODEV) { > - pr_debug("%s: %s module removed\n", > - __func__, dma_chan_name(chan)); > - list_del_rcu(&device->global_node); > - } else if (err) > - pr_debug("%s: failed to get %s: (%d)\n", > - __func__, dma_chan_name(chan), err); > - else > - break; > - if (--device->privatecnt == 0) > - dma_cap_clear(DMA_PRIVATE, device->cap_mask); > - chan = NULL; > - } > + chan = NULL; > } > mutex_unlock(&dma_list_mutex); > > -- > 2.6.3 > -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/