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

[Yangbo Lu]

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 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 at 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
> >>
> >