| Message ID | ec929c37c655ede7bb42e426354093c8a1377a0b.1668947686.git.christophe.jaillet@wanadoo.fr |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp1098160wrr;
Sun, 20 Nov 2022 04:43:33 -0800 (PST)
X-Google-Smtp-Source:
AA0mqf46Dv9hMqBxX1gG8ffFffEikZceycokgZPFidEXRXAdB0VTRAbfCX7b6+39mhYBNOQyceoM
X-Received: by 2002:a17:90b:19c3:b0:212:de1a:355b with SMTP id
nm3-20020a17090b19c300b00212de1a355bmr16306037pjb.1.1668948213490;
Sun, 20 Nov 2022 04:43:33 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1668948213; cv=none;
d=google.com; s=arc-20160816;
b=0lfSJKl9Z/iEwlrGhWW66TuSHbP4gkI/VGA+GDzZDXL2lX84073F4hzzyjsG/nF4bw
+frpWrauflUBxhWGV6eibV1HZSJYOPLEilaI2dz2+jDwagGaofEOh5dio0nN4Pc6ALEf
WlMUFxmJPhPlwlS4JnbC6WiQwn002gfv5pnXgFWHrr/XAEgN++1Au7TO47TNL2aCnIyb
GRv3t06paT5oIGWl6ml61mLbTbhvVJe6jZ2lO+z901ZKbD4355w3xz7IqzIqoLGJQ7QQ
nWwEUhGw5jvohoec1osJEN6N/IfnvFFVMq1gD6xA+fDjzYEZLpX6OSDGAn4UAt4eM//A
NERA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:content-transfer-encoding:mime-version
:message-id:date:subject:cc:to:from;
bh=8PAWmW4pCdn9ryF8cQFs/v1qcaJMHMIlRf4tCBu8kI4=;
b=qzkfjdXf+z9IFknnBySWTuXCXWY29PHxrKNRcJLpGCIiZQ1w10gIY/Bo153vqE62EW
vRdEnrveXCnaV4WWUg4vnBGMUZ5MvZBYodpFXKpuibehbleiR/z2sQwNbB9GJtUEjqwr
aSat3nj4ULbAty/orBhjk29krP3Lq1zNYCQxvnb29DR4y0SeGCp9uEH4SGzbLkpIJr/1
NWzy6ZTUlxUN760wHkhgogKwdyPxp2kDvCfqext4BSWsqpgAIB0Cacndhowke2Y1NMAV
yVp0BX6CwT0yPw0EcRBla2+ypeTaV0g1nVS/A9BdFsqy4P16BoN+xNn5yEguY7xRN/Np
6Fqg==
ARC-Authentication-Results: i=1; mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
ob10-20020a17090b390a00b002130d0f6df8si9672978pjb.30.2022.11.20.04.43.16;
Sun, 20 Nov 2022 04:43:33 -0800 (PST)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S229634AbiKTMfK (ORCPT <rfc822;aposhian.dev@gmail.com>
+ 99 others); Sun, 20 Nov 2022 07:35:10 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35484 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S229520AbiKTMfJ (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Sun, 20 Nov 2022 07:35:09 -0500
Received: from smtp.smtpout.orange.fr (smtp-30.smtpout.orange.fr
[80.12.242.30])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 62DC3558F
for <linux-kernel@vger.kernel.org>;
Sun, 20 Nov 2022 04:35:07 -0800 (PST)
Received: from pop-os.home ([86.243.100.34])
by smtp.orange.fr with ESMTPA
id wjXMofNQANKuIwjXMoFRmK; Sun, 20 Nov 2022 13:35:05 +0100
X-ME-Helo: pop-os.home
X-ME-Auth: Y2hyaXN0b3BoZS5qYWlsbGV0QHdhbmFkb28uZnI=
X-ME-Date: Sun, 20 Nov 2022 13:35:05 +0100
X-ME-IP: 86.243.100.34
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
To: Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Georgi Djakov <djakov@kernel.org>,
Luca Weiss <luca.weiss@fairphone.com>
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org
Subject: [PATCH] interconnect: qcom: icc-rpmh: Fix an error handling path in
qcom_icc_rpmh_probe()
Date: Sun, 20 Nov 2022 13:35:03 +0100
Message-Id:
<ec929c37c655ede7bb42e426354093c8a1377a0b.1668947686.git.christophe.jaillet@wanadoo.fr>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,
RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS
autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1750019041830319801?=
X-GMAIL-MSGID: =?utf-8?q?1750019041830319801?=
|
| Series |
interconnect: qcom: icc-rpmh: Fix an error handling path in qcom_icc_rpmh_probe()
|
|
Commit Message
Christophe JAILLET
Nov. 20, 2022, 12:35 p.m. UTC
If of_platform_populate() fails, some resources need to be freed as already
done in the other error handling paths.
Fixes: 57eb14779dfd ("interconnect: qcom: icc-rpmh: Support child NoC device probe")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/interconnect/qcom/icc-rpmh.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Comments
Hi Christophe, On Sun Nov 20, 2022 at 1:35 PM CET, Christophe JAILLET wrote: > If of_platform_populate() fails, some resources need to be freed as already > done in the other error handling paths. > > Fixes: 57eb14779dfd ("interconnect: qcom: icc-rpmh: Support child NoC device probe") I believe the same needs to be applied to icc-rpm.c. Also there shouldn't be an empty line here between Fixes: and Signed-off-by: Regards Luca > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > drivers/interconnect/qcom/icc-rpmh.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c > index fd17291c61eb..5168bbf3d92f 100644 > --- a/drivers/interconnect/qcom/icc-rpmh.c > +++ b/drivers/interconnect/qcom/icc-rpmh.c > @@ -235,8 +235,11 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, qp); > > /* Populate child NoC devices if any */ > - if (of_get_child_count(dev->of_node) > 0) > - return of_platform_populate(dev->of_node, NULL, NULL, dev); > + if (of_get_child_count(dev->of_node) > 0) { > + ret = of_platform_populate(dev->of_node, NULL, NULL, dev); > + if (ret) > + goto err; > + } > > return 0; > err: > -- > 2.34.1
Le 21/11/2022 à 08:42, Luca Weiss a écrit : > Hi Christophe, > > On Sun Nov 20, 2022 at 1:35 PM CET, Christophe JAILLET wrote: >> If of_platform_populate() fails, some resources need to be freed as already >> done in the other error handling paths. >> >> Fixes: 57eb14779dfd ("interconnect: qcom: icc-rpmh: Support child NoC device probe") > > I believe the same needs to be applied to icc-rpm.c. I'll give it a look and send a v2. CJ > > Also there shouldn't be an empty line here between Fixes: and Signed-off-by: > > Regards > Luca > >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> drivers/interconnect/qcom/icc-rpmh.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c >> index fd17291c61eb..5168bbf3d92f 100644 >> --- a/drivers/interconnect/qcom/icc-rpmh.c >> +++ b/drivers/interconnect/qcom/icc-rpmh.c >> @@ -235,8 +235,11 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev) >> platform_set_drvdata(pdev, qp); >> >> /* Populate child NoC devices if any */ >> - if (of_get_child_count(dev->of_node) > 0) >> - return of_platform_populate(dev->of_node, NULL, NULL, dev); >> + if (of_get_child_count(dev->of_node) > 0) { >> + ret = of_platform_populate(dev->of_node, NULL, NULL, dev); >> + if (ret) >> + goto err; >> + } >> >> return 0; >> err: >> -- >> 2.34.1 > >
Le 21/11/2022 à 08:42, Luca Weiss a écrit : > Hi Christophe, > > On Sun Nov 20, 2022 at 1:35 PM CET, Christophe JAILLET wrote: >> If of_platform_populate() fails, some resources need to be freed as already >> done in the other error handling paths. >> >> Fixes: 57eb14779dfd ("interconnect: qcom: icc-rpmh: Support child NoC device probe") > > I believe the same needs to be applied to icc-rpm.c. Their are other issues in the error handling path of this file. 1) if (desc->has_bus_pd) { ret = dev_pm_domain_attach(dev, true); is unbalanced, both in the error handling path and in the remove function. 2) clk_bulk_prepare_enable() is not balanced in all error handling paths. 3) the same issue about error handling if of_platform_populate() fails. These issues have been introduced in different commits. Would you prefer several patches, each related to a given Fixes: tag, or one bigger "fix all error handling paths"? Anyway, fixing the points above would require moving some code around. Usually I try to avoid it, because it is not always easy to see associated side effects. CJ > > Also there shouldn't be an empty line here between Fixes: and Signed-off-by: > > Regards > Luca > >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> drivers/interconnect/qcom/icc-rpmh.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c >> index fd17291c61eb..5168bbf3d92f 100644 >> --- a/drivers/interconnect/qcom/icc-rpmh.c >> +++ b/drivers/interconnect/qcom/icc-rpmh.c >> @@ -235,8 +235,11 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev) >> platform_set_drvdata(pdev, qp); >> >> /* Populate child NoC devices if any */ >> - if (of_get_child_count(dev->of_node) > 0) >> - return of_platform_populate(dev->of_node, NULL, NULL, dev); >> + if (of_get_child_count(dev->of_node) > 0) { >> + ret = of_platform_populate(dev->of_node, NULL, NULL, dev); >> + if (ret) >> + goto err; >> + } >> >> return 0; >> err: >> -- >> 2.34.1 > >
Hi Christophe, On Tue Nov 22, 2022 at 10:45 PM CET, Christophe JAILLET wrote: > > I believe the same needs to be applied to icc-rpm.c. > > Their are other issues in the error handling path of this file. > > 1) if (desc->has_bus_pd) { > ret = dev_pm_domain_attach(dev, true); > is unbalanced, both in the error handling path and in the remove function. > > > 2) clk_bulk_prepare_enable() > is not balanced in all error handling paths. > > > 3) the same issue about error handling if of_platform_populate() fails. > > > These issues have been introduced in different commits. > > > Would you prefer several patches, each related to a given Fixes: tag, or > one bigger "fix all error handling paths"? I don't really have an opinion on this, I guess if the issues were introduced in different commits, make separate fixes? If it's the wrong way somebody will tell you anyways ;) Regards Luca > > > Anyway, fixing the points above would require moving some code around. > Usually I try to avoid it, because it is not always easy to see > associated side effects. > > CJ
diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c index fd17291c61eb..5168bbf3d92f 100644 --- a/drivers/interconnect/qcom/icc-rpmh.c +++ b/drivers/interconnect/qcom/icc-rpmh.c @@ -235,8 +235,11 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev) platform_set_drvdata(pdev, qp); /* Populate child NoC devices if any */ - if (of_get_child_count(dev->of_node) > 0) - return of_platform_populate(dev->of_node, NULL, NULL, dev); + if (of_get_child_count(dev->of_node) > 0) { + ret = of_platform_populate(dev->of_node, NULL, NULL, dev); + if (ret) + goto err; + } return 0; err: