2020-06-26 11:37:34

by Yoshihiro Shimoda

[permalink] [raw]
Subject: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

If regulator_is_enabled() of both vmmc and vqmmc returns false,
_mmc_suspend() should call mmc_poweroff_nofity() instead of
mmc_sleep().

Note that this is possible to happen when the regulator-fixed driver
turns the vmmc and vqmmc off by firmware like PSCI while the system
is suspended.

Signed-off-by: Yoshihiro Shimoda <[email protected]>
---
drivers/mmc/core/mmc.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 4203303..75df5f8 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <linux/stat.h>
#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>

#include <linux/mmc/host.h>
#include <linux/mmc/card.h>
@@ -2022,6 +2023,18 @@ static void mmc_detect(struct mmc_host *host)
}
}

+static bool mmc_regulators_are_disabled(struct mmc_host *host)
+{
+ if (IS_ERR(host->supply.vmmc) ||
+ regulator_is_enabled(host->supply.vmmc))
+ return false;
+ if (IS_ERR(host->supply.vqmmc) ||
+ regulator_is_enabled(host->supply.vqmmc))
+ return false;
+
+ return true;
+}
+
static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
{
int err = 0;
@@ -2038,7 +2051,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
goto out;

if (mmc_can_poweroff_notify(host->card) &&
- ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
+ ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
+ mmc_regulators_are_disabled(host)))
err = mmc_poweroff_notify(host->card, notify_type);
else if (mmc_can_sleep(host->card))
err = mmc_sleep(host);
--
2.7.4


2020-06-26 16:47:34

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

On Fri, Jun 26, 2020 at 06:32:21PM +0900, Yoshihiro Shimoda wrote:

> If regulator_is_enabled() of both vmmc and vqmmc returns false,
> _mmc_suspend() should call mmc_poweroff_nofity() instead of
> mmc_sleep().

This is possibly something it makes sense to do, if the power did
suddenly vanish on the device then using a separate cleanup path for
that seems sensible (it's probably something worth complaining loudly
about). Registering for notification on power loss might also be
sensible.

> Note that this is possible to happen when the regulator-fixed driver
> turns the vmmc and vqmmc off by firmware like PSCI while the system
> is suspended.

This is not a good interface, if there's a need to query the state over
suspend then we should query the state over suspend rather than trying
to somehow shoehorn it via the runtime enable state which is going to
break any other users and relies on the regulator driver doing dodgy
stuff representing the enable state.


Attachments:
(No filename) (992.00 B)
signature.asc (499.00 B)
Download all attachments

2020-06-26 17:11:50

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

Hello!

On 06/26/2020 12:32 PM, Yoshihiro Shimoda wrote:

> If regulator_is_enabled() of both vmmc and vqmmc returns false,
> _mmc_suspend() should call mmc_poweroff_nofity() instead of
^^^^^^
That hard word again. :-)

> mmc_sleep().
>
> Note that this is possible to happen when the regulator-fixed driver
> turns the vmmc and vqmmc off by firmware like PSCI while the system
> is suspended.
>
> Signed-off-by: Yoshihiro Shimoda <[email protected]>
[...]

MBR, Sergei

2020-06-29 02:50:01

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

Hi Mark,

> From: Mark Brown, Sent: Saturday, June 27, 2020 12:14 AM
>
> On Fri, Jun 26, 2020 at 06:32:21PM +0900, Yoshihiro Shimoda wrote:
> > Note that this is possible to happen when the regulator-fixed driver
> > turns the vmmc and vqmmc off by firmware like PSCI while the system
> > is suspended.
>
> This is not a good interface, if there's a need to query the state over
> suspend then we should query the state over suspend rather than trying
> to somehow shoehorn it via the runtime enable state which is going to
> break any other users and relies on the regulator driver doing dodgy
> stuff representing the enable state.

I understood it.
So, as I mentioned other email thread, I'm thinking adding a new property
into MMC is better.

Best regards,
Yoshihiro Shimoda

2020-06-29 20:41:32

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled

Hello!

> From: Sergei Shtylyov, Sent: Saturday, June 27, 2020 12:54 AM
>
> Hello!
>
> On 06/26/2020 12:32 PM, Yoshihiro Shimoda wrote:
>
> > If regulator_is_enabled() of both vmmc and vqmmc returns false,
> > _mmc_suspend() should call mmc_poweroff_nofity() instead of
> ^^^^^^
> That hard word again. :-)

Oops! Thank you for pointed it out!
# Also, the subject was incorrect...

Best regards,
Yoshihiro Shimoda