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@xxxxxxxxxx> 于2021年3月22日周一 下午6:26写道：
>
> On Sat, 20 Mar 2021 at 05:57, kehuanlin <chgokhl@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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
> >
