[PATCH 0/5] mmc: Miscellaneous fixes for SDHCI and MMC core

[PATCH 0/5] mmc: Miscellaneous fixes for SDHCI and MMC core

[Andrew Gabbasov]

Andrew Gabbasov (5):
  mmc: sdhci: Balance vmmc regulator_disable()
  mmc: sdhci: fix error conditions for controller reset
  mmc: core: Initialize SET_BLOCK_COUNT request fields
  mmc: core: Add debug message for SET_BLOCK_COUNT result
  mmc: core: Fix error paths and messages in mmc_init_card

 drivers/mmc/core/core.c  | 12 ++++++++++++
 drivers/mmc/core/mmc.c   | 16 ++++++++--------
 drivers/mmc/host/sdhci.c | 10 ++++------
 3 files changed, 24 insertions(+), 14 deletions(-)

-- 
2.1.0

[PATCH 1/5] mmc: sdhci: Balance vmmc regulator_disable()

[Andrew Gabbasov]

As a follow-up of commit
"mmc: sdhci: Balance vmmc regulator_enable(), and always enable vqmmc"
vmmc regulator disable is also not needed in sdhci_remove_host.
The regulator is completely controlled by mmc_power_up and mmc_power_off
functions and is already disabled by the time of removing the host.
Extra regulator_disable call in sdhci_remove_host is unbalanced and
causes a warning reported by regulator core, so should be removed.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mmc/host/sdhci.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 37b2a9a..0272021 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3286,9 +3286,6 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 
 	tasklet_kill(&host->finish_tasklet);
 
-	if (!IS_ERR(mmc->supply.vmmc))
-		regulator_disable(mmc->supply.vmmc);
-
 	if (!IS_ERR(mmc->supply.vqmmc))
 		regulator_disable(mmc->supply.vqmmc);
 
-- 
2.1.0

[PATCH 2/5] mmc: sdhci: fix error conditions for controller reset

[Andrew Gabbasov]

Add the case of SET_BLOCK_COUNT command error to the error conditions
check for making a controller reset at request handling finish.
Otherwise, if the SET_BLOCK_COUNT command failed, e.g. with a timeout,
the controller state was not reset, and the next command failed too.

In the case of data error the controller reset is already done in
finish_data() function before sending stop command (if present),
so the finish tasklet should make a reset after data error only
if no stop command existed in the request.

