mmc: core: retries counter for data command initialization.

DISCUSSION Konstantin Dorfman 2 participants 4 messages Apr 29, 2014 – May 13, 2014 MMC, SD and SDIO

Share & tools

Konstantin Dorfman

author

2014-04-29 07:55 UTC #0 lore raw

Hi All,

After 6035d9730d5825e6e3c225b721a5847a521d6556 "mmc: fix async request mechanism for sequential read scenarios" mmc layer data path async request handling was separated from mmc commands path.

mmc_wait_for_data_req_done() function is used to wait for completion of the currently running data request, while for commands mmc_wait_for_req_done() is used.

There is code that repeats request cmd->retries times in case of error.
The problem that for data command this counter is not initialized properly as it does for mmc command sending:

int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries)

{
	struct mmc_request mrq = {NULL};

	WARN_ON(!host->claimed);

	memset(cmd->resp, 0, sizeof(cmd->resp));
	cmd->retries = retries;                 <-- supplied by
                                                    caller

	mrq.cmd = cmd;
	cmd->data = NULL;

	mmc_wait_for_req(host, &mrq);

	return cmd->error;
}

I found that there is no similar initialization for data command processing code, so the counter cmd->retries remains always 0.

Lets discuss:
- do we need retries for data request
- what value is good: for command there are 0 or 5 retries
- where to do this initialization:card/block.c:mmc_wait_for_req_done() ?

Thanks,
Kostya

Signature · 3 lines

sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Konstantin Dorfman

author

2014-04-29 11:48 UTC #1 lore raw

On 04/29/14 10:55, Konstantin Dorfman wrote:

Correction inside

Quoted 34 lines

Hi All,
After 6035d9730d5825e6e3c225b721a5847a521d6556 "mmc: fix async request mechanism for sequential read scenarios" mmc layer data path async request handling was separated from mmc commands path.
mmc_wait_for_data_req_done() function is used to wait for completion of the currently running data request, while for commands mmc_wait_for_req_done() is used.
There is code that repeats request cmd->retries times in case of error. The problem that for data command this counter is not initialized properly as it does for mmc command sending:
int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries)
{
     struct mmc_request mrq = {NULL};

     WARN_ON(!host->claimed);

     memset(cmd->resp, 0, sizeof(cmd->resp));
     cmd->retries = retries;                 <-- supplied by
                                                     caller

     mrq.cmd = cmd;
     cmd->data = NULL;

     mmc_wait_for_req(host, &mrq);

     return cmd->error;
}

Quoted 7 lines

I found that there is no similar initialization for data command processing code, so the counter cmd->retries remains always 0.
Lets discuss:
- do we need retries for data request
- what value is good: for command there are 0 or 5 retries
- where to do this initialization:

card/block.c: static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_card *card,
int disable_multi,
struct mmc_queue *mq)

Quoted 4 lines

Thanks,
Kostya

Signature · 3 lines

sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Konstantin Dorfman

author

2014-05-12 14:33 UTC #2 lore raw

Hello Chris, Ulf,

Sorry for nagging.

Do you have opinion on issue with retries counter for data command?

Since till now the code behave as there is no retry, may be it is better to remove dead code? Or the counter need to be initialized properly.

Quoted 34 lines · Konstantin Dorfman

Hi All,
After 6035d9730d5825e6e3c225b721a5847a521d6556 "mmc: fix async request mechanism for sequential read scenarios" mmc layer data path async request handling was separated from mmc commands path.
mmc_wait_for_data_req_done() function is used to wait for completion of the currently running data request, while for commands mmc_wait_for_req_done() is used.
There is code that repeats request cmd->retries times in case of error. The problem that for data command this counter is not initialized properly as it does for mmc command sending:
int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries)
{
     struct mmc_request mrq = {NULL};

     WARN_ON(!host->claimed);

     memset(cmd->resp, 0, sizeof(cmd->resp));
     cmd->retries = retries;                 <-- supplied by
                                                     caller

     mrq.cmd = cmd;
     cmd->data = NULL;

     mmc_wait_for_req(host, &mrq);

     return cmd->error;
}

Quoted 7 lines

I found that there is no similar initialization for data command processing code, so the counter cmd->retries remains always 0.
Lets discuss:
- do we need retries for data request
- what value is good: for command there are 0 or 5 retries
- where to do this initialization:

card/block.c: static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_card *card,
int disable_multi,
struct mmc_queue *mq)

Signature · 7 lines

sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
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

Ulf Hansson

2014-05-13 08:19 UTC #3 lore raw

Quoted 4 lines · Konstantin Dorfman

Hello Chris, Ulf,
Sorry for nagging.

No worries. Let me have some more thinking before I come back to you though.

Kind regards
Ulf Hansson

Quoted 7 lines

Do you have opinion on issue with retries counter for data command?
Since till now the code behave as there is no retry, may be it is better to remove dead code? Or the counter need to be initialized properly.