[U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards

[Jaehoon Chung <jh80.chung@samsung.com>]

On 08/02/2016 04:03 PM, Yangbo Lu wrote:
> Hi Jaehoon,
> 
> 
>> -----Original Message-----
>> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
>> Sent: Thursday, July 28, 2016 4:40 PM
>> To: Yangbo Lu; Ziyuan Xu; u-boot at lists.denx.de; Tom Rini
>> Cc: Pantelis Antoniou
>> Subject: Re: [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC
>> cards
>>
>> Hi Yangbo,
>>
>> On 07/28/2016 11:45 AM, Yangbo Lu wrote:
>>> Hi Ziyuan and Jaehoon,
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ziyuan Xu [mailto:xzy.xu at rock-chips.com]
>>>> Sent: Wednesday, July 27, 2016 9:37 PM
>>>> To: Jaehoon Chung; Yangbo Lu; u-boot at lists.denx.de; Tom Rini
>>>> Cc: Pantelis Antoniou
>>>> Subject: Re: [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC
>>>> cards
>>>>
>>>>
>>>>
>>>> On 2016?07?27? 19:15, Jaehoon Chung wrote:
>>>>> On 07/27/2016 04:28 PM, Yangbo Lu wrote:
>>>>>> Hi Tom,
>>>>>>
>>>>>> Could you help to assign this mmc patch reviewing to right person?
>>>>>> It seems no one had reviewed it for almost half year.
>>>>>>
>>>>>> And another my mmc patch also needs to be reviewed.
>>>>>> I submitted in May. Please help.
>>>>>> http://patchwork.ozlabs.org/patch/624448/
>>>>>>
>>>>>>
>>>>>> Thank you very much.
>>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Yangbo Lu
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Yangbo Lu [mailto:yangbo.lu at nxp.com]
>>>>>>> Sent: Wednesday, March 09, 2016 11:00 AM
>>>>>>> To: u-boot at lists.denx.de
>>>>>>> Cc: Pantelis Antoniou; Yangbo Lu
>>>>>>> Subject: [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
>>>>>>>
>>>>>>> When the MMC framework was added in u-boot, the mmc_go_idle was
>>>>>>> added before mmc_send_op_cond_iter in function mmc_send_op_cond
>>>>>>> annotating that some cards seemed to need this. Actually, we still
>>>>>>> need to do this in function mmc_complete_op_cond for those cards.
>>>>>>> This has been verified on Micron MTFC4GACAECN eMMC chip.
>>>>> If there is no go_idle(), then what happen?
>>>>> If you share the information more, i can check the more..
>>>> Sounds interesting, I also want want to know what happen?
>>>> It seems like you failed in CMD1? The eMMC device was always in busy
>>>> device within 1 second?
>>>
>>> [Lu Yangbo-B47093] This was an issue which our customer reported and
>> required us to fix in March.
>>> They used NXP LS1020A platform and Micron MTFC4GACAECN eMMC, and
>> reported they had to add CMD0 as below.
>>> Otherwise it couldn?t read OCR.
>>>
>>> static int mmc_complete_op_cond(struct mmc *mmc) {
>>> 	struct mmc_cmd cmd;
>>> 	int timeout = 1000;
>>> 	uint start;
>>> 	int err;
>>>
>>> #if defined (XXX_CHANGED)
>>> 	// our eMMC chip (Micron MTFC4GACAECN) requires that it be put in
>> idle mode before
>>> 	// negociating the operating voltage levels.
>>> 	mmc_go_idle(mmc);
>>> #endif
>>
>> Well, it seems to fix workaround. mmc_go_idle() means Device Reset.
>>
>> mmc_complete_op_cond() function has added for reducing the booting time.
>> If mmc_go_idle() is added at here, there is no benefit, and it should be
>> back to old concept.
>>
>> I don't agree this patch..now.
>>
> 
> [Lu Yangbo-B47093] Did you notice mmc_send_op_cond function? Before mmc_send_op_cond_iter sending CMD1, there always was mmc_go_idle.
> I don?t know why said 'Some cards seem to need this', but it must fix some issue.
> 
> static int mmc_send_op_cond(struct mmc *mmc)
> {
>         int err, i;
> 
>         /* 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);
>                 if (err)
>                         return err;
> 
>                 /* exit if not busy (flag seems to be inverted) */
>                 if (mmc->ocr & OCR_BUSY)
>                         break;
>         }
>         mmc->op_cond_pending = 1;
>         return 0;
> }
> 
> Now in mmc_complete_op_cond function, there may be the same issue. Without the mmc_go_idle, mmc_send_op_cond_iter failed to get ocr.
> Maybe I should move mmc_go_idle just before mmc_send_op_cond_iter, like this.
> 
> static int mmc_complete_op_cond(struct mmc *mmc)
> {
>         struct mmc_cmd cmd;
>         int timeout = 1000;
>         uint start;
>         int err;
> 
>         mmc->op_cond_pending = 0;
>         if (!(mmc->ocr & OCR_BUSY)) {
>                 start = get_timer(0);
>                 while (1) {
> 				/* Some cards seem to need this */
> 				mmc_go_idle(mmc);
>                         err = mmc_send_op_cond_iter(mmc, 1);

If you need to add the mmc_go_idle(), then I think this point is right. :)

Best Regards,
Jaehoon Chung

> 
> If you think it's not proper, do you have any suggestion?
> :)
> 
> Thanks a lot.
> 
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>>
>>> I hadn?t reproduce this to get more details about this issue since I
>> didn?t have one this kind eMMC card that time.
>>> Thanks.
>>>
>>>>>
>>>>> Best Regards,
>>>>> Jaehoon Chung
>>>>>
>>>>>>> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
>>>>>>> ---
>>>>>>>   drivers/mmc/mmc.c | 3 +++
>>>>>>>   1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
>>>>>>> ede5d6e..82e3268
>>>>>>> 100644
>>>>>>> --- a/drivers/mmc/mmc.c
>>>>>>> +++ b/drivers/mmc/mmc.c
>>>>>>> @@ -418,6 +418,9 @@ static int mmc_complete_op_cond(struct mmc *mmc)
>>>>>>>   	uint start;
>>>>>>>   	int err;
>>>>>>>
>>>>>>> +	/* Some cards seem to need this */
>>>>>>> +	mmc_go_idle(mmc);
>>>>>>> +
>>>>>>>   	mmc->op_cond_pending = 0;
>>>>>>>   	if (!(mmc->ocr & OCR_BUSY)) {
>>>>>>>   		start = get_timer(0);
>>>>>>> --
>>>>>>> 2.1.0.27.g96db324
>>>>>> _______________________________________________
>>>>>> U-Boot mailing list
>>>>>> U-Boot at lists.denx.de
>>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>>>
>>>>>>
>>>>>>
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot at lists.denx.de
>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>
>>>
>