[U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
Yangbo Lu
yangbo.lu at nxp.com
Tue Aug 2 09:23:45 CEST 2016
> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: Tuesday, August 02, 2016 3:18 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
>
> On 08/02/2016 04:13 PM, Jaehoon Chung wrote:
> > 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. :)
>
> One more..i don't check in more detail.
> Could you consider the below things?
>
> 1. located mmc_go_idle() before while()
>
> 2. included mmc_go_idle in while()
>
[Lu Yangbo-B47093] Thank you for your reminding.
From the customer code, I think locating it before while() should be what they want.
Anyway, we didn’t know why some cards need this. But doing this really work:)
> Best Regards,
> Jaehoon Chung
>
> >
> > 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 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
> >>>>>
> >>>>
> >>
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
More information about the U-Boot
mailing list