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=-17.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 E4502C433E0 for ; Mon, 28 Dec 2020 05:38:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A9FD9207CC for ; Mon, 28 Dec 2020 05:38:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727975AbgL1FiX (ORCPT ); Mon, 28 Dec 2020 00:38:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36248 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726657AbgL1FiW (ORCPT ); Mon, 28 Dec 2020 00:38:22 -0500 Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0E06EC061795 for ; Sun, 27 Dec 2020 21:37:41 -0800 (PST) Received: by mail-pj1-x1031.google.com with SMTP id f14so5396236pju.4 for ; Sun, 27 Dec 2020 21:37:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=zegu0X9sZDxNvLaLEb6B1P2y6YF1mpAuGwZH8pLcUFA=; b=kGu4z+nkVQp+gc3xdhla18BclFTp67yzoxD+9RMY/GNIXQuJexMdOsKGqMSyUf1E/l 5CB7kIpM6JGNNbciDg+zesEBaWdz1pBhFlY7EJdHT1T+liJT84FxCNu+mMoBZeWEkzJ6 Q52JQkCXPH4rhP8Y3QYUtl/ZOCyEeePFYh74dpzixQjAYGJzhX0cD3gFD3E4Eid1d7Cx dbWFydxsZnVIzQxYWP6sy6nbUdVlaeyP3LjdaasrPTmMxEQNIXRasuR6qjl9MS/p7E33 gSVdelpIQSxrGGuumL9cdg5LYxRoRR5Z2A6zoM3Tu5mCXTZu/cfiGCDdoolQJxZB1rfa axIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=zegu0X9sZDxNvLaLEb6B1P2y6YF1mpAuGwZH8pLcUFA=; b=bH4wIbdIfO2Q+RDZTIMZNCF2kU+yP1zJlrBUV3PXz5uw/8AEsJW4ky3kBMYMCd2LSH bl9owjDXljiNOw/rXX/n92tgjUsFYdTXlJ8iOBctXxAu46bqfRzkWELj6tjGINWgVrOu 0isnYsbFjhhO6khj/TvZokO5RohoaHvL3IVYK1QT5lmCTa8GbC3Z3gtNF5wrx/hpLbrj EMVwdDLm8yw813xv9mlbyqAMIyFbgTk+ny0HZMJhItq+utVHwyBeJ4LDN0jr/sKn6VpA n41EGnrMVQeKsl3cVcpuby3PBJ8dqnmaa/DjZXcKi+Oe00Tyf80nHGfEYmaw5DAkVOAN gazg== X-Gm-Message-State: AOAM530t1lXwr324kUerg5nV1hFZFERWWReWSnY3Gwv1+cCRGrtwT/5H rURVKl7K6gLBTTdRC3HyxsFW6g== X-Google-Smtp-Source: ABdhPJxZONjSIasQCRp/1pDOfNWPBn0hTvHIEA8MIs0tOmtvQDHZvqlafaqvzNUcqKO4wHrWxx/3HA== X-Received: by 2002:a17:90a:ee8e:: with SMTP id i14mr19562217pjz.190.1609133861327; Sun, 27 Dec 2020 21:37:41 -0800 (PST) Received: from localhost ([122.172.20.109]) by smtp.gmail.com with ESMTPSA id d124sm36711730pgc.68.2020.12.27.21.37.40 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 27 Dec 2020 21:37:40 -0800 (PST) Date: Mon, 28 Dec 2020 11:07:38 +0530 From: Viresh Kumar To: Christophe JAILLET Cc: mmayer@broadcom.com, bcm-kernel-feedback-list@broadcom.com, rjw@rjwysocki.net, f.fainelli@gmail.com, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH] cpufreq: brcmstb-avs-cpufreq: Fix some resource leaks in the error handling path of the probe function Message-ID: <20201228053738.6b6uaz2ipzjpwzet@vireshk-i7> References: <20201219101751.181783-1-christophe.jaillet@wanadoo.fr> <20201222043505.rq3cmajc3mxv3p2z@vireshk-i7> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20180716-391-311a52 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27-12-20, 18:22, Christophe JAILLET wrote: > Le 22/12/2020 à 05:35, Viresh Kumar a écrit : > > On 19-12-20, 11:17, Christophe JAILLET wrote: > > > If 'cpufreq_register_driver()' fails, we must release the resources > > > allocated in 'brcm_avs_prepare_init()' as already done in the remove > > > function. > > > > > > To do that, introduce a new function 'brcm_avs_prepare_uninit()' in order > > > to avoid code duplication. This also makes the code more readable (IMHO). > > > > > > Fixes: de322e085995 ("cpufreq: brcmstb-avs-cpufreq: AVS CPUfreq driver for Broadcom STB SoCs") > > > Signed-off-by: Christophe JAILLET > > > --- > > > I'm not sure that the existing error handling in the remove function is > > > correct and/or needed. > > > --- > > > drivers/cpufreq/brcmstb-avs-cpufreq.c | 25 ++++++++++++++++++++----- > > > 1 file changed, 20 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/cpufreq/brcmstb-avs-cpufreq.c b/drivers/cpufreq/brcmstb-avs-cpufreq.c > > > index 3e31e5d28b79..750ca7cfccb0 100644 > > > --- a/drivers/cpufreq/brcmstb-avs-cpufreq.c > > > +++ b/drivers/cpufreq/brcmstb-avs-cpufreq.c > > > @@ -597,6 +597,16 @@ static int brcm_avs_prepare_init(struct platform_device *pdev) > > > return ret; > > > } > > > +static void brcm_avs_prepare_uninit(struct platform_device *pdev) > > > +{ > > > + struct private_data *priv; > > > + > > > + priv = platform_get_drvdata(pdev); > > > + > > > + iounmap(priv->avs_intr_base); > > > + iounmap(priv->base); > > > +} > > > + > > > static int brcm_avs_cpufreq_init(struct cpufreq_policy *policy) > > > { > > > struct cpufreq_frequency_table *freq_table; > > > @@ -732,21 +742,26 @@ static int brcm_avs_cpufreq_probe(struct platform_device *pdev) > > > brcm_avs_driver.driver_data = pdev; > > > - return cpufreq_register_driver(&brcm_avs_driver); > > > + ret = cpufreq_register_driver(&brcm_avs_driver); > > > + if (ret) > > > + goto err_uninit; > > > + > > > + return 0; > > > + > > > +err_uninit: > > > + brcm_avs_prepare_uninit(pdev); > > > + return ret; > > > > Maybe rewrite as: > > > > ret = cpufreq_register_driver(&brcm_avs_driver); > > if (ret) > > brcm_avs_prepare_uninit(pdev); > > return ret; > > > > Personlaly, I prefer what I have proposed. Having a clear and dedicated > error handling path is more future proff, IMHO. I would have agreed to that if there were other things we were handling in the error path, but right now we are adding an extra label, goto, etc without any need. If in future this needs a change, we can always come back to it and update with a label. But right now I would suggest to keep it simple. > > > } > > > static int brcm_avs_cpufreq_remove(struct platform_device *pdev) > > > { > > > - struct private_data *priv; > > > int ret; > > > ret = cpufreq_unregister_driver(&brcm_avs_driver); > > > if (ret) > > > return ret; > > > > Instead of returning here, it can be just WARN_ON(ret); and then go on and free > > the resources and this needs to be done in a separate patch. > > Ok, I agree (see my comment below the --- in my patch). > I'll send a patch for it when the first patch will be applied, unless you > prefer if I resend as a serie. Based on the above comment from me, I am expecting another version from you for this patch. So you can fix both the issues in the same patchset. -- viresh From mboxrd@z Thu Jan 1 00:00:00 1970 From: Viresh Kumar Date: Mon, 28 Dec 2020 05:49:38 +0000 Subject: Re: [PATCH] cpufreq: brcmstb-avs-cpufreq: Fix some resource leaks in the error handling path of the Message-Id: <20201228053738.6b6uaz2ipzjpwzet@vireshk-i7> List-Id: References: <20201219101751.181783-1-christophe.jaillet@wanadoo.fr> <20201222043505.rq3cmajc3mxv3p2z@vireshk-i7> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Christophe JAILLET Cc: f.fainelli@gmail.com, linux-pm@vger.kernel.org, kernel-janitors@vger.kernel.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, mmayer@broadcom.com, linux-arm-kernel@lists.infradead.org On 27-12-20, 18:22, Christophe JAILLET wrote: > Le 22/12/2020 =E0 05:35, Viresh Kumar a =E9crit=A0: > > On 19-12-20, 11:17, Christophe JAILLET wrote: > > > If 'cpufreq_register_driver()' fails, we must release the resources > > > allocated in 'brcm_avs_prepare_init()' as already done in the remove > > > function. > > >=20 > > > To do that, introduce a new function 'brcm_avs_prepare_uninit()' in o= rder > > > to avoid code duplication. This also makes the code more readable (IM= HO). > > >=20 > > > Fixes: de322e085995 ("cpufreq: brcmstb-avs-cpufreq: AVS CPUfreq drive= r for Broadcom STB SoCs") > > > Signed-off-by: Christophe JAILLET > > > --- > > > I'm not sure that the existing error handling in the remove function = is > > > correct and/or needed. > > > --- > > > drivers/cpufreq/brcmstb-avs-cpufreq.c | 25 ++++++++++++++++++++----- > > > 1 file changed, 20 insertions(+), 5 deletions(-) > > >=20 > > > diff --git a/drivers/cpufreq/brcmstb-avs-cpufreq.c b/drivers/cpufreq/= brcmstb-avs-cpufreq.c > > > index 3e31e5d28b79..750ca7cfccb0 100644 > > > --- a/drivers/cpufreq/brcmstb-avs-cpufreq.c > > > +++ b/drivers/cpufreq/brcmstb-avs-cpufreq.c > > > @@ -597,6 +597,16 @@ static int brcm_avs_prepare_init(struct platform= _device *pdev) > > > return ret; > > > } > > > +static void brcm_avs_prepare_uninit(struct platform_device *pdev) > > > +{ > > > + struct private_data *priv; > > > + > > > + priv =3D platform_get_drvdata(pdev); > > > + > > > + iounmap(priv->avs_intr_base); > > > + iounmap(priv->base); > > > +} > > > + > > > static int brcm_avs_cpufreq_init(struct cpufreq_policy *policy) > > > { > > > struct cpufreq_frequency_table *freq_table; > > > @@ -732,21 +742,26 @@ static int brcm_avs_cpufreq_probe(struct platfo= rm_device *pdev) > > > brcm_avs_driver.driver_data =3D pdev; > > > - return cpufreq_register_driver(&brcm_avs_driver); > > > + ret =3D cpufreq_register_driver(&brcm_avs_driver); > > > + if (ret) > > > + goto err_uninit; > > > + > > > + return 0; > > > + > > > +err_uninit: > > > + brcm_avs_prepare_uninit(pdev); > > > + return ret; > >=20 > > Maybe rewrite as: > >=20 > > ret =3D cpufreq_register_driver(&brcm_avs_driver); > > if (ret) > > brcm_avs_prepare_uninit(pdev); > > return ret; > >=20 >=20 > Personlaly, I prefer what I have proposed. Having a clear and dedicated > error handling path is more future proff, IMHO. I would have agreed to that if there were other things we were handling in = the error path, but right now we are adding an extra label, goto, etc without a= ny need. If in future this needs a change, we can always come back to it and u= pdate with a label. But right now I would suggest to keep it simple. > > > } > > > static int brcm_avs_cpufreq_remove(struct platform_device *pdev) > > > { > > > - struct private_data *priv; > > > int ret; > > > ret =3D cpufreq_unregister_driver(&brcm_avs_driver); > > > if (ret) > > > return ret; > >=20 > > Instead of returning here, it can be just WARN_ON(ret); and then go on = and free > > the resources and this needs to be done in a separate patch. >=20 > Ok, I agree (see my comment below the --- in my patch). > I'll send a patch for it when the first patch will be applied, unless you > prefer if I resend as a serie. Based on the above comment from me, I am expecting another version from you= for this patch. So you can fix both the issues in the same patchset. --=20 viresh 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=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 EA1EBC433E0 for ; Mon, 28 Dec 2020 05:40:46 +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 5D280207CC for ; Mon, 28 Dec 2020 05:40:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5D280207CC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.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:In-Reply-To:MIME-Version:References: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=uQa7n8ECnpFmyzx5Fd0HUYv3vkmaY9Ykj7fLbMtv5po=; b=TBtC5AGKCg3cuEKsNCzhFrzr3 E6dnS5+vlrh4Do8HX1Pgk1H3usQulC4BBb21xcQBMqdzb3j6xy7joYo8h4wznWsBktZ8PFG2lxog/ 4LrLV2oj3tUhXKWiMFNUtmyGa4nPYyEYM8DgyI/d8pLeeVRe+jUNyx2rImXUzdTb1WAHA9Ac57Sj5 BnN0z0E1zdxZF7sA2s3v5kNDkGi3/rnDdhPRprz+twib49q7B4jQNUxWqrZ53tMTbLUInPx3iRXJ7 P2wWKShgE4aBJy9lrybXL++5/mENkPpEpTMN7kqkedGeDg+P4ZROT9oXhWoC7Q60c8ypLRJIHa1CE KIljok6Pg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ktlE5-0007RF-Qy; Mon, 28 Dec 2020 05:37:49 +0000 Received: from mail-pj1-x1033.google.com ([2607:f8b0:4864:20::1033]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1ktlE3-0007Qg-BV for linux-arm-kernel@lists.infradead.org; Mon, 28 Dec 2020 05:37:48 +0000 Received: by mail-pj1-x1033.google.com with SMTP id z12so5398580pjn.1 for ; Sun, 27 Dec 2020 21:37:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=zegu0X9sZDxNvLaLEb6B1P2y6YF1mpAuGwZH8pLcUFA=; b=kGu4z+nkVQp+gc3xdhla18BclFTp67yzoxD+9RMY/GNIXQuJexMdOsKGqMSyUf1E/l 5CB7kIpM6JGNNbciDg+zesEBaWdz1pBhFlY7EJdHT1T+liJT84FxCNu+mMoBZeWEkzJ6 Q52JQkCXPH4rhP8Y3QYUtl/ZOCyEeePFYh74dpzixQjAYGJzhX0cD3gFD3E4Eid1d7Cx dbWFydxsZnVIzQxYWP6sy6nbUdVlaeyP3LjdaasrPTmMxEQNIXRasuR6qjl9MS/p7E33 gSVdelpIQSxrGGuumL9cdg5LYxRoRR5Z2A6zoM3Tu5mCXTZu/cfiGCDdoolQJxZB1rfa axIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=zegu0X9sZDxNvLaLEb6B1P2y6YF1mpAuGwZH8pLcUFA=; b=dGDLxXIcLr5cZJCs+w+6JXYdL5jOypjzCJG33q+H6Fjw7K+Vfv9/TsgQcfoVOKyCll C1De6gzy6Ti2gQVJcpveiIw5AHsAKVFt8jt6Cb87cwqpY2pAdGx+nCFR0+ONp6aStlVl F/b3hZS/LBqGfwoqFUhiLa/6PXyZBlvHtMjY96F+kz9sHHuRjZA2y3D2qR8lJXF5ubN6 rmJg97QWwLgBLrKiuFfXbzzOpxPddw3vE2QZh6NlDMTQa3Gs/t5oZVHjUGhYrCni/N3X +JjI1/2N2qp16VuDuwEk83tg9tLIGsdKoYHRF45AclEZsQFXBPR1K1mwADR3az9W8L0o QDWA== X-Gm-Message-State: AOAM530zrW8brCV/we1HRQACCYU7N9Cyk2Wr9yK4TCTnhJc1XSn68TZn afgK+lrJPd3DH8a0H2ny7mlocQ== X-Google-Smtp-Source: ABdhPJxZONjSIasQCRp/1pDOfNWPBn0hTvHIEA8MIs0tOmtvQDHZvqlafaqvzNUcqKO4wHrWxx/3HA== X-Received: by 2002:a17:90a:ee8e:: with SMTP id i14mr19562217pjz.190.1609133861327; Sun, 27 Dec 2020 21:37:41 -0800 (PST) Received: from localhost ([122.172.20.109]) by smtp.gmail.com with ESMTPSA id d124sm36711730pgc.68.2020.12.27.21.37.40 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 27 Dec 2020 21:37:40 -0800 (PST) Date: Mon, 28 Dec 2020 11:07:38 +0530 From: Viresh Kumar To: Christophe JAILLET Subject: Re: [PATCH] cpufreq: brcmstb-avs-cpufreq: Fix some resource leaks in the error handling path of the probe function Message-ID: <20201228053738.6b6uaz2ipzjpwzet@vireshk-i7> References: <20201219101751.181783-1-christophe.jaillet@wanadoo.fr> <20201222043505.rq3cmajc3mxv3p2z@vireshk-i7> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20180716-391-311a52 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201228_003747_534043_17010493 X-CRM114-Status: GOOD ( 33.75 ) 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: f.fainelli@gmail.com, linux-pm@vger.kernel.org, kernel-janitors@vger.kernel.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, mmayer@broadcom.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 27-12-20, 18:22, Christophe JAILLET wrote: > Le 22/12/2020 =E0 05:35, Viresh Kumar a =E9crit=A0: > > On 19-12-20, 11:17, Christophe JAILLET wrote: > > > If 'cpufreq_register_driver()' fails, we must release the resources > > > allocated in 'brcm_avs_prepare_init()' as already done in the remove > > > function. > > > = > > > To do that, introduce a new function 'brcm_avs_prepare_uninit()' in o= rder > > > to avoid code duplication. This also makes the code more readable (IM= HO). > > > = > > > Fixes: de322e085995 ("cpufreq: brcmstb-avs-cpufreq: AVS CPUfreq drive= r for Broadcom STB SoCs") > > > Signed-off-by: Christophe JAILLET > > > --- > > > I'm not sure that the existing error handling in the remove function = is > > > correct and/or needed. > > > --- > > > drivers/cpufreq/brcmstb-avs-cpufreq.c | 25 ++++++++++++++++++++----- > > > 1 file changed, 20 insertions(+), 5 deletions(-) > > > = > > > diff --git a/drivers/cpufreq/brcmstb-avs-cpufreq.c b/drivers/cpufreq/= brcmstb-avs-cpufreq.c > > > index 3e31e5d28b79..750ca7cfccb0 100644 > > > --- a/drivers/cpufreq/brcmstb-avs-cpufreq.c > > > +++ b/drivers/cpufreq/brcmstb-avs-cpufreq.c > > > @@ -597,6 +597,16 @@ static int brcm_avs_prepare_init(struct platform= _device *pdev) > > > return ret; > > > } > > > +static void brcm_avs_prepare_uninit(struct platform_device *pdev) > > > +{ > > > + struct private_data *priv; > > > + > > > + priv =3D platform_get_drvdata(pdev); > > > + > > > + iounmap(priv->avs_intr_base); > > > + iounmap(priv->base); > > > +} > > > + > > > static int brcm_avs_cpufreq_init(struct cpufreq_policy *policy) > > > { > > > struct cpufreq_frequency_table *freq_table; > > > @@ -732,21 +742,26 @@ static int brcm_avs_cpufreq_probe(struct platfo= rm_device *pdev) > > > brcm_avs_driver.driver_data =3D pdev; > > > - return cpufreq_register_driver(&brcm_avs_driver); > > > + ret =3D cpufreq_register_driver(&brcm_avs_driver); > > > + if (ret) > > > + goto err_uninit; > > > + > > > + return 0; > > > + > > > +err_uninit: > > > + brcm_avs_prepare_uninit(pdev); > > > + return ret; > > = > > Maybe rewrite as: > > = > > ret =3D cpufreq_register_driver(&brcm_avs_driver); > > if (ret) > > brcm_avs_prepare_uninit(pdev); > > return ret; > > = > = > Personlaly, I prefer what I have proposed. Having a clear and dedicated > error handling path is more future proff, IMHO. I would have agreed to that if there were other things we were handling in = the error path, but right now we are adding an extra label, goto, etc without a= ny need. If in future this needs a change, we can always come back to it and u= pdate with a label. But right now I would suggest to keep it simple. > > > } > > > static int brcm_avs_cpufreq_remove(struct platform_device *pdev) > > > { > > > - struct private_data *priv; > > > int ret; > > > ret =3D cpufreq_unregister_driver(&brcm_avs_driver); > > > if (ret) > > > return ret; > > = > > Instead of returning here, it can be just WARN_ON(ret); and then go on = and free > > the resources and this needs to be done in a separate patch. > = > Ok, I agree (see my comment below the --- in my patch). > I'll send a patch for it when the first patch will be applied, unless you > prefer if I resend as a serie. Based on the above comment from me, I am expecting another version from you= for this patch. So you can fix both the issues in the same patchset. -- = viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel