Skip to contentSkip to conversation

Move the judgement of timeout into mmc_switch_status_error()

PATCH NAK 장영진 2 participants 3 messages Dec 24, 2015 MMC, SD and SDIO

lore
Share & tools
lore

장영진

author
#0 lore raw

PATCH 1/1 Move the judgement of timeout into mmc_switch_status_error()

Quoted 1 line

From bc1e491b29d9199f16c6e002a69f9377677b93af Mon Sep 17 00:00:00 2001

From: "yj84.jang" <[email protected]>

Date: Wed, 16 Dec 2015 08:52:47 +0900

Subject: [PATCH] Move the judgement of timeout into mmc_switch_status_error()

__mmc_switch() is possible to misjudge error.

Although jiffies was overed timeout,

the chance about "R1_CURRENT_STATE(status) != R1_STATE_PRG" is exists.

In other words, Successful completion of CMD6 with timeout error.

Kernel expects failure of __mmc_switch,

but card is completed CMD6 in this case.

It makes mismatch status between mmc subsystem and mmc card.

Especially, in case of partition switch,

If timeout occurs with normal status of CMD13 response unfortunatley,

"main_md->curr" will not be set, even card switched successfully.

Checking state of mmc card is needed when judging an timeout error,

if it is just timeout or timeout with error.

---

drivers/mmc/core/mmc_ops.c | 5 +++--

1 file changed, 3 insertions(+), 2 deletions(-)
drivers/mmc/core/mmc_ops.c +0 -0

index 1f44426..033f475 100644

--- a/drivers/mmc/core/mmc_ops.c

+++ b/drivers/mmc/core/mmc_ops.c

@@ -456,6 +456,8 @@ int mmc_switch_status_error(struct mmc_host *host, u32 status)

                    if (status & R1_SPI_ILLEGAL_COMMAND)

                               return -EBADMSG;

         } else {

+ if (R1_CURRENT_STATE(status) == R1_STATE_PRG)

+ return -ETIMEDOUT;

if (status & 0xFDFFA000)

pr_warn("%s: unexpected status %#x after
switch\n",

                                         mmc_hostname(host), status);

@@ -568,8 +570,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,

if (time_after(jiffies, timeout)) {

pr_err("%s: Card stuck in programming state!
%s\n",

                                         mmc_hostname(host), __func__);

- err = -ETIMEDOUT;

- goto out;

+ break;

                    }

         } while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
Signature · 1 line
1.7.9.5
#1 lore raw

Hi,

Before send the patch at mailing list, you need to make the patch as mainline style. Maybe..maintainer should not review this patch.

Refer to "Linux kernel coding style" and "submitting Patches". Also set your email environment. (Documentation/email-clients.txt)

Best Regards,
Jaehoon Chung

Quoted 105 lines · 장영진
Quoted 1 line

From bc1e491b29d9199f16c6e002a69f9377677b93af Mon Sep 17 00:00:00 2001

From: "yj84.jang" <[email protected]>

Date: Wed, 16 Dec 2015 08:52:47 +0900

Subject: [PATCH] Move the judgement of timeout into mmc_switch_status_error()

__mmc_switch() is possible to misjudge error.

Although jiffies was overed timeout,

the chance about "R1_CURRENT_STATE(status) != R1_STATE_PRG" is exists.

In other words, Successful completion of CMD6 with timeout error.

Kernel expects failure of __mmc_switch,

but card is completed CMD6 in this case.

It makes mismatch status between mmc subsystem and mmc card.

Especially, in case of partition switch,

If timeout occurs with normal status of CMD13 response unfortunatley,

"main_md->curr" will not be set, even card switched successfully.

Checking state of mmc card is needed when judging an timeout error,

if it is just timeout or timeout with error.

---

drivers/mmc/core/mmc_ops.c | 5 +++--

1 file changed, 3 insertions(+), 2 deletions(-)
drivers/mmc/core/mmc_ops.c +0 -0

index 1f44426..033f475 100644

--- a/drivers/mmc/core/mmc_ops.c

+++ b/drivers/mmc/core/mmc_ops.c

drivers/mmc/core/mmc_ops.c +2 -0
@@ -456,6 +456,8 @@ int mmc_switch_status_error(struct mmc_host *host, u32
 status)
 
                    if (status & R1_SPI_ILLEGAL_COMMAND)
 
                               return -EBADMSG;
 
         } else {
 
+                   if (R1_CURRENT_STATE(status) == R1_STATE_PRG)
 
+                              return -ETIMEDOUT;
 
                    if (status & 0xFDFFA000)
 
                               pr_warn("%s: unexpected status %#x after

switch\n",

                                         mmc_hostname(host), status);
drivers/mmc/core/mmc_ops.c +1 -2
@@ -568,8 +570,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8
 index, u8 value,
 
                    if (time_after(jiffies, timeout)) {
 
                               pr_err("%s: Card stuck in programming state!
 %s\n",
 
                                         mmc_hostname(host), __func__);
 
-                               err = -ETIMEDOUT;
 
-                               goto out;
 
+                              break;
 
                    }
 
         } while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
#2 lore raw

Hi,

NAK.

Maybe..It seems that this is not the patch based on latest kernel.. Which kernel version did you use? Plz, check the latest kernel.

Best Regards,
Jaehoon Chung

Quoted 124 lines · Jaehoon Chung

Hi,

Before send the patch at mailing list, you need to make the patch as mainline style. Maybe..maintainer should not review this patch.

Refer to "Linux kernel coding style" and "submitting Patches". Also set your email environment. (Documentation/email-clients.txt)

Best Regards,
Jaehoon Chung

Quoted 95 lines · 장영진
Quoted 1 line

From bc1e491b29d9199f16c6e002a69f9377677b93af Mon Sep 17 00:00:00 2001

From: "yj84.jang" <[email protected]>

Date: Wed, 16 Dec 2015 08:52:47 +0900

Subject: [PATCH] Move the judgement of timeout into mmc_switch_status_error()

__mmc_switch() is possible to misjudge error.

Although jiffies was overed timeout,

the chance about "R1_CURRENT_STATE(status) != R1_STATE_PRG" is exists.

In other words, Successful completion of CMD6 with timeout error.

Kernel expects failure of __mmc_switch,

but card is completed CMD6 in this case.

It makes mismatch status between mmc subsystem and mmc card.

Especially, in case of partition switch,

If timeout occurs with normal status of CMD13 response unfortunatley,

"main_md->curr" will not be set, even card switched successfully.

Checking state of mmc card is needed when judging an timeout error,

if it is just timeout or timeout with error.

---

drivers/mmc/core/mmc_ops.c | 5 +++--

1 file changed, 3 insertions(+), 2 deletions(-)
drivers/mmc/core/mmc_ops.c +0 -0

index 1f44426..033f475 100644

--- a/drivers/mmc/core/mmc_ops.c

+++ b/drivers/mmc/core/mmc_ops.c

drivers/mmc/core/mmc_ops.c +2 -0
@@ -456,6 +456,8 @@ int mmc_switch_status_error(struct mmc_host *host, u32
 status)
 
                    if (status & R1_SPI_ILLEGAL_COMMAND)
 
                               return -EBADMSG;
 
         } else {
 
+                   if (R1_CURRENT_STATE(status) == R1_STATE_PRG)
 
+                              return -ETIMEDOUT;
 
                    if (status & 0xFDFFA000)
 
                               pr_warn("%s: unexpected status %#x after

switch\n",

                                         mmc_hostname(host), status);
drivers/mmc/core/mmc_ops.c +0 -0
@@ -568,8 +570,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8
 index, u8 value,
 
                    if (time_after(jiffies, timeout)) {
 
                               pr_err("%s: Card stuck in programming state!
 %s\n",
 
                                         mmc_hostname(host), __func__);

drivers/mmc/core/mmc_ops.c int __mmc_switch(struct mmc_card *card, u8 set, u8

Quoted 10 lines
drivers/mmc/core/mmc_ops.c +1 -2
 
-                               err = -ETIMEDOUT;
 
-                               goto out;
 
+                              break;
 
                    }
 
         } while (R1_CURRENT_STATE(status) == R1_STATE_PRG);

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html