Also, fix the indentation of this condition check to make it more logical.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mmc/host/sdhci.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0272021..46e84a0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2122,9 +2122,10 @@ static void sdhci_tasklet_finish(unsigned long param)
 	 */
 	if (!(host->flags & SDHCI_DEVICE_DEAD) &&
 	    ((mrq->cmd && mrq->cmd->error) ||
-		 (mrq->data && (mrq->data->error ||
-		  (mrq->data->stop && mrq->data->stop->error))) ||
-		   (host->quirks & SDHCI_QUIRK_RESET_AFTER_REQUEST))) {
+	     (mrq->sbc && mrq->sbc->error) ||
+	     (mrq->data && ((mrq->data->error && !mrq->data->stop) ||
+			    (mrq->data->stop && mrq->data->stop->error))) ||
+	     (host->quirks & SDHCI_QUIRK_RESET_AFTER_REQUEST))) {
 
 		/* Some controllers need this kick or reset won't work here */
 		if (host->quirks & SDHCI_QUIRK_CLOCK_BEFORE_RESET)
-- 
2.1.0

[PATCH 3/5] mmc: core: Initialize SET_BLOCK_COUNT request fields

[Andrew Gabbasov]

Some request fields are initialized just before request processing
for sanity purposes. This is done for command, data, and stop parts
of the request, but not for sbc (set block count) part. Add such
initialization for that part too.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mmc/core/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d03a080..b8e57db 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -214,6 +214,10 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 
 	mrq->cmd->error = 0;
 	mrq->cmd->mrq = mrq;
+	if (mrq->sbc) {
+		mrq->sbc->error = 0;
+		mrq->sbc->mrq = mrq;
+	}
 	if (mrq->data) {
 		BUG_ON(mrq->data->blksz > host->max_blk_size);
 		BUG_ON(mrq->data->blocks > host->max_blk_count);
-- 
2.1.0

[PATCH 4/5] mmc: core: Add debug message for SET_BLOCK_COUNT result

[Andrew Gabbasov]

The debug messages with commands execution results, that are printed
after processing the request, do not include results of sbc (set block count)
part of request. Add the debug message for that part too.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mmc/core/core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index b8e57db..162bcd3 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -149,6 +149,14 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
 
 		led_trigger_event(host->led, LED_OFF);
 
+		if (mrq->sbc) {
+			pr_debug("%s: req done <CMD%u>: %d: %08x %08x %08x %08x\n",
+				mmc_hostname(host), mrq->sbc->opcode,
+				mrq->sbc->error,
+				mrq->sbc->resp[0], mrq->sbc->resp[1],
+				mrq->sbc->resp[2], mrq->sbc->resp[3]);
+		}
+
 		pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x %08x\n",
 			mmc_hostname(host), cmd->opcode, err,
 			cmd->resp[0], cmd->resp[1],
-- 
2.1.0

[PATCH 5/5] mmc: core: Fix error paths and messages in mmc_init_card

[Andrew Gabbasov]

In mmc_init_card function some of the branches in error handling paths
go to "err" label, which skips removing of newly allocated card structure,
that will actually not be used. Fix that by using proper "free_card" label.

Also, some messages in these branches are reported as warnings,
although the operation processing is not continued. Change these
messages to error level.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mmc/core/mmc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1eda8dd..a818703 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -973,7 +973,7 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
 			ext_csd_bits,
 			card->ext_csd.generic_cmd6_time);
 	if (err) {
-		pr_warn("%s: switch to bus width %d ddr failed\n",
+		pr_err("%s: switch to bus width %d ddr failed\n",
 			mmc_hostname(host), 1 << bus_width);
 		return err;
 	}
@@ -1028,7 +1028,7 @@ static int mmc_select_hs400(struct mmc_card *card)
 			   card->ext_csd.generic_cmd6_time,
 			   true, true, true);
 	if (err) {
-		pr_warn("%s: switch to high-speed from hs200 failed, err:%d\n",
+		pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
 			mmc_hostname(host), err);
 		return err;
 	}
@@ -1038,7 +1038,7 @@ static int mmc_select_hs400(struct mmc_card *card)
 			 EXT_CSD_DDR_BUS_WIDTH_8,
 			 card->ext_csd.generic_cmd6_time);
 	if (err) {
-		pr_warn("%s: switch to bus width for hs400 failed, err:%d\n",
+		pr_err("%s: switch to bus width for hs400 failed, err:%d\n",
 			mmc_hostname(host), err);
 		return err;
 	}
@@ -1048,7 +1048,7 @@ static int mmc_select_hs400(struct mmc_card *card)
 			   card->ext_csd.generic_cmd6_time,
 			   true, true, true);
 	if (err) {
-		pr_warn("%s: switch to hs400 failed, err:%d\n",
+		pr_err("%s: switch to hs400 failed, err:%d\n",
 			 mmc_hostname(host), err);
 		return err;
 	}
@@ -1159,7 +1159,7 @@ static int mmc_hs200_tuning(struct mmc_card *card)
 		mmc_host_clk_release(host);
 
 		if (err)
-			pr_warn("%s: tuning execution failed\n",
+			pr_err("%s: tuning execution failed\n",
 				mmc_hostname(host));
 	}
 
@@ -1378,18 +1378,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	if (mmc_card_hs200(card)) {
 		err = mmc_hs200_tuning(card);
 		if (err)
-			goto err;
+			goto free_card;
 
 		err = mmc_select_hs400(card);
 		if (err)
-			goto err;
+			goto free_card;
 	} else if (mmc_card_hs(card)) {
 		/* Select the desired bus width optionally */
 		err = mmc_select_bus_width(card);
 		if (!IS_ERR_VALUE(err)) {
 			err = mmc_select_hs_ddr(card);
 			if (err)
-				goto err;
+				goto free_card;
 		}
 	}
 
-- 
2.1.0

RE: [PATCH 0/5] mmc: Miscellaneous fixes for SDHCI and MMC core

[Andrew Gabbasov]

Hi,

Any comments on this?

> -----Original Message-----
> From: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> Sent: Wednesday, October 01, 2014 4:14 PM
> To: linux-mmc@vger.kernel.org
> Subject: [PATCH 0/5] mmc: Miscellaneous fixes for SDHCI and MMC core
> 
> Andrew Gabbasov (5):
>   mmc: sdhci: Balance vmmc regulator_disable()
>   mmc: sdhci: fix error conditions for controller reset
>   mmc: core: Initialize SET_BLOCK_COUNT request fields
>   mmc: core: Add debug message for SET_BLOCK_COUNT result
>   mmc: core: Fix error paths and messages in mmc_init_card
> 
>  drivers/mmc/core/core.c  | 12 ++++++++++++
>  drivers/mmc/core/mmc.c   | 16 ++++++++--------
>  drivers/mmc/host/sdhci.c | 10 ++++------
>  3 files changed, 24 insertions(+), 14 deletions(-)
> 
> --
> 2.1.0

Thanks.

Best regards,
Andrew Gabbasov

Re: [PATCH 0/5] mmc: Miscellaneous fixes for SDHCI and MMC core

[Ulf Hansson]

On 1 October 2014 14:14, Andrew Gabbasov <andrew_gabbasov@mentor.com> wrote:
> Andrew Gabbasov (5):
>   mmc: sdhci: Balance vmmc regulator_disable()
>   mmc: sdhci: fix error conditions for controller reset
>   mmc: core: Initialize SET_BLOCK_COUNT request fields
>   mmc: core: Add debug message for SET_BLOCK_COUNT result
>   mmc: core: Fix error paths and messages in mmc_init_card
>
>  drivers/mmc/core/core.c  | 12 ++++++++++++
>  drivers/mmc/core/mmc.c   | 16 ++++++++--------
>  drivers/mmc/host/sdhci.c | 10 ++++------
>  3 files changed, 24 insertions(+), 14 deletions(-)

Thanks! Applied for next!

Kind regards
Uffe

>
> --
> 2.1.0
>
> --
> 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