[U-Boot] [PATCH 37/41] mmc: fsl_esdhc: fix sd/mmc ddr mode clock setting issue

[Peng Fan]

> -----Original Message-----
> From: Benoît Thébaudeau [mailto:benoit.thebaudeau.dev at gmail.com]
> Sent: 2018年5月29日 6:32
> To: Peng Fan <peng.fan at nxp.com>
> Cc: sbabic at denx.de; Fabio Estevam <fabio.estevam at nxp.com>; U-Boot
> <u-boot at lists.denx.de>; Bough Chen <haibo.chen at nxp.com>
> Subject: Re: [U-Boot] [PATCH 37/41] mmc: fsl_esdhc: fix sd/mmc ddr mode clock
> setting issue
> 
> Dear Peng Fan,
> 
> On Mon, May 28, 2018 at 2:25 PM, Peng Fan <peng.fan at nxp.com> wrote:
> > From: Ye Li <ye.li at nxp.com>
> >
> > When sd/mmc work at DDR mode, like HS400/HS400ES/DDR52/DDR50 mode,
> the
> > actual clock rate is just half of the expected clock.
> >
> > This patch set the DDR_EN bit first for DDR mode, hardware divide the
> > usdhc clock automatically, then follow the original sdr clock setting
> > method.
> >
> > Signed-off-by: Haibo Chen <haibo.chen at nxp.com>
> > Signed-off-by: Ye Li <ye.li at nxp.com>
> > ---
> >  drivers/mmc/fsl_esdhc.c | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index
> > 1b062ff06d..bf4ae74847 100644
> > --- a/drivers/mmc/fsl_esdhc.c
> > +++ b/drivers/mmc/fsl_esdhc.c
> > @@ -583,18 +583,32 @@ static void set_sysctl(struct fsl_esdhc_priv
> > *priv, struct mmc *mmc, uint clock)  #else
> >         int pre_div = 2;
> >  #endif
> > -       int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
> >         int sdhc_clk = priv->sdhc_clk;
> >         uint clk;
> >
> > +       /*
> > +        * For ddr mode, usdhc need to enable DDR mode first, after select
> > +        * this DDR mode, usdhc will automatically divide the usdhc clock
> > +        */
> > +       if (mmc->ddr_mode) {
> > +               writel(readl(&regs->mixctrl) | MIX_CTRL_DDREN,
> &regs->mixctrl);
> > +               sdhc_clk >>= 1;
> > +       }
> > +
> >         if (clock < mmc->cfg->f_min)
> >                 clock = mmc->cfg->f_min;
> >
> > -       while (sdhc_clk / (16 * pre_div * ddr_pre_div) > clock && pre_div <
> 256)
> > -               pre_div *= 2;
> > +       if ((sdhc_clk / 16) > clock) {
> > +               for (; pre_div < 256; pre_div *= 2)
> > +                       if ((sdhc_clk / pre_div) <= (clock * 16))
> > +                               break;
> > +       } else {
> > +               pre_div = 1;
> 
> This value is not always available for this divider. See the conditions in the
> initialization of this variable giving its minimum value.

The else {pre_div = 1;} could be removed then.

Thanks,
Peng.

> 
> > +       }
> >
> > -       while (sdhc_clk / (div * pre_div * ddr_pre_div) > clock && div < 16)
> > -               div++;
> > +       for (div = 1; div <= 16; div++)
> > +               if ((sdhc_clk / (div * pre_div)) <= clock)
> > +                       break;
> >
> >         pre_div >>= 1;
> >         div -= 1;
> 
> Best regards,
> Benoît