[v1] watchdog: starfive: Fix the probe return error if PM and early_enable are both disabled
| Message ID | 20230425100456.32718-1-xingyu.wu@starfivetech.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp3294009vqo;
Tue, 25 Apr 2023 03:07:33 -0700 (PDT)
X-Google-Smtp-Source:
AKy350aCk4BuJhblPcVFe22Jj//LGoBGJ/GHONsWDrfSm+9Eg4MrccjDYuG1Ye4pVOZiXYydRHkS
X-Received: by 2002:a17:902:d484:b0:1a9:712d:18b5 with SMTP id
c4-20020a170902d48400b001a9712d18b5mr8331104plg.67.1682417253366;
Tue, 25 Apr 2023 03:07:33 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; t=1682417253; cv=none;
d=google.com; s=arc-20160816;
b=yq6SGwxSKPWxuWkCCRBugpEAaGpMj29036NK07WiWKFlN38BkYdPxMbrBko63EOe+B
CpY66XIxvmRUtlIlI7DCjZ+8rvTyR2H5ykkoPbvJ9ncVTqQtbUtEBm6+leHfcd5NR9I1
3qyc4eYiGDfIFzdr7R8FmeMLRdxaEq9r9zeiWPhXDAVYxgfjt9Y+GqPqNzDjFrUWApYa
eUuioREVECYBywsmhSXd05phxGgV2/4oD8h/FKBpGSt1y3a4rN2KkQ1oBNTNDr4intUj
EBpFXs4BurRxpmcWcyRC9C1ZE350etWv7+afEBKLTiH7tRZqyiy5TJrOBEGKuzQgBacq
JI/Q==
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=xSFg5KsU5qT6n2KjKcvApauUwVCi4kh3ti1k2bvYuT8=;
b=YyHVgsWk1BNDwQd4upVXvOAoZDym4EiJ6YVdS+34AIKtMUBuC3rxQKebh7EdKh9WHG
YZ+++2+3UBkzyfPhDt0l1xSYxim8vc9c9WBSs6t9ZJOhIWCe7goT9utRKqJyjuPdr8Aa
JRZDIsiVs/H2dKWBlE5Uunk5vC49fe4kCvtXQtG3DvkVAKEVuP1RbKM7iKNZLy5RXv6P
++oZJfggCyAvDppwO1e0moJxPbCm+ulVxe2AtsH+VFn1UqjRLRZr1nVPy3lujnwD+weW
ykPdV2fHz2npzncYBUsdYSTXTg3z8SoMq8HeJROQX3UPl3vbooYmsHiLgWgKGOOrEL+w
HbDQ==
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
x14-20020a170902ec8e00b001a97f458716si3586936plg.618.2023.04.25.03.07.21;
Tue, 25 Apr 2023 03:07:33 -0700 (PDT)
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 S233565AbjDYKFP convert rfc822-to-8bit (ORCPT
<rfc822;zxc52fgh@gmail.com> + 99 others);
Tue, 25 Apr 2023 06:05:15 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54224 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S233560AbjDYKFK (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Tue, 25 Apr 2023 06:05:10 -0400
Received: from ex01.ufhost.com (ex01.ufhost.com [61.152.239.75])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BC3DECC11;
Tue, 25 Apr 2023 03:05:07 -0700 (PDT)
Received: from EXMBX166.cuchost.com (unknown [175.102.18.54])
(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
(Client CN "EXMBX166", Issuer "EXMBX166" (not verified))
by ex01.ufhost.com (Postfix) with ESMTP id 5D1B624E2D6;
Tue, 25 Apr 2023 18:04:57 +0800 (CST)
Received: from EXMBX061.cuchost.com (172.16.6.61) by EXMBX166.cuchost.com
(172.16.6.76) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Tue, 25 Apr
2023 18:04:57 +0800
Received: from localhost.localdomain (113.72.145.137) by EXMBX061.cuchost.com
(172.16.6.61) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Tue, 25 Apr
2023 18:04:56 +0800
From: Xingyu Wu <xingyu.wu@starfivetech.com>
To: <linux-watchdog@vger.kernel.org>,
Wim Van Sebroeck <wim@linux-watchdog.org>,
Guenter Roeck <linux@roeck-us.net>
CC: Xingyu Wu <xingyu.wu@starfivetech.com>,
Samin Guo <samin.guo@starfivetech.com>,
<linux-kernel@vger.kernel.org>
Subject: [PATCH v1] watchdog: starfive: Fix the probe return error if PM and
early_enable are both disabled
Date: Tue, 25 Apr 2023 18:04:56 +0800
Message-ID: <20230425100456.32718-1-xingyu.wu@starfivetech.com>
X-Mailer: git-send-email 2.25.1
MIME-Version: 1.0
Content-Type: text/plain
X-Originating-IP: [113.72.145.137]
X-ClientProxiedBy: EXCAS066.cuchost.com (172.16.6.26) To EXMBX061.cuchost.com
(172.16.6.61)
X-YovoleRuleAgent: yovoleflag
Content-Transfer-Encoding: 8BIT
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE,
SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham 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?1764142353402256179?=
X-GMAIL-MSGID: =?utf-8?q?1764142353402256179?=
|
| Series |
[v1] watchdog: starfive: Fix the probe return error if PM and early_enable are both disabled
|
|
Commit Message
Xingyu Wu
April 25, 2023, 10:04 a.m. UTC
When the starfive watchdog driver uses 'pm_runtime_put_sync()' as probe
return value at last and 'early_enable' is disabled, it could return the
error '-ENOSYS' if the CONFIG_PM is disabled, but the driver should works
normally.
Add a check to make sure the PM is enabled and then use
'pm_runtime_put_sync()' as return value when 'early_enable' is disabled.
Fixes: db728ea9c7be ("drivers: watchdog: Add StarFive Watchdog driver")
Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
Hi, Guenter and Wim,
This patch fixes the issue of StarFive watchdog driver and rebases on
the master branch of linux-next.
Thanks.
---
drivers/watchdog/starfive-wdt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
On 4/25/23 03:04, Xingyu Wu wrote: > When the starfive watchdog driver uses 'pm_runtime_put_sync()' as probe > return value at last and 'early_enable' is disabled, it could return the > error '-ENOSYS' if the CONFIG_PM is disabled, but the driver should works > normally. > > Add a check to make sure the PM is enabled and then use > 'pm_runtime_put_sync()' as return value when 'early_enable' is disabled. > > Fixes: db728ea9c7be ("drivers: watchdog: Add StarFive Watchdog driver") > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > --- > > Hi, Guenter and Wim, > > This patch fixes the issue of StarFive watchdog driver and rebases on > the master branch of linux-next. > > Thanks. > > --- > drivers/watchdog/starfive-wdt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c > index 1995cceca51e..51e487e09960 100644 > --- a/drivers/watchdog/starfive-wdt.c > +++ b/drivers/watchdog/starfive-wdt.c > @@ -492,7 +492,8 @@ static int starfive_wdt_probe(struct platform_device *pdev) > goto err_exit; > > if (!early_enable) > - return pm_runtime_put_sync(&pdev->dev); > + if (pm_runtime_enabled(&pdev->dev)) > + return pm_runtime_put_sync(&pdev->dev); > Why not just if (!early_enable) pm_runtime_put_sync(&pdev->dev) like almost every other caller of pm_runtime_put_sync() ? Guenter
On 2023/4/25 22:06, Guenter Roeck wrote: > On 4/25/23 03:04, Xingyu Wu wrote: >> When the starfive watchdog driver uses 'pm_runtime_put_sync()' as probe >> return value at last and 'early_enable' is disabled, it could return the >> error '-ENOSYS' if the CONFIG_PM is disabled, but the driver should works >> normally. >> >> Add a check to make sure the PM is enabled and then use >> 'pm_runtime_put_sync()' as return value when 'early_enable' is disabled. >> >> Fixes: db728ea9c7be ("drivers: watchdog: Add StarFive Watchdog driver") >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> --- >> >> Hi, Guenter and Wim, >> >> This patch fixes the issue of StarFive watchdog driver and rebases on >> the master branch of linux-next. >> >> Thanks. >> --- >> drivers/watchdog/starfive-wdt.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c >> index 1995cceca51e..51e487e09960 100644 >> --- a/drivers/watchdog/starfive-wdt.c >> +++ b/drivers/watchdog/starfive-wdt.c >> @@ -492,7 +492,8 @@ static int starfive_wdt_probe(struct platform_device *pdev) >> goto err_exit; >> if (!early_enable) >> - return pm_runtime_put_sync(&pdev->dev); >> + if (pm_runtime_enabled(&pdev->dev)) >> + return pm_runtime_put_sync(&pdev->dev); >> > > Why not just > > if (!early_enable) > pm_runtime_put_sync(&pdev->dev) > > like almost every other caller of pm_runtime_put_sync() ? > The function of pm_runtime_put_sync() is that: static inline int pm_runtime_put_sync(struct device *dev) { return __pm_runtime_idle(dev, RPM_GET_PUT); } and when do not enable CONFIG_PM, the __pm_runtime_idle() is that: static inline int __pm_runtime_idle(struct device *dev, int rpmflags) { return -ENOSYS; } If I do not open PM, it will return error saying probe failed but it works fine without PM. I had tested that and probe really was failed. So I should add this check before using pm_runtime_put_sync(). Best regards, Xingyu Wu
On 4/25/23 18:49, Xingyu Wu wrote: > On 2023/4/25 22:06, Guenter Roeck wrote: >> On 4/25/23 03:04, Xingyu Wu wrote: >>> When the starfive watchdog driver uses 'pm_runtime_put_sync()' as probe >>> return value at last and 'early_enable' is disabled, it could return the >>> error '-ENOSYS' if the CONFIG_PM is disabled, but the driver should works >>> normally. >>> >>> Add a check to make sure the PM is enabled and then use >>> 'pm_runtime_put_sync()' as return value when 'early_enable' is disabled. >>> >>> Fixes: db728ea9c7be ("drivers: watchdog: Add StarFive Watchdog driver") >>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >>> --- >>> >>> Hi, Guenter and Wim, >>> >>> This patch fixes the issue of StarFive watchdog driver and rebases on >>> the master branch of linux-next. >>> >>> Thanks. >>> --- >>> drivers/watchdog/starfive-wdt.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c >>> index 1995cceca51e..51e487e09960 100644 >>> --- a/drivers/watchdog/starfive-wdt.c >>> +++ b/drivers/watchdog/starfive-wdt.c >>> @@ -492,7 +492,8 @@ static int starfive_wdt_probe(struct platform_device *pdev) >>> goto err_exit; >>> if (!early_enable) >>> - return pm_runtime_put_sync(&pdev->dev); >>> + if (pm_runtime_enabled(&pdev->dev)) >>> + return pm_runtime_put_sync(&pdev->dev); >>> >> >> Why not just >> >> if (!early_enable) >> pm_runtime_put_sync(&pdev->dev) >> >> like almost every other caller of pm_runtime_put_sync() ? >> > > The function of pm_runtime_put_sync() is that: > > static inline int pm_runtime_put_sync(struct device *dev) > { > return __pm_runtime_idle(dev, RPM_GET_PUT); > } > > and when do not enable CONFIG_PM, the __pm_runtime_idle() is that: > > static inline int __pm_runtime_idle(struct device *dev, int rpmflags) > { > return -ENOSYS; > } > > If I do not open PM, it will return error saying probe failed > but it works fine without PM. I had tested that and probe really > was failed. So I should add this check before using pm_runtime_put_sync(). > You did not answer my question. Guenter
On 2023/4/26 10:01, Guenter Roeck wrote: > On 4/25/23 18:49, Xingyu Wu wrote: >> On 2023/4/25 22:06, Guenter Roeck wrote: >>> On 4/25/23 03:04, Xingyu Wu wrote: >>>> When the starfive watchdog driver uses 'pm_runtime_put_sync()' as probe >>>> return value at last and 'early_enable' is disabled, it could return the >>>> error '-ENOSYS' if the CONFIG_PM is disabled, but the driver should works >>>> normally. >>>> >>>> Add a check to make sure the PM is enabled and then use >>>> 'pm_runtime_put_sync()' as return value when 'early_enable' is disabled. >>>> >>>> Fixes: db728ea9c7be ("drivers: watchdog: Add StarFive Watchdog driver") >>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >>>> --- >>>> >>>> Hi, Guenter and Wim, >>>> >>>> This patch fixes the issue of StarFive watchdog driver and rebases on >>>> the master branch of linux-next. >>>> >>>> Thanks. >>>> --- >>>> drivers/watchdog/starfive-wdt.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c >>>> index 1995cceca51e..51e487e09960 100644 >>>> --- a/drivers/watchdog/starfive-wdt.c >>>> +++ b/drivers/watchdog/starfive-wdt.c >>>> @@ -492,7 +492,8 @@ static int starfive_wdt_probe(struct platform_device *pdev) >>>> goto err_exit; >>>> if (!early_enable) >>>> - return pm_runtime_put_sync(&pdev->dev); >>>> + if (pm_runtime_enabled(&pdev->dev)) >>>> + return pm_runtime_put_sync(&pdev->dev); >>>> >>> >>> Why not just >>> >>> if (!early_enable) >>> pm_runtime_put_sync(&pdev->dev) >>> >>> like almost every other caller of pm_runtime_put_sync() ? >>> >> >> The function of pm_runtime_put_sync() is that: >> >> static inline int pm_runtime_put_sync(struct device *dev) >> { >> return __pm_runtime_idle(dev, RPM_GET_PUT); >> } >> >> and when do not enable CONFIG_PM, the __pm_runtime_idle() is that: >> >> static inline int __pm_runtime_idle(struct device *dev, int rpmflags) >> { >> return -ENOSYS; >> } >> >> If I do not open PM, it will return error saying probe failed >> but it works fine without PM. I had tested that and probe really >> was failed. So I should add this check before using pm_runtime_put_sync(). >> > > You did not answer my question. > Oh sorry, I did not notice that you dropped the 'return'. It looks simpler. Best regards, Xingyu Wu
diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c index 1995cceca51e..51e487e09960 100644 --- a/drivers/watchdog/starfive-wdt.c +++ b/drivers/watchdog/starfive-wdt.c @@ -492,7 +492,8 @@ static int starfive_wdt_probe(struct platform_device *pdev) goto err_exit; if (!early_enable) - return pm_runtime_put_sync(&pdev->dev); + if (pm_runtime_enabled(&pdev->dev)) + return pm_runtime_put_sync(&pdev->dev); return 0;