[PATCH] mmc: retry CMD1 in mmc_send_op_cond() until the eMMC is ready

[BOUGH CHEN]

> -----Original Message-----
> From: Peng Fan <peng.fan at nxp.com>
> Sent: 2020年5月19日 11:38
> To: BOUGH CHEN <haibo.chen at nxp.com>; u-boot at lists.denx.de
> Cc: dl-uboot-imx <uboot-imx at nxp.com>
> Subject: RE: [PATCH] mmc: retry CMD1 in mmc_send_op_cond() until the
> eMMC is ready
> 
> > Subject: [PATCH] mmc: retry CMD1 in mmc_send_op_cond() until the eMMC
> > is ready
> >
> > From: Haibo Chen <haibo.chen at nxp.com>
> >
> > According to eMMC specification v5.1 section 6.4.3, we should issue
> > CMD1 repeatedly in the idle state until the eMMC is ready even if
> > mmc_send_op_cond() send CMD1 with argument = 0. Otherwise some
> eMMC
> > devices seems to enter the inactive mode after
> > mmc_complete_op_cond() issued CMD0 when the eMMC device is busy. This
> > patch also align with Linux 5.7.
> >
> > Signed-off-by: Haibo Chen <haibo.chen at nxp.com>
> > ---
> >  drivers/mmc/mmc.c | 41 +++++++++++++++++++++++++++++++++--------
> >  1 file changed, 33 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > 523c055967..509549756b 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -664,21 +664,46 @@ static int mmc_send_op_cond_iter(struct mmc
> > *mmc, int use_arg)
> >
> >  static int mmc_send_op_cond(struct mmc *mmc)  {
> > +	struct mmc_cmd cmd;
> >  	int err, i;
> > +	int timeout = 1000;
> > +	ulong start;
> >
> >  	/* 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);
> > +	cmd.cmdidx = MMC_CMD_SEND_OP_COND;
> > +	cmd.resp_type = MMC_RSP_R3;
> > +	cmd.cmdarg = 0;
> > +
> > +	start = get_timer(0);
> > +	/*
> > +	 * According to eMMC specification v5.1 section 6.4.3, we
> > +	 * should issue CMD1 repeatedly in the idle state until
> > +	 * the eMMC is ready. Otherwise some eMMC devices seem to enter
> > +	 * the inactive mode after mmc_complete_op_cond() issued CMD0
> > when
> > +	 * the eMMC device is busy.
> > +	 */
> 
> Could we just extend the previous for loop to fix the issue?
> Your piece code duplicate much of the code and changed the original behavior.
> 
> > +	while (1) {
> > +		err = mmc_send_cmd(mmc, &cmd, NULL);
> >  		if (err)
> >  			return err;
> > -
> > -		/* exit if not busy (flag seems to be inverted) */
> > -		if (mmc->ocr & OCR_BUSY)
> > -			break;
> > +		if (mmc_host_is_spi(mmc)) {
> > +			if (!(cmd.response[0] & (1 << 0)))
> > +				break;
> > +		} else {
> > +			mmc->ocr = cmd.response[0];
> > +			/* exit if not busy (flag seems to be inverted) */
> > +			if (mmc->ocr & OCR_BUSY)
> > +				break;
> > +		}
> > +		if (get_timer(start) > timeout)
> > +			return -EOPNOTSUPP;
> 
> TIMEOUT, please.
> 
> > +		udelay(100);
> 
> This will change the original behavior by adding a delay.
> 
> > +		if (!mmc_host_is_spi(mmc))
> > +			cmd.cmdarg = cmd.response[0] | (1 << 30);
> 
> Do we need "1 << 30" ?
> 
> >  	}
> > +
> >  	mmc->op_cond_pending = 1;
> >  	return 0;
> >  }
> > @@ -691,7 +716,7 @@ static int mmc_complete_op_cond(struct mmc
> > *mmc)
> >  	int err;
> >
> >  	mmc->op_cond_pending = 0;
> > -	if (!(mmc->ocr & OCR_BUSY)) {
> > +	if (mmc->ocr & OCR_BUSY) {
> 
> When card not go out busy, it means card not finish power up seq. Why drop !?
> 
> >  		/* Some cards seem to need this */
> >  		mmc_go_idle(mmc);
> >
> > --
> 
> Would the following patch help?
> 
Hi Peng,

Thanks for your carefully review and suggestion, most eMMC chip can work normal on the original code, only some Toshiba eMMC need more time to get out of busy after cmd1. 
The patch I send out just refer to the Linux driver code, and pass on the customer side.
Currently I'm not sure whether the bit 30 of the argument for cmd1 is necessary for this eMMC, I will get the reworked board which soldered this Toshiba eMMC in a few days, then I can test on my hand.
Will test this then and give you response.

Best Regards
Haibo Chen

> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> 523c055967..d3a0538cdb 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -665,12 +665,15 @@ static int mmc_send_op_cond_iter(struct mmc
> *mmc, int use_arg)  static int mmc_send_op_cond(struct mmc *mmc)  {
>         int err, i;
> +       int timeout = 1000;
> +       ulong start;
> 
>         /* Some cards seem to need this */
>         mmc_go_idle(mmc);
> 
> +       start = get_timer(0);
>         /* Asking to the card its capabilities */
> -       for (i = 0; i < 2; i++) {
> +       for (i = 0; ; i++) {
>                 err = mmc_send_op_cond_iter(mmc, i != 0);
>                 if (err)
>                         return err;
> @@ -678,6 +681,9 @@ static int mmc_send_op_cond(struct mmc *mmc)
>                 /* exit if not busy (flag seems to be inverted) */
>                 if (mmc->ocr & OCR_BUSY)
>                         break;
> +
> +               if (get_timer(start) > timeout)
> +                       return -ETIMEDOUT;
>         }
>         mmc->op_cond_pending = 1;
>         return 0;
> 
> Thanks,
> Peng.
> > 2.17.1