RE: [PATCH 2/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code.

[Bough Chen]

> -----Original Message-----
> From: ZHIZHIKIN Andrey [mailto:andrey.zhizhi...@leica-geosystems.com]
> Sent: 2021年3月3日 19:00
> To: Bough Chen <haibo.c...@nxp.com>; Peng Fan <peng....@nxp.com>;
> u-boot@lists.denx.de; sba...@denx.de
> Cc: dl-uboot-imx <uboot-...@nxp.com>; thar...@gateworks.com;
> feste...@gmail.com; Ye Li <ye...@nxp.com>
> Subject: RE: [PATCH 2/2] mmc: fsl_esdhc_imx: remove redundant cmd11
> related code.
> 
> 
> 
> > -----Original Message-----
> > From: haibo.c...@nxp.com <haibo.c...@nxp.com>
> > Sent: Wednesday, March 3, 2021 10:06 AM
> > To: peng....@nxp.com; u-boot@lists.denx.de; sba...@denx.de
> > Cc: haibo.c...@nxp.com; uboot-...@nxp.com; thar...@gateworks.com;
> > ZHIZHIKIN Andrey <andrey.zhizhi...@leica-geosystems.com>;
> > feste...@gmail.com; ye...@nxp.com
> > Subject: [PATCH 2/2] mmc: fsl_esdhc_imx: remove redundant cmd11
> > related code.
> >
> > From: Haibo Chen <haibo.c...@nxp.com>
> >
> > Common code already handle the voltage switch sequence based on spec,
> > so remove the redundant voltage switch code.
> >
> > Signed-off-by: Haibo Chen <haibo.c...@nxp.com>
> > ---
> >  drivers/mmc/fsl_esdhc_imx.c | 10 +---------
> >  1 file changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> > index
> > af36558b3c..a199cf3df6 100644
> > --- a/drivers/mmc/fsl_esdhc_imx.c
> > +++ b/drivers/mmc/fsl_esdhc_imx.c
> > @@ -515,15 +515,6 @@ static int esdhc_send_cmd_common(struct
> > fsl_esdhc_priv *priv, struct mmc *mmc,
> >                 goto out;
> >         }
> >
> > -       /* Switch voltage to 1.8V if CMD11 succeeded */
> > -       if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) {
> > -               esdhc_setbits32(&regs->vendorspec,
> ESDHC_VENDORSPEC_VSELECT);
> > -
> > -               printf("Run CMD11 1.8V switch\n");
> > -               /* Sleep for 5 ms - max time for card to switch to 1.8V */
> > -               udelay(5000);
> > -       }
> > -
> >         /* Workaround for ESDHC errata ENGcm03648 */
> >         if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
> >                 int timeout = 50000;
> > @@ -839,6 +830,7 @@ static int esdhc_set_voltage(struct mmc *mmc)
> >                 }
> >  #endif
> >                 esdhc_setbits32(&regs->vendorspec,
> > ESDHC_VENDORSPEC_VSELECT);
> > +               mdelay(5);
> 
> Why is this delay introduced here? It is not clear from the commit message
> whether and why it is required here.
> 
> If this is kept from the removed block - maybe it is better to move the
> corresponding comment here as well.

Hi Andrev,

Thanks for your careful review!
Without this 5ms delay, with patch1 and remove the upper redundant cmd11 
related code,
mmc_switch_voltage() will fail, due to the second mmc_wait_dat0() return 
timeout. Seems for usdhc,
gate off clock for 10ms is not enough. If total delay 15ms, then the second 
mmc_wait_dat0() can return normal.
So I add 5ms delay here. Yes, I should add a comment for this 5ms in the code.
You can also do the test on your side.

Best Regards
Haibo

> 
> >                 if (esdhc_read32(&regs->vendorspec) &
> ESDHC_VENDORSPEC_VSELECT)
> >                         return 0;
> >
> > --
> > 2.17.1
> 
> -- andrey

smime.p7s
Description: S/MIME cryptographic signature