Set sdio CMD52/53's retry time as MMC_CMD_RETRIES. As a result,
sdio might benefit from core-internal CMD retry mechanism when
some errors happen(CRC, etc).
Signed-off-by: yongd <[email protected]>
---
drivers/mmc/core/sdio_ops.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
index d29e206..884c27e 100644
--- a/drivers/mmc/core/sdio_ops.c
+++ b/drivers/mmc/core/sdio_ops.c
@@ -86,7 +86,7 @@ static int mmc_io_rw_direct_host(struct mmc_host *host, int write, unsigned fn,
cmd.arg |= in;
cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_AC;
- err = mmc_wait_for_cmd(host, &cmd, 0);
+ err = mmc_wait_for_cmd(host, &cmd, MMC_CMD_RETRIES);
if (err)
return err;
@@ -147,6 +147,7 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
else
cmd.arg |= 0x08000000 | blocks; /* block mode */
cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
+ cmd.retries = MMC_CMD_RETRIES;
data.blksz = blksz;
/* Code in host drivers/fwk assumes that "blocks" always is >=1 */
--
1.7.9.5
On Mon, May 14, 2012 at 2:09 PM, yongd <[email protected]> wrote:
> ?Set sdio CMD52/53's retry time as MMC_CMD_RETRIES. As a result,
> sdio might benefit from core-internal CMD retry mechanism when
> some errors happen(CRC, etc).
>
> Signed-off-by: yongd <[email protected]>
> ---
> ?drivers/mmc/core/sdio_ops.c | ? ?3 ++-
> ?1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
> index d29e206..884c27e 100644
> --- a/drivers/mmc/core/sdio_ops.c
> +++ b/drivers/mmc/core/sdio_ops.c
> @@ -86,7 +86,7 @@ static int mmc_io_rw_direct_host(struct mmc_host *host, int write, unsigned fn,
> ? ? ? ?cmd.arg |= in;
> ? ? ? ?cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_AC;
>
> - ? ? ? err = mmc_wait_for_cmd(host, &cmd, 0);
> + ? ? ? err = mmc_wait_for_cmd(host, &cmd, MMC_CMD_RETRIES);
> ? ? ? ?if (err)
> ? ? ? ? ? ? ? ?return err;
>
> @@ -147,6 +147,7 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
> ? ? ? ?else
> ? ? ? ? ? ? ? ?cmd.arg |= 0x08000000 | blocks; ? ? ? ? /* block mode */
> ? ? ? ?cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
> + ? ? ? cmd.retries = MMC_CMD_RETRIES;
>
> ? ? ? ?data.blksz = blksz;
> ? ? ? ?/* Code in host drivers/fwk assumes that "blocks" always is >=1 */
> --
Looks right to me.
Reviewed-by: Venkatraman S <[email protected]>
Hi Yongd,
From my perspective I don't think this patch is wanted.
A retry mechanism is likely very much hw-SDIO device dependent. Upper
layers (SDIO func driver) is thus the only software that is able to
handle retries correctly.
Kind regards
Ulf Hansson
On 05/14/2012 10:39 AM, yongd wrote:
> Set sdio CMD52/53's retry time as MMC_CMD_RETRIES. As a result,
> sdio might benefit from core-internal CMD retry mechanism when
> some errors happen(CRC, etc).
>
> Signed-off-by: yongd<[email protected]>
> ---
> drivers/mmc/core/sdio_ops.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
> index d29e206..884c27e 100644
> --- a/drivers/mmc/core/sdio_ops.c
> +++ b/drivers/mmc/core/sdio_ops.c
> @@ -86,7 +86,7 @@ static int mmc_io_rw_direct_host(struct mmc_host *host, int write, unsigned fn,
> cmd.arg |= in;
> cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_AC;
>
> - err = mmc_wait_for_cmd(host,&cmd, 0);
> + err = mmc_wait_for_cmd(host,&cmd, MMC_CMD_RETRIES);
> if (err)
> return err;
>
> @@ -147,6 +147,7 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
> else
> cmd.arg |= 0x08000000 | blocks; /* block mode */
> cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
> + cmd.retries = MMC_CMD_RETRIES;
>
> data.blksz = blksz;
> /* Code in host drivers/fwk assumes that "blocks" always is>=1 */
Hi Ulf Hansson,
Thanks for your comments.
Such retry mechanism is a simple CMD-level or sdio-core-level retry which takes advantage of the mmc core internal retry. There is not any side effect, but with it, even those SDIO function driver's without its own retrying mechanism can benefit in some conditions.
Besides, for CMD52, it can be used during SDIO card initialization/detection phase. At that time, SDIO function driver hasn't started to work. Of course we can probably encounter CMD52 error due to some reasons(eg, board-level schematic bad behavior), and such retry can be a potential workaround. Actually, we've encountered exactly such case in our development.
Best Wishes,
Yong Ding
Operating Systems Engineering,
Application Processor Systems Engineering
-----Original Message-----
From: Ulf Hansson [mailto:[email protected]]
Sent: Monday, May 14, 2012 8:58 PM
To: Yong Ding
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc:sdio:retry CMD52/53 when error happens
Hi Yongd,
From my perspective I don't think this patch is wanted.
A retry mechanism is likely very much hw-SDIO device dependent. Upper
layers (SDIO func driver) is thus the only software that is able to
handle retries correctly.
Kind regards
Ulf Hansson
On 05/14/2012 10:39 AM, yongd wrote:
> Set sdio CMD52/53's retry time as MMC_CMD_RETRIES. As a result,
> sdio might benefit from core-internal CMD retry mechanism when
> some errors happen(CRC, etc).
>
> Signed-off-by: yongd<[email protected]>
> ---
> drivers/mmc/core/sdio_ops.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
> index d29e206..884c27e 100644
> --- a/drivers/mmc/core/sdio_ops.c
> +++ b/drivers/mmc/core/sdio_ops.c
> @@ -86,7 +86,7 @@ static int mmc_io_rw_direct_host(struct mmc_host *host, int write, unsigned fn,
> cmd.arg |= in;
> cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_AC;
>
> - err = mmc_wait_for_cmd(host,&cmd, 0);
> + err = mmc_wait_for_cmd(host,&cmd, MMC_CMD_RETRIES);
> if (err)
> return err;
>
> @@ -147,6 +147,7 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
> else
> cmd.arg |= 0x08000000 | blocks; /* block mode */
> cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
> + cmd.retries = MMC_CMD_RETRIES;
>
> data.blksz = blksz;
> /* Code in host drivers/fwk assumes that "blocks" always is>=1 */
Hi Yong Ding,
On 05/15/2012 05:14 AM, Yong Ding wrote:
> Hi Ulf Hansson, Thanks for your comments.
>
> Such retry mechanism is a simple CMD-level or sdio-core-level retry
> which takes advantage of the mmc core internal retry. There is not
> any side effect, but with it, even those SDIO function driver's
> without its own retrying mechanism can benefit in some conditions.
I am not sure about that. It all depends on what kind of protocol the
SDIO func driver is handling and as well what SDIO hw that is connected.
For a WLAN interface you might end up in reading/writing the next
buffer, maybe messing up even more data. Have you considered this?
>
> Besides, for CMD52, it can be used during SDIO card
> initialization/detection phase. At that time, SDIO function driver
> hasn't started to work. Of course we can probably encounter CMD52
> error due to some reasons(eg, board-level schematic bad behavior),
> and such retry can be a potential workaround. Actually, we've
> encountered exactly such case in our development.
I realize that there is a need for this, but I am not sure we always
would like to have "retries" for every CMD52/53 request.
During SDIO initialization before SDIO func driver is started, retries
should be safe since there is no upper protocol to consider yet. Maybe
we should try to create a patch which only enables retries during SDIO
init sequence instead!?
Kind regards
Ulf Hansson