* [PATCH] mmc: core: in mmc_hw_reset(), allow power cycle.
@ 2016-04-01 16:46 Gwendal Grignou
2016-04-01 17:25 ` [PATCH v2] " Gwendal Grignou
0 siblings, 1 reply; 6+ messages in thread
From: Gwendal Grignou @ 2016-04-01 16:46 UTC (permalink / raw)
To: ulf.hansson; +Cc: linux-mmc
When the eMMC device does not support/allow sending RST_n signal,
do a brute force power cycle instead of returning EOPNOTSUPP.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
drivers/mmc/core/core.c | 5 +++--
drivers/mmc/core/mmc.c | 18 ++++++++----------
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d8bbc78..d5bc2d8 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2468,8 +2468,9 @@ int mmc_hw_reset(struct mmc_host *host)
ret = host->bus_ops->reset(host);
mmc_bus_put(host);
- if (ret != -EOPNOTSUPP)
- pr_warn("%s: tried to reset card\n", mmc_hostname(host));
+ if (ret)
+ pr_warn("%s: tried to reset card, got error %d\n",
+ mmc_hostname(host), ret);
return ret;
}
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 96cc7e2..deb8c16 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1988,19 +1988,17 @@ static int mmc_reset(struct mmc_host *host)
{
struct mmc_card *card = host->card;
- if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
- return -EOPNOTSUPP;
-
- if (!mmc_can_reset(card))
- return -EOPNOTSUPP;
-
mmc_set_clock(host, host->f_init);
-
- host->ops->hw_reset(host);
-
+ if ((host->caps & MMC_CAP_HW_RESET) && host->ops->hw_reset &&
+ mmc_can_reset(card)) {
+ /* If the card accept RST_n signal, send it. */
+ host->ops->hw_reset(host);
+ } else {
+ /* Do a brute force power cycle */
+ mmc_power_cycle(host, host->card->ocr);
+ }
/* Set initial state and call mmc_set_ios */
mmc_set_initial_state(host);
-
return mmc_init_card(host, card->ocr, card);
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] mmc: core: in mmc_hw_reset(), allow power cycle.
2016-04-01 16:46 [PATCH] mmc: core: in mmc_hw_reset(), allow power cycle Gwendal Grignou
@ 2016-04-01 17:25 ` Gwendal Grignou
2016-04-01 23:04 ` [PATCH v3] " Gwendal Grignou
0 siblings, 1 reply; 6+ messages in thread
From: Gwendal Grignou @ 2016-04-01 17:25 UTC (permalink / raw)
To: ulf.hansson; +Cc: linux-mmc
When the eMMC device does not support/allow sending RST_n signal,
do a brute force power cycle instead of returning EOPNOTSUPP.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes in v2:
- Fix indentation.
- Use card instead of host->card when available.
drivers/mmc/core/core.c | 5 +++--
drivers/mmc/core/mmc.c | 18 ++++++++----------
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d8bbc78..d5bc2d8 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2468,8 +2468,9 @@ int mmc_hw_reset(struct mmc_host *host)
ret = host->bus_ops->reset(host);
mmc_bus_put(host);
- if (ret != -EOPNOTSUPP)
- pr_warn("%s: tried to reset card\n", mmc_hostname(host));
+ if (ret)
+ pr_warn("%s: tried to reset card, got error %d\n",
+ mmc_hostname(host), ret);
return ret;
}
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 96cc7e2..deb8c16 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1988,19 +1988,17 @@ static int mmc_reset(struct mmc_host *host)
{
struct mmc_card *card = host->card;
- if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
- return -EOPNOTSUPP;
-
- if (!mmc_can_reset(card))
- return -EOPNOTSUPP;
-
mmc_set_clock(host, host->f_init);
-
- host->ops->hw_reset(host);
-
+ if ((host->caps & MMC_CAP_HW_RESET) && host->ops->hw_reset &&
+ mmc_can_reset(card)) {
+ /* If the card accept RST_n signal, send it. */
+ host->ops->hw_reset(host);
+ } else {
+ /* Do a brute force power cycle */
+ mmc_power_cycle(host, card->ocr);
+ }
/* Set initial state and call mmc_set_ios */
mmc_set_initial_state(host);
-
return mmc_init_card(host, card->ocr, card);
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3] mmc: core: in mmc_hw_reset(), allow power cycle.
2016-04-01 17:25 ` [PATCH v2] " Gwendal Grignou
@ 2016-04-01 23:04 ` Gwendal Grignou
2016-04-04 12:39 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Gwendal Grignou @ 2016-04-01 23:04 UTC (permalink / raw)
To: ulf.hansson; +Cc: linux-mmc
When the eMMC device does not support/allow sending RST_n signal,
try to do a brute force power cycle instead of returning EOPNOTSUPP.
If power cycle is not supported by the hardware, mmc_init_card still
send a CMD0, performing a Software Reset.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes in v3:
- clock setting in already done in power_cycle.
- clarify behavior when power cycle is not implemented. .
drivers/mmc/core/core.c | 5 +++--
drivers/mmc/core/mmc.c | 24 +++++++++++-------------
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d8bbc78..d5bc2d8 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2468,8 +2468,9 @@ int mmc_hw_reset(struct mmc_host *host)
ret = host->bus_ops->reset(host);
mmc_bus_put(host);
- if (ret != -EOPNOTSUPP)
- pr_warn("%s: tried to reset card\n", mmc_hostname(host));
+ if (ret)
+ pr_warn("%s: tried to reset card, got error %d\n",
+ mmc_hostname(host), ret);
return ret;
}
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 96cc7e2..92b7bea 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1988,19 +1988,17 @@ static int mmc_reset(struct mmc_host *host)
{
struct mmc_card *card = host->card;
- if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
- return -EOPNOTSUPP;
-
- if (!mmc_can_reset(card))
- return -EOPNOTSUPP;
-
- mmc_set_clock(host, host->f_init);
-
- host->ops->hw_reset(host);
-
- /* Set initial state and call mmc_set_ios */
- mmc_set_initial_state(host);
-
+ if ((host->caps & MMC_CAP_HW_RESET) && host->ops->hw_reset &&
+ mmc_can_reset(card)) {
+ /* If the card accept RST_n signal, send it. */
+ mmc_set_clock(host, host->f_init);
+ host->ops->hw_reset(host);
+ /* Set initial state and call mmc_set_ios */
+ mmc_set_initial_state(host);
+ } else {
+ /* Do a brute force power cycle */
+ mmc_power_cycle(host, card->ocr);
+ }
return mmc_init_card(host, card->ocr, card);
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] mmc: core: in mmc_hw_reset(), allow power cycle.
2016-04-01 23:04 ` [PATCH v3] " Gwendal Grignou
@ 2016-04-04 12:39 ` Ulf Hansson
2016-04-07 18:14 ` [PATCH v4] " Gwendal Grignou
2016-04-11 13:33 ` [PATCH v3] " Ulf Hansson
0 siblings, 2 replies; 6+ messages in thread
From: Ulf Hansson @ 2016-04-04 12:39 UTC (permalink / raw)
To: Gwendal Grignou; +Cc: linux-mmc
On 2 April 2016 at 01:04, Gwendal Grignou <gwendal@chromium.org> wrote:
> When the eMMC device does not support/allow sending RST_n signal,
> try to do a brute force power cycle instead of returning EOPNOTSUPP.
> If power cycle is not supported by the hardware, mmc_init_card still
> send a CMD0, performing a Software Reset.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes in v3:
> - clock setting in already done in power_cycle.
> - clarify behavior when power cycle is not implemented. .
> drivers/mmc/core/core.c | 5 +++--
> drivers/mmc/core/mmc.c | 24 +++++++++++-------------
> 2 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index d8bbc78..d5bc2d8 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2468,8 +2468,9 @@ int mmc_hw_reset(struct mmc_host *host)
> ret = host->bus_ops->reset(host);
> mmc_bus_put(host);
>
> - if (ret != -EOPNOTSUPP)
> - pr_warn("%s: tried to reset card\n", mmc_hostname(host));
> + if (ret)
> + pr_warn("%s: tried to reset card, got error %d\n",
> + mmc_hostname(host), ret);
>
> return ret;
> }
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 96cc7e2..92b7bea 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1988,19 +1988,17 @@ static int mmc_reset(struct mmc_host *host)
> {
> struct mmc_card *card = host->card;
>
> - if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
> - return -EOPNOTSUPP;
> -
> - if (!mmc_can_reset(card))
> - return -EOPNOTSUPP;
> -
> - mmc_set_clock(host, host->f_init);
> -
> - host->ops->hw_reset(host);
> -
> - /* Set initial state and call mmc_set_ios */
> - mmc_set_initial_state(host);
> -
> + if ((host->caps & MMC_CAP_HW_RESET) && host->ops->hw_reset &&
> + mmc_can_reset(card)) {
> + /* If the card accept RST_n signal, send it. */
> + mmc_set_clock(host, host->f_init);
> + host->ops->hw_reset(host);
> + /* Set initial state and call mmc_set_ios */
> + mmc_set_initial_state(host);
> + } else {
> + /* Do a brute force power cycle */
> + mmc_power_cycle(host, card->ocr);
> + }
> return mmc_init_card(host, card->ocr, card);
> }
>
> --
> 2.8.0.rc3.226.g39d4020
>
There are more two callers triggering the host->ops->hw_reset() to be
called, which also have special treatment of the error code
-EOPNOTSUPP. These are mmc_hw_reset() and mmc_blk_reset().
Please make sure to update their error handling accordingly. Otherwise
this looks okay to me.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4] mmc: core: in mmc_hw_reset(), allow power cycle.
2016-04-04 12:39 ` Ulf Hansson
@ 2016-04-07 18:14 ` Gwendal Grignou
2016-04-11 13:33 ` [PATCH v3] " Ulf Hansson
1 sibling, 0 replies; 6+ messages in thread
From: Gwendal Grignou @ 2016-04-07 18:14 UTC (permalink / raw)
To: ulf.hansson; +Cc: linux-mmc
When the eMMC device does not support/allow sending RST_n signal,
try to do a brute force power cycle instead of returning EOPNOTSUPP.
If power cycle is not supported by the hardware, mmc_init_card still
send a CMD0, performing a Software Reset.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes in v4:
- Integrate Ulf's comment: update mmc_blk_reset()
drivers/mmc/card/block.c | 4 ++--
drivers/mmc/core/core.c | 5 +++--
drivers/mmc/core/mmc.c | 24 +++++++++++-------------
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index a4bf0f7..44e220a 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1141,7 +1141,7 @@ static int mmc_blk_reset(struct mmc_blk_data *md, struct mmc_host *host,
md->reset_done |= type;
err = mmc_hw_reset(host);
/* Ensure we switch back to the correct partition */
- if (err != -EOPNOTSUPP) {
+ if (!err) {
struct mmc_blk_data *main_md =
dev_get_drvdata(&host->card->dev);
int part_err;
@@ -1153,7 +1153,7 @@ static int mmc_blk_reset(struct mmc_blk_data *md, struct mmc_host *host,
* We have failed to get back into the correct
* partition, so we need to abort the whole request.
*/
- return -ENODEV;
+ err = -ENODEV;
}
}
return err;
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d8bbc78..d5bc2d8 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2468,8 +2468,9 @@ int mmc_hw_reset(struct mmc_host *host)
ret = host->bus_ops->reset(host);
mmc_bus_put(host);
- if (ret != -EOPNOTSUPP)
- pr_warn("%s: tried to reset card\n", mmc_hostname(host));
+ if (ret)
+ pr_warn("%s: tried to reset card, got error %d\n",
+ mmc_hostname(host), ret);
return ret;
}
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 96cc7e2..92b7bea 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1988,19 +1988,17 @@ static int mmc_reset(struct mmc_host *host)
{
struct mmc_card *card = host->card;
- if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
- return -EOPNOTSUPP;
-
- if (!mmc_can_reset(card))
- return -EOPNOTSUPP;
-
- mmc_set_clock(host, host->f_init);
-
- host->ops->hw_reset(host);
-
- /* Set initial state and call mmc_set_ios */
- mmc_set_initial_state(host);
-
+ if ((host->caps & MMC_CAP_HW_RESET) && host->ops->hw_reset &&
+ mmc_can_reset(card)) {
+ /* If the card accept RST_n signal, send it. */
+ mmc_set_clock(host, host->f_init);
+ host->ops->hw_reset(host);
+ /* Set initial state and call mmc_set_ios */
+ mmc_set_initial_state(host);
+ } else {
+ /* Do a brute force power cycle */
+ mmc_power_cycle(host, card->ocr);
+ }
return mmc_init_card(host, card->ocr, card);
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] mmc: core: in mmc_hw_reset(), allow power cycle.
2016-04-04 12:39 ` Ulf Hansson
2016-04-07 18:14 ` [PATCH v4] " Gwendal Grignou
@ 2016-04-11 13:33 ` Ulf Hansson
1 sibling, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2016-04-11 13:33 UTC (permalink / raw)
To: Gwendal Grignou; +Cc: linux-mmc
On 4 April 2016 at 14:39, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 2 April 2016 at 01:04, Gwendal Grignou <gwendal@chromium.org> wrote:
>> When the eMMC device does not support/allow sending RST_n signal,
>> try to do a brute force power cycle instead of returning EOPNOTSUPP.
>> If power cycle is not supported by the hardware, mmc_init_card still
>> send a CMD0, performing a Software Reset.
>>
>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>> ---
>> Changes in v3:
>> - clock setting in already done in power_cycle.
>> - clarify behavior when power cycle is not implemented. .
>> drivers/mmc/core/core.c | 5 +++--
>> drivers/mmc/core/mmc.c | 24 +++++++++++-------------
>> 2 files changed, 14 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index d8bbc78..d5bc2d8 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -2468,8 +2468,9 @@ int mmc_hw_reset(struct mmc_host *host)
>> ret = host->bus_ops->reset(host);
>> mmc_bus_put(host);
>>
>> - if (ret != -EOPNOTSUPP)
>> - pr_warn("%s: tried to reset card\n", mmc_hostname(host));
>> + if (ret)
>> + pr_warn("%s: tried to reset card, got error %d\n",
>> + mmc_hostname(host), ret);
>>
>> return ret;
>> }
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 96cc7e2..92b7bea 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1988,19 +1988,17 @@ static int mmc_reset(struct mmc_host *host)
>> {
>> struct mmc_card *card = host->card;
>>
>> - if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
>> - return -EOPNOTSUPP;
>> -
>> - if (!mmc_can_reset(card))
>> - return -EOPNOTSUPP;
>> -
>> - mmc_set_clock(host, host->f_init);
>> -
>> - host->ops->hw_reset(host);
>> -
>> - /* Set initial state and call mmc_set_ios */
>> - mmc_set_initial_state(host);
>> -
>> + if ((host->caps & MMC_CAP_HW_RESET) && host->ops->hw_reset &&
>> + mmc_can_reset(card)) {
>> + /* If the card accept RST_n signal, send it. */
>> + mmc_set_clock(host, host->f_init);
>> + host->ops->hw_reset(host);
>> + /* Set initial state and call mmc_set_ios */
>> + mmc_set_initial_state(host);
>> + } else {
>> + /* Do a brute force power cycle */
>> + mmc_power_cycle(host, card->ocr);
>> + }
>> return mmc_init_card(host, card->ocr, card);
>> }
>>
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>
> There are more two callers triggering the host->ops->hw_reset() to be
> called, which also have special treatment of the error code
> -EOPNOTSUPP. These are mmc_hw_reset() and mmc_blk_reset().
>
> Please make sure to update their error handling accordingly. Otherwise
> this looks okay to me.
After some more investigations, I realized that the error path in the
mmc block layer needed some additional cleanups.
Therefore I have decided to apply the v3 version of $subject patch for
next, then I am going to post some cleanups changes on top.
I also took the liberty to update the changelog as I think it needed
to be extended with some more information.
Thanks and kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-11 13:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-01 16:46 [PATCH] mmc: core: in mmc_hw_reset(), allow power cycle Gwendal Grignou
2016-04-01 17:25 ` [PATCH v2] " Gwendal Grignou
2016-04-01 23:04 ` [PATCH v3] " Gwendal Grignou
2016-04-04 12:39 ` Ulf Hansson
2016-04-07 18:14 ` [PATCH v4] " Gwendal Grignou
2016-04-11 13:33 ` [PATCH v3] " Ulf Hansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.