All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
@ 2016-03-09  3:00 Yangbo Lu
  2016-04-21  0:38 ` Yangbo Lu
  2016-07-27  7:28 ` Yangbo Lu
  0 siblings, 2 replies; 12+ messages in thread
From: Yangbo Lu @ 2016-03-09  3:00 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-03-09  3:00 [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards Yangbo Lu
@ 2016-04-21  0:38 ` Yangbo Lu
  2016-07-27  7:28 ` Yangbo Lu
  1 sibling, 0 replies; 12+ messages in thread
From: Yangbo Lu @ 2016-04-21  0:38 UTC (permalink / raw)
  To: u-boot

Any comments?
Thanks.


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-03-09  3:00 [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards Yangbo Lu
  2016-04-21  0:38 ` Yangbo Lu
@ 2016-07-27  7:28 ` Yangbo Lu
  2016-07-27 11:15   ` Jaehoon Chung
  1 sibling, 1 reply; 12+ messages in thread
From: Yangbo Lu @ 2016-07-27  7:28 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-07-27  7:28 ` Yangbo Lu
@ 2016-07-27 11:15   ` Jaehoon Chung
  2016-07-27 13:37     ` Ziyuan Xu
  0 siblings, 1 reply; 12+ messages in thread
From: Jaehoon Chung @ 2016-07-27 11:15 UTC (permalink / raw)
  To: u-boot

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-07-27 11:15   ` Jaehoon Chung
@ 2016-07-27 13:37     ` Ziyuan Xu
  2016-07-28  2:45       ` Yangbo Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Ziyuan Xu @ 2016-07-27 13:37 UTC (permalink / raw)
  To: 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 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?
>
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-07-27 13:37     ` Ziyuan Xu
@ 2016-07-28  2:45       ` Yangbo Lu
  2016-07-28  8:39         ` Jaehoon Chung
  0 siblings, 1 reply; 12+ messages in thread
From: Yangbo Lu @ 2016-07-28  2:45 UTC (permalink / raw)
  To: u-boot

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	


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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-07-28  2:45       ` Yangbo Lu
@ 2016-07-28  8:39         ` Jaehoon Chung
  2016-08-02  7:03           ` Yangbo Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Jaehoon Chung @ 2016-07-28  8:39 UTC (permalink / raw)
  To: 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 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.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-07-28  8:39         ` Jaehoon Chung
@ 2016-08-02  7:03           ` Yangbo Lu
  2016-08-02  7:13             ` Jaehoon Chung
  0 siblings, 1 reply; 12+ messages in thread
From: Yangbo Lu @ 2016-08-02  7:03 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-08-02  7:03           ` Yangbo Lu
@ 2016-08-02  7:13             ` Jaehoon Chung
  2016-08-02  7:16               ` Yangbo Lu
  2016-08-02  7:18               ` Jaehoon Chung
  0 siblings, 2 replies; 12+ messages in thread
From: Jaehoon Chung @ 2016-08-02  7:13 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-08-02  7:13             ` Jaehoon Chung
@ 2016-08-02  7:16               ` Yangbo Lu
  2016-08-02  7:18               ` Jaehoon Chung
  1 sibling, 0 replies; 12+ messages in thread
From: Yangbo Lu @ 2016-08-02  7:16 UTC (permalink / raw)
  To: 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 at samsung.com]
> Sent: Tuesday, August 02, 2016 3:13 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: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
> >>>>
> >>>
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-08-02  7:13             ` Jaehoon Chung
  2016-08-02  7:16               ` Yangbo Lu
@ 2016-08-02  7:18               ` Jaehoon Chung
  2016-08-02  7:23                 ` Yangbo Lu
  1 sibling, 1 reply; 12+ messages in thread
From: Jaehoon Chung @ 2016-08-02  7:18 UTC (permalink / raw)
  To: 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 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()

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards
  2016-08-02  7:18               ` Jaehoon Chung
@ 2016-08-02  7:23                 ` Yangbo Lu
  0 siblings, 0 replies; 12+ messages in thread
From: Yangbo Lu @ 2016-08-02  7:23 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-08-02  7:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09  3:00 [U-Boot] [PATCH] mmc: send CMD0 before CMD1 for some MMC cards Yangbo Lu
2016-04-21  0:38 ` Yangbo Lu
2016-07-27  7:28 ` Yangbo Lu
2016-07-27 11:15   ` Jaehoon Chung
2016-07-27 13:37     ` Ziyuan Xu
2016-07-28  2:45       ` Yangbo Lu
2016-07-28  8:39         ` Jaehoon Chung
2016-08-02  7:03           ` Yangbo Lu
2016-08-02  7:13             ` Jaehoon Chung
2016-08-02  7:16               ` Yangbo Lu
2016-08-02  7:18               ` Jaehoon Chung
2016-08-02  7:23                 ` Yangbo Lu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.