From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AA831C52D7C for ; Fri, 23 Aug 2024 12:02:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:CC:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=q6gB4pAMNkq5ECow59XXOReLYiUE0/uaqXABV1NoxXI=; b=xmHeQgqBr1f9vgmScgbUVcsidd YzA0djQlataHqMC4FMCeAw9qaTZvisBa3NU74Kn2qhWF5Us04LZ8+jgJCc3s9bBmQBMtK7Sa48Gjs +3Qs1eZ5kGVjgwi/tjloxHjZpEjpL3/Nbeuy3AunKmZKD2uFeZCcj1a7rBzrFMyDZ2BTZ3wLkSZFT Wou58uVtXRpLNKwinPZyWrkdrX7jpu3LZLSIrzL1WYFOkfU0cPjUSyvx3Au4ssJYx1oA8Qb1o1+S7 W89LCNCm4Gbzws6LxnDaB+77MUQeZNm86s3nUfQTbFcpELDT4HD0180VPnkY6EYkhGk3SMGc+EyKC Jt9e06eQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1shSzx-0000000Gcrr-1VwR; Fri, 23 Aug 2024 12:02:33 +0000 Received: from frasgout.his.huawei.com ([185.176.79.56]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1shSyq-0000000GcZx-079s for linux-arm-kernel@lists.infradead.org; Fri, 23 Aug 2024 12:01:25 +0000 Received: from mail.maildlp.com (unknown [172.18.186.216]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4Wqz8Y2c24z6GBpb; Fri, 23 Aug 2024 19:57:33 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id 57A781400DB; Fri, 23 Aug 2024 20:01:18 +0800 (CST) Received: from localhost (10.203.177.66) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Fri, 23 Aug 2024 13:01:17 +0100 Date: Fri, 23 Aug 2024 13:01:17 +0100 From: Jonathan Cameron To: Liao Yuanhong CC: , , , Subject: Re: [PATCH 2/6] dma:dma-jz4780:Use devm_clk_get_enabled() helpers Message-ID: <20240823130117.00007e25@Huawei.com> In-Reply-To: <20240823101933.9517-3-liaoyuanhong@vivo.com> References: <20240823101933.9517-1-liaoyuanhong@vivo.com> <20240823101933.9517-3-liaoyuanhong@vivo.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.177.66] X-ClientProxiedBy: lhrpeml100001.china.huawei.com (7.191.160.183) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240823_050124_392114_9D59DCA8 X-CRM114-Status: GOOD ( 22.34 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 23 Aug 2024 18:19:29 +0800 Liao Yuanhong wrote: > Use devm_clk_get_enabled() instead of clk functions in dma-jz4780. > > Signed-off-by: Liao Yuanhong Original code has some odd ordering. The error path order and remove order should be the same and aren't. Even with that tidied up this reorders clock disable and tasklet killing. So I think this is fine, but needs more eyes on it. > --- > drivers/dma/dma-jz4780.c | 18 ++++++------------ > 1 file changed, 6 insertions(+), 12 deletions(-) > > diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c > index c9cfa341db51..151a85516419 100644 > --- a/drivers/dma/dma-jz4780.c > +++ b/drivers/dma/dma-jz4780.c > @@ -149,7 +149,6 @@ struct jz4780_dma_dev { > struct dma_device dma_device; > void __iomem *chn_base; > void __iomem *ctrl_base; > - struct clk *clk; > unsigned int irq; > const struct jz4780_dma_soc_data *soc_data; > > @@ -857,6 +856,7 @@ static int jz4780_dma_probe(struct platform_device *pdev) > struct dma_device *dd; > struct resource *res; > int i, ret; > + struct clk *clk; > > if (!dev->of_node) { > dev_err(dev, "This driver must be probed from devicetree\n"); > @@ -896,15 +896,13 @@ static int jz4780_dma_probe(struct platform_device *pdev) > return -EINVAL; > } > > - jzdma->clk = devm_clk_get(dev, NULL); > - if (IS_ERR(jzdma->clk)) { > + clk = devm_clk_get_enabled(dev, NULL); > + if (IS_ERR(clk)) { > dev_err(dev, "failed to get clock\n"); > - ret = PTR_ERR(jzdma->clk); > + ret = PTR_ERR(clk); > return ret; > } > > - clk_prepare_enable(jzdma->clk); > - > /* Property is optional, if it doesn't exist the value will remain 0. */ > of_property_read_u32_index(dev->of_node, "ingenic,reserved-channels", > 0, &jzdma->chan_reserved); > @@ -972,7 +970,7 @@ static int jz4780_dma_probe(struct platform_device *pdev) > > ret = platform_get_irq(pdev, 0); > if (ret < 0) > - goto err_disable_clk; > + return ret; > > jzdma->irq = ret; > > @@ -980,7 +978,7 @@ static int jz4780_dma_probe(struct platform_device *pdev) > jzdma); > if (ret) { > dev_err(dev, "failed to request IRQ %u!\n", jzdma->irq); > - goto err_disable_clk; > + return ret; > } > > ret = dmaenginem_async_device_register(dd); > @@ -1002,9 +1000,6 @@ static int jz4780_dma_probe(struct platform_device *pdev) > > err_free_irq: > free_irq(jzdma->irq, jzdma); > - > -err_disable_clk: > - clk_disable_unprepare(jzdma->clk); > return ret; > } > > @@ -1015,7 +1010,6 @@ static void jz4780_dma_remove(struct platform_device *pdev) > > of_dma_controller_free(pdev->dev.of_node); > > - clk_disable_unprepare(jzdma->clk); > free_irq(jzdma->irq, jzdma); Hmm. Ordering wise this is not great. > > for (i = 0; i < jzdma->soc_data->nb_channels; i++)