Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752243AbdLMEuu (ORCPT ); Tue, 12 Dec 2017 23:50:50 -0500 Received: from mga09.intel.com ([134.134.136.24]:1024 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751499AbdLMEur (ORCPT ); Tue, 12 Dec 2017 23:50:47 -0500 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,396,1508828400"; d="scan'208";a="2334157" Date: Wed, 13 Dec 2017 10:24:33 +0530 From: Vinod Koul To: Andreas Platschek Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, dan.j.williams@intel.com Subject: Re: [PATCH v2] dmaengine: fsl-edma: disable clks on all error paths Message-ID: <20171213045432.GM18649@localhost> References: <1512568223-13258-1-git-send-email-andreas.platschek@opentech.at> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1512568223-13258-1-git-send-email-andreas.platschek@opentech.at> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1748 Lines: 51 On Wed, Dec 06, 2017 at 02:50:23PM +0100, Andreas Platschek wrote: > Previously enabled clks are only disabled if clk_prepare_enable() fails. > However, there are other error paths were the previously enabled > clocks are not disabled. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Andreas Platschek > --- > drivers/dma/fsl-edma.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/fsl-edma.c b/drivers/dma/fsl-edma.c > index 6775f2c74e25..9940f2f7039f 100644 > --- a/drivers/dma/fsl-edma.c > +++ b/drivers/dma/fsl-edma.c > @@ -878,7 +878,7 @@ static int fsl_edma_probe(struct platform_device *pdev) > struct fsl_edma_chan *fsl_chan; > struct resource *res; > int len, chans; > - int ret, i; > + int ret, i, j; > > ret = of_property_read_u32(np, "dma-channels", &chans); > if (ret) { > @@ -904,12 +904,21 @@ static int fsl_edma_probe(struct platform_device *pdev) > > res = platform_get_resource(pdev, IORESOURCE_MEM, 1 + i); > fsl_edma->muxbase[i] = devm_ioremap_resource(&pdev->dev, res); > - if (IS_ERR(fsl_edma->muxbase[i])) > + if (IS_ERR(fsl_edma->muxbase[i])) { > + /* disable only clks which were enabled on error */ > + for (j = i-1; j >= 0; j--) ^^^ space around - pls > + clk_disable_unprepare(fsl_edma->muxclk[j]); why not modify fsl_disable_clocks() and invoke that...? > > sprintf(clkname, "dmamux%d", i); > fsl_edma->muxclk[i] = devm_clk_get(&pdev->dev, clkname); > if (IS_ERR(fsl_edma->muxclk[i])) { > + /* disable only clks which were enabled on error */ > + for (j = i-1; j >= 0; j--) > + clk_disable_unprepare(fsl_edma->muxclk[j]); ditto -- ~Vinod