[3/3] mmc: fsl_esdhc_imx: correct the actual card clock

From: Haibo Chen <haibo.chen@nxp.com>

The original code logic can not show the correct card clock, and also
has one risk when the div is 0. Because there is div -=1 before.

So move the operation before div -=1, and also involve ddr_pre_div
to get the correct value.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/fsl_esdhc_imx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

On 2/11/22 12:16, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The original code logic can not show the correct card clock, and also
> has one risk when the div is 0. Because there is div -=1 before.
> 
> So move the operation before div -=1, and also involve ddr_pre_div
> to get the correct value.

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks !

> From: Haibo Chen <haibo.chen@nxp.com>
> The original code logic can not show the correct card clock, and also
> has one risk when the div is 0. Because there is div -=1 before.
> So move the operation before div -=1, and also involve ddr_pre_div
> to get the correct value.
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> Reviewed-by: Marek Vasut <marex@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

On Fri, Feb 11, 2022 at 3:48 AM <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> The original code logic can not show the correct card clock, and also
> has one risk when the div is 0. Because there is div -=1 before.
>
> So move the operation before div -=1, and also involve ddr_pre_div
> to get the correct value.
>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mmc/fsl_esdhc_imx.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 0be7cae1e5..0ea7b0b50c 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -609,6 +609,8 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>         while (sdhc_clk / (div * pre_div * ddr_pre_div) > clock && div < 16)
>                 div++;
>
> +       mmc->clock = sdhc_clk / pre_div / div / ddr_pre_div;
> +
>         pre_div >>= 1;
>         div -= 1;
>
> @@ -630,7 +632,6 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>         else
>                 esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
>
> -       mmc->clock = sdhc_clk / pre_div / div;
>         priv->clock = clock;
>  }
>
> --
> 2.17.1
>

Haibo,

I found that this particular patch keeps an imx8mm-venice-gw7901 board
that has a viking vwsdinbdg4 eMMC from booting to Linux. While u-boot
appears to work ok, as soon as I load a kernel (from emmc or even
network) and boot to it I hang at 'starting kernel' even with early
debug turned on.

u-boot=> mmc list
FSL_SDHC: 0
FSL_SDHC: 1
FSL_SDHC: 2 (eMMC)
u-boot=> mmc dev 2
switch to partitions #0, OK
mmc2(part 0) is current device
u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 45
OEM: 0
Name: DG4008
Bus Speed: 200000000
Mode: HS400ES (200MHz)
Rd Block Len: 512
MMC version 5.1
High Capacity: Yes
Capacity: 7.3 GiB
Bus Width: 8-bit DDR
Erase Group Size: 512 KiB
HC WP Group Size: 8 MiB
User Capacity: 7.3 GiB WRREL
Boot Capacity: 4 MiB ENH
RPMB Capacity: 4 MiB ENH
Boot area 0 is not write protected
Boot area 1 is not write protected

I have other boards with a Micron MTFC8GAKAJCN non HS400ES that don't
have any issue so it appears to be something to do with HS400ES
support and I find if I disable CONFIG_MMC_HS400_ES_SUPPORT or revert
this patch the issue goes away.

Any idea what might be going on here?

Best Regards,

Tim

> -----Original Message-----
> From: Tim Harvey <tharvey@gateworks.com>
> Sent: 2022年7月23日 10:23
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>; Jaehoon Chung
> <jh80.chung@samsung.com>; Fabio Estevam <festevam@gmail.com>; Sean
> Anderson <sean.anderson@seco.com>; u-boot <u-boot@lists.denx.de>; Marek
> Vasut <marex@denx.de>; Adam Ford <aford173@gmail.com>; Andrey Zhizhikin
> <andrey.zhizhikin@leica-geosystems.com>; dl-uboot-imx <uboot-imx@nxp.com>
> Subject: Re: [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock
> 
> On Fri, Feb 11, 2022 at 3:48 AM <haibo.chen@nxp.com> wrote:
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The original code logic can not show the correct card clock, and also
> > has one risk when the div is 0. Because there is div -=1 before.
> >
> > So move the operation before div -=1, and also involve ddr_pre_div to
> > get the correct value.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/mmc/fsl_esdhc_imx.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> > index 0be7cae1e5..0ea7b0b50c 100644
> > --- a/drivers/mmc/fsl_esdhc_imx.c
> > +++ b/drivers/mmc/fsl_esdhc_imx.c
> > @@ -609,6 +609,8 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct
> mmc *mmc, uint clock)
> >         while (sdhc_clk / (div * pre_div * ddr_pre_div) > clock && div < 16)
> >                 div++;
> >
> > +       mmc->clock = sdhc_clk / pre_div / div / ddr_pre_div;
> > +
> >         pre_div >>= 1;
> >         div -= 1;
> >
> > @@ -630,7 +632,6 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct
> mmc *mmc, uint clock)
> >         else
> >                 esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN |
> > SYSCTL_CKEN);
> >
> > -       mmc->clock = sdhc_clk / pre_div / div;
> >         priv->clock = clock;
> >  }
> >
> > --
> > 2.17.1
> >
> 
> Haibo,
> 
> I found that this particular patch keeps an imx8mm-venice-gw7901 board that
> has a viking vwsdinbdg4 eMMC from booting to Linux. While u-boot appears to
> work ok, as soon as I load a kernel (from emmc or even
> network) and boot to it I hang at 'starting kernel' even with early debug turned
> on.
> 
> u-boot=> mmc list
> FSL_SDHC: 0
> FSL_SDHC: 1
> FSL_SDHC: 2 (eMMC)
> u-boot=> mmc dev 2
> switch to partitions #0, OK
> mmc2(part 0) is current device
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 45
> OEM: 0
> Name: DG4008
> Bus Speed: 200000000
> Mode: HS400ES (200MHz)
> Rd Block Len: 512
> MMC version 5.1
> High Capacity: Yes
> Capacity: 7.3 GiB
> Bus Width: 8-bit DDR
> Erase Group Size: 512 KiB
> HC WP Group Size: 8 MiB
> User Capacity: 7.3 GiB WRREL
> Boot Capacity: 4 MiB ENH
> RPMB Capacity: 4 MiB ENH
> Boot area 0 is not write protected
> Boot area 1 is not write protected
> 
> I have other boards with a Micron MTFC8GAKAJCN non HS400ES that don't have
> any issue so it appears to be something to do with HS400ES support and I find if I
> disable CONFIG_MMC_HS400_ES_SUPPORT or revert this patch the issue goes
> away.
> 
> Any idea what might be going on here?

Hi Tim,

I think this should be related with the HS400ES downgrade miss. Before boot linux, uboot will do some cleanup, remove all devices. Call mmc_blk_remove-> mmc_deinit-> mmc_select_mode_and_width-> mmc_set_card_speed(mmc, MMC_HS, true);

Please make sure in your environment,  finally call mmc_set_card_speed(mmc, MMC_HS, true); which means the HS400ES mode first downgrade to HS mode, then switch to legacy mode. Otherwise, emmc mode switch may meet issue. Some emmc may hang in somewhere, we did meet that before. To debug this, you can enable the debug info, and see the detail steps of mmc related. Not sure in your side, you miss some config which cause this issue.

For why this is related with the clock related patch, I think the wrong clock config happed workaround it.

Best Regards
Haibo Chen


> 
> Best Regards,
> 
> Tim