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 X-Spam-Level: X-Spam-Status: No, score=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D8BCC2D0A3 for ; Thu, 29 Oct 2020 15:40:15 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2B40120759 for ; Thu, 29 Oct 2020 15:40:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="tJsB1z8F"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="eVaXIUx8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2B40120759 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Message-ID: Subject: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=IMHl5nq9suY4WVa4LvmmBcLwJ3Iv39X28PzL0SxESRg=; b=tJsB1z8F0Dzws7M/ZbuC/JGi6 s3lwMuL6Z9ioMvOnto0CHVmsk6NkFQNG1JBlIpzAd++CkvZ6019Bbrh27/F145XzLCR25XIomhNl4 C+qWp551k89d9oasPQ+5jChdUqdInycH6naExFoi3h45qmykFlQM7XtFQ3ugnJc6a/7s5gO1H9pfJ +igKrMEKfh9jbbIemQuo7VCWFshb4CysBUQXP/hme5fAn5EW65f39i3WKFhd5of9RizkcvzMb/6c8 j9PsjZbsH4nhsEsbey9ck6hXAoiuUiNk/n/EZ9RzGKNiWRrzVnG37ucgy2UZLJv9JiLO+VEGAHq66 n4dFzHVTQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kYA0m-0003Eu-2i; Thu, 29 Oct 2020 15:38:48 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kYA0j-0003Dx-NJ for linux-arm-kernel@lists.infradead.org; Thu, 29 Oct 2020 15:38:46 +0000 Received: from archlinux (cpc149474-cmbg20-2-0-cust94.5-4.cable.virginm.net [82.4.196.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 3AFCC2076E; Thu, 29 Oct 2020 15:38:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1603985923; bh=Y2j8bDq8gMUZy8zARJTkEfGUrc17A2GF2McIejrCvnE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=eVaXIUx8A/CDynW1RHxXxSbPP4KcQYMa9FdVzvFN9SMyMuzX2xNEeTYxrPsd7rhqu nApNa0Veb+0QDZAl2TfI7We4Lz33f3MRqfGjgm1pDUmz85t8lLLHeETr8CiJKGPsEk +pxaHqHmeK/TY9aMcfjBX4js2q8dxGRtOBQGrmdY= Date: Thu, 29 Oct 2020 15:38:38 +0000 From: Jonathan Cameron To: Bartosz Golaszewski Subject: Re: [PATCH 3/5] iio: adc: xilinx: use a devres action to disable and unprepare the clock Message-ID: <20201029153838.069b46f5@archlinux> In-Reply-To: <20201026133609.24262-4-brgl@bgdev.pl> References: <20201026133609.24262-1-brgl@bgdev.pl> <20201026133609.24262-4-brgl@bgdev.pl> X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201029_113845_939210_FACD78F6 X-CRM114-Status: GOOD ( 23.55 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lars-Peter Clausen , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, Michal Simek , Bartosz Golaszewski , Peter Meerwald-Stadler , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 26 Oct 2020 14:36:07 +0100 Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > In order to simplify resource management and error paths in probe(), add > a devm action that calls clk_disable_unprepare() at driver detach. > > Signed-off-by: Bartosz Golaszewski > --- > drivers/iio/adc/xilinx-xadc-core.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c > index b516280ccbd4..e0da6258092c 100644 > --- a/drivers/iio/adc/xilinx-xadc-core.c > +++ b/drivers/iio/adc/xilinx-xadc-core.c > @@ -1185,6 +1185,13 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > return 0; > } > > +static void xadc_clk_disable_unprepare(void *data) > +{ > + struct clk *clk = data; > + > + clk_disable_unprepare(clk); > +} > + > static int xadc_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1262,6 +1269,11 @@ static int xadc_probe(struct platform_device *pdev) > if (ret) > goto err_free_samplerate_trigger; > Hi Bartosz, > + ret = devm_add_action_or_reset(dev, > + xadc_clk_disable_unprepare, xadc->clk); Take advantage of flexibility in line length limit and put that on line line. The moment I see a devm call with a goto in the error path I get suspicious about ordering. So why do we have actions in the error paths of probe that don't turn up in the remove function? Fix that before you do this change. There are some cases in here already that push the limits on this such as clock allocation but they are easily argued as safe. This is getting less obvious. Jonathan > + if (ret) > + goto err_free_samplerate_trigger; > + > /* > * Make sure not to exceed the maximum samplerate since otherwise the > * resulting interrupt storm will soft-lock the system. > @@ -1280,7 +1292,7 @@ static int xadc_probe(struct platform_device *pdev) > ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0, > dev_name(dev), indio_dev); > if (ret) > - goto err_clk_disable_unprepare; > + goto err_free_samplerate_trigger; > > ret = xadc->ops->setup(pdev, indio_dev, xadc->irq); > if (ret) > @@ -1344,8 +1356,6 @@ static int xadc_probe(struct platform_device *pdev) > err_free_irq: > free_irq(xadc->irq, indio_dev); > cancel_delayed_work_sync(&xadc->zynq_unmask_work); > -err_clk_disable_unprepare: > - clk_disable_unprepare(xadc->clk); > err_free_samplerate_trigger: > if (xadc->ops->flags & XADC_FLAGS_BUFFERED) > iio_trigger_free(xadc->samplerate_trigger); > @@ -1372,7 +1382,6 @@ static int xadc_remove(struct platform_device *pdev) > } > free_irq(xadc->irq, indio_dev); > cancel_delayed_work_sync(&xadc->zynq_unmask_work); > - clk_disable_unprepare(xadc->clk); > > return 0; > } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel