| Message ID | 1457492423-29558-1-git-send-email-yangbo.lu@nxp.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jaehoon Chung |
| Headers | show |
Any comments? Thanks. Best regards, Yangbo Lu > -----Original Message----- > From: Yangbo Lu [mailto:yangbo.lu@nxp.com] > Sent: Wednesday, March 09, 2016 11:00 AM > To: u-boot@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. > > 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
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@nxp.com] > Sent: Wednesday, March 09, 2016 11:00 AM > To: u-boot@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. > > 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
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@nxp.com] >> Sent: Wednesday, March 09, 2016 11:00 AM >> To: u-boot@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.. 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@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > > >
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@nxp.com] >>> Sent: Wednesday, March 09, 2016 11:00 AM >>> To: u-boot@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? > > 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@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> >> >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Ziyuan and Jaehoon, > -----Original Message----- > From: Ziyuan Xu [mailto:xzy.xu@rock-chips.com] > Sent: Wednesday, July 27, 2016 9:37 PM > To: Jaehoon Chung; Yangbo Lu; u-boot@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@nxp.com] > >>> Sent: Wednesday, March 09, 2016 11:00 AM > >>> To: u-boot@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 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@lists.denx.de > >> http://lists.denx.de/mailman/listinfo/u-boot > >> > >> > >> > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot >
Hi Yangbo, On 07/28/2016 11:45 AM, Yangbo Lu wrote: > Hi Ziyuan and Jaehoon, > > >> -----Original Message----- >> From: Ziyuan Xu [mailto:xzy.xu@rock-chips.com] >> Sent: Wednesday, July 27, 2016 9:37 PM >> To: Jaehoon Chung; Yangbo Lu; u-boot@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@nxp.com] >>>>> Sent: Wednesday, March 09, 2016 11:00 AM >>>>> To: u-boot@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. 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@lists.denx.de >>>> http://lists.denx.de/mailman/listinfo/u-boot >>>> >>>> >>>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot >> >
Hi Jaehoon, > -----Original Message----- > From: Jaehoon Chung [mailto:jh80.chung@samsung.com] > Sent: Thursday, July 28, 2016 4:40 PM > To: Yangbo Lu; Ziyuan Xu; u-boot@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@rock-chips.com] > >> Sent: Wednesday, July 27, 2016 9:37 PM > >> To: Jaehoon Chung; Yangbo Lu; u-boot@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@nxp.com] > >>>>> Sent: Wednesday, March 09, 2016 11:00 AM > >>>>> To: u-boot@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@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@lists.denx.de > >>>> http://lists.denx.de/mailman/listinfo/u-boot > >>>> > >>>> > >>>> > >>> _______________________________________________ > >>> U-Boot mailing list > >>> U-Boot@lists.denx.de > >>> http://lists.denx.de/mailman/listinfo/u-boot > >> > >
On 08/02/2016 04:03 PM, Yangbo Lu wrote: > Hi Jaehoon, > > >> -----Original Message----- >> From: Jaehoon Chung [mailto:jh80.chung@samsung.com] >> Sent: Thursday, July 28, 2016 4:40 PM >> To: Yangbo Lu; Ziyuan Xu; u-boot@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@rock-chips.com] >>>> Sent: Wednesday, July 27, 2016 9:37 PM >>>> To: Jaehoon Chung; Yangbo Lu; u-boot@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@nxp.com] >>>>>>> Sent: Wednesday, March 09, 2016 11:00 AM >>>>>>> To: u-boot@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@lists.denx.de >>>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>>>> >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> U-Boot mailing list >>>>> U-Boot@lists.denx.de >>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>> >>> >
Thank you, Jaehoon. So I will send the new version later :) Best regards, Yangbo Lu > -----Original Message----- > From: Jaehoon Chung [mailto:jh80.chung@samsung.com] > Sent: Tuesday, August 02, 2016 3:13 PM > To: Yangbo Lu; Ziyuan Xu; u-boot@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:03 PM, Yangbo Lu wrote: > > Hi Jaehoon, > > > > > >> -----Original Message----- > >> From: Jaehoon Chung [mailto:jh80.chung@samsung.com] > >> Sent: Thursday, July 28, 2016 4:40 PM > >> To: Yangbo Lu; Ziyuan Xu; u-boot@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@rock-chips.com] > >>>> Sent: Wednesday, July 27, 2016 9:37 PM > >>>> To: Jaehoon Chung; Yangbo Lu; u-boot@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@nxp.com] > >>>>>>> Sent: Wednesday, March 09, 2016 11:00 AM > >>>>>>> To: u-boot@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@lists.denx.de > >>>>>> http://lists.denx.de/mailman/listinfo/u-boot > >>>>>> > >>>>>> > >>>>>> > >>>>> _______________________________________________ > >>>>> U-Boot mailing list > >>>>> U-Boot@lists.denx.de > >>>>> http://lists.denx.de/mailman/listinfo/u-boot > >>>> > >>> > >
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@samsung.com] >>> Sent: Thursday, July 28, 2016 4:40 PM >>> To: Yangbo Lu; Ziyuan Xu; u-boot@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@rock-chips.com] >>>>> Sent: Wednesday, July 27, 2016 9:37 PM >>>>> To: Jaehoon Chung; Yangbo Lu; u-boot@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@nxp.com] >>>>>>>> Sent: Wednesday, March 09, 2016 11:00 AM >>>>>>>> To: u-boot@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() 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@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@lists.denx.de >>>>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>>>>> >>>>>>> >>>>>>> >>>>>> _______________________________________________ >>>>>> U-Boot mailing list >>>>>> U-Boot@lists.denx.de >>>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>>> >>>> >> > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
> -----Original Message----- > From: Jaehoon Chung [mailto:jh80.chung@samsung.com] > Sent: Tuesday, August 02, 2016 3:18 PM > To: Yangbo Lu; Ziyuan Xu; u-boot@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@samsung.com] > >>> Sent: Thursday, July 28, 2016 4:40 PM > >>> To: Yangbo Lu; Ziyuan Xu; u-boot@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@rock-chips.com] > >>>>> Sent: Wednesday, July 27, 2016 9:37 PM > >>>>> To: Jaehoon Chung; Yangbo Lu; u-boot@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@nxp.com] > >>>>>>>> Sent: Wednesday, March 09, 2016 11:00 AM > >>>>>>>> To: u-boot@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@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@lists.denx.de > >>>>>>> http://lists.denx.de/mailman/listinfo/u-boot > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>> _______________________________________________ > >>>>>> U-Boot mailing list > >>>>>> U-Boot@lists.denx.de > >>>>>> http://lists.denx.de/mailman/listinfo/u-boot > >>>>> > >>>> > >> > > > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot > >
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);
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. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- drivers/mmc/mmc.c | 3 +++ 1 file changed, 3 insertions(+)