On 5/19/20 12:37 PM, Peng Fan wrote:
>> Subject: [PATCH] mmc: retry CMD1 in mmc_send_op_cond() until the eMMC
>> is ready
>>
>> From: Haibo Chen <haibo.c...@nxp.com>
>>
>> According to eMMC specification v5.1 section 6.4.3, we should issue
>> CMD1 repeatedly in the idle state until the eMMC is ready even if
>> mmc_send_op_cond() send CMD1 with argument = 0. Otherwise some
>> eMMC devices seems to enter the inactive mode after
>> mmc_complete_op_cond() issued CMD0 when the eMMC device is busy. This
>> patch also align with Linux 5.7.
Well, i think that it can't compare to linux driver.
is your issue fixed with this patch?
As i know, this sequence is not follow correct spec.
To enhance boot, it was skipping the polling time.
If there is a problem, could you explain in more detail?
Then I will also check on my targets.
>>
>> Signed-off-by: Haibo Chen <haibo.c...@nxp.com>
>> ---
>> drivers/mmc/mmc.c | 41 +++++++++++++++++++++++++++++++++--------
>> 1 file changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
>> 523c055967..509549756b 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -664,21 +664,46 @@ static int mmc_send_op_cond_iter(struct mmc
>> *mmc, int use_arg)
>>
>> static int mmc_send_op_cond(struct mmc *mmc) {
>> + struct mmc_cmd cmd;
>> int err, i;
>> + int timeout = 1000;
>> + ulong start;
>>
>> /* Some cards seem to need this */
>> mmc_go_idle(mmc);
>>
>> - /* Asking to the card its capabilities */
>> - for (i = 0; i < 2; i++) {
>> - err = mmc_send_op_cond_iter(mmc, i != 0);
>> + cmd.cmdidx = MMC_CMD_SEND_OP_COND;
>> + cmd.resp_type = MMC_RSP_R3;
>> + cmd.cmdarg = 0;
>> +
>> + start = get_timer(0);
>> + /*
>> + * According to eMMC specification v5.1 section 6.4.3, we
>> + * should issue CMD1 repeatedly in the idle state until
>> + * the eMMC is ready. Otherwise some eMMC devices seem to enter
>> + * the inactive mode after mmc_complete_op_cond() issued CMD0
>> when
>> + * the eMMC device is busy.
>> + */
>
> Could we just extend the previous for loop to fix the issue?
> Your piece code duplicate much of the code and changed
> the original behavior.
>
>> + while (1) {
>> + err = mmc_send_cmd(mmc, &cmd, NULL);
>> if (err)
>> return err;
>> -
>> - /* exit if not busy (flag seems to be inverted) */
>> - if (mmc->ocr & OCR_BUSY)
>> - break;
>> + if (mmc_host_is_spi(mmc)) {
>> + if (!(cmd.response[0] & (1 << 0)))
>> + break;
>> + } else {
>> + mmc->ocr = cmd.response[0];
>> + /* exit if not busy (flag seems to be inverted) */
>> + if (mmc->ocr & OCR_BUSY)
>> + break;
>> + }
>> + if (get_timer(start) > timeout)
>> + return -EOPNOTSUPP;
>
> TIMEOUT, please.
>
>> + udelay(100);
>
> This will change the original behavior by adding a delay.
>
>> + if (!mmc_host_is_spi(mmc))
>> + cmd.cmdarg = cmd.response[0] | (1 << 30);
>
> Do we need "1 << 30" ?
>
>> }
>> +
>> mmc->op_cond_pending = 1;
>> return 0;
>> }
>> @@ -691,7 +716,7 @@ static int mmc_complete_op_cond(struct mmc
>> *mmc)
>> int err;
>>
>> mmc->op_cond_pending = 0;
>> - if (!(mmc->ocr & OCR_BUSY)) {
>> + if (mmc->ocr & OCR_BUSY) {
>
> When card not go out busy, it means card not
> finish power up seq. Why drop !?
Right. It doesn't finish power-ups seq.
If it's working with this patch, it seems something wrong.
>
>> /* Some cards seem to need this */
>> mmc_go_idle(mmc);
>>
>> --
>
> Would the following patch help?
yes. it's more better.
If need to increase polling time, i think it's correct place in
mmc_complete_op_cond().
Best Regards,
Jaehoon Chung
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 523c055967..d3a0538cdb 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -665,12 +665,15 @@ static int mmc_send_op_cond_iter(struct mmc *mmc, int
> use_arg)
> static int mmc_send_op_cond(struct mmc *mmc)
> {
> int err, i;
> + int timeout = 1000;
> + ulong start;
>
> /* Some cards seem to need this */
> mmc_go_idle(mmc);
>
> + start = get_timer(0);
> /* Asking to the card its capabilities */
> - for (i = 0; i < 2; i++) {
> + for (i = 0; ; i++) {
> err = mmc_send_op_cond_iter(mmc, i != 0);
> if (err)
> return err;
> @@ -678,6 +681,9 @@ static int mmc_send_op_cond(struct mmc *mmc)
> /* exit if not busy (flag seems to be inverted) */
> if (mmc->ocr & OCR_BUSY)
> break;
> +
> + if (get_timer(start) > timeout)
> + return -ETIMEDOUT;
> }
> mmc->op_cond_pending = 1;
> return 0;
>
> Thanks,
> Peng.
>> 2.17.1
>
>
