* [PATCH] mmc: mtk-sd: optimize register settings for hs400(es) mode
@ 2025-01-23 7:44 Andy-ld Lu
2025-01-23 8:44 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 3+ messages in thread
From: Andy-ld Lu @ 2025-01-23 7:44 UTC (permalink / raw)
To: ulf.hansson, matthias.bgg, angelogioacchino.delregno, wenbin.mei
Cc: linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek,
Andy-ld Lu
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.
Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
---
drivers/mmc/host/mtk-sd.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 4b6e91372526..4e3e31ddf84b 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 */
@@ -317,9 +318,10 @@
#define PAD_CMD_TX_DLY GENMASK(16, 12) /* RW */
/* EMMC50_PAD_DS_TUNE mask */
-#define PAD_DS_DLY_SEL BIT(16) /* RW */
-#define PAD_DS_DLY1 GENMASK(14, 10) /* RW */
-#define PAD_DS_DLY3 GENMASK(4, 0) /* RW */
+#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 */
/* LOOP_TEST_CONTROL mask */
#define TEST_LOOP_DSCLK_MUX_SEL BIT(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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] mmc: mtk-sd: optimize register settings for hs400(es) mode
2025-01-23 7:44 [PATCH] mmc: mtk-sd: optimize register settings for hs400(es) mode Andy-ld Lu
@ 2025-01-23 8:44 ` AngeloGioacchino Del Regno
2025-01-23 9:02 ` Andy-ld Lu (卢东)
0 siblings, 1 reply; 3+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-01-23 8:44 UTC (permalink / raw)
To: Andy-ld Lu, ulf.hansson, matthias.bgg, wenbin.mei
Cc: linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek
Il 23/01/25 08:44, Andy-ld Lu ha scritto:
> 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.
The commit title is a bit misleading: you're not "optimizing" the settings,
but actually "fixing" a bug here!
Please fix the commit title.
>
> Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
This commit needs a Fixes tag, please add the relevant one and resend.
> ---
> drivers/mmc/host/mtk-sd.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 4b6e91372526..4e3e31ddf84b 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 */
> @@ -317,9 +318,10 @@
> #define PAD_CMD_TX_DLY GENMASK(16, 12) /* RW */
>
> /* EMMC50_PAD_DS_TUNE mask */
> -#define PAD_DS_DLY_SEL BIT(16) /* RW */
> -#define PAD_DS_DLY1 GENMASK(14, 10) /* RW */
> -#define PAD_DS_DLY3 GENMASK(4, 0) /* RW */
> +#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 */
Why are you changing tabulations with spaces?
Please use tabs.
Cheers,
Angelo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mmc: mtk-sd: optimize register settings for hs400(es) mode
2025-01-23 8:44 ` AngeloGioacchino Del Regno
@ 2025-01-23 9:02 ` Andy-ld Lu (卢东)
0 siblings, 0 replies; 3+ messages in thread
From: Andy-ld Lu (卢东) @ 2025-01-23 9:02 UTC (permalink / raw)
To: ulf.hansson@linaro.org, matthias.bgg@gmail.com,
AngeloGioacchino Del Regno, Wenbin Mei (梅文彬)
Cc: linux-arm-kernel@lists.infradead.org, linux-mmc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org
On Thu, 2025-01-23 at 09:44 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 23/01/25 08:44, Andy-ld Lu ha scritto:
> > 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.
>
> The commit title is a bit misleading: you're not "optimizing" the
> settings,
> but actually "fixing" a bug here!
>
> Please fix the commit title.
>
> >
> > Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
>
> This commit needs a Fixes tag, please add the relevant one and
> resend.
>
Thanks for your review, I will update in next change.
>
> > ---
> > drivers/mmc/host/mtk-sd.c | 37 +++++++++++++++++++++++-----------
> > ---
> > 1 file changed, 23 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> > index 4b6e91372526..4e3e31ddf84b 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 */
> > @@ -317,9 +318,10 @@
> > #define PAD_CMD_TX_DLY GENMASK(16, 12) /* RW */
> >
> > /* EMMC50_PAD_DS_TUNE mask */
> > -#define PAD_DS_DLY_SEL BIT(16) /* RW */
> > -#define PAD_DS_DLY1 GENMASK(14, 10) /* RW */
> > -#define PAD_DS_DLY3 GENMASK(4, 0) /* RW */
> > +#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 */
>
> Why are you changing tabulations with spaces?
> Please use tabs.
>
> Cheers,
> Angelo
>
Because the new added line seems a little misaligned in the patch file,
I have used spaces instead. However, I will change back to tabs in next
change.
Best regards
Andy-ld Lu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-23 9:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-23 7:44 [PATCH] mmc: mtk-sd: optimize register settings for hs400(es) mode Andy-ld Lu
2025-01-23 8:44 ` AngeloGioacchino Del Regno
2025-01-23 9:02 ` Andy-ld Lu (卢东)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox