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=ham 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 E75CEC4363A for ; Thu, 29 Oct 2020 15:42:56 +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 53AD5207DE for ; Thu, 29 Oct 2020 15:42:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="WvuBs3SA"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="x6mAJBBK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 53AD5207DE 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=8iCsg6Yj7fcBkI6nvHNE9AmFLvCYdwA0dNkVJOgx4Ug=; b=WvuBs3SABlYf06irIfn9zoWja 5nOM9w3ZG53t6X+CQss25FESqbau01k37Sx3TAnY5MhxcozCXCCI4hFGyqofrTkPRWIadWLXr4hdu /czbgNj7gHyJaus81b2v1TqwW4wpqSlEeCZSzakNH0d4QE84PNdNLkQsoaHW6xkUbqkNsQeCDLk8n ASZych1tKfTmg7W6UXc2nyRO28J+4hadp9IPRx1W69SfTP4e63YN+wIts8R+vhoyGGcuj+FcKUf5v KJSRChuLH/mFvlssoS61wsyQyptr3r7M+wYH4vvd2fTQzEZEJxkEeUcyHRglUBtoolnAA/jrEQERG kVgB/V6ug==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kYA3L-000401-9B; Thu, 29 Oct 2020 15:41:27 +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 1kYA3J-0003z5-3s for linux-arm-kernel@lists.infradead.org; Thu, 29 Oct 2020 15:41:26 +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 4247620759; Thu, 29 Oct 2020 15:41:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1603986084; bh=NH3QL4UBUna3hBHsBKAlVHRlZtIKuYG5/lyMVyxAwLs=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=x6mAJBBKFfyV8rgvaJzNVB64oMV9PZDjki2vOWlra0tOQnFS3w71leg99fuXtmvUi mRIbfIA0QHW86A/hFArT/PVEvBWZU2C/bAY6dCqUV8ntN2wGq8sA7T2M8tY3nNxVtj HIKz2jScu08jy3kyfxhSPYc8G3RBHgC17WiZ8NJ0= Date: Thu, 29 Oct 2020 15:41:18 +0000 From: Jonathan Cameron To: Bartosz Golaszewski Subject: Re: [PATCH 4/5] iio: adc: xilinx: use devres for irq handling Message-ID: <20201029154118.12fd6c23@archlinux> In-Reply-To: <20201026133609.24262-5-brgl@bgdev.pl> References: <20201026133609.24262-1-brgl@bgdev.pl> <20201026133609.24262-5-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_114125_350016_CB35FDFF X-CRM114-Status: GOOD ( 25.40 ) 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:08 +0100 Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Further simplify the remove() callback and error paths in probe() by > using the managed variant of request_irq() as well as using a devm action > for cancelling the delayed work at driver detach. > > Signed-off-by: Bartosz Golaszewski Again, this is potentially fine but I'd rather you cleaned up the ordering first rather than doing things in this order. The end result of the whole series looks like it will be correct, but that isn't so obvious for the intermediate patches on their own. Also, you end up with a lot of noise renaming gotos that then go away at the end. Jonathan > --- > drivers/iio/adc/xilinx-xadc-core.c | 35 ++++++++++++++++++------------ > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c > index e0da6258092c..4440b7a9bd36 100644 > --- a/drivers/iio/adc/xilinx-xadc-core.c > +++ b/drivers/iio/adc/xilinx-xadc-core.c > @@ -1192,6 +1192,13 @@ static void xadc_clk_disable_unprepare(void *data) > clk_disable_unprepare(clk); > } > > +static void xadc_cancel_delayed_work(void *data) > +{ > + struct delayed_work *work = data; > + > + cancel_delayed_work_sync(work); > +} > + > static int xadc_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1289,14 +1296,19 @@ static int xadc_probe(struct platform_device *pdev) > } > } > > - ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0, > - dev_name(dev), indio_dev); > + ret = devm_request_irq(dev, xadc->irq, xadc->ops->interrupt_handler, 0, > + dev_name(dev), indio_dev); > + if (ret) > + goto err_free_samplerate_trigger; > + > + ret = devm_add_action_or_reset(dev, xadc_cancel_delayed_work, > + &xadc->zynq_unmask_work); > if (ret) > goto err_free_samplerate_trigger; > > ret = xadc->ops->setup(pdev, indio_dev, xadc->irq); > if (ret) > - goto err_free_irq; > + goto err_free_samplerate_trigger; > > for (i = 0; i < 16; i++) > xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i), > @@ -1304,7 +1316,7 @@ static int xadc_probe(struct platform_device *pdev) > > ret = xadc_write_adc_reg(xadc, XADC_REG_CONF0, conf0); > if (ret) > - goto err_free_irq; > + goto err_free_samplerate_trigger; > > bipolar_mask = 0; > for (i = 0; i < indio_dev->num_channels; i++) { > @@ -1314,17 +1326,17 @@ static int xadc_probe(struct platform_device *pdev) > > ret = xadc_write_adc_reg(xadc, XADC_REG_INPUT_MODE(0), bipolar_mask); > if (ret) > - goto err_free_irq; > + goto err_free_samplerate_trigger; > ret = xadc_write_adc_reg(xadc, XADC_REG_INPUT_MODE(1), > bipolar_mask >> 16); > if (ret) > - goto err_free_irq; > + goto err_free_samplerate_trigger; > > /* Disable all alarms */ > ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK, > XADC_CONF1_ALARM_MASK); > if (ret) > - goto err_free_irq; > + goto err_free_samplerate_trigger; > > /* Set thresholds to min/max */ > for (i = 0; i < 16; i++) { > @@ -1339,7 +1351,7 @@ static int xadc_probe(struct platform_device *pdev) > ret = xadc_write_adc_reg(xadc, XADC_REG_THRESHOLD(i), > xadc->threshold[i]); > if (ret) > - goto err_free_irq; > + goto err_free_samplerate_trigger; > } > > /* Go to non-buffered mode */ > @@ -1347,15 +1359,12 @@ static int xadc_probe(struct platform_device *pdev) > > ret = iio_device_register(indio_dev); > if (ret) > - goto err_free_irq; > + goto err_free_samplerate_trigger; > > platform_set_drvdata(pdev, indio_dev); > > return 0; > > -err_free_irq: > - free_irq(xadc->irq, indio_dev); > - cancel_delayed_work_sync(&xadc->zynq_unmask_work); > err_free_samplerate_trigger: > if (xadc->ops->flags & XADC_FLAGS_BUFFERED) > iio_trigger_free(xadc->samplerate_trigger); > @@ -1380,8 +1389,6 @@ static int xadc_remove(struct platform_device *pdev) > iio_trigger_free(xadc->convst_trigger); > iio_triggered_buffer_cleanup(indio_dev); > } > - free_irq(xadc->irq, indio_dev); > - cancel_delayed_work_sync(&xadc->zynq_unmask_work); > > return 0; > } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel