On Thu, 23 Jan 2025 at 10:26, Andy-ld Lu <andy-ld.lu@xxxxxxxxxxxx> wrote: > > For hs400(es) mode, the 'hs400-ds-delay' is typically configured in the > dts. However, some projects may only define 'mediatek,hs400-ds-dly3', > which can lead to initialization failures in hs400es mode. CMD13 reported > response crc error in the mmc_switch_status() just after switching to > hs400es mode. > > [ 1.914038][ T82] mmc0: mmc_select_hs400es failed, error -84 > [ 1.914954][ T82] mmc0: error -84 whilst initialising MMC card > > Currently, the hs400_ds_dly3 value is set within the tuning function. This > means that the PAD_DS_DLY3 field is not configured before tuning process, > which is the reason for the above-mentioned CMD13 response crc error. > > Move the PAD_DS_DLY3 field configuration into msdc_prepare_hs400_tuning(), > and add a value check of hs400_ds_delay to prevent overwriting by zero when > the 'hs400-ds-delay' is not set in the dts. In addition, since hs400(es) > only tune the PAD_DS_DLY1, the PAD_DS_DLY2_SEL bit should be cleared to > bypass it. > > Fixes: c4ac38c6539b ("mmc: mtk-sd: Add HS400 online tuning support") > Signed-off-by: Andy-ld Lu <andy-ld.lu@xxxxxxxxxxxx> Applied for fixes and by adding a stable-tags, thanks! Kind regards Uffe > --- > Changes in v2: > - Change commit title from 'optimize' to 'Fix' > - Add a Fixes tag > - Change spaces back to tabs > > --- > drivers/mmc/host/mtk-sd.c | 31 ++++++++++++++++++++----------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > index 4b6e91372526..345ea91629e0 100644 > --- a/drivers/mmc/host/mtk-sd.c > +++ b/drivers/mmc/host/mtk-sd.c > @@ -273,6 +273,7 @@ > #define MSDC_PAD_TUNE_CMD2_SEL BIT(21) /* RW */ > > #define PAD_DS_TUNE_DLY_SEL BIT(0) /* RW */ > +#define PAD_DS_TUNE_DLY2_SEL BIT(1) /* RW */ > #define PAD_DS_TUNE_DLY1 GENMASK(6, 2) /* RW */ > #define PAD_DS_TUNE_DLY2 GENMASK(11, 7) /* RW */ > #define PAD_DS_TUNE_DLY3 GENMASK(16, 12) /* RW */ > @@ -318,6 +319,7 @@ > > /* EMMC50_PAD_DS_TUNE mask */ > #define PAD_DS_DLY_SEL BIT(16) /* RW */ > +#define PAD_DS_DLY2_SEL BIT(15) /* RW */ > #define PAD_DS_DLY1 GENMASK(14, 10) /* RW */ > #define PAD_DS_DLY3 GENMASK(4, 0) /* RW */ > > @@ -2504,13 +2506,23 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode) > static int msdc_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios) > { > struct msdc_host *host = mmc_priv(mmc); > + > host->hs400_mode = true; > > - if (host->top_base) > - writel(host->hs400_ds_delay, > - host->top_base + EMMC50_PAD_DS_TUNE); > - else > - writel(host->hs400_ds_delay, host->base + PAD_DS_TUNE); > + if (host->top_base) { > + if (host->hs400_ds_dly3) > + sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE, > + PAD_DS_DLY3, host->hs400_ds_dly3); > + if (host->hs400_ds_delay) > + writel(host->hs400_ds_delay, > + host->top_base + EMMC50_PAD_DS_TUNE); > + } else { > + if (host->hs400_ds_dly3) > + sdr_set_field(host->base + PAD_DS_TUNE, > + PAD_DS_TUNE_DLY3, host->hs400_ds_dly3); > + if (host->hs400_ds_delay) > + writel(host->hs400_ds_delay, host->base + PAD_DS_TUNE); > + } > /* hs400 mode must set it to 0 */ > sdr_clr_bits(host->base + MSDC_PATCH_BIT2, MSDC_PATCH_BIT2_CFGCRCSTS); > /* to improve read performance, set outstanding to 2 */ > @@ -2530,14 +2542,11 @@ static int msdc_execute_hs400_tuning(struct mmc_host *mmc, struct mmc_card *card > if (host->top_base) { > sdr_set_bits(host->top_base + EMMC50_PAD_DS_TUNE, > PAD_DS_DLY_SEL); > - if (host->hs400_ds_dly3) > - sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE, > - PAD_DS_DLY3, host->hs400_ds_dly3); > + sdr_clr_bits(host->top_base + EMMC50_PAD_DS_TUNE, > + PAD_DS_DLY2_SEL); > } else { > sdr_set_bits(host->base + PAD_DS_TUNE, PAD_DS_TUNE_DLY_SEL); > - if (host->hs400_ds_dly3) > - sdr_set_field(host->base + PAD_DS_TUNE, > - PAD_DS_TUNE_DLY3, host->hs400_ds_dly3); > + sdr_clr_bits(host->base + PAD_DS_TUNE, PAD_DS_TUNE_DLY2_SEL); > } > > host->hs400_tuning = true; > -- > 2.46.0 >