* [PATCH 2/3] mmc: core: add ignorance case for CMD13 CRC error
@ 2013-08-21 12:42 Seungwon Jeon
2013-08-23 9:21 ` Ulf Hansson
0 siblings, 1 reply; 9+ messages in thread
From: Seungwon Jeon @ 2013-08-21 12:42 UTC (permalink / raw)
To: linux-mmc; +Cc: 'Chris Ball'
While speed mode is changed, CMD13 cannot be guaranteed.
According to the spec., it is not recommended to use CMD13
to check the busy completion of the timing change.
If CMD13 is used in this case, CRC error must be ignored.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
drivers/mmc/core/mmc_ops.c | 23 ++++++++++++++++++++---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 837fc73..f5dfa9c 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -23,6 +23,9 @@
#define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
+static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
+ bool ignore_crc);
+
static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
{
int err;
@@ -380,6 +383,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
struct mmc_command cmd = {0};
unsigned long timeout;
u32 status;
+ bool ignore_crc;
BUG_ON(!card);
BUG_ON(!card->host);
@@ -408,10 +412,15 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
if (!use_busy_signal)
return 0;
- /* Must check status to be sure of no errors */
+ /*
+ * Must check status to be sure of no errors
+ * If CMD13 is to check the busy completion of the timing change,
+ * disable the check of CRC error.
+ */
+ ignore_crc = (index == EXT_CSD_HS_TIMING) ? true : false;
timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
do {
- err = mmc_send_status(card, &status);
+ err = __mmc_send_status(card, &status, ignore_crc);
if (err)
return err;
if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
@@ -449,7 +458,8 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
}
EXPORT_SYMBOL_GPL(mmc_switch);
-int mmc_send_status(struct mmc_card *card, u32 *status)
+static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
+ bool ignore_crc)
{
int err;
struct mmc_command cmd = {0};
@@ -461,6 +471,8 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
if (!mmc_host_is_spi(card->host))
cmd.arg = card->rca << 16;
cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
+ if (ignore_crc)
+ cmd.flags &= ~MMC_RSP_CRC;
err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
if (err)
@@ -475,6 +487,11 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
return 0;
}
+int mmc_send_status(struct mmc_card *card, u32 *status)
+{
+ return __mmc_send_status(card, status, false);
+}
+
static int
mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
u8 len)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] mmc: core: add ignorance case for CMD13 CRC error
2013-08-21 12:42 [PATCH 2/3] mmc: core: add ignorance case for CMD13 CRC error Seungwon Jeon
@ 2013-08-23 9:21 ` Ulf Hansson
2013-08-26 7:20 ` [PATCH v2] mmc: " Seungwon Jeon
0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2013-08-23 9:21 UTC (permalink / raw)
To: Seungwon Jeon; +Cc: linux-mmc, Chris Ball
On 21 August 2013 14:42, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> While speed mode is changed, CMD13 cannot be guaranteed.
> According to the spec., it is not recommended to use CMD13
> to check the busy completion of the timing change.
> If CMD13 is used in this case, CRC error must be ignored.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> drivers/mmc/core/mmc_ops.c | 23 ++++++++++++++++++++---
> 1 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 837fc73..f5dfa9c 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -23,6 +23,9 @@
>
> #define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
>
> +static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
> + bool ignore_crc);
> +
I suggest to move the implementation here as well. Thus no specific
pre-declaration is needed.
Otherwise it looks good!
Kind regards
Ulf Hansson
> static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> {
> int err;
> @@ -380,6 +383,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> struct mmc_command cmd = {0};
> unsigned long timeout;
> u32 status;
> + bool ignore_crc;
>
> BUG_ON(!card);
> BUG_ON(!card->host);
> @@ -408,10 +412,15 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> if (!use_busy_signal)
> return 0;
>
> - /* Must check status to be sure of no errors */
> + /*
> + * Must check status to be sure of no errors
> + * If CMD13 is to check the busy completion of the timing change,
> + * disable the check of CRC error.
> + */
> + ignore_crc = (index == EXT_CSD_HS_TIMING) ? true : false;
> timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
> do {
> - err = mmc_send_status(card, &status);
> + err = __mmc_send_status(card, &status, ignore_crc);
> if (err)
> return err;
> if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
> @@ -449,7 +458,8 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> }
> EXPORT_SYMBOL_GPL(mmc_switch);
>
> -int mmc_send_status(struct mmc_card *card, u32 *status)
> +static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
> + bool ignore_crc)
> {
> int err;
> struct mmc_command cmd = {0};
> @@ -461,6 +471,8 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
> if (!mmc_host_is_spi(card->host))
> cmd.arg = card->rca << 16;
> cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> + if (ignore_crc)
> + cmd.flags &= ~MMC_RSP_CRC;
>
> err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> if (err)
> @@ -475,6 +487,11 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
> return 0;
> }
>
> +int mmc_send_status(struct mmc_card *card, u32 *status)
> +{
> + return __mmc_send_status(card, status, false);
> +}
> +
> static int
> mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> u8 len)
> --
> 1.7.0.4
>
>
> --
> 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] 9+ messages in thread* [PATCH v2] mmc: add ignorance case for CMD13 CRC error
2013-08-23 9:21 ` Ulf Hansson
@ 2013-08-26 7:20 ` Seungwon Jeon
2013-09-03 9:59 ` Seungwon Jeon
2013-09-03 12:58 ` Ulf Hansson
0 siblings, 2 replies; 9+ messages in thread
From: Seungwon Jeon @ 2013-08-26 7:20 UTC (permalink / raw)
To: 'linux-mmc'; +Cc: 'Chris Ball', 'Ulf Hansson'
While speed mode is changed, CMD13 cannot be guaranteed.
According to the spec., it is not recommended to use CMD13
to check the busy completion of the timing change.
If CMD13 is used in this case, CRC error must be ignored.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Change in v2:
- Removed function declaration.(From Ulf Hansson)
drivers/mmc/core/mmc_ops.c | 70 ++++++++++++++++++++++++++-----------------
1 files changed, 42 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index ef18348..1464c1e 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -23,6 +23,40 @@
#define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
+static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
+ bool ignore_crc)
+{
+ int err;
+ struct mmc_command cmd = {0};
+
+ BUG_ON(!card);
+ BUG_ON(!card->host);
+
+ cmd.opcode = MMC_SEND_STATUS;
+ if (!mmc_host_is_spi(card->host))
+ cmd.arg = card->rca << 16;
+ cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
+ if (ignore_crc)
+ cmd.flags &= ~MMC_RSP_CRC;
+
+ err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
+ if (err)
+ return err;
+
+ /* NOTE: callers are required to understand the difference
+ * between "native" and SPI format status words!
+ */
+ if (status)
+ *status = cmd.resp[0];
+
+ return 0;
+}
+
+int mmc_send_status(struct mmc_card *card, u32 *status)
+{
+ return __mmc_send_status(card, status, false);
+}
+
static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
{
int err;
@@ -380,6 +414,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
struct mmc_command cmd = {0};
unsigned long timeout;
u32 status;
+ bool ignore_crc;
BUG_ON(!card);
BUG_ON(!card->host);
@@ -408,10 +443,15 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
if (!use_busy_signal)
return 0;
- /* Must check status to be sure of no errors */
+ /*
+ * Must check status to be sure of no errors
+ * If CMD13 is to check the busy completion of the timing change,
+ * disable the check of CRC error.
+ */
+ ignore_crc = (index == EXT_CSD_HS_TIMING) ? true : false;
timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
do {
- err = mmc_send_status(card, &status);
+ err = __mmc_send_status(card, &status, ignore_crc);
if (err)
return err;
if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
@@ -449,32 +489,6 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
}
EXPORT_SYMBOL_GPL(mmc_switch);
-int mmc_send_status(struct mmc_card *card, u32 *status)
-{
- int err;
- struct mmc_command cmd = {0};
-
- BUG_ON(!card);
- BUG_ON(!card->host);
-
- cmd.opcode = MMC_SEND_STATUS;
- if (!mmc_host_is_spi(card->host))
- cmd.arg = card->rca << 16;
- cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
-
- err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
- if (err)
- return err;
-
- /* NOTE: callers are required to understand the difference
- * between "native" and SPI format status words!
- */
- if (status)
- *status = cmd.resp[0];
-
- return 0;
-}
-
static int
mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
u8 len)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* RE: [PATCH v2] mmc: add ignorance case for CMD13 CRC error
2013-08-26 7:20 ` [PATCH v2] mmc: " Seungwon Jeon
@ 2013-09-03 9:59 ` Seungwon Jeon
2013-09-03 12:58 ` Ulf Hansson
1 sibling, 0 replies; 9+ messages in thread
From: Seungwon Jeon @ 2013-09-03 9:59 UTC (permalink / raw)
To: 'Seungwon Jeon', 'linux-mmc'
Cc: 'Chris Ball', 'Ulf Hansson'
On Mon, August 26, 2013, Seungwon Jeon wrote:
> While speed mode is changed, CMD13 cannot be guaranteed.
> According to the spec., it is not recommended to use CMD13
> to check the busy completion of the timing change.
> If CMD13 is used in this case, CRC error must be ignored.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> Change in v2:
> - Removed function declaration.(From Ulf Hansson)
Hi Ulf,
I hope that you find this change.
Thanks,
Seungwon Jeon
>
> drivers/mmc/core/mmc_ops.c | 70 ++++++++++++++++++++++++++-----------------
> 1 files changed, 42 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index ef18348..1464c1e 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -23,6 +23,40 @@
>
> #define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
>
> +static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
> + bool ignore_crc)
> +{
> + int err;
> + struct mmc_command cmd = {0};
> +
> + BUG_ON(!card);
> + BUG_ON(!card->host);
> +
> + cmd.opcode = MMC_SEND_STATUS;
> + if (!mmc_host_is_spi(card->host))
> + cmd.arg = card->rca << 16;
> + cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> + if (ignore_crc)
> + cmd.flags &= ~MMC_RSP_CRC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> + if (err)
> + return err;
> +
> + /* NOTE: callers are required to understand the difference
> + * between "native" and SPI format status words!
> + */
> + if (status)
> + *status = cmd.resp[0];
> +
> + return 0;
> +}
> +
> +int mmc_send_status(struct mmc_card *card, u32 *status)
> +{
> + return __mmc_send_status(card, status, false);
> +}
> +
> static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> {
> int err;
> @@ -380,6 +414,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> struct mmc_command cmd = {0};
> unsigned long timeout;
> u32 status;
> + bool ignore_crc;
>
> BUG_ON(!card);
> BUG_ON(!card->host);
> @@ -408,10 +443,15 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> if (!use_busy_signal)
> return 0;
>
> - /* Must check status to be sure of no errors */
> + /*
> + * Must check status to be sure of no errors
> + * If CMD13 is to check the busy completion of the timing change,
> + * disable the check of CRC error.
> + */
> + ignore_crc = (index == EXT_CSD_HS_TIMING) ? true : false;
> timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
> do {
> - err = mmc_send_status(card, &status);
> + err = __mmc_send_status(card, &status, ignore_crc);
> if (err)
> return err;
> if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
> @@ -449,32 +489,6 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> }
> EXPORT_SYMBOL_GPL(mmc_switch);
>
> -int mmc_send_status(struct mmc_card *card, u32 *status)
> -{
> - int err;
> - struct mmc_command cmd = {0};
> -
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> -
> - cmd.opcode = MMC_SEND_STATUS;
> - if (!mmc_host_is_spi(card->host))
> - cmd.arg = card->rca << 16;
> - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> -
> - err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> - if (err)
> - return err;
> -
> - /* NOTE: callers are required to understand the difference
> - * between "native" and SPI format status words!
> - */
> - if (status)
> - *status = cmd.resp[0];
> -
> - return 0;
> -}
> -
> static int
> mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> u8 len)
> --
> 1.7.0.4
>
>
> --
> 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] 9+ messages in thread* Re: [PATCH v2] mmc: add ignorance case for CMD13 CRC error
2013-08-26 7:20 ` [PATCH v2] mmc: " Seungwon Jeon
2013-09-03 9:59 ` Seungwon Jeon
@ 2013-09-03 12:58 ` Ulf Hansson
2013-09-04 1:53 ` Seungwon Jeon
1 sibling, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2013-09-03 12:58 UTC (permalink / raw)
To: Seungwon Jeon; +Cc: linux-mmc, Chris Ball
On 26 August 2013 09:20, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> While speed mode is changed, CMD13 cannot be guaranteed.
> According to the spec., it is not recommended to use CMD13
> to check the busy completion of the timing change.
> If CMD13 is used in this case, CRC error must be ignored.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> Change in v2:
> - Removed function declaration.(From Ulf Hansson)
>
> drivers/mmc/core/mmc_ops.c | 70 ++++++++++++++++++++++++++-----------------
> 1 files changed, 42 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index ef18348..1464c1e 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -23,6 +23,40 @@
>
> #define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
>
> +static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
> + bool ignore_crc)
Is there any specific reason to you made this function inline?
> +{
> + int err;
> + struct mmc_command cmd = {0};
> +
> + BUG_ON(!card);
> + BUG_ON(!card->host);
> +
> + cmd.opcode = MMC_SEND_STATUS;
> + if (!mmc_host_is_spi(card->host))
> + cmd.arg = card->rca << 16;
> + cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> + if (ignore_crc)
> + cmd.flags &= ~MMC_RSP_CRC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> + if (err)
> + return err;
> +
> + /* NOTE: callers are required to understand the difference
> + * between "native" and SPI format status words!
> + */
> + if (status)
> + *status = cmd.resp[0];
> +
> + return 0;
> +}
> +
> +int mmc_send_status(struct mmc_card *card, u32 *status)
> +{
> + return __mmc_send_status(card, status, false);
> +}
> +
> static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> {
> int err;
> @@ -380,6 +414,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> struct mmc_command cmd = {0};
> unsigned long timeout;
> u32 status;
> + bool ignore_crc;
>
> BUG_ON(!card);
> BUG_ON(!card->host);
> @@ -408,10 +443,15 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> if (!use_busy_signal)
> return 0;
>
> - /* Must check status to be sure of no errors */
> + /*
> + * Must check status to be sure of no errors
> + * If CMD13 is to check the busy completion of the timing change,
> + * disable the check of CRC error.
> + */
> + ignore_crc = (index == EXT_CSD_HS_TIMING) ? true : false;
You need to consider MMC_CAP_WAIT_WHILE_BUSY in the condition for
"ignore_crc" as well. Otherwise you will ignore CRC in cases where we
actually can detect it.
Kind regards
Ulf Hansson
> timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
> do {
> - err = mmc_send_status(card, &status);
> + err = __mmc_send_status(card, &status, ignore_crc);
> if (err)
> return err;
> if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
> @@ -449,32 +489,6 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> }
> EXPORT_SYMBOL_GPL(mmc_switch);
>
> -int mmc_send_status(struct mmc_card *card, u32 *status)
> -{
> - int err;
> - struct mmc_command cmd = {0};
> -
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> -
> - cmd.opcode = MMC_SEND_STATUS;
> - if (!mmc_host_is_spi(card->host))
> - cmd.arg = card->rca << 16;
> - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> -
> - err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> - if (err)
> - return err;
> -
> - /* NOTE: callers are required to understand the difference
> - * between "native" and SPI format status words!
> - */
> - if (status)
> - *status = cmd.resp[0];
> -
> - return 0;
> -}
> -
> static int
> mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> u8 len)
> --
> 1.7.0.4
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH v2] mmc: add ignorance case for CMD13 CRC error
2013-09-03 12:58 ` Ulf Hansson
@ 2013-09-04 1:53 ` Seungwon Jeon
2013-09-04 12:21 ` [PATCH v3] " Seungwon Jeon
0 siblings, 1 reply; 9+ messages in thread
From: Seungwon Jeon @ 2013-09-04 1:53 UTC (permalink / raw)
To: 'Ulf Hansson'; +Cc: 'linux-mmc', 'Chris Ball'
On Tue, September 03, 2013, Ulf Hansson wrote:
> On 26 August 2013 09:20, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > While speed mode is changed, CMD13 cannot be guaranteed.
> > According to the spec., it is not recommended to use CMD13
> > to check the busy completion of the timing change.
> > If CMD13 is used in this case, CRC error must be ignored.
> >
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> > Change in v2:
> > - Removed function declaration.(From Ulf Hansson)
> >
> > drivers/mmc/core/mmc_ops.c | 70 ++++++++++++++++++++++++++-----------------
> > 1 files changed, 42 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> > index ef18348..1464c1e 100644
> > --- a/drivers/mmc/core/mmc_ops.c
> > +++ b/drivers/mmc/core/mmc_ops.c
> > @@ -23,6 +23,40 @@
> >
> > #define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
> >
> > +static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
> > + bool ignore_crc)
>
> Is there any specific reason to you made this function inline?
First, when considering that mmc_send_status() just wraps __mmc_send_status(),
we can reduce call stack though it seems trivial.
And __mmc_send_status() will be used only in 'mmc_ops.c'. Currently it is called
in do~while loop. inline function could be helpful to avoid frequent call.
>
> > +{
> > + int err;
> > + struct mmc_command cmd = {0};
> > +
> > + BUG_ON(!card);
> > + BUG_ON(!card->host);
> > +
> > + cmd.opcode = MMC_SEND_STATUS;
> > + if (!mmc_host_is_spi(card->host))
> > + cmd.arg = card->rca << 16;
> > + cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> > + if (ignore_crc)
> > + cmd.flags &= ~MMC_RSP_CRC;
> > +
> > + err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> > + if (err)
> > + return err;
> > +
> > + /* NOTE: callers are required to understand the difference
> > + * between "native" and SPI format status words!
> > + */
> > + if (status)
> > + *status = cmd.resp[0];
> > +
> > + return 0;
> > +}
> > +
> > +int mmc_send_status(struct mmc_card *card, u32 *status)
> > +{
> > + return __mmc_send_status(card, status, false);
> > +}
> > +
> > static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> > {
> > int err;
> > @@ -380,6 +414,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> > struct mmc_command cmd = {0};
> > unsigned long timeout;
> > u32 status;
> > + bool ignore_crc;
> >
> > BUG_ON(!card);
> > BUG_ON(!card->host);
> > @@ -408,10 +443,15 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> > if (!use_busy_signal)
> > return 0;
> >
> > - /* Must check status to be sure of no errors */
> > + /*
> > + * Must check status to be sure of no errors
> > + * If CMD13 is to check the busy completion of the timing change,
> > + * disable the check of CRC error.
> > + */
> > + ignore_crc = (index == EXT_CSD_HS_TIMING) ? true : false;
>
> You need to consider MMC_CAP_WAIT_WHILE_BUSY in the condition for
> "ignore_crc" as well. Otherwise you will ignore CRC in cases where we
> actually can detect it.
Ok, it would be good.
Thanks,
Seungwon Jeon
>
> Kind regards
> Ulf Hansson
>
>
> > timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
> > do {
> > - err = mmc_send_status(card, &status);
> > + err = __mmc_send_status(card, &status, ignore_crc);
> > if (err)
> > return err;
> > if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
> > @@ -449,32 +489,6 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> > }
> > EXPORT_SYMBOL_GPL(mmc_switch);
> >
> > -int mmc_send_status(struct mmc_card *card, u32 *status)
> > -{
> > - int err;
> > - struct mmc_command cmd = {0};
> > -
> > - BUG_ON(!card);
> > - BUG_ON(!card->host);
> > -
> > - cmd.opcode = MMC_SEND_STATUS;
> > - if (!mmc_host_is_spi(card->host))
> > - cmd.arg = card->rca << 16;
> > - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> > -
> > - err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> > - if (err)
> > - return err;
> > -
> > - /* NOTE: callers are required to understand the difference
> > - * between "native" and SPI format status words!
> > - */
> > - if (status)
> > - *status = cmd.resp[0];
> > -
> > - return 0;
> > -}
> > -
> > static int
> > mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> > u8 len)
> > --
> > 1.7.0.4
> >
> >
> --
> 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] 9+ messages in thread* [PATCH v3] mmc: add ignorance case for CMD13 CRC error
2013-09-04 1:53 ` Seungwon Jeon
@ 2013-09-04 12:21 ` Seungwon Jeon
2013-09-04 12:31 ` Ulf Hansson
0 siblings, 1 reply; 9+ messages in thread
From: Seungwon Jeon @ 2013-09-04 12:21 UTC (permalink / raw)
To: 'linux-mmc'; +Cc: 'Chris Ball', 'Ulf Hansson'
While speed mode is changed, CMD13 cannot be guaranteed.
According to the spec., it is not recommended to use CMD13
to check the busy completion of the timing change.
If CMD13 is used in this case, CRC error must be ignored.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Change in v3:
- Adjusted condition with MMC_CAP_WAIT_WHILE_BUSY.(From Ulf Hansson)
Change in v2:
- Removed function declaration.(From Ulf Hansson)
drivers/mmc/core/mmc_ops.c | 73 +++++++++++++++++++++++++++-----------------
1 files changed, 45 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index ef18348..37f7d70 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -23,6 +23,40 @@
#define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
+static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
+ bool ignore_crc)
+{
+ int err;
+ struct mmc_command cmd = {0};
+
+ BUG_ON(!card);
+ BUG_ON(!card->host);
+
+ cmd.opcode = MMC_SEND_STATUS;
+ if (!mmc_host_is_spi(card->host))
+ cmd.arg = card->rca << 16;
+ cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
+ if (ignore_crc)
+ cmd.flags &= ~MMC_RSP_CRC;
+
+ err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
+ if (err)
+ return err;
+
+ /* NOTE: callers are required to understand the difference
+ * between "native" and SPI format status words!
+ */
+ if (status)
+ *status = cmd.resp[0];
+
+ return 0;
+}
+
+int mmc_send_status(struct mmc_card *card, u32 *status)
+{
+ return __mmc_send_status(card, status, false);
+}
+
static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
{
int err;
@@ -380,6 +414,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
struct mmc_command cmd = {0};
unsigned long timeout;
u32 status;
+ bool ignore_crc = false;
BUG_ON(!card);
BUG_ON(!card->host);
@@ -408,10 +443,18 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
if (!use_busy_signal)
return 0;
- /* Must check status to be sure of no errors */
+ /*
+ * Must check status to be sure of no errors
+ * If CMD13 is to check the busy completion of the timing change,
+ * disable the check of CRC error.
+ */
+ if (index == EXT_CSD_HS_TIMING &&
+ !(card->host->caps & MMC_CAP_WAIT_WHILE_BUSY))
+ ignore_crc = true;
+
timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
do {
- err = mmc_send_status(card, &status);
+ err = __mmc_send_status(card, &status, ignore_crc);
if (err)
return err;
if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
@@ -449,32 +492,6 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
}
EXPORT_SYMBOL_GPL(mmc_switch);
-int mmc_send_status(struct mmc_card *card, u32 *status)
-{
- int err;
- struct mmc_command cmd = {0};
-
- BUG_ON(!card);
- BUG_ON(!card->host);
-
- cmd.opcode = MMC_SEND_STATUS;
- if (!mmc_host_is_spi(card->host))
- cmd.arg = card->rca << 16;
- cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
-
- err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
- if (err)
- return err;
-
- /* NOTE: callers are required to understand the difference
- * between "native" and SPI format status words!
- */
- if (status)
- *status = cmd.resp[0];
-
- return 0;
-}
-
static int
mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
u8 len)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3] mmc: add ignorance case for CMD13 CRC error
2013-09-04 12:21 ` [PATCH v3] " Seungwon Jeon
@ 2013-09-04 12:31 ` Ulf Hansson
2013-09-26 1:41 ` Chris Ball
0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2013-09-04 12:31 UTC (permalink / raw)
To: Seungwon Jeon; +Cc: linux-mmc, Chris Ball
On 4 September 2013 14:21, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> While speed mode is changed, CMD13 cannot be guaranteed.
> According to the spec., it is not recommended to use CMD13
> to check the busy completion of the timing change.
> If CMD13 is used in this case, CRC error must be ignored.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> Change in v3:
> - Adjusted condition with MMC_CAP_WAIT_WHILE_BUSY.(From Ulf Hansson)
>
> Change in v2:
> - Removed function declaration.(From Ulf Hansson)
>
> drivers/mmc/core/mmc_ops.c | 73 +++++++++++++++++++++++++++-----------------
> 1 files changed, 45 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index ef18348..37f7d70 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -23,6 +23,40 @@
>
> #define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
>
> +static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
> + bool ignore_crc)
> +{
> + int err;
> + struct mmc_command cmd = {0};
> +
> + BUG_ON(!card);
> + BUG_ON(!card->host);
> +
> + cmd.opcode = MMC_SEND_STATUS;
> + if (!mmc_host_is_spi(card->host))
> + cmd.arg = card->rca << 16;
> + cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> + if (ignore_crc)
> + cmd.flags &= ~MMC_RSP_CRC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> + if (err)
> + return err;
> +
> + /* NOTE: callers are required to understand the difference
> + * between "native" and SPI format status words!
> + */
> + if (status)
> + *status = cmd.resp[0];
> +
> + return 0;
> +}
> +
> +int mmc_send_status(struct mmc_card *card, u32 *status)
> +{
> + return __mmc_send_status(card, status, false);
> +}
> +
> static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> {
> int err;
> @@ -380,6 +414,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> struct mmc_command cmd = {0};
> unsigned long timeout;
> u32 status;
> + bool ignore_crc = false;
>
> BUG_ON(!card);
> BUG_ON(!card->host);
> @@ -408,10 +443,18 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> if (!use_busy_signal)
> return 0;
>
> - /* Must check status to be sure of no errors */
> + /*
> + * Must check status to be sure of no errors
> + * If CMD13 is to check the busy completion of the timing change,
> + * disable the check of CRC error.
> + */
> + if (index == EXT_CSD_HS_TIMING &&
> + !(card->host->caps & MMC_CAP_WAIT_WHILE_BUSY))
> + ignore_crc = true;
> +
> timeout = jiffies + msecs_to_jiffies(MMC_OPS_TIMEOUT_MS);
> do {
> - err = mmc_send_status(card, &status);
> + err = __mmc_send_status(card, &status, ignore_crc);
> if (err)
> return err;
> if (card->host->caps & MMC_CAP_WAIT_WHILE_BUSY)
> @@ -449,32 +492,6 @@ int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> }
> EXPORT_SYMBOL_GPL(mmc_switch);
>
> -int mmc_send_status(struct mmc_card *card, u32 *status)
> -{
> - int err;
> - struct mmc_command cmd = {0};
> -
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> -
> - cmd.opcode = MMC_SEND_STATUS;
> - if (!mmc_host_is_spi(card->host))
> - cmd.arg = card->rca << 16;
> - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
> -
> - err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> - if (err)
> - return err;
> -
> - /* NOTE: callers are required to understand the difference
> - * between "native" and SPI format status words!
> - */
> - if (status)
> - *status = cmd.resp[0];
> -
> - return 0;
> -}
> -
> static int
> mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> u8 len)
> --
> 1.7.0.4
>
>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3] mmc: add ignorance case for CMD13 CRC error
2013-09-04 12:31 ` Ulf Hansson
@ 2013-09-26 1:41 ` Chris Ball
0 siblings, 0 replies; 9+ messages in thread
From: Chris Ball @ 2013-09-26 1:41 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Seungwon Jeon, linux-mmc
Hi,
On Wed, Sep 04 2013, Ulf Hansson wrote:
> On 4 September 2013 14:21, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> While speed mode is changed, CMD13 cannot be guaranteed.
>> According to the spec., it is not recommended to use CMD13
>> to check the busy completion of the timing change.
>> If CMD13 is used in this case, CRC error must be ignored.
>>
>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Thanks, pushed to mmc-next for 3.13 with Ulf's ACK.
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-09-26 1:41 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-21 12:42 [PATCH 2/3] mmc: core: add ignorance case for CMD13 CRC error Seungwon Jeon
2013-08-23 9:21 ` Ulf Hansson
2013-08-26 7:20 ` [PATCH v2] mmc: " Seungwon Jeon
2013-09-03 9:59 ` Seungwon Jeon
2013-09-03 12:58 ` Ulf Hansson
2013-09-04 1:53 ` Seungwon Jeon
2013-09-04 12:21 ` [PATCH v3] " Seungwon Jeon
2013-09-04 12:31 ` Ulf Hansson
2013-09-26 1:41 ` Chris Ball
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox