> -----Original Message-----
> From: Peng Fan <peng....@nxp.com>
> Sent: 2020年5月19日 11:38
> To: BOUGH CHEN <haibo.c...@nxp.com>; u-boot@lists.denx.de
> Cc: dl-uboot-imx <uboot-...@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.c...@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.c...@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

Reply via email to