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 AA505C77B61 for ; Sun, 16 Apr 2023 12:29:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id: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=HhxZRAo9ujexzxbj20Q6g3JcnMtf2CKSkoBHfruuf1Y=; b=0hs9FKe6lAK6O8 eYcKyQgqMKSUJzbhxPN0AmeiBnxTwRreBIQNGEyHYFlszNyh6g/hVnqjxNmufXDx1h2x9bjv+Xpps gwL7zjbxTCUYMVm8uf0hcKvhmUrlMCGqhjffHtM5LUS7d1POHraHXQudul9ekd+k1EK6QGAYgps3L H5qafAIgRfVXNf4dyxclY1XOSMbHjHRjDHpv9EUZjrdAKt1xtfG3U/EVwCrNUxFQKgJwblsv9MwFc eHzxZxVL027ptH7tXULpQtJUtn8FG7QCXe4AnWktrQcKHtirUSKuuiCqFyhSsP8p2QahTOhDJAtkO qZFS8rfuuDvlll9yVaLA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1po1UT-00DnNR-0n; Sun, 16 Apr 2023 12:28:21 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1po1UQ-00DnN3-09 for linux-arm-kernel@lists.infradead.org; Sun, 16 Apr 2023 12:28:19 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id EA77960AD6; Sun, 16 Apr 2023 12:28:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E5FA3C433EF; Sun, 16 Apr 2023 12:28:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1681648096; bh=LBtp8u40d7gaHGLY8w2ydHcEPeO7+ToYI4YBDXA5b8I=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=UFqYPpaG6h7nQ3kOO/JHgzS6QChXYLuOAzpeXPCB4TVFBHAqkvh0bSZMgSQ3uUPDf yA6Esk0IKIAppGJ9ijREuT1M77OkO9LlsohEt5jHRN10gWHj9FuAa/670DeWnznJYQ ubGfhgKkT2sEcSqpixPZeiPdrxb+D2Atzc9lr0a1wReuHcDOWZNooiHEuKPVteabP9 T9+zzlqlSGspes0idWKeHzH3SBaF0Bs0xmcvdsBUnlntFNnGIr4kyf3y2hWvp9dVGl A9yu5a1yRYyNKQzK8APXYg8OtElohSydQ7CIm6HOMpEL0FGDUaLYw0T+rQ9o0Jbu99 XayvnHKYhwOvw== Date: Sun, 16 Apr 2023 13:28:16 +0100 From: Jonathan Cameron To: Jiakai Luo Cc: Lars-Peter Clausen , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Ksenija Stanojevic , Lee Jones , Marek Vasut , linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] iio: adc: mxs-lradc: fix the order of two cleanup operations Message-ID: <20230416132816.70f4814e@jic23-huawei> In-Reply-To: <20230416072157.57388-1-jkluo@hust.edu.cn> References: <20230416072157.57388-1-jkluo@hust.edu.cn> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.37; 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-20230416_052818_186478_65867A07 X-CRM114-Status: GOOD ( 23.87 ) 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: , 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 Sun, 16 Apr 2023 00:21:57 -0700 Jiakai Luo wrote: > Smatch reports: > drivers/iio/adc/mxs-lradc-adc.c:766 mxs_lradc_adc_probe() warn: > missing unwind goto? > > the order of three init operation: > 1.mxs_lradc_adc_trigger_init > 2.iio_triggered_buffer_setup > 3.mxs_lradc_adc_hw_init > > thus, the order of three cleanup operation should be: > 1.mxs_lradc_adc_hw_stop > 2.iio_triggered_buffer_cleanup > 3.mxs_lradc_adc_trigger_remove > > we exchange the order of two cleanup operations, > introducing the following differences: > 1.if mxs_lradc_adc_trigger_init fails, returns directly; > 2.if trigger_init succeeds but iio_triggered_buffer_setup fails, > goto err_trig and remove the trigger. > > v1->v2: exchange the order of mxs_lradc_adc_trigger_remove() > and iio_triggered_buffer_cleanup() in error handling labels > > Fixes: 6dd112b9f85e ("iio: adc: mxs-lradc: Add support for ADC driver") > Signed-off-by: Jiakai Luo > Reviewed-by: Dongliang Mu Thanks for the patch. I agree with your analysis. Please also reorder the unwind that goes on in the remove() callback to match the new ordering. That way things remain consistent between the remove() calls and the error handling. I doubt there is a bug due to the ordering in remove() but there might be. Jonathan > --- > drivers/iio/adc/mxs-lradc-adc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c > index bca79a93cbe4..d32e9b1d03ce 100644 > --- a/drivers/iio/adc/mxs-lradc-adc.c > +++ b/drivers/iio/adc/mxs-lradc-adc.c > @@ -757,13 +757,13 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev) > > ret = mxs_lradc_adc_trigger_init(iio); > if (ret) > - goto err_trig; > + return ret; > > ret = iio_triggered_buffer_setup(iio, &iio_pollfunc_store_time, > &mxs_lradc_adc_trigger_handler, > &mxs_lradc_adc_buffer_ops); > if (ret) > - return ret; > + goto err_trig; > > adc->vref_mv = mxs_lradc_adc_vref_mv[lradc->soc]; > > @@ -801,9 +801,9 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev) > > err_dev: > mxs_lradc_adc_hw_stop(adc); > - mxs_lradc_adc_trigger_remove(iio); > -err_trig: > iio_triggered_buffer_cleanup(iio); > +err_trig: > + mxs_lradc_adc_trigger_remove(iio); > return ret; > } > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel