[PATCH v7 0/3] mmc: core: hw_reset changes

[PATCH v7 0/3] mmc: core: hw_reset changes

[Johan Rudholm]

Make the mmc_hw_reset routine more generic, by putting the (e)MMC-
specific stuff in the new bus_ops->reset in mmc.c. Add a
bus_ops->reset for SD cards, allowing them to be restarted when
errors occur.

For MMC cards, always check if the reset was sucessful. This allows
us to remove the mmc_hw_reset_check() interface.

As I don't have an eMMC device myself, much less one with a reset line,
I'd be very happy if someone could help me test the code with an eMMC?

v7:
 - Rename bus_ops->hw_reset to reset
 - Move the send_status check to mmc.c
 - Put the changes concerning mmc_hw_reset_check in a separate patch

v6:
 - Always perform the mmc_send_status reset check, which allows us to
   have only one interface to the card reset functions
 - Because of this, add the bus_ops->hw_reset to sd.c instead of
   falling back to a power_cycle

v5:
 - Move the mmc_test-specific code to mmc_test.c
 - Fall back to a power_cycle if the bus_ops->hw_reset is missing
 - Because of this, skip the bus_ops->hw_reset in sd.c

v4:
 - Rebase onto next

v3:
 - Keep mmc_can_reset
 - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state()

v2:
 - Call the new bus_ops member hw_reset instead of power_reset
 - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset
   instead of mmc_power_up
 - Keep "mmc_hw_reset" naming

Johan Rudholm (3):
  mmc: core: always check status after reset
  mmc: core: refactor the hw_reset routines
  mmc: sd: add reset bus_ops callback

 drivers/mmc/card/mmc_test.c |   18 +++++--------
 drivers/mmc/core/core.c     |   59 +++++++-----------------------------------
 drivers/mmc/core/core.h     |    1 +
 drivers/mmc/core/mmc.c      |   41 +++++++++++++++++++++++++++++
 drivers/mmc/core/sd.c       |    7 +++++
 include/linux/mmc/core.h    |    1 -
 6 files changed, 66 insertions(+), 61 deletions(-)

-- 
1.7.2.5

[PATCH v7 1/3] mmc: core: always check status after reset

[Johan Rudholm]

Always check if the card is alive after a successful reset. This allows
us to remove mmc_hw_reset_check(), leaving mmc_hw_reset() as the only
card reset interface.

Signed-off-by: Johan Rudholm <johanru@axis.com>
---
 drivers/mmc/card/mmc_test.c |   18 +++++++-----------
 drivers/mmc/core/core.c     |   24 +++++-------------------
 include/linux/mmc/core.h    |    1 -
 3 files changed, 12 insertions(+), 31 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 0a7430f..7dac469 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -2342,20 +2342,16 @@ static int mmc_test_hw_reset(struct mmc_test_card *test)
 	struct mmc_host *host = card->host;
 	int err;
 
-	err = mmc_hw_reset_check(host);
+	if (!mmc_card_mmc(card) || !mmc_can_reset(card))
+		return RESULT_UNSUP_CARD;
+
+	err = mmc_hw_reset(host);
 	if (!err)
 		return RESULT_OK;
+	else if (err == -EOPNOTSUPP)
+		return RESULT_UNSUP_HOST;
 
-	if (err == -ENOSYS)
-		return RESULT_FAIL;
-
-	if (err != -EOPNOTSUPP)
-		return err;
-
-	if (!mmc_can_reset(card))
-		return RESULT_UNSUP_CARD;
-
-	return RESULT_UNSUP_HOST;
+	return RESULT_FAIL;
 }
 
 static const struct mmc_test_case mmc_test_cases[] = {
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d3bfbdf..9959997e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2286,9 +2286,10 @@ int mmc_can_reset(struct mmc_card *card)
 }
 EXPORT_SYMBOL(mmc_can_reset);
 
-static int mmc_do_hw_reset(struct mmc_host *host, int check)
+static int mmc_hw_reset(struct mmc_host *host)
 {
 	struct mmc_card *card = host->card;
+	u32 status;
 
 	if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
 		return -EOPNOTSUPP;
@@ -2305,13 +2306,9 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check)
 	host->ops->hw_reset(host);
 
 	/* If the reset has happened, then a status command will fail */
-	if (check) {
-		u32 status;
-
-		if (!mmc_send_status(card, &status)) {
-			mmc_host_clk_release(host);
-			return -ENOSYS;
-		}
+	if (!mmc_send_status(card, &status)) {
+		mmc_host_clk_release(host);
+		return -ENOSYS;
 	}
 
 	/* Set initial state and call mmc_set_ios */
@@ -2321,19 +2318,8 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check)
 
 	return host->bus_ops->power_restore(host);
 }
-
-int mmc_hw_reset(struct mmc_host *host)
-{
-	return mmc_do_hw_reset(host, 0);
-}
 EXPORT_SYMBOL(mmc_hw_reset);
 
-int mmc_hw_reset_check(struct mmc_host *host)
-{
-	return mmc_do_hw_reset(host, 1);
-}
-EXPORT_SYMBOL(mmc_hw_reset_check);
-
 static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
 {
 	host->f_init = freq;
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index cb2b040..160448f 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -182,7 +182,6 @@ extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
 extern int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
 			      bool is_rel_write);
 extern int mmc_hw_reset(struct mmc_host *host);
-extern int mmc_hw_reset_check(struct mmc_host *host);
 extern int mmc_can_reset(struct mmc_card *card);
 
 extern void mmc_set_data_timeout(struct mmc_data *, const struct mmc_card *);
-- 
1.7.2.5

[PATCH v7 2/3] mmc: core: refactor the hw_reset routines

[Johan Rudholm]

Move the (e)MMC specific hw_reset code from core.c into mmc.c. Call the
code from the new bus_ops member "reset". This also allows for adding
a SD card specific reset procedure.

Signed-off-by: Johan Rudholm <johanru@axis.com>
---
 drivers/mmc/core/core.c |   45 ++++++++++-----------------------------------
 drivers/mmc/core/core.h |    1 +
 drivers/mmc/core/mmc.c  |   41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 9959997e..2cdb06e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2273,50 +2273,25 @@ static void mmc_hw_reset_for_init(struct mmc_host *host)
 	mmc_host_clk_release(host);
 }
 
-int mmc_can_reset(struct mmc_card *card)
+int mmc_hw_reset(struct mmc_host *host)
 {
-	u8 rst_n_function;
-
-	if (!mmc_card_mmc(card))
-		return 0;
-	rst_n_function = card->ext_csd.rst_n_function;
-	if ((rst_n_function & EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED)
-		return 0;
-	return 1;
-}
-EXPORT_SYMBOL(mmc_can_reset);
-
-static int mmc_hw_reset(struct mmc_host *host)
-{
-	struct mmc_card *card = host->card;
-	u32 status;
-
-	if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
-		return -EOPNOTSUPP;
+	int ret;
 
-	if (!card)
+	if (!host->card)
 		return -EINVAL;
 
-	if (!mmc_can_reset(card))
+	mmc_bus_get(host);
+	if (!host->bus_ops || host->bus_dead || !host->bus_ops->reset) {
+		mmc_bus_put(host);
 		return -EOPNOTSUPP;
-
-	mmc_host_clk_hold(host);
-	mmc_set_clock(host, host->f_init);
-
-	host->ops->hw_reset(host);
-
-	/* If the reset has happened, then a status command will fail */
-	if (!mmc_send_status(card, &status)) {
-		mmc_host_clk_release(host);
-		return -ENOSYS;
 	}
 
-	/* Set initial state and call mmc_set_ios */
-	mmc_set_initial_state(host);
+	ret = host->bus_ops->reset(host);
+	mmc_bus_put(host);
 
-	mmc_host_clk_release(host);
+	pr_warn("%s: tried to reset card\n", mmc_hostname(host));
 
-	return host->bus_ops->power_restore(host);
+	return ret;
 }
 EXPORT_SYMBOL(mmc_hw_reset);
 
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index b528c0e..a0bccbc 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -27,6 +27,7 @@ struct mmc_bus_ops {
 	int (*power_restore)(struct mmc_host *);
 	int (*alive)(struct mmc_host *);
 	int (*shutdown)(struct mmc_host *);
+	int (*reset)(struct mmc_host *);
 };
 
 void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 0b8ec87..d5d576a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1795,6 +1795,46 @@ static int mmc_power_restore(struct mmc_host *host)
 	return ret;
 }
 
+int mmc_can_reset(struct mmc_card *card)
+{
+	u8 rst_n_function;
+
+	rst_n_function = card->ext_csd.rst_n_function;
+	if ((rst_n_function & EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED)
+		return 0;
+	return 1;
+}
+EXPORT_SYMBOL(mmc_can_reset);
+
+static int mmc_reset(struct mmc_host *host)
+{
+	struct mmc_card *card = host->card;
+	u32 status;
+
+	if (!(host->caps & MMC_CAP_HW_RESET) || !host->ops->hw_reset)
+		return -EOPNOTSUPP;
+
+	if (!mmc_can_reset(card))
+		return -EOPNOTSUPP;
+
+	mmc_host_clk_hold(host);
+	mmc_set_clock(host, host->f_init);
+
+	host->ops->hw_reset(host);
+
+	/* If the reset has happened, then a status command will fail */
+	if (!mmc_send_status(card, &status)) {
+		mmc_host_clk_release(host);
+		return -ENOSYS;
+	}
+
+	/* Set initial state and call mmc_set_ios */
+	mmc_set_initial_state(host);
+	mmc_host_clk_release(host);
+
+	return mmc_power_restore(host);
+}
+
 static const struct mmc_bus_ops mmc_ops = {
 	.remove = mmc_remove,
 	.detect = mmc_detect,
@@ -1805,6 +1845,7 @@ static const struct mmc_bus_ops mmc_ops = {
 	.power_restore = mmc_power_restore,
 	.alive = mmc_alive,
 	.shutdown = mmc_shutdown,
+	.reset = mmc_reset,
 };
 
 /*
-- 
1.7.2.5

[PATCH v7 3/3] mmc: sd: add reset bus_ops callback

[Johan Rudholm]

Enable power cycle and re-initialization of SD cards via the reset
bus_ops. Power cycling a buggy SD card sometimes helps it get back on
track.

Signed-off-by: Johan Rudholm <johanru@axis.com>
---
 drivers/mmc/core/sd.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 29fccdc..36d5333 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1197,6 +1197,12 @@ static int mmc_sd_power_restore(struct mmc_host *host)
 	return ret;
 }
 
+static int mmc_sd_reset(struct mmc_host *host)
+{
+	mmc_power_cycle(host, host->card->ocr);
+	return mmc_sd_power_restore(host);
+}
+
 static const struct mmc_bus_ops mmc_sd_ops = {
 	.remove = mmc_sd_remove,
 	.detect = mmc_sd_detect,
@@ -1207,6 +1213,7 @@ static const struct mmc_bus_ops mmc_sd_ops = {
 	.power_restore = mmc_sd_power_restore,
 	.alive = mmc_sd_alive,
 	.shutdown = mmc_sd_suspend,
+	.reset = mmc_sd_reset,
 };
 
 /*
-- 
1.7.2.5

Re: [PATCH v7 0/3] mmc: core: hw_reset changes

[Ulf Hansson]

On 12 January 2015 at 15:38, Johan Rudholm <johan.rudholm@axis.com> wrote:
> Make the mmc_hw_reset routine more generic, by putting the (e)MMC-
> specific stuff in the new bus_ops->reset in mmc.c. Add a
> bus_ops->reset for SD cards, allowing them to be restarted when
> errors occur.
>
> For MMC cards, always check if the reset was sucessful. This allows
> us to remove the mmc_hw_reset_check() interface.
>
> As I don't have an eMMC device myself, much less one with a reset line,
> I'd be very happy if someone could help me test the code with an eMMC?
>
> v7:
>  - Rename bus_ops->hw_reset to reset
>  - Move the send_status check to mmc.c
>  - Put the changes concerning mmc_hw_reset_check in a separate patch
>
> v6:
>  - Always perform the mmc_send_status reset check, which allows us to
>    have only one interface to the card reset functions
>  - Because of this, add the bus_ops->hw_reset to sd.c instead of
>    falling back to a power_cycle
>
> v5:
>  - Move the mmc_test-specific code to mmc_test.c
>  - Fall back to a power_cycle if the bus_ops->hw_reset is missing
>  - Because of this, skip the bus_ops->hw_reset in sd.c
>
> v4:
>  - Rebase onto next
>
> v3:
>  - Keep mmc_can_reset
>  - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state()
>
> v2:
>  - Call the new bus_ops member hw_reset instead of power_reset
>  - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset
>    instead of mmc_power_up
>  - Keep "mmc_hw_reset" naming
>
> Johan Rudholm (3):
>   mmc: core: always check status after reset
>   mmc: core: refactor the hw_reset routines
>   mmc: sd: add reset bus_ops callback
>
>  drivers/mmc/card/mmc_test.c |   18 +++++--------
>  drivers/mmc/core/core.c     |   59 +++++++-----------------------------------
>  drivers/mmc/core/core.h     |    1 +
>  drivers/mmc/core/mmc.c      |   41 +++++++++++++++++++++++++++++
>  drivers/mmc/core/sd.c       |    7 +++++
>  include/linux/mmc/core.h    |    1 -
>  6 files changed, 66 insertions(+), 61 deletions(-)
>
> --
> 1.7.2.5
>

Thanks! Applied for next.

Kind regards
Uffe

Re: [PATCH v7 0/3] mmc: core: hw_reset changes

[Ulf Hansson]

On 12 January 2015 at 16:11, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 12 January 2015 at 15:38, Johan Rudholm <johan.rudholm@axis.com> wrote:
>> Make the mmc_hw_reset routine more generic, by putting the (e)MMC-
>> specific stuff in the new bus_ops->reset in mmc.c. Add a
>> bus_ops->reset for SD cards, allowing them to be restarted when
>> errors occur.
>>
>> For MMC cards, always check if the reset was sucessful. This allows
>> us to remove the mmc_hw_reset_check() interface.
>>
>> As I don't have an eMMC device myself, much less one with a reset line,
>> I'd be very happy if someone could help me test the code with an eMMC?
>>
>> v7:
>>  - Rename bus_ops->hw_reset to reset
>>  - Move the send_status check to mmc.c
>>  - Put the changes concerning mmc_hw_reset_check in a separate patch
>>
>> v6:
>>  - Always perform the mmc_send_status reset check, which allows us to
>>    have only one interface to the card reset functions
>>  - Because of this, add the bus_ops->hw_reset to sd.c instead of
>>    falling back to a power_cycle
>>
>> v5:
>>  - Move the mmc_test-specific code to mmc_test.c
>>  - Fall back to a power_cycle if the bus_ops->hw_reset is missing
>>  - Because of this, skip the bus_ops->hw_reset in sd.c
>>
>> v4:
>>  - Rebase onto next
>>
>> v3:
>>  - Keep mmc_can_reset
>>  - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state()
>>
>> v2:
>>  - Call the new bus_ops member hw_reset instead of power_reset
>>  - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset
>>    instead of mmc_power_up
>>  - Keep "mmc_hw_reset" naming
>>
>> Johan Rudholm (3):
>>   mmc: core: always check status after reset
>>   mmc: core: refactor the hw_reset routines
>>   mmc: sd: add reset bus_ops callback
>>
>>  drivers/mmc/card/mmc_test.c |   18 +++++--------
>>  drivers/mmc/core/core.c     |   59 +++++++-----------------------------------
>>  drivers/mmc/core/core.h     |    1 +
>>  drivers/mmc/core/mmc.c      |   41 +++++++++++++++++++++++++++++
>>  drivers/mmc/core/sd.c       |    7 +++++
>>  include/linux/mmc/core.h    |    1 -
>>  6 files changed, 66 insertions(+), 61 deletions(-)
>>
>> --
>> 1.7.2.5
>>
>
> Thanks! Applied for next.

Patch 1 caused compiler error and thus breaking bisectability. I
decided to fix it up myself this time, but future wise please make
sure each patch builds separately.

Kind regards
Uffe

Re: [PATCH v7 0/3] mmc: core: hw_reset changes

[Johan Rudholm]

2015-01-13 9:19 GMT+01:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 12 January 2015 at 16:11, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 12 January 2015 at 15:38, Johan Rudholm <johan.rudholm@axis.com> wrote:
>>> Make the mmc_hw_reset routine more generic, by putting the (e)MMC-
>>> specific stuff in the new bus_ops->reset in mmc.c. Add a
>>> bus_ops->reset for SD cards, allowing them to be restarted when
>>> errors occur.
>>>
>>> For MMC cards, always check if the reset was sucessful. This allows
>>> us to remove the mmc_hw_reset_check() interface.
>>>
>>> As I don't have an eMMC device myself, much less one with a reset line,
>>> I'd be very happy if someone could help me test the code with an eMMC?
>>>
>>> v7:
>>>  - Rename bus_ops->hw_reset to reset
>>>  - Move the send_status check to mmc.c
>>>  - Put the changes concerning mmc_hw_reset_check in a separate patch
>>>
>>> v6:
>>>  - Always perform the mmc_send_status reset check, which allows us to
>>>    have only one interface to the card reset functions
>>>  - Because of this, add the bus_ops->hw_reset to sd.c instead of
>>>    falling back to a power_cycle
>>>
>>> v5:
>>>  - Move the mmc_test-specific code to mmc_test.c
>>>  - Fall back to a power_cycle if the bus_ops->hw_reset is missing
>>>  - Because of this, skip the bus_ops->hw_reset in sd.c
>>>
>>> v4:
>>>  - Rebase onto next
>>>
>>> v3:
>>>  - Keep mmc_can_reset
>>>  - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state()
>>>
>>> v2:
>>>  - Call the new bus_ops member hw_reset instead of power_reset
>>>  - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset
>>>    instead of mmc_power_up
>>>  - Keep "mmc_hw_reset" naming
>>>
>>> Johan Rudholm (3):
>>>   mmc: core: always check status after reset
>>>   mmc: core: refactor the hw_reset routines
>>>   mmc: sd: add reset bus_ops callback
>>>
>>>  drivers/mmc/card/mmc_test.c |   18 +++++--------
>>>  drivers/mmc/core/core.c     |   59 +++++++-----------------------------------
>>>  drivers/mmc/core/core.h     |    1 +
>>>  drivers/mmc/core/mmc.c      |   41 +++++++++++++++++++++++++++++
>>>  drivers/mmc/core/sd.c       |    7 +++++
>>>  include/linux/mmc/core.h    |    1 -
>>>  6 files changed, 66 insertions(+), 61 deletions(-)
>>>
>>> --
>>> 1.7.2.5
>>>
>>
>> Thanks! Applied for next.
>
> Patch 1 caused compiler error and thus breaking bisectability. I
> decided to fix it up myself this time, but future wise please make
> sure each patch builds separately.

Very sorry about that, won't happen again. Thanks!

//Johan