From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756311AbcECUA4 (ORCPT ); Tue, 3 May 2016 16:00:56 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:44992 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752195AbcECUAy (ORCPT ); Tue, 3 May 2016 16:00:54 -0400 Subject: Re: [PATCH] mmc: omap: Use dma_request_chan() for requesting DMA channel To: Ulf Hansson References: <1461935197-28664-1-git-send-email-peter.ujfalusi@ti.com> CC: Kishon , Tony Lindgren , Roger Quadros , linux-mmc , linux-omap , "linux-kernel@vger.kernel.org" From: Peter Ujfalusi Message-ID: Date: Tue, 3 May 2016 23:00:47 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: 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 On 05/03/16 11:46, Ulf Hansson wrote: > On 29 April 2016 at 15:06, Peter Ujfalusi wrote: >> With the new dma_request_chan() the client driver does not need to look for >> the DMA resource and it does not need to pass filter_fn anymore. >> By switching to the new API the driver can now support deferred probing >> against DMA. >> >> Signed-off-by: Peter Ujfalusi >> CC: Ulf Hansson >> CC: Jarkko Nikula >> --- >> drivers/mmc/host/omap.c | 45 ++++++++++++++++++++++----------------------- >> 1 file changed, 22 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c >> index b9958a123594..a8d9228657b2 100644 >> --- a/drivers/mmc/host/omap.c >> +++ b/drivers/mmc/host/omap.c >> @@ -23,7 +23,6 @@ >> #include >> #include >> #include >> -#include >> #include >> #include >> #include >> @@ -1321,8 +1320,6 @@ static int mmc_omap_probe(struct platform_device *pdev) >> struct omap_mmc_platform_data *pdata = pdev->dev.platform_data; >> struct mmc_omap_host *host = NULL; >> struct resource *res; >> - dma_cap_mask_t mask; >> - unsigned sig = 0; >> int i, ret = 0; >> int irq; >> >> @@ -1382,29 +1379,31 @@ static int mmc_omap_probe(struct platform_device *pdev) >> goto err_free_iclk; >> } >> >> - dma_cap_zero(mask); >> - dma_cap_set(DMA_SLAVE, mask); >> - >> host->dma_tx_burst = -1; >> host->dma_rx_burst = -1; >> >> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx"); >> - if (res) >> - sig = res->start; >> - host->dma_tx = dma_request_slave_channel_compat(mask, >> - omap_dma_filter_fn, &sig, &pdev->dev, "tx"); >> - if (!host->dma_tx) >> - dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n", >> - sig); >> - >> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx"); >> - if (res) >> - sig = res->start; >> - host->dma_rx = dma_request_slave_channel_compat(mask, >> - omap_dma_filter_fn, &sig, &pdev->dev, "rx"); >> - if (!host->dma_rx) >> - dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n", >> - sig); >> + host->dma_tx = dma_request_chan(&pdev->dev, "tx"); >> + if (IS_ERR(host->dma_tx)) { >> + ret = PTR_ERR(host->dma_tx); >> + if (ret == -EPROBE_DEFER) >> + goto err_free_iclk; and one clk_put(host->fclk) is missing from here. >> + >> + host->dma_tx = NULL; > > Instead of setting this to NULL, let's keep its value and later check > it with IS_ERR() when needed. I thought about it, but decided against without need to have changes in other places in the driver. > >> + dev_warn(host->dev, "TX DMA channel request failed\n"); >> + } >> + >> + host->dma_rx = dma_request_chan(&pdev->dev, "rx"); >> + if (IS_ERR(host->dma_rx)) { >> + ret = PTR_ERR(host->dma_rx); >> + if (ret == -EPROBE_DEFER) { >> + dma_release_channel(host->dma_tx); > > host->dma_tx can be NULL here, so this isn't safe. Oh, true. > >> + clk_put(host->fclk); >> + goto err_free_iclk; >> + } >> + >> + host->dma_rx = NULL; > > Instead of setting this to NULL, let's keep its value and later check > it with IS_ERR() when needed. > >> + dev_warn(host->dev, "RX DMA channel request failed\n"); >> + } >> >> ret = request_irq(host->irq, mmc_omap_irq, 0, DRIVER_NAME, host); >> if (ret) >> -- >> 2.8.1 >> > > Kind regards > Uffe > -- Péter