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 5ADCBC531DC for ; Fri, 23 Aug 2024 12:09:42 +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=DvL9WfEX1UJRppkC98DjXufKYaiP2fkvb1seUQWbezQ=; b=qDTVmf6acX0exUuvoCoqzrjEHM Wdl7sXRwRYikvnB0GCDNt+xraI0pJJDhcnYeCkZIgWpWs76Vqr2/lQgiktPteXrWZIXD2wEbqs1x2 TugOrY9JqnpIVWb5m8X5QT7WustTpasowAmWbxCwltEwYp6olCAyfr3noj5/Ey3VgHFZAxm9SghOV ji8aZrowubytivGs01hBH2xA9hzYKL75DxUzHlqVC/GT9dsPumphR6mu5tOXIAsbw7m8OqhJCqQp9 0lTLrcQgsvA7m8rn5/eLeL78/oPyeD4eDEngODuDvFe5ram20N4hFcTaIyqGGGN2+nVZ3cegfgjA2 x2lN5iAg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1shT6g-0000000GeYC-0rz9; Fri, 23 Aug 2024 12:09:30 +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 1shT5u-0000000GeOm-1aCd for linux-arm-kernel@lists.infradead.org; Fri, 23 Aug 2024 12:08:43 +0000 Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4WqzJz6wy7z6G9M8; Fri, 23 Aug 2024 20:04:51 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id F1A4B1400DD; Fri, 23 Aug 2024 20:08:36 +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:08:36 +0100 Date: Fri, 23 Aug 2024 13:08:35 +0100 From: Jonathan Cameron To: Liao Yuanhong CC: , , , Subject: Re: [PATCH 4/6] dma:imx-sdma:Use devm_clk_get_enabled() helpers Message-ID: <20240823130835.00003495@Huawei.com> In-Reply-To: <20240823101933.9517-5-liaoyuanhong@vivo.com> References: <20240823101933.9517-1-liaoyuanhong@vivo.com> <20240823101933.9517-5-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_050842_739538_7F3F4E0E X-CRM114-Status: GOOD ( 19.69 ) 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:31 +0800 Liao Yuanhong wrote: > Use devm_clk_get_enabled() instead of clk functions in imx-sdma. > > Signed-off-by: Liao Yuanhong No. Consider why the clocks are adjusted where they are in existing code before 'cleaning' it up. > --- > drivers/dma/imx-sdma.c | 57 ++++-------------------------------------- > 1 file changed, 5 insertions(+), 52 deletions(-) > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c > index 72299a08af44..af972a4b6ce1 100644 > --- a/drivers/dma/imx-sdma.c > +++ b/drivers/dma/imx-sdma.c > @@ -1493,24 +1493,11 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan) > sdmac->event_id0 = data->dma_request; > sdmac->event_id1 = data->dma_request2; > > - ret = clk_enable(sdmac->sdma->clk_ipg); > - if (ret) > - return ret; > - ret = clk_enable(sdmac->sdma->clk_ahb); > - if (ret) > - goto disable_clk_ipg; > - > ret = sdma_set_channel_priority(sdmac, prio); > if (ret) > - goto disable_clk_ahb; > + return ret; > > return 0; > - > -disable_clk_ahb: > - clk_disable(sdmac->sdma->clk_ahb); > -disable_clk_ipg: > - clk_disable(sdmac->sdma->clk_ipg); > - return ret; > } > > static void sdma_free_chan_resources(struct dma_chan *chan) > @@ -1530,9 +1517,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan) > sdmac->event_id1 = 0; > > sdma_set_channel_priority(sdmac, 0); > - > - clk_disable(sdma->clk_ipg); > - clk_disable(sdma->clk_ahb); > } > > static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac, > @@ -2015,14 +1999,10 @@ static void sdma_load_firmware(const struct firmware *fw, void *context) > addr = (void *)header + header->script_addrs_start; > ram_code = (void *)header + header->ram_code_start; > > - clk_enable(sdma->clk_ipg); > - clk_enable(sdma->clk_ahb); > /* download the RAM image for SDMA */ > sdma_load_script(sdma, ram_code, > header->ram_code_size, > addr->ram_code_start_addr); > - clk_disable(sdma->clk_ipg); > - clk_disable(sdma->clk_ahb); Why do you think it is suddenly fine to leave the locks on here and it wasn't before? Check all the paths. > > sdma_add_scripts(sdma, addr); > > @@ -2119,13 +2099,6 @@ static int sdma_init(struct sdma_engine *sdma) > dma_addr_t ccb_phys; > int ccbsize; > > - ret = clk_enable(sdma->clk_ipg); > - if (ret) > - return ret; > - ret = clk_enable(sdma->clk_ahb); > - if (ret) > - goto disable_clk_ipg; > - > if (sdma->drvdata->check_ratio && > (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))) > sdma->clk_ratio = 1; > @@ -2180,15 +2153,9 @@ static int sdma_init(struct sdma_engine *sdma) > /* Initializes channel's priorities */ > sdma_set_channel_priority(&sdma->channel[0], 7); > > - clk_disable(sdma->clk_ipg); > - clk_disable(sdma->clk_ahb); > - > return 0; > > err_dma_alloc: > - clk_disable(sdma->clk_ahb); > -disable_clk_ipg: > - clk_disable(sdma->clk_ipg); > dev_err(sdma->dev, "initialisation failed with %d\n", ret); > return ret; > } > @@ -2266,33 +2233,25 @@ static int sdma_probe(struct platform_device *pdev) > if (IS_ERR(sdma->regs)) > return PTR_ERR(sdma->regs); > > - sdma->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); > + sdma->clk_ipg = devm_clk_get_enabled(&pdev->dev, "ipg"); > if (IS_ERR(sdma->clk_ipg)) > return PTR_ERR(sdma->clk_ipg); > > - sdma->clk_ahb = devm_clk_get(&pdev->dev, "ahb"); > + sdma->clk_ahb = devm_clk_get_enabled(&pdev->dev, "ahb"); > if (IS_ERR(sdma->clk_ahb)) > return PTR_ERR(sdma->clk_ahb); > > - ret = clk_prepare(sdma->clk_ipg); > - if (ret) > - return ret; > - > - ret = clk_prepare(sdma->clk_ahb); > - if (ret) > - goto err_clk; > - > ret = devm_request_irq(&pdev->dev, irq, sdma_int_handler, 0, > dev_name(&pdev->dev), sdma); > if (ret) > - goto err_irq; > + return ret; > > sdma->irq = irq; > > sdma->script_addrs = kzalloc(sizeof(*sdma->script_addrs), GFP_KERNEL); > if (!sdma->script_addrs) { > ret = -ENOMEM; > - goto err_irq; > + return ret; > } > > /* initially no scripts available */ > @@ -2407,10 +2366,6 @@ static int sdma_probe(struct platform_device *pdev) > dma_async_device_unregister(&sdma->dma_device); > err_init: > kfree(sdma->script_addrs); > -err_irq: > - clk_unprepare(sdma->clk_ahb); > -err_clk: > - clk_unprepare(sdma->clk_ipg); > return ret; > } > > @@ -2422,8 +2377,6 @@ static void sdma_remove(struct platform_device *pdev) > devm_free_irq(&pdev->dev, sdma->irq, sdma); > dma_async_device_unregister(&sdma->dma_device); > kfree(sdma->script_addrs); > - clk_unprepare(sdma->clk_ahb); > - clk_unprepare(sdma->clk_ipg); > /* Kill the tasklet */ > for (i = 0; i < MAX_DMA_CHANNELS; i++) { > struct sdma_channel *sdmac = &sdma->channel[i];