All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] mmc: sd: Remove superfluous error code assignment
@ 2015-09-09 16:33 Yousong Zhou
  2015-09-09 16:33 ` [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries Yousong Zhou
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Yousong Zhou @ 2015-09-09 16:33 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Hans de Goede, Shawn Lin, linux-mmc, Yousong Zhou

Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
---
 drivers/mmc/core/sd.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 4e7366a..e28ebf3 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -357,8 +357,6 @@ int mmc_sd_switch_hs(struct mmc_card *card)
 	if (card->sw_caps.hs_max_dtr == 0)
 		return 0;
 
-	err = -EIO;
-
 	status = kmalloc(64, GFP_KERNEL);
 	if (!status) {
 		pr_err("%s: could not allocate a buffer for "
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries
  2015-09-09 16:33 [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Yousong Zhou
@ 2015-09-09 16:33 ` Yousong Zhou
  2015-09-10  0:33   ` Shawn Lin
  2015-09-09 16:33 ` [PATCH 3/3] mmc: sd: Retry switching to highspeed mode in case of error Yousong Zhou
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Yousong Zhou @ 2015-09-09 16:33 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Hans de Goede, Shawn Lin, linux-mmc, Yousong Zhou

This will allow retrying access mode switch to High-Speed in the
following commit.

Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
---
 drivers/mmc/core/core.c   |    4 ++++
 drivers/mmc/core/sd_ops.c |    5 +++--
 drivers/mmc/core/sd_ops.h |   10 ++++++++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 9ad73f3..e726bb1 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -468,11 +468,13 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
 				  struct mmc_request *mrq)
 {
 	struct mmc_command *cmd;
+	struct mmc_data *data;
 
 	while (1) {
 		wait_for_completion(&mrq->completion);
 
 		cmd = mrq->cmd;
+		data = mrq->data;
 
 		/*
 		 * If host has timed out waiting for the sanitize
@@ -501,6 +503,8 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
 			 mmc_hostname(host), cmd->opcode, cmd->error);
 		cmd->retries--;
 		cmd->error = 0;
+		if (data)
+			data->error = 0;
 		__mmc_start_request(host, mrq);
 	}
 
diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
index 48d0c93..22bef3c 100644
--- a/drivers/mmc/core/sd_ops.c
+++ b/drivers/mmc/core/sd_ops.c
@@ -304,8 +304,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
 	return 0;
 }
 
-int mmc_sd_switch(struct mmc_card *card, int mode, int group,
-	u8 value, u8 *resp)
+int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
+	u8 value, u8 *resp, int retries)
 {
 	struct mmc_request mrq = {NULL};
 	struct mmc_command cmd = {0};
@@ -328,6 +328,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int group,
 	cmd.arg &= ~(0xF << (group * 4));
 	cmd.arg |= value << (group * 4);
 	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
+	cmd.retries = retries;
 
 	data.blksz = 64;
 	data.blocks = 1;
diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
index ffc2305..a53c51e 100644
--- a/drivers/mmc/core/sd_ops.h
+++ b/drivers/mmc/core/sd_ops.h
@@ -17,9 +17,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
 int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
 int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
 int mmc_app_send_scr(struct mmc_card *card, u32 *scr);
-int mmc_sd_switch(struct mmc_card *card, int mode, int group,
-	u8 value, u8 *resp);
 int mmc_app_sd_status(struct mmc_card *card, void *ssr);
 
+int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
+	u8 value, u8 *resp, int retries);
+static inline int mmc_sd_switch(struct mmc_card *card, int mode, int group,
+	u8 value, u8 *resp)
+{
+	return __mmc_sd_switch(card, mode, group, value, resp, 0);
+}
+
 #endif
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] mmc: sd: Retry switching to highspeed mode in case of error
  2015-09-09 16:33 [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Yousong Zhou
  2015-09-09 16:33 ` [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries Yousong Zhou
@ 2015-09-09 16:33 ` Yousong Zhou
  2015-09-09 19:29 ` [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Hans de Goede
  2015-09-15 11:34 ` Ulf Hansson
  3 siblings, 0 replies; 11+ messages in thread
From: Yousong Zhou @ 2015-09-09 16:33 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Hans de Goede, Shawn Lin, linux-mmc, Yousong Zhou

A SD card with sunxi-mmc can fail with the following error message (RCE for
response CRC error) when trying to switch to highspeed mode.  But the mode
switch can almost always succeed on the second try.

Dmesg before this commit.

    [    1.112060] mmc0: host does not support reading read-only switch, assuming write-enable
    [    1.126527] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 6, RD RCE !!
    [    1.132388] sunxi-mmc 1c0f000.mmc: data error, sending stop command
    [    1.139451] sunxi-mmc 1c0f000.mmc: send stop command failed
    [    1.145056] mmc0: error -110 whilst initialising SD card
    [    1.150424] ehci-platform 1c1c000.usb: USB 2.0 started, EHCI 1.00
    [    1.156533] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 1, RTO !!

Dmesg after this commit.

    [    1.062306] mmc0: host does not support reading read-only switch, assuming write-enable
    [    1.070635] sunxi-mmc 1c0f000.mmc: smc 0 err, cmd 6, RD RCE !!
    [    1.295939] sunxi-mmc 1c0f000.mmc: data error, sending stop command
    [    1.322635] Waiting for root device /dev/mmcblk0p2...
    [    1.322691] sunxi-mmc 1c0f000.mmc: send stop command failed
    [    1.335686] mmc0: new high speed SD card at address 0002
    [    1.341557] mmcblk0: mmc0:0002 00000 974 MiB
    [    1.347021]  mmcblk0: p1 p2

Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
---
 drivers/mmc/core/sd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index e28ebf3..7465b16 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -364,7 +364,7 @@ int mmc_sd_switch_hs(struct mmc_card *card)
 		return -ENOMEM;
 	}
 
-	err = mmc_sd_switch(card, 1, 0, 1, status);
+	err = __mmc_sd_switch(card, 1, 0, 1, status, MMC_CMD_RETRIES);
 	if (err)
 		goto out;
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] mmc: sd: Remove superfluous error code assignment
  2015-09-09 16:33 [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Yousong Zhou
  2015-09-09 16:33 ` [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries Yousong Zhou
  2015-09-09 16:33 ` [PATCH 3/3] mmc: sd: Retry switching to highspeed mode in case of error Yousong Zhou
@ 2015-09-09 19:29 ` Hans de Goede
  2015-09-15 11:34 ` Ulf Hansson
  3 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2015-09-09 19:29 UTC (permalink / raw)
  To: Yousong Zhou, Ulf Hansson; +Cc: Shawn Lin, linux-mmc

Hi All,

I requested yousong to send this set to further discuss it,
and hopefully get some fix for this in the kernel.

Yousong, I actually intended you to do a patch to the sunxi-mmc
code, rather then too the core. Ah well. Since we've this set
now lets see if people like it, it may help other cases
too I guess.

Regards,

Hans


On 09-09-15 18:33, Yousong Zhou wrote:
> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
> ---
>   drivers/mmc/core/sd.c |    2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 4e7366a..e28ebf3 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -357,8 +357,6 @@ int mmc_sd_switch_hs(struct mmc_card *card)
>   	if (card->sw_caps.hs_max_dtr == 0)
>   		return 0;
>
> -	err = -EIO;
> -
>   	status = kmalloc(64, GFP_KERNEL);
>   	if (!status) {
>   		pr_err("%s: could not allocate a buffer for "
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries
  2015-09-09 16:33 ` [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries Yousong Zhou
@ 2015-09-10  0:33   ` Shawn Lin
  2015-09-10  3:02     ` Yousong Zhou
  0 siblings, 1 reply; 11+ messages in thread
From: Shawn Lin @ 2015-09-10  0:33 UTC (permalink / raw)
  To: Yousong Zhou, Ulf Hansson; +Cc: shawn.lin, Hans de Goede, linux-mmc

On 2015/9/10 0:33, Yousong Zhou wrote:
> This will allow retrying access mode switch to High-Speed in the
> following commit.
>
> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
> ---
>   drivers/mmc/core/core.c   |    4 ++++
>   drivers/mmc/core/sd_ops.c |    5 +++--
>   drivers/mmc/core/sd_ops.h |   10 ++++++++--
>   3 files changed, 15 insertions(+), 4 deletions(-)
>

I test this patchset with my already-working sd cards, seems ok.
Actually I don't have a card to test like yours that need retry switch 
HS, but from the changes itself, it's harmless to other cards.

so,

Tested-by: Shawn Lin <shawn.lin@rock-chips.com>

BTW, I can't find the cover letter? And another should be clarified, plz 
see comment blow


> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 9ad73f3..e726bb1 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -468,11 +468,13 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
>   				  struct mmc_request *mrq)
>   {
>   	struct mmc_command *cmd;
> +	struct mmc_data *data;
>
>   	while (1) {
>   		wait_for_completion(&mrq->completion);
>
>   		cmd = mrq->cmd;
> +		data = mrq->data;
>
>   		/*
>   		 * If host has timed out waiting for the sanitize
> @@ -501,6 +503,8 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
>   			 mmc_hostname(host), cmd->opcode, cmd->error);
>   		cmd->retries--;
>   		cmd->error = 0;
> +		if (data)
> +			data->error = 0;

What's this change for?

>   		__mmc_start_request(host, mrq);
>   	}
>
> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
> index 48d0c93..22bef3c 100644
> --- a/drivers/mmc/core/sd_ops.c
> +++ b/drivers/mmc/core/sd_ops.c
> @@ -304,8 +304,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
>   	return 0;
>   }
>
> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
> -	u8 value, u8 *resp)
> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
> +	u8 value, u8 *resp, int retries)
>   {
>   	struct mmc_request mrq = {NULL};
>   	struct mmc_command cmd = {0};
> @@ -328,6 +328,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>   	cmd.arg &= ~(0xF << (group * 4));
>   	cmd.arg |= value << (group * 4);
>   	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> +	cmd.retries = retries;
>
>   	data.blksz = 64;
>   	data.blocks = 1;
> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
> index ffc2305..a53c51e 100644
> --- a/drivers/mmc/core/sd_ops.h
> +++ b/drivers/mmc/core/sd_ops.h
> @@ -17,9 +17,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
>   int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
>   int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
>   int mmc_app_send_scr(struct mmc_card *card, u32 *scr);
> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
> -	u8 value, u8 *resp);
>   int mmc_app_sd_status(struct mmc_card *card, void *ssr);
>
> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
> +	u8 value, u8 *resp, int retries);
> +static inline int mmc_sd_switch(struct mmc_card *card, int mode, int group,
> +	u8 value, u8 *resp)
> +{
> +	return __mmc_sd_switch(card, mode, group, value, resp, 0);
> +}
> +
>   #endif
>
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries
  2015-09-10  0:33   ` Shawn Lin
@ 2015-09-10  3:02     ` Yousong Zhou
  2015-09-16  2:36       ` Jaehoon Chung
  0 siblings, 1 reply; 11+ messages in thread
From: Yousong Zhou @ 2015-09-10  3:02 UTC (permalink / raw)
  To: Shawn Lin; +Cc: Ulf Hansson, Hans de Goede, linux-mmc

On 10 September 2015 at 08:33, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> On 2015/9/10 0:33, Yousong Zhou wrote:
>>
>> This will allow retrying access mode switch to High-Speed in the
>> following commit.
>>
>> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
>> ---
>>   drivers/mmc/core/core.c   |    4 ++++
>>   drivers/mmc/core/sd_ops.c |    5 +++--
>>   drivers/mmc/core/sd_ops.h |   10 ++++++++--
>>   3 files changed, 15 insertions(+), 4 deletions(-)
>>
>
> I test this patchset with my already-working sd cards, seems ok.
> Actually I don't have a card to test like yours that need retry switch HS,
> but from the changes itself, it's harmless to other cards.

Thanks for the testing.

Those cards normally won't burn out after MMC_CMD_RETRIES access mode switch
failures, right?

>
> so,
>
> Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> BTW, I can't find the cover letter? And another should be clarified, plz see
> comment blow
>
>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 9ad73f3..e726bb1 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -468,11 +468,13 @@ static void mmc_wait_for_req_done(struct mmc_host
>> *host,
>>                                   struct mmc_request *mrq)
>>   {
>>         struct mmc_command *cmd;
>> +       struct mmc_data *data;
>>
>>         while (1) {
>>                 wait_for_completion(&mrq->completion);
>>
>>                 cmd = mrq->cmd;
>> +               data = mrq->data;
>>
>>                 /*
>>                  * If host has timed out waiting for the sanitize
>> @@ -501,6 +503,8 @@ static void mmc_wait_for_req_done(struct mmc_host
>> *host,
>>                          mmc_hostname(host), cmd->opcode, cmd->error);
>>                 cmd->retries--;
>>                 cmd->error = 0;
>> +               if (data)
>> +                       data->error = 0;
>
>
> What's this change for?

sunxi-mmc will set both cmd->error and data->error to -ETIMEDOUT in case of any
interrupt error bit was set (related code within `sunxi_finalize_request()').

`__mmc_sd_switch()' thought it was a error case if data->error != 0.

So the data->error bit needs to be cleared before next retry.

Regards,

                yousnog

>
>
>>                 __mmc_start_request(host, mrq);
>>         }
>>
>> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
>> index 48d0c93..22bef3c 100644
>> --- a/drivers/mmc/core/sd_ops.c
>> +++ b/drivers/mmc/core/sd_ops.c
>> @@ -304,8 +304,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
>>         return 0;
>>   }
>>
>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>> -       u8 value, u8 *resp)
>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>> +       u8 value, u8 *resp, int retries)
>>   {
>>         struct mmc_request mrq = {NULL};
>>         struct mmc_command cmd = {0};
>> @@ -328,6 +328,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int
>> group,
>>         cmd.arg &= ~(0xF << (group * 4));
>>         cmd.arg |= value << (group * 4);
>>         cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>> +       cmd.retries = retries;
>>
>>         data.blksz = 64;
>>         data.blocks = 1;
>> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
>> index ffc2305..a53c51e 100644
>> --- a/drivers/mmc/core/sd_ops.h
>> +++ b/drivers/mmc/core/sd_ops.h
>> @@ -17,9 +17,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32
>> ocr, u32 *rocr);
>>   int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
>>   int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
>>   int mmc_app_send_scr(struct mmc_card *card, u32 *scr);
>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>> -       u8 value, u8 *resp);
>>   int mmc_app_sd_status(struct mmc_card *card, void *ssr);
>>
>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>> +       u8 value, u8 *resp, int retries);
>> +static inline int mmc_sd_switch(struct mmc_card *card, int mode, int
>> group,
>> +       u8 value, u8 *resp)
>> +{
>> +       return __mmc_sd_switch(card, mode, group, value, resp, 0);
>> +}
>> +
>>   #endif
>>
>>
>
>
> --
> Best Regards
> Shawn Lin
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] mmc: sd: Remove superfluous error code assignment
  2015-09-09 16:33 [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Yousong Zhou
                   ` (2 preceding siblings ...)
  2015-09-09 19:29 ` [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Hans de Goede
@ 2015-09-15 11:34 ` Ulf Hansson
  3 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2015-09-15 11:34 UTC (permalink / raw)
  To: Yousong Zhou; +Cc: Hans de Goede, Shawn Lin, linux-mmc

On 9 September 2015 at 18:33, Yousong Zhou <yszhou4tech@gmail.com> wrote:
> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>

Thanks, applied for next!

I will comment on patch2 and patch3 separately.

Kind regards
Uffe

> ---
>  drivers/mmc/core/sd.c |    2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 4e7366a..e28ebf3 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -357,8 +357,6 @@ int mmc_sd_switch_hs(struct mmc_card *card)
>         if (card->sw_caps.hs_max_dtr == 0)
>                 return 0;
>
> -       err = -EIO;
> -
>         status = kmalloc(64, GFP_KERNEL);
>         if (!status) {
>                 pr_err("%s: could not allocate a buffer for "
> --
> 1.7.10.4
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries
  2015-09-10  3:02     ` Yousong Zhou
@ 2015-09-16  2:36       ` Jaehoon Chung
  2015-09-16  3:09         ` Yousong Zhou
  0 siblings, 1 reply; 11+ messages in thread
From: Jaehoon Chung @ 2015-09-16  2:36 UTC (permalink / raw)
  To: Yousong Zhou, Shawn Lin; +Cc: Ulf Hansson, Hans de Goede, linux-mmc, CPGS

Hi, 

On 09/10/2015 12:02 PM, Yousong Zhou wrote:
> On 10 September 2015 at 08:33, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> On 2015/9/10 0:33, Yousong Zhou wrote:
>>>
>>> This will allow retrying access mode switch to High-Speed in the
>>> following commit.

Well, it's not solution. (It's my preference.)
Did you consider the card removable case?

Could you share which vendor's card needs to retry?

Best Regards,
Jaehoon Chung

>>>
>>> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
>>> ---
>>>   drivers/mmc/core/core.c   |    4 ++++
>>>   drivers/mmc/core/sd_ops.c |    5 +++--
>>>   drivers/mmc/core/sd_ops.h |   10 ++++++++--
>>>   3 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>
>> I test this patchset with my already-working sd cards, seems ok.
>> Actually I don't have a card to test like yours that need retry switch HS,
>> but from the changes itself, it's harmless to other cards.
> 
> Thanks for the testing.
> 
> Those cards normally won't burn out after MMC_CMD_RETRIES access mode switch
> failures, right?
> 
>>
>> so,
>>
>> Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> BTW, I can't find the cover letter? And another should be clarified, plz see
>> comment blow
>>
>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index 9ad73f3..e726bb1 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -468,11 +468,13 @@ static void mmc_wait_for_req_done(struct mmc_host
>>> *host,
>>>                                   struct mmc_request *mrq)
>>>   {
>>>         struct mmc_command *cmd;
>>> +       struct mmc_data *data;
>>>
>>>         while (1) {
>>>                 wait_for_completion(&mrq->completion);
>>>
>>>                 cmd = mrq->cmd;
>>> +               data = mrq->data;
>>>
>>>                 /*
>>>                  * If host has timed out waiting for the sanitize
>>> @@ -501,6 +503,8 @@ static void mmc_wait_for_req_done(struct mmc_host
>>> *host,
>>>                          mmc_hostname(host), cmd->opcode, cmd->error);
>>>                 cmd->retries--;
>>>                 cmd->error = 0;
>>> +               if (data)
>>> +                       data->error = 0;
>>
>>
>> What's this change for?
> 
> sunxi-mmc will set both cmd->error and data->error to -ETIMEDOUT in case of any
> interrupt error bit was set (related code within `sunxi_finalize_request()').
> 
> `__mmc_sd_switch()' thought it was a error case if data->error != 0.
> 
> So the data->error bit needs to be cleared before next retry.
> 
> Regards,
> 
>                 yousnog
> 
>>
>>
>>>                 __mmc_start_request(host, mrq);
>>>         }
>>>
>>> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
>>> index 48d0c93..22bef3c 100644
>>> --- a/drivers/mmc/core/sd_ops.c
>>> +++ b/drivers/mmc/core/sd_ops.c
>>> @@ -304,8 +304,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
>>>         return 0;
>>>   }
>>>
>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>> -       u8 value, u8 *resp)
>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>> +       u8 value, u8 *resp, int retries)
>>>   {
>>>         struct mmc_request mrq = {NULL};
>>>         struct mmc_command cmd = {0};
>>> @@ -328,6 +328,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int
>>> group,
>>>         cmd.arg &= ~(0xF << (group * 4));
>>>         cmd.arg |= value << (group * 4);
>>>         cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>>> +       cmd.retries = retries;
>>>
>>>         data.blksz = 64;
>>>         data.blocks = 1;
>>> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
>>> index ffc2305..a53c51e 100644
>>> --- a/drivers/mmc/core/sd_ops.h
>>> +++ b/drivers/mmc/core/sd_ops.h
>>> @@ -17,9 +17,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32
>>> ocr, u32 *rocr);
>>>   int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
>>>   int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
>>>   int mmc_app_send_scr(struct mmc_card *card, u32 *scr);
>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>> -       u8 value, u8 *resp);
>>>   int mmc_app_sd_status(struct mmc_card *card, void *ssr);
>>>
>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>> +       u8 value, u8 *resp, int retries);
>>> +static inline int mmc_sd_switch(struct mmc_card *card, int mode, int
>>> group,
>>> +       u8 value, u8 *resp)
>>> +{
>>> +       return __mmc_sd_switch(card, mode, group, value, resp, 0);
>>> +}
>>> +
>>>   #endif
>>>
>>>
>>
>>
>> --
>> Best Regards
>> Shawn Lin
>>
> --
> 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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries
  2015-09-16  2:36       ` Jaehoon Chung
@ 2015-09-16  3:09         ` Yousong Zhou
  2015-09-16  4:17           ` Jaehoon Chung
  0 siblings, 1 reply; 11+ messages in thread
From: Yousong Zhou @ 2015-09-16  3:09 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: Shawn Lin, Ulf Hansson, Hans de Goede, linux-mmc, CPGS

Hi,

On 16 September 2015 at 10:36, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi,
>
> On 09/10/2015 12:02 PM, Yousong Zhou wrote:
>> On 10 September 2015 at 08:33, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>> On 2015/9/10 0:33, Yousong Zhou wrote:
>>>>
>>>> This will allow retrying access mode switch to High-Speed in the
>>>> following commit.
>
> Well, it's not solution. (It's my preference.)
> Did you consider the card removable case?
>
> Could you share which vendor's card needs to retry?
>

It's a SD card labeled as "pqi" made in Korea.  The issue was found on
sunxi-mmc controller where it reported response crc error when trying
to switch the card into highspeed mode.

The idea about retrying the access mode switch was from U-Boot code of
Allwinner (vendor of sunxi SoCs).  The code comments there said this
issue can happen with eMMC cards from Toshiba [1].

The above info was also said in a previous discussion [1].  A
sunxi-mmc specific change as suggested by Hans de Geode should
definitely be a safer move.  But that requires more fundamental
framework code change I guess.

[1] https://github.com/allwinner-zh/bootloader/blob/master/u-boot-2011.09/drivers/mmc/mmc.c#L2220
[2] http://thread.gmane.org/gmane.comp.hardware.netbook.arm.sunxi/18442/focus=18480

Regards,

                yousong

> Best Regards,
> Jaehoon Chung
>
>>>>
>>>> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
>>>> ---
>>>>   drivers/mmc/core/core.c   |    4 ++++
>>>>   drivers/mmc/core/sd_ops.c |    5 +++--
>>>>   drivers/mmc/core/sd_ops.h |   10 ++++++++--
>>>>   3 files changed, 15 insertions(+), 4 deletions(-)
>>>>
>>>
>>> I test this patchset with my already-working sd cards, seems ok.
>>> Actually I don't have a card to test like yours that need retry switch HS,
>>> but from the changes itself, it's harmless to other cards.
>>
>> Thanks for the testing.
>>
>> Those cards normally won't burn out after MMC_CMD_RETRIES access mode switch
>> failures, right?
>>
>>>
>>> so,
>>>
>>> Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>
>>> BTW, I can't find the cover letter? And another should be clarified, plz see
>>> comment blow
>>>
>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index 9ad73f3..e726bb1 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -468,11 +468,13 @@ static void mmc_wait_for_req_done(struct mmc_host
>>>> *host,
>>>>                                   struct mmc_request *mrq)
>>>>   {
>>>>         struct mmc_command *cmd;
>>>> +       struct mmc_data *data;
>>>>
>>>>         while (1) {
>>>>                 wait_for_completion(&mrq->completion);
>>>>
>>>>                 cmd = mrq->cmd;
>>>> +               data = mrq->data;
>>>>
>>>>                 /*
>>>>                  * If host has timed out waiting for the sanitize
>>>> @@ -501,6 +503,8 @@ static void mmc_wait_for_req_done(struct mmc_host
>>>> *host,
>>>>                          mmc_hostname(host), cmd->opcode, cmd->error);
>>>>                 cmd->retries--;
>>>>                 cmd->error = 0;
>>>> +               if (data)
>>>> +                       data->error = 0;
>>>
>>>
>>> What's this change for?
>>
>> sunxi-mmc will set both cmd->error and data->error to -ETIMEDOUT in case of any
>> interrupt error bit was set (related code within `sunxi_finalize_request()').
>>
>> `__mmc_sd_switch()' thought it was a error case if data->error != 0.
>>
>> So the data->error bit needs to be cleared before next retry.
>>
>> Regards,
>>
>>                 yousnog
>>
>>>
>>>
>>>>                 __mmc_start_request(host, mrq);
>>>>         }
>>>>
>>>> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
>>>> index 48d0c93..22bef3c 100644
>>>> --- a/drivers/mmc/core/sd_ops.c
>>>> +++ b/drivers/mmc/core/sd_ops.c
>>>> @@ -304,8 +304,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
>>>>         return 0;
>>>>   }
>>>>
>>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>> -       u8 value, u8 *resp)
>>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>> +       u8 value, u8 *resp, int retries)
>>>>   {
>>>>         struct mmc_request mrq = {NULL};
>>>>         struct mmc_command cmd = {0};
>>>> @@ -328,6 +328,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int
>>>> group,
>>>>         cmd.arg &= ~(0xF << (group * 4));
>>>>         cmd.arg |= value << (group * 4);
>>>>         cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>>>> +       cmd.retries = retries;
>>>>
>>>>         data.blksz = 64;
>>>>         data.blocks = 1;
>>>> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
>>>> index ffc2305..a53c51e 100644
>>>> --- a/drivers/mmc/core/sd_ops.h
>>>> +++ b/drivers/mmc/core/sd_ops.h
>>>> @@ -17,9 +17,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32
>>>> ocr, u32 *rocr);
>>>>   int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
>>>>   int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
>>>>   int mmc_app_send_scr(struct mmc_card *card, u32 *scr);
>>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>> -       u8 value, u8 *resp);
>>>>   int mmc_app_sd_status(struct mmc_card *card, void *ssr);
>>>>
>>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>> +       u8 value, u8 *resp, int retries);
>>>> +static inline int mmc_sd_switch(struct mmc_card *card, int mode, int
>>>> group,
>>>> +       u8 value, u8 *resp)
>>>> +{
>>>> +       return __mmc_sd_switch(card, mode, group, value, resp, 0);
>>>> +}
>>>> +
>>>>   #endif
>>>>
>>>>
>>>
>>>
>>> --
>>> Best Regards
>>> Shawn Lin
>>>
>> --
>> 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
>>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries
  2015-09-16  3:09         ` Yousong Zhou
@ 2015-09-16  4:17           ` Jaehoon Chung
  2015-09-17  8:29             ` Yousong Zhou
  0 siblings, 1 reply; 11+ messages in thread
From: Jaehoon Chung @ 2015-09-16  4:17 UTC (permalink / raw)
  To: Yousong Zhou, Jaehoon Chung
  Cc: Shawn Lin, Ulf Hansson, Hans de Goede, linux-mmc, CPGS

Hi, Yousong.

On 09/16/2015 12:09 PM, Yousong Zhou wrote:
> Hi,
> 
> On 16 September 2015 at 10:36, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi,
>>
>> On 09/10/2015 12:02 PM, Yousong Zhou wrote:
>>> On 10 September 2015 at 08:33, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>>> On 2015/9/10 0:33, Yousong Zhou wrote:
>>>>>
>>>>> This will allow retrying access mode switch to High-Speed in the
>>>>> following commit.
>>
>> Well, it's not solution. (It's my preference.)
>> Did you consider the card removable case?
>>
>> Could you share which vendor's card needs to retry?
>>
> 
> It's a SD card labeled as "pqi" made in Korea.  The issue was found on
> sunxi-mmc controller where it reported response crc error when trying
> to switch the card into highspeed mode.

Did you test on other target with that card?
On other words, is Sd-card working fine on your real phone or other target?
If it's occurred on only sunxi-mmc controller, this is not solution.

And Your comment means "have only to retry when failed to switch high-speed mode.", doesn't?
Do you feel it's reasonable? It's same that adds the delay.

> 
> The idea about retrying the access mode switch was from U-Boot code of
> Allwinner (vendor of sunxi SoCs).  The code comments there said this
> issue can happen with eMMC cards from Toshiba [1].

Why did you compare to U-boot? There are too many differences for user-case.

> 
> The above info was also said in a previous discussion [1].  A
> sunxi-mmc specific change as suggested by Hans de Geode should
> definitely be a safer move.  But that requires more fundamental
> framework code change I guess.

[1] maintained at local git? not mainline..

> 
> [1] https://github.com/allwinner-zh/bootloader/blob/master/u-boot-2011.09/drivers/mmc/mmc.c#L2220
> [2] http://thread.gmane.org/gmane.comp.hardware.netbook.arm.sunxi/18442/focus=18480

Well, i don't know how ulf think about this.
I want to find the root cause and fix it.
If this is sunxi-mmc controller's problem, i think it can be fixed in sunxi-mmc controller.


Best Regards,
Jaehoon Chung
> 
> Regards,
> 
>                 yousong
> 
>> Best Regards,
>> Jaehoon Chung
>>
>>>>>
>>>>> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
>>>>> ---
>>>>>   drivers/mmc/core/core.c   |    4 ++++
>>>>>   drivers/mmc/core/sd_ops.c |    5 +++--
>>>>>   drivers/mmc/core/sd_ops.h |   10 ++++++++--
>>>>>   3 files changed, 15 insertions(+), 4 deletions(-)
>>>>>
>>>>
>>>> I test this patchset with my already-working sd cards, seems ok.
>>>> Actually I don't have a card to test like yours that need retry switch HS,
>>>> but from the changes itself, it's harmless to other cards.
>>>
>>> Thanks for the testing.
>>>
>>> Those cards normally won't burn out after MMC_CMD_RETRIES access mode switch
>>> failures, right?
>>>
>>>>
>>>> so,
>>>>
>>>> Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>>
>>>> BTW, I can't find the cover letter? And another should be clarified, plz see
>>>> comment blow
>>>>
>>>>
>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>> index 9ad73f3..e726bb1 100644
>>>>> --- a/drivers/mmc/core/core.c
>>>>> +++ b/drivers/mmc/core/core.c
>>>>> @@ -468,11 +468,13 @@ static void mmc_wait_for_req_done(struct mmc_host
>>>>> *host,
>>>>>                                   struct mmc_request *mrq)
>>>>>   {
>>>>>         struct mmc_command *cmd;
>>>>> +       struct mmc_data *data;
>>>>>
>>>>>         while (1) {
>>>>>                 wait_for_completion(&mrq->completion);
>>>>>
>>>>>                 cmd = mrq->cmd;
>>>>> +               data = mrq->data;
>>>>>
>>>>>                 /*
>>>>>                  * If host has timed out waiting for the sanitize
>>>>> @@ -501,6 +503,8 @@ static void mmc_wait_for_req_done(struct mmc_host
>>>>> *host,
>>>>>                          mmc_hostname(host), cmd->opcode, cmd->error);
>>>>>                 cmd->retries--;
>>>>>                 cmd->error = 0;
>>>>> +               if (data)
>>>>> +                       data->error = 0;
>>>>
>>>>
>>>> What's this change for?
>>>
>>> sunxi-mmc will set both cmd->error and data->error to -ETIMEDOUT in case of any
>>> interrupt error bit was set (related code within `sunxi_finalize_request()').
>>>
>>> `__mmc_sd_switch()' thought it was a error case if data->error != 0.
>>>
>>> So the data->error bit needs to be cleared before next retry.
>>>
>>> Regards,
>>>
>>>                 yousnog
>>>
>>>>
>>>>
>>>>>                 __mmc_start_request(host, mrq);
>>>>>         }
>>>>>
>>>>> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
>>>>> index 48d0c93..22bef3c 100644
>>>>> --- a/drivers/mmc/core/sd_ops.c
>>>>> +++ b/drivers/mmc/core/sd_ops.c
>>>>> @@ -304,8 +304,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
>>>>>         return 0;
>>>>>   }
>>>>>
>>>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>> -       u8 value, u8 *resp)
>>>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>> +       u8 value, u8 *resp, int retries)
>>>>>   {
>>>>>         struct mmc_request mrq = {NULL};
>>>>>         struct mmc_command cmd = {0};
>>>>> @@ -328,6 +328,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int
>>>>> group,
>>>>>         cmd.arg &= ~(0xF << (group * 4));
>>>>>         cmd.arg |= value << (group * 4);
>>>>>         cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>>>>> +       cmd.retries = retries;
>>>>>
>>>>>         data.blksz = 64;
>>>>>         data.blocks = 1;
>>>>> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
>>>>> index ffc2305..a53c51e 100644
>>>>> --- a/drivers/mmc/core/sd_ops.h
>>>>> +++ b/drivers/mmc/core/sd_ops.h
>>>>> @@ -17,9 +17,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32
>>>>> ocr, u32 *rocr);
>>>>>   int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
>>>>>   int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
>>>>>   int mmc_app_send_scr(struct mmc_card *card, u32 *scr);
>>>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>> -       u8 value, u8 *resp);
>>>>>   int mmc_app_sd_status(struct mmc_card *card, void *ssr);
>>>>>
>>>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>> +       u8 value, u8 *resp, int retries);
>>>>> +static inline int mmc_sd_switch(struct mmc_card *card, int mode, int
>>>>> group,
>>>>> +       u8 value, u8 *resp)
>>>>> +{
>>>>> +       return __mmc_sd_switch(card, mode, group, value, resp, 0);
>>>>> +}
>>>>> +
>>>>>   #endif
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Best Regards
>>>> Shawn Lin
>>>>
>>> --
>>> 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
>>>
>>
> --
> 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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries
  2015-09-16  4:17           ` Jaehoon Chung
@ 2015-09-17  8:29             ` Yousong Zhou
  0 siblings, 0 replies; 11+ messages in thread
From: Yousong Zhou @ 2015-09-17  8:29 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: Shawn Lin, Ulf Hansson, Hans de Goede, linux-mmc, CPGS

On 16 September 2015 at 12:17, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi, Yousong.
>
> On 09/16/2015 12:09 PM, Yousong Zhou wrote:
>> Hi,
>>
>> On 16 September 2015 at 10:36, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi,
>>>
>>> On 09/10/2015 12:02 PM, Yousong Zhou wrote:
>>>> On 10 September 2015 at 08:33, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>>>> On 2015/9/10 0:33, Yousong Zhou wrote:
>>>>>>
>>>>>> This will allow retrying access mode switch to High-Speed in the
>>>>>> following commit.
>>>
>>> Well, it's not solution. (It's my preference.)
>>> Did you consider the card removable case?
>>>
>>> Could you share which vendor's card needs to retry?
>>>
>>
>> It's a SD card labeled as "pqi" made in Korea.  The issue was found on
>> sunxi-mmc controller where it reported response crc error when trying
>> to switch the card into highspeed mode.
>
> Did you test on other target with that card?
> On other words, is Sd-card working fine on your real phone or other target?
> If it's occurred on only sunxi-mmc controller, this is not solution.
>
> And Your comment means "have only to retry when failed to switch high-speed mode.", doesn't?
> Do you feel it's reasonable? It's same that adds the delay.
>

Not reasonable at all...  I expected those cards would just work out
of the box and never supposed that I needed to dig into so many places
to debug such problems.  sorry for the grunt

>>
>> The idea about retrying the access mode switch was from U-Boot code of
>> Allwinner (vendor of sunxi SoCs).  The code comments there said this
>> issue can happen with eMMC cards from Toshiba [1].
>
> Why did you compare to U-boot? There are too many differences for user-case.
>

Because I was then guessing whether the vendor had also encountered
the same issue and if so did they actually fixed or worked around it.
Then viola, there they wrote in the comments in natural language
describing exact the same error I encountered both in U-Boot and
Linux.

>>
>> The above info was also said in a previous discussion [1].  A
>> sunxi-mmc specific change as suggested by Hans de Geode should
>> definitely be a safer move.  But that requires more fundamental
>> framework code change I guess.
>
> [1] maintained at local git? not mainline..
>
>>
>> [1] https://github.com/allwinner-zh/bootloader/blob/master/u-boot-2011.09/drivers/mmc/mmc.c#L2220
>> [2] http://thread.gmane.org/gmane.comp.hardware.netbook.arm.sunxi/18442/focus=18480
>
> Well, i don't know how ulf think about this.
> I want to find the root cause and fix it.
> If this is sunxi-mmc controller's problem, i think it can be fixed in sunxi-mmc controller.
>

I agree.  Root cause should be found and fix may eventually come.
Other false rumours were already there in the Internet that sunxi mmc
controller did not support cards smaller than 512MB...  Well, I may
try (not in the near future) how vendor's linux-sunxi-3.4 will work
with this card.

Cheers,

                yousong


>
> Best Regards,
> Jaehoon Chung
>>
>> Regards,
>>
>>                 yousong
>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>>>
>>>>>> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
>>>>>> ---
>>>>>>   drivers/mmc/core/core.c   |    4 ++++
>>>>>>   drivers/mmc/core/sd_ops.c |    5 +++--
>>>>>>   drivers/mmc/core/sd_ops.h |   10 ++++++++--
>>>>>>   3 files changed, 15 insertions(+), 4 deletions(-)
>>>>>>
>>>>>
>>>>> I test this patchset with my already-working sd cards, seems ok.
>>>>> Actually I don't have a card to test like yours that need retry switch HS,
>>>>> but from the changes itself, it's harmless to other cards.
>>>>
>>>> Thanks for the testing.
>>>>
>>>> Those cards normally won't burn out after MMC_CMD_RETRIES access mode switch
>>>> failures, right?
>>>>
>>>>>
>>>>> so,
>>>>>
>>>>> Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>>>
>>>>> BTW, I can't find the cover letter? And another should be clarified, plz see
>>>>> comment blow
>>>>>
>>>>>
>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>>> index 9ad73f3..e726bb1 100644
>>>>>> --- a/drivers/mmc/core/core.c
>>>>>> +++ b/drivers/mmc/core/core.c
>>>>>> @@ -468,11 +468,13 @@ static void mmc_wait_for_req_done(struct mmc_host
>>>>>> *host,
>>>>>>                                   struct mmc_request *mrq)
>>>>>>   {
>>>>>>         struct mmc_command *cmd;
>>>>>> +       struct mmc_data *data;
>>>>>>
>>>>>>         while (1) {
>>>>>>                 wait_for_completion(&mrq->completion);
>>>>>>
>>>>>>                 cmd = mrq->cmd;
>>>>>> +               data = mrq->data;
>>>>>>
>>>>>>                 /*
>>>>>>                  * If host has timed out waiting for the sanitize
>>>>>> @@ -501,6 +503,8 @@ static void mmc_wait_for_req_done(struct mmc_host
>>>>>> *host,
>>>>>>                          mmc_hostname(host), cmd->opcode, cmd->error);
>>>>>>                 cmd->retries--;
>>>>>>                 cmd->error = 0;
>>>>>> +               if (data)
>>>>>> +                       data->error = 0;
>>>>>
>>>>>
>>>>> What's this change for?
>>>>
>>>> sunxi-mmc will set both cmd->error and data->error to -ETIMEDOUT in case of any
>>>> interrupt error bit was set (related code within `sunxi_finalize_request()').
>>>>
>>>> `__mmc_sd_switch()' thought it was a error case if data->error != 0.
>>>>
>>>> So the data->error bit needs to be cleared before next retry.
>>>>
>>>> Regards,
>>>>
>>>>                 yousnog
>>>>
>>>>>
>>>>>
>>>>>>                 __mmc_start_request(host, mrq);
>>>>>>         }
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
>>>>>> index 48d0c93..22bef3c 100644
>>>>>> --- a/drivers/mmc/core/sd_ops.c
>>>>>> +++ b/drivers/mmc/core/sd_ops.c
>>>>>> @@ -304,8 +304,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
>>>>>>         return 0;
>>>>>>   }
>>>>>>
>>>>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>>> -       u8 value, u8 *resp)
>>>>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>>> +       u8 value, u8 *resp, int retries)
>>>>>>   {
>>>>>>         struct mmc_request mrq = {NULL};
>>>>>>         struct mmc_command cmd = {0};
>>>>>> @@ -328,6 +328,7 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int
>>>>>> group,
>>>>>>         cmd.arg &= ~(0xF << (group * 4));
>>>>>>         cmd.arg |= value << (group * 4);
>>>>>>         cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>>>>>> +       cmd.retries = retries;
>>>>>>
>>>>>>         data.blksz = 64;
>>>>>>         data.blocks = 1;
>>>>>> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
>>>>>> index ffc2305..a53c51e 100644
>>>>>> --- a/drivers/mmc/core/sd_ops.h
>>>>>> +++ b/drivers/mmc/core/sd_ops.h
>>>>>> @@ -17,9 +17,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32
>>>>>> ocr, u32 *rocr);
>>>>>>   int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
>>>>>>   int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
>>>>>>   int mmc_app_send_scr(struct mmc_card *card, u32 *scr);
>>>>>> -int mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>>> -       u8 value, u8 *resp);
>>>>>>   int mmc_app_sd_status(struct mmc_card *card, void *ssr);
>>>>>>
>>>>>> +int __mmc_sd_switch(struct mmc_card *card, int mode, int group,
>>>>>> +       u8 value, u8 *resp, int retries);
>>>>>> +static inline int mmc_sd_switch(struct mmc_card *card, int mode, int
>>>>>> group,
>>>>>> +       u8 value, u8 *resp)
>>>>>> +{
>>>>>> +       return __mmc_sd_switch(card, mode, group, value, resp, 0);
>>>>>> +}
>>>>>> +
>>>>>>   #endif
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best Regards
>>>>> Shawn Lin
>>>>>
>>>> --
>>>> 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
>>>>
>>>
>> --
>> 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
>>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-09-17  8:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-09 16:33 [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Yousong Zhou
2015-09-09 16:33 ` [PATCH 2/3] mmc: sd: Allow calling sd mode switch with retries Yousong Zhou
2015-09-10  0:33   ` Shawn Lin
2015-09-10  3:02     ` Yousong Zhou
2015-09-16  2:36       ` Jaehoon Chung
2015-09-16  3:09         ` Yousong Zhou
2015-09-16  4:17           ` Jaehoon Chung
2015-09-17  8:29             ` Yousong Zhou
2015-09-09 16:33 ` [PATCH 3/3] mmc: sd: Retry switching to highspeed mode in case of error Yousong Zhou
2015-09-09 19:29 ` [PATCH 1/3] mmc: sd: Remove superfluous error code assignment Hans de Goede
2015-09-15 11:34 ` 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.