diff mbox

Revert "mmc: core: Remove redundant ->power_restore() callback"

Message ID 1459455931-19538-1-git-send-email-gwendal@chromium.org
State New
Headers show

Commit Message

Gwendal Grignou March 31, 2016, 8:25 p.m. UTC
This reverts commit 364549ddc29d ("mmc: core: Remove redundant
->power_restore() callback for MMC")
and commit 3056c49c35c1 ("mmc: core: Remove redundant
 ->power_restore() callback for SD")

->power_restore() is called by mmc_host_power_restore(). For MMC, We can
not use mmc_reset() instead, because the device may not have
RST_n_FUNCTION set up.
However we have to call mmc_init_card when we power back eMMC card,
and it is only possible through a call to ->power_restore().

For symmetry, also reinstantiate the function for SD.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/mmc/core/mmc.c | 14 +++++++++++++-
 drivers/mmc/core/sd.c  | 14 +++++++++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

Comments

Ulf Hansson April 1, 2016, 10:06 a.m. UTC | #1
On 31 March 2016 at 22:25, Gwendal Grignou <gwendal@chromium.org> wrote:
> This reverts commit 364549ddc29d ("mmc: core: Remove redundant
> ->power_restore() callback for MMC")
> and commit 3056c49c35c1 ("mmc: core: Remove redundant
>  ->power_restore() callback for SD")
>
> ->power_restore() is called by mmc_host_power_restore(). For MMC, We can
> not use mmc_reset() instead, because the device may not have
> RST_n_FUNCTION set up.

I don't want to restore the callbacks.

Instead, let's be a bit more clever and try to extend mmc_reset() to
cover the MMC case as well. I haven't thought about that in detail,
but I think it should work.

[...]

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gwendal Grignou April 1, 2016, 4:43 p.m. UTC | #2
On Fri, Apr 1, 2016 at 3:06 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>
> I don't want to restore the callbacks.
>
> Instead, let's be a bit more clever and try to extend mmc_reset() to
> cover the MMC case as well. I haven't thought about that in detail,
> but I think it should work.

I am posting a different path soon.
Also, given power_restore is gone, mmc_host_power_restore is going to
crash when called for a MMC or SD device.
Shall we remove the interface  mmc_power_save_host/mmc_power_restore_host?
It is only supported for sdio devices, and used only by the wl1271 to
power cycle the controller via sysfs. I am wondering if we could use
the suspend/resume entry point instead.

Gwendal.
>
>
> [...]
>
> Kind regards
> Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson April 4, 2016, 11:59 a.m. UTC | #3
On 1 April 2016 at 18:41, Gwendal Grignou <gwendal@google.com> wrote:
>
>
> On Fri, Apr 1, 2016 at 3:06 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>>
>> I don't want to restore the callbacks.
>>
>> Instead, let's be a bit more clever and try to extend mmc_reset() to
>> cover the MMC case as well. I haven't thought about that in detail,
>> but I think it should work.
>
> I am posting a different path soon.
> Also, given power_restore is gone, mmc_host_power_restore is going to crash
> when called for a MMC or SD device.
> Shall we remove the interface  mmc_power_save_host/mmc_power_restore_host?

It's in my long term plan, yes.

> It is only supported for sdio devices, and used only by the wl1271 to power
> cycle the controller via sysfs. I am wondering if we could use the
> suspend/resume entry point instead.

Regarding SDIO power management support in the mmc core, it's
fragile/broken. I have gotten error reports several times, unfortunate
it's not a trivial task to fix them but needs some structural changes
in the mmc core for SDIO.
I have plans to look into this, but I can't make any promises when that will be.

In your case I would start by extending mmc_reset() to serve MMC cards
as well, that seems like a fairly simple change. Then we can discuss
SDIO separately, if you are still interested. :-)

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index bf49e44..97fbe50 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1942,6 +1942,17 @@  static int mmc_runtime_resume(struct mmc_host *host)
 	return 0;
 }
 
+static int mmc_power_restore(struct mmc_host *host)
+{
+	int ret;
+
+	mmc_claim_host(host);
+	ret = mmc_init_card(host, host->card->ocr, host->card);
+	mmc_release_host(host);
+
+	return ret;
+}
+
 int mmc_can_reset(struct mmc_card *card)
 {
 	u8 rst_n_function;
@@ -1970,7 +1981,7 @@  static int mmc_reset(struct mmc_host *host)
 	/* Set initial state and call mmc_set_ios */
 	mmc_set_initial_state(host);
 
-	return mmc_init_card(host, card->ocr, card);
+	return mmc_power_restore(host);
 }
 
 static const struct mmc_bus_ops mmc_ops = {
@@ -1980,6 +1991,7 @@  static const struct mmc_bus_ops mmc_ops = {
 	.resume = mmc_resume,
 	.runtime_suspend = mmc_runtime_suspend,
 	.runtime_resume = mmc_runtime_resume,
+	.power_restore = mmc_power_restore,
 	.alive = mmc_alive,
 	.shutdown = mmc_shutdown,
 	.reset = mmc_reset,
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index bb39a29..c7ee77d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1177,10 +1177,21 @@  static int mmc_sd_runtime_resume(struct mmc_host *host)
 	return 0;
 }
 
+static int mmc_sd_power_restore(struct mmc_host *host)
+{
+	int ret;
+
+	mmc_claim_host(host);
+	ret = mmc_sd_init_card(host, host->card->ocr, host->card);
+	mmc_release_host(host);
+
+	return ret;
+}
+
 static int mmc_sd_reset(struct mmc_host *host)
 {
 	mmc_power_cycle(host, host->card->ocr);
-	return mmc_sd_init_card(host, host->card->ocr, host->card);
+	return mmc_sd_power_restore(host);
 }
 
 static const struct mmc_bus_ops mmc_sd_ops = {
@@ -1190,6 +1201,7 @@  static const struct mmc_bus_ops mmc_sd_ops = {
 	.runtime_resume = mmc_sd_runtime_resume,
 	.suspend = mmc_sd_suspend,
 	.resume = mmc_sd_resume,
+	.power_restore = mmc_sd_power_restore,
 	.alive = mmc_sd_alive,
 	.shutdown = mmc_sd_suspend,
 	.reset = mmc_sd_reset,