From: hieagle <chgokhl@gmail.com>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-mmc <linux-mmc@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
kehuanlin@fishsemi.com
Subject: Re: [PATCH] mmc: core: Mark mmc_host device with pm_runtime_no_callbacks
Date: Mon, 21 Jun 2021 09:38:34 +0800 [thread overview]
Message-ID: <CAMjpFAUTsei-PWLDdM+_78qXTo=QFdw-Wbgo9QA2Jnss+i4yRA@mail.gmail.com> (raw)
In-Reply-To: <CAPDyKFqXtKPngfnQZXXaF=rvqw0=nWzDc7P++QxNHPwGHHSxUA@mail.gmail.com>
Sorry, I don't receive the reply email in my gmail.
Normally the mmc_host's power.disable_depth is large than zero, the
rpm_resume(mmc:0001) will not be called recursively for parent. This is
the most case.
Although the mmc class device never calls pm_runtime_enable() directly,
there are still some cases as below to call pm_runtime_enable(), which
may cause it's power.disable_depth decremented to zero.
case1: device_resume_early->pm_runtime_enable
case2: device_resume->pm_runtime_enable
Anything that can go wrong will go wrong. Unfortunately we meet the case.
If you trigger to set the mmc_host's power.disable_depth value to zero
after mmc suspended, you can find the issue.
In our platform the mmc device's parent list is as below:
mmc0:0001->mmc_host mmc0->fa630000.mmc->soc.
The rpm_resume call trace is as below in our scenario:
rpm_resume(mmc0:0001)
|
if (!parent && dev->parent) //true
if (!parent->power.disable_depth
&& !parent->power.ignore_children) //true
rpm_resume(parent, 0) ---> rpm_resume(mmc_host, 0)
| |
| callback = RPM_GET_CALLBACK(mmc_host, ...) = NULL
| retval = rpm_callback(callback, mmc_host) = -ENOSYS
| |
| return retval = -ENOSYS
if (retval) goto out; //skip rpm_callback()
return retval = -ENOSYS
The scenario is rare, but anything that can go wrong will go wrong.
The patch can enhance the code to avoid this scenario.
Ulf Hansson <ulf.hansson@linaro.org> 于2021年3月22日周一 下午6:26写道:
>
> On Sat, 20 Mar 2021 at 05:57, kehuanlin <chgokhl@gmail.com> wrote:
> >
> > The rpm_resume() will call parent's resume callback recursively.
> > Since mmc_host has no its own pm_runtime callbacks, the mmc devices
> > may fail to resume (-ENOSYS in rpm_callback) sometimes. Mark mmc_host
> > device with pm_runtime_no_callbacks can fix the issue.
>
> Can you please elaborate more on this? What do you mean by "sometimes"?
>
> More precisely, how do you trigger the rpm_callback() for mmc class
> device to return -ENOSYS?
>
> Don't get me wrong, the patch is fine, but I want to understand if it
> actually solves a problem for you - or that it's better considered as
> an optimization?
>
> Kind regards
> Uffe
>
> >
> > Signed-off-by: kehuanlin <chgokhl@gmail.com>
> > ---
> > drivers/mmc/core/host.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> > index 9b89a91b6b47..177bebd9a6c4 100644
> > --- a/drivers/mmc/core/host.c
> > +++ b/drivers/mmc/core/host.c
> > @@ -15,6 +15,7 @@
> > #include <linux/of.h>
> > #include <linux/of_gpio.h>
> > #include <linux/pagemap.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/pm_wakeup.h>
> > #include <linux/export.h>
> > #include <linux/leds.h>
> > @@ -480,6 +481,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
> > host->class_dev.class = &mmc_host_class;
> > device_initialize(&host->class_dev);
> > device_enable_async_suspend(&host->class_dev);
> > + pm_runtime_no_callbacks(&host->class_dev);
> >
> > if (mmc_gpio_alloc(host)) {
> > put_device(&host->class_dev);
> > --
> > 2.30.0
> >
next prev parent reply other threads:[~2021-06-21 1:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-20 4:57 [PATCH] mmc: core: Mark mmc_host device with pm_runtime_no_callbacks kehuanlin
2021-03-22 10:25 ` Ulf Hansson
2021-03-23 10:49 ` hieagle
2021-03-23 14:00 ` Ulf Hansson
2021-05-07 11:38 ` Ulf Hansson
2021-05-26 12:18 ` Ulf Hansson
2021-06-21 1:38 ` hieagle [this message]
2021-06-21 9:20 ` Ulf Hansson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMjpFAUTsei-PWLDdM+_78qXTo=QFdw-Wbgo9QA2Jnss+i4yRA@mail.gmail.com' \
--to=chgokhl@gmail.com \
--cc=chgokhl@163.com \
--cc=kehuanlin@fishsemi.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).