* [PATCH] mmc: mediatek: Add system suspend/resume interface
@ 2020-11-18 6:34 Wenbin Mei
2020-11-23 16:06 ` Ulf Hansson
0 siblings, 1 reply; 5+ messages in thread
From: Wenbin Mei @ 2020-11-18 6:34 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chaotian Jing, Matthias Brugger, linux-mmc, linux-arm-kernel,
linux-mediatek, linux-kernel, srv_heupstream, Wenbin Mei
Before we got these errors on MT8192 platform:
[ 59.153891] Restarting tasks ...
[ 59.154540] done.
[ 59.159175] PM: suspend exit
[ 59.218724] mtk-msdc 11f60000.mmc: phase: [map:fffffffe] [maxlen:31]
[final:16]
[ 119.776083] mmc0: cqhci: timeout for tag 9
[ 119.780196] mmc0: cqhci: ============ CQHCI REGISTER DUMP ===========
[ 119.786709] mmc0: cqhci: Caps: 0x100020b6 | Version: 0x00000510
[ 119.793225] mmc0: cqhci: Config: 0x00000101 | Control: 0x00000000
[ 119.799706] mmc0: cqhci: Int stat: 0x00000000 | Int enab: 0x00000000
[ 119.806177] mmc0: cqhci: Int sig: 0x00000000 | Int Coal: 0x00000000
[ 119.812670] mmc0: cqhci: TDL base: 0x00000000 | TDL up32: 0x00000000
[ 119.819149] mmc0: cqhci: Doorbell: 0x003ffc00 | TCN: 0x00000200
[ 119.825656] mmc0: cqhci: Dev queue: 0x00000000 | Dev Pend: 0x00000000
[ 119.832155] mmc0: cqhci: Task clr: 0x00000000 | SSC1: 0x00001000
[ 119.838627] mmc0: cqhci: SSC2: 0x00000000 | DCMD rsp: 0x00000000
[ 119.845174] mmc0: cqhci: RED mask: 0xfdf9a080 | TERRI: 0x0000891c
[ 119.851654] mmc0: cqhci: Resp idx: 0x00000000 | Resp arg: 0x00000000
[ 119.865773] mmc0: cqhci: : ===========================================
[ 119.872358] mmc0: running CQE recovery
From these logs, we found TDL base was back to the default value.
After suspend, the mmc host is powered off by HW, and bring CQE register
to the default value, so we add system suspend/resume interface, then bring
CQE to deactivated state before suspend, it will be enabled by CQE first
request after resume.
Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
---
drivers/mmc/host/mtk-sd.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index fc5ee5df91ad..c5f9cd6fc951 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -2758,11 +2758,29 @@ static int msdc_runtime_resume(struct device *dev)
msdc_restore_reg(host);
return 0;
}
+
+static int msdc_sys_suspend(struct device *dev)
+{
+ struct mmc_host *mmc = dev_get_drvdata(dev);
+ int ret;
+
+ if (mmc->caps2 & MMC_CAP2_CQE) {
+ ret = cqhci_suspend(mmc);
+ if (ret)
+ return ret;
+ }
+
+ return pm_runtime_force_suspend(dev);
+}
+
+static int msdc_sys_resume(struct device *dev)
+{
+ return pm_runtime_force_resume(dev);
+}
#endif
static const struct dev_pm_ops msdc_dev_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
- pm_runtime_force_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(msdc_sys_suspend, msdc_sys_resume)
SET_RUNTIME_PM_OPS(msdc_runtime_suspend, msdc_runtime_resume, NULL)
};
--
2.18.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: mediatek: Add system suspend/resume interface
2020-11-18 6:34 [PATCH] mmc: mediatek: Add system suspend/resume interface Wenbin Mei
@ 2020-11-23 16:06 ` Ulf Hansson
2020-11-25 1:10 ` Wenbin Mei
0 siblings, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2020-11-23 16:06 UTC (permalink / raw)
To: Wenbin Mei
Cc: Chaotian Jing, Matthias Brugger, linux-mmc, Linux ARM,
moderated list:ARM/Mediatek SoC support,
Linux Kernel Mailing List, srv_heupstream
On Wed, 18 Nov 2020 at 07:34, Wenbin Mei <wenbin.mei@mediatek.com> wrote:
>
> Before we got these errors on MT8192 platform:
> [ 59.153891] Restarting tasks ...
> [ 59.154540] done.
> [ 59.159175] PM: suspend exit
> [ 59.218724] mtk-msdc 11f60000.mmc: phase: [map:fffffffe] [maxlen:31]
> [final:16]
> [ 119.776083] mmc0: cqhci: timeout for tag 9
> [ 119.780196] mmc0: cqhci: ============ CQHCI REGISTER DUMP ===========
> [ 119.786709] mmc0: cqhci: Caps: 0x100020b6 | Version: 0x00000510
> [ 119.793225] mmc0: cqhci: Config: 0x00000101 | Control: 0x00000000
> [ 119.799706] mmc0: cqhci: Int stat: 0x00000000 | Int enab: 0x00000000
> [ 119.806177] mmc0: cqhci: Int sig: 0x00000000 | Int Coal: 0x00000000
> [ 119.812670] mmc0: cqhci: TDL base: 0x00000000 | TDL up32: 0x00000000
> [ 119.819149] mmc0: cqhci: Doorbell: 0x003ffc00 | TCN: 0x00000200
> [ 119.825656] mmc0: cqhci: Dev queue: 0x00000000 | Dev Pend: 0x00000000
> [ 119.832155] mmc0: cqhci: Task clr: 0x00000000 | SSC1: 0x00001000
> [ 119.838627] mmc0: cqhci: SSC2: 0x00000000 | DCMD rsp: 0x00000000
> [ 119.845174] mmc0: cqhci: RED mask: 0xfdf9a080 | TERRI: 0x0000891c
> [ 119.851654] mmc0: cqhci: Resp idx: 0x00000000 | Resp arg: 0x00000000
> [ 119.865773] mmc0: cqhci: : ===========================================
> [ 119.872358] mmc0: running CQE recovery
> From these logs, we found TDL base was back to the default value.
>
> After suspend, the mmc host is powered off by HW, and bring CQE register
> to the default value, so we add system suspend/resume interface, then bring
> CQE to deactivated state before suspend, it will be enabled by CQE first
> request after resume.
>
> Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
I took the liberty of renaming msdc_sys_suspend|resume to
msdc_suspend|resume, as I think the "_sys" is a bit superfluous.
Additionally, I added a fixes+stable tag, then I applied this for fixes, thanks!
Please tell me, if there is anything you would like me to change.
Kind regards
Uffe
> ---
> drivers/mmc/host/mtk-sd.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index fc5ee5df91ad..c5f9cd6fc951 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -2758,11 +2758,29 @@ static int msdc_runtime_resume(struct device *dev)
> msdc_restore_reg(host);
> return 0;
> }
> +
> +static int msdc_sys_suspend(struct device *dev)
> +{
> + struct mmc_host *mmc = dev_get_drvdata(dev);
> + int ret;
> +
> + if (mmc->caps2 & MMC_CAP2_CQE) {
> + ret = cqhci_suspend(mmc);
> + if (ret)
> + return ret;
> + }
> +
> + return pm_runtime_force_suspend(dev);
> +}
> +
> +static int msdc_sys_resume(struct device *dev)
> +{
> + return pm_runtime_force_resume(dev);
> +}
> #endif
>
> static const struct dev_pm_ops msdc_dev_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> - pm_runtime_force_resume)
> + SET_SYSTEM_SLEEP_PM_OPS(msdc_sys_suspend, msdc_sys_resume)
> SET_RUNTIME_PM_OPS(msdc_runtime_suspend, msdc_runtime_resume, NULL)
> };
>
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: mediatek: Add system suspend/resume interface
2020-11-23 16:06 ` Ulf Hansson
@ 2020-11-25 1:10 ` Wenbin Mei
2020-12-03 1:29 ` Nicolas Boichat
0 siblings, 1 reply; 5+ messages in thread
From: Wenbin Mei @ 2020-11-25 1:10 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chaotian Jing, Matthias Brugger, linux-mmc, Linux ARM,
moderated list:ARM/Mediatek SoC support,
Linux Kernel Mailing List, srv_heupstream
On Mon, 2020-11-23 at 17:06 +0100, Ulf Hansson wrote:
> On Wed, 18 Nov 2020 at 07:34, Wenbin Mei <wenbin.mei@mediatek.com> wrote:
> >
> > Before we got these errors on MT8192 platform:
> > [ 59.153891] Restarting tasks ...
> > [ 59.154540] done.
> > [ 59.159175] PM: suspend exit
> > [ 59.218724] mtk-msdc 11f60000.mmc: phase: [map:fffffffe] [maxlen:31]
> > [final:16]
> > [ 119.776083] mmc0: cqhci: timeout for tag 9
> > [ 119.780196] mmc0: cqhci: ============ CQHCI REGISTER DUMP ===========
> > [ 119.786709] mmc0: cqhci: Caps: 0x100020b6 | Version: 0x00000510
> > [ 119.793225] mmc0: cqhci: Config: 0x00000101 | Control: 0x00000000
> > [ 119.799706] mmc0: cqhci: Int stat: 0x00000000 | Int enab: 0x00000000
> > [ 119.806177] mmc0: cqhci: Int sig: 0x00000000 | Int Coal: 0x00000000
> > [ 119.812670] mmc0: cqhci: TDL base: 0x00000000 | TDL up32: 0x00000000
> > [ 119.819149] mmc0: cqhci: Doorbell: 0x003ffc00 | TCN: 0x00000200
> > [ 119.825656] mmc0: cqhci: Dev queue: 0x00000000 | Dev Pend: 0x00000000
> > [ 119.832155] mmc0: cqhci: Task clr: 0x00000000 | SSC1: 0x00001000
> > [ 119.838627] mmc0: cqhci: SSC2: 0x00000000 | DCMD rsp: 0x00000000
> > [ 119.845174] mmc0: cqhci: RED mask: 0xfdf9a080 | TERRI: 0x0000891c
> > [ 119.851654] mmc0: cqhci: Resp idx: 0x00000000 | Resp arg: 0x00000000
> > [ 119.865773] mmc0: cqhci: : ===========================================
> > [ 119.872358] mmc0: running CQE recovery
> > From these logs, we found TDL base was back to the default value.
> >
> > After suspend, the mmc host is powered off by HW, and bring CQE register
> > to the default value, so we add system suspend/resume interface, then bring
> > CQE to deactivated state before suspend, it will be enabled by CQE first
> > request after resume.
> >
> > Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
>
> I took the liberty of renaming msdc_sys_suspend|resume to
> msdc_suspend|resume, as I think the "_sys" is a bit superfluous.
>
> Additionally, I added a fixes+stable tag, then I applied this for fixes, thanks!
>
> Please tell me, if there is anything you would like me to change.
>
> Kind regards
> Uffe
>
> It is OK for me, thanks for your help.
> > ---
> > drivers/mmc/host/mtk-sd.c | 22 ++++++++++++++++++++--
> > 1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> > index fc5ee5df91ad..c5f9cd6fc951 100644
> > --- a/drivers/mmc/host/mtk-sd.c
> > +++ b/drivers/mmc/host/mtk-sd.c
> > @@ -2758,11 +2758,29 @@ static int msdc_runtime_resume(struct device *dev)
> > msdc_restore_reg(host);
> > return 0;
> > }
> > +
> > +static int msdc_sys_suspend(struct device *dev)
> > +{
> > + struct mmc_host *mmc = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + if (mmc->caps2 & MMC_CAP2_CQE) {
> > + ret = cqhci_suspend(mmc);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return pm_runtime_force_suspend(dev);
> > +}
> > +
> > +static int msdc_sys_resume(struct device *dev)
> > +{
> > + return pm_runtime_force_resume(dev);
> > +}
> > #endif
> >
> > static const struct dev_pm_ops msdc_dev_pm_ops = {
> > - SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > - pm_runtime_force_resume)
> > + SET_SYSTEM_SLEEP_PM_OPS(msdc_sys_suspend, msdc_sys_resume)
> > SET_RUNTIME_PM_OPS(msdc_runtime_suspend, msdc_runtime_resume, NULL)
> > };
> >
> > --
> > 2.18.0
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: mediatek: Add system suspend/resume interface
2020-11-25 1:10 ` Wenbin Mei
@ 2020-12-03 1:29 ` Nicolas Boichat
2020-12-04 15:07 ` Ulf Hansson
0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Boichat @ 2020-12-03 1:29 UTC (permalink / raw)
To: Wenbin Mei
Cc: Ulf Hansson, Chaotian Jing, Matthias Brugger, linux-mmc,
Linux ARM, moderated list:ARM/Mediatek SoC support,
Linux Kernel Mailing List, srv_heupstream, Guenter Roeck
This causes a 0-day warning (on our chromeos-5.4 backports but I don't
see why upstream would not be affected):
https://groups.google.com/g/cros-kernel-buildreports/c/MfS3SInT5jg/m/Hkzxh_U7AwAJ
Didn't look at the details of the config, but SET_SYSTEM_SLEEP_PM_OPS
is a noop if CONFIG_PM_SLEEP is not set (while SET_RUNTIME_PM_OPS is
noop-ed by CONFIG_PM).
So I guess msdc_suspend/msdc_resume should be guarded by
CONFIG_PM_SLEEP instead of CONFIG_PM.
On Wed, Nov 25, 2020 at 9:12 AM Wenbin Mei <wenbin.mei@mediatek.com> wrote:
>
> On Mon, 2020-11-23 at 17:06 +0100, Ulf Hansson wrote:
> > On Wed, 18 Nov 2020 at 07:34, Wenbin Mei <wenbin.mei@mediatek.com> wrote:
> > >
> > > Before we got these errors on MT8192 platform:
> > > [ 59.153891] Restarting tasks ...
> > > [ 59.154540] done.
> > > [ 59.159175] PM: suspend exit
> > > [ 59.218724] mtk-msdc 11f60000.mmc: phase: [map:fffffffe] [maxlen:31]
> > > [final:16]
> > > [ 119.776083] mmc0: cqhci: timeout for tag 9
> > > [ 119.780196] mmc0: cqhci: ============ CQHCI REGISTER DUMP ===========
> > > [ 119.786709] mmc0: cqhci: Caps: 0x100020b6 | Version: 0x00000510
> > > [ 119.793225] mmc0: cqhci: Config: 0x00000101 | Control: 0x00000000
> > > [ 119.799706] mmc0: cqhci: Int stat: 0x00000000 | Int enab: 0x00000000
> > > [ 119.806177] mmc0: cqhci: Int sig: 0x00000000 | Int Coal: 0x00000000
> > > [ 119.812670] mmc0: cqhci: TDL base: 0x00000000 | TDL up32: 0x00000000
> > > [ 119.819149] mmc0: cqhci: Doorbell: 0x003ffc00 | TCN: 0x00000200
> > > [ 119.825656] mmc0: cqhci: Dev queue: 0x00000000 | Dev Pend: 0x00000000
> > > [ 119.832155] mmc0: cqhci: Task clr: 0x00000000 | SSC1: 0x00001000
> > > [ 119.838627] mmc0: cqhci: SSC2: 0x00000000 | DCMD rsp: 0x00000000
> > > [ 119.845174] mmc0: cqhci: RED mask: 0xfdf9a080 | TERRI: 0x0000891c
> > > [ 119.851654] mmc0: cqhci: Resp idx: 0x00000000 | Resp arg: 0x00000000
> > > [ 119.865773] mmc0: cqhci: : ===========================================
> > > [ 119.872358] mmc0: running CQE recovery
> > > From these logs, we found TDL base was back to the default value.
> > >
> > > After suspend, the mmc host is powered off by HW, and bring CQE register
> > > to the default value, so we add system suspend/resume interface, then bring
> > > CQE to deactivated state before suspend, it will be enabled by CQE first
> > > request after resume.
> > >
> > > Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
> >
> > I took the liberty of renaming msdc_sys_suspend|resume to
> > msdc_suspend|resume, as I think the "_sys" is a bit superfluous.
> >
> > Additionally, I added a fixes+stable tag, then I applied this for fixes, thanks!
> >
> > Please tell me, if there is anything you would like me to change.
> >
> > Kind regards
> > Uffe
> >
> > It is OK for me, thanks for your help.
>
> > > ---
> > > drivers/mmc/host/mtk-sd.c | 22 ++++++++++++++++++++--
> > > 1 file changed, 20 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> > > index fc5ee5df91ad..c5f9cd6fc951 100644
> > > --- a/drivers/mmc/host/mtk-sd.c
> > > +++ b/drivers/mmc/host/mtk-sd.c
> > > @@ -2758,11 +2758,29 @@ static int msdc_runtime_resume(struct device *dev)
> > > msdc_restore_reg(host);
> > > return 0;
> > > }
> > > +
> > > +static int msdc_sys_suspend(struct device *dev)
> > > +{
> > > + struct mmc_host *mmc = dev_get_drvdata(dev);
> > > + int ret;
> > > +
> > > + if (mmc->caps2 & MMC_CAP2_CQE) {
> > > + ret = cqhci_suspend(mmc);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return pm_runtime_force_suspend(dev);
> > > +}
> > > +
> > > +static int msdc_sys_resume(struct device *dev)
> > > +{
> > > + return pm_runtime_force_resume(dev);
> > > +}
> > > #endif
> > >
> > > static const struct dev_pm_ops msdc_dev_pm_ops = {
> > > - SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > > - pm_runtime_force_resume)
> > > + SET_SYSTEM_SLEEP_PM_OPS(msdc_sys_suspend, msdc_sys_resume)
> > > SET_RUNTIME_PM_OPS(msdc_runtime_suspend, msdc_runtime_resume, NULL)
> > > };
> > >
> > > --
> > > 2.18.0
> > >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: mediatek: Add system suspend/resume interface
2020-12-03 1:29 ` Nicolas Boichat
@ 2020-12-04 15:07 ` Ulf Hansson
0 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2020-12-04 15:07 UTC (permalink / raw)
To: Nicolas Boichat
Cc: Wenbin Mei, Chaotian Jing, Matthias Brugger, linux-mmc,
Linux ARM, moderated list:ARM/Mediatek SoC support,
Linux Kernel Mailing List, srv_heupstream, Guenter Roeck
On Thu, 3 Dec 2020 at 02:29, Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> This causes a 0-day warning (on our chromeos-5.4 backports but I don't
> see why upstream would not be affected):
> https://groups.google.com/g/cros-kernel-buildreports/c/MfS3SInT5jg/m/Hkzxh_U7AwAJ
>
> Didn't look at the details of the config, but SET_SYSTEM_SLEEP_PM_OPS
> is a noop if CONFIG_PM_SLEEP is not set (while SET_RUNTIME_PM_OPS is
> noop-ed by CONFIG_PM).
>
> So I guess msdc_suspend/msdc_resume should be guarded by
> CONFIG_PM_SLEEP instead of CONFIG_PM.
Yep.
There is already a patch [1] in my queue for this. It uses the
_maybe_unused approach (Arnd prefers that and I don't mind).
[...]
Kind regards
Uffe
[1]
https://patchwork.kernel.org/project/linux-mediatek/patch/20201203222922.1067522-1-arnd@kernel.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-04 15:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-18 6:34 [PATCH] mmc: mediatek: Add system suspend/resume interface Wenbin Mei
2020-11-23 16:06 ` Ulf Hansson
2020-11-25 1:10 ` Wenbin Mei
2020-12-03 1:29 ` Nicolas Boichat
2020-12-04 15:07 ` Ulf Hansson
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).