From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peng Fan <Peng.Fan@freescale.com>
Date: Sat, 21 Mar 2015 18:33:34 +0800
Subject: [U-Boot] [PATCH] mmc: fix OCR Polling
In-Reply-To: <000001d062de$30a89240$91f9b6c0$@mentor.com>
References: <000001d062de$30a89240$91f9b6c0$@mentor.com>
Message-ID: <550D48FE.1040205@freescale.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi, Andrew

On 3/20/2015 3:19 PM, Andrew Gabbasov wrote:
> Hi Peng,
>
>> From: Peng.Fan at freescale.com (Peng Fan)
>> Date: Thu, 19 Mar 2015 16:22:46 +0800
>> Subject: [U-Boot] [PATCH] mmc: fix OCR Polling
>>
>> If in mmc_send_op_cond, OCR_BUSY is set in CMD1's response, then
>> state is transfered to Ready state, and there is no need to send
>> CMD1 again. Otherwise following CMD1 will recieve no response, or
>> timeour error from driver such as fsl_esdhc.c.
>>
>> If not into Ready state in previous CMD1, then continue CMD1 command.
>>
>> In mmc_complete_op_cond, we use the value mmc->op_cond_response
>> from mmc_send_op_cond, since there should be no CMD1 command between
>> mmc_send_op_cond and mmc_complete_op_cond
>>
>> Before fixing this, uboot log shows:
>> "
>> CMD_SEND:0
>>                  ARG                      0x00000000
>>                  MMC_RSP_NONE
>> CMD_SEND:8
>>                  ARG                      0x000001AA
>>                  MMC_RSP_R1,5,6,7         0x18EC1504
>> CMD_SEND:55
>>                  ARG                      0x00000000
>>                  MMC_RSP_R1,5,6,7         0x18EC1504
>> CMD_SEND:0
>>                  ARG                      0x00000000
>>                  MMC_RSP_NONE
>> CMD_SEND:1
>>                  ARG                      0x00000000
>>                  MMC_RSP_R3,4             0x00FF8080
>> CMD_SEND:1
>>                  ARG                      0x40300000
>>                  MMC_RSP_R3,4             0xC0FF8080 --> Already OCR_BUSY
> set
>> CMD_SEND:1
>>                  ARG                      0x40300000
>>                  MMC_RSP_R3,4             0x0096850A --> Failed CMD1
>> MMC init failed
>> "
>>
>> Using this patch, this issue is fixed, emmc can be detected correctly.
>>
>>
>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>> ---
>>   drivers/mmc/mmc.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index a13769e..43a9a8a 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -406,14 +406,23 @@ static int mmc_complete_op_cond(struct mmc *mmc)
>>   
>>       mmc->op_cond_pending = 0;
>>       start = get_timer(0);
>> -    do {
>> +    /*
>> +     * If in mmc_send_op_cond, OCR_BUSY is set in CMD1's response, then
>> +     * state is transfered to Ready state, and there is no need to
>> +     * send CMD1 again. Otherwise following CMD1 will recieve no
> response,
>> +     * or timeour error from driver such as fsl_esdhc.c.
>> +     *
>> +     * If not into Ready state in previous CMD1, then continue CMD1
>> +     * command.
>> +     */
>> +    while (!(mmc->op_cond_response & OCR_BUSY)) {
>>           err = mmc_send_op_cond_iter(mmc, &cmd, 1);
>>           if (err)
>>               return err;
>>           if (get_timer(start) > timeout)
>>               return UNUSABLE_ERR;
>>           udelay(100);
>> -    } while (!(mmc->op_cond_response & OCR_BUSY));
>> +    }
>>   
>>       if (mmc_host_is_spi(mmc)) { /* read OCR for spi */
>>           cmd.cmdidx = MMC_CMD_SPI_READ_OCR;
>> -- 
>> 1.8.4
> After this patch, if the busy flag is indeed already set (so that the loop
> body
> is not executed), and it is not an SPI case (mmc_host_is_spi(mmc) is false),
> the "cmd" structure (which is local to mmc_complete_op_cond() function)
> is left uninitialized, and using cmd.response[0] later in the function
> becomes
> incorrect. And the OCR register value and the high capacity flag may be set
> incorrectly.
Yeah. you are right.
Maybe the following piece of code should be added to replace mmc->ocr = 
cmd.response[0]:
"
if (mmc_host_is_spi(mmc))
     mmc->ocr = cmd.response[0];
else
     mmc->ocr = mmc->op_cond_response;
"
Thanks for correcting me.
>
> So, this patch is not enough by itself, it needs to be modified (appended)
> in order to get the correct code.
>
> Coincidentally, the same day I submitted a patch series
> ("[U-Boot] [PATCH 0/6] mmc: Fix OCR polling and splitted initialization"),
> that, among other changes, fixes the OCR polling problem too
> ("[U-Boot] [PATCH 4/6] mmc: Continue polling MMC card for OCR only if it is
> still not ready"),
> but in a slightly different way, that hopefully doesn't have the mentioned
> issue.
Your's does not have such issue:)
> Thanks.
>
> Best regards,
> Andrew
>
>
> .
>
Thanks
Peng.