Shimoda-san
> Anyway, I tested this patch. And the mmc core said some error happened.
> I think this is suitable behavior. So,
>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
Thank you very much for the quick response and testing!
> < Question >
> In mmc_blk_cmd_recovery(), if "brq->stop.resp[0] & R1_CARD_ECC_FAILED" is true, ecc_err is set to true.
> However, in mmc_blk_err_check(), ecc_err is only referred when brq->data.error is true.
> So, I guess we need a patch like below as another patch. What do you think?
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 0e838b0..99c937b 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1374,7 +1374,7 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_ca
> return MMC_BLK_RETRY;
> }
>
> - if (brq->data.error) {
> + if (brq->data.error || ecc_err) {
> if (need_retune && !brq->retune_retry_done) {
> pr_debug("%s: retrying because a re-tune was needed\n",
> req->rq_disk->disk_name);
>
> After that, the mmc core also output the following message:
>
> mmcblk1: error 0 transferring data, sector 0, nr 128, cmd response 0x900, card status 0x200b00
> mmcblk1: retrying using single block read
I think you are right that we need to change something somewhere in this
code block. I was about to suggest adding "|| brq->stop.error" instead
of "|| ecc_err". However, I am not so sure about this as well, since the
code is a little confusing to me. Especially this:
1381 if (rq_data_dir(req) == READ) {
1382 if (ecc_err)
1383 return MMC_BLK_ECC_ERR;
1384 return MMC_BLK_DATA_ERR;
1385 } else {
1386 return MMC_BLK_CMD_ERR;
1387 }
If the data_directions is WRITE, then we return a CMD_ERR instead of a
DATA_ERR? And all that in a code path which is only run when
brq->data.error, i.e. brq->cmd.err is not even touched?
I'd think the if-block in question wants to handle errors which happened
while transferring data. As such, I'd still think adding
"|| brq->stop.error" might be a good idea. And revisiting if this block
should really return CMD_ERR.
But I may be wrong and misunderstanding, so I am happy for further
opinions. Maybe the code needs just some more comments.
Thanks and regards,
Wolfram
Attachment:
signature.asc
Description: PGP signature
