linux-kernel - Re: [PATCH 2/4] mmc: mtk-sd: add default tuning parameters for mt7620

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <71fbe6ac-c89b-46ea-b0d2-31a829c0a2a0@collabora.com>
Date: Mon, 23 Jun 2025 11:57:01 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Shiji Yang <yangshiji66@...look.com>, linux-mmc@...r.kernel.org
Cc: Chaotian Jing <chaotian.jing@...iatek.com>,
 Ulf Hansson <ulf.hansson@...aro.org>,
 Matthias Brugger <matthias.bgg@...il.com>, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 2/4] mmc: mtk-sd: add default tuning parameters for mt7620

Il 19/06/25 07:35, Shiji Yang ha scritto:
> The MIPS MT762x SoCs require some specific tuning parameters at
> different clock frequencies. These legacy SoCs only support max
> 48~50 MHz High-Speed SD mode. Therefore, the standard tuning step
> is not available. We have to hardcode these tuning parameters to
> make them work properly.
> 
> Signed-off-by: Shiji Yang <yangshiji66@...look.com>
> ---
>   drivers/mmc/host/mtk-sd.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 53d63bb4e..52198daef 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -79,6 +79,8 @@
>   #define MSDC_PATCH_BIT2  0xb8
>   #define MSDC_PAD_TUNE    0xec
>   #define MSDC_PAD_TUNE0   0xf0
> +#define MSDC_DAT_RDDLY0  0xf0
> +#define MSDC_DAT_RDDLY1  0xf4
>   #define PAD_DS_TUNE      0x188
>   #define PAD_CMD_TUNE     0x18c
>   #define EMMC51_CFG0	 0x204
> @@ -449,6 +451,7 @@ struct mtk_mmc_compatible {
>   	bool use_internal_cd;
>   	bool support_new_tx;
>   	bool support_new_rx;
> +	bool mips_mt762x;
>   };
>   
>   struct msdc_tune_para {
> @@ -595,6 +598,7 @@ static const struct mtk_mmc_compatible mt7620_compat = {
>   	.enhance_rx = false,
>   	.support_cmd23 = false,
>   	.use_internal_cd = true,
> +	.mips_mt762x = true,

No, you can't do that - this needs to be done in a clean manner.

Please map that to something that makes sense, as in, add the register definitions
and add something like...

.tune_para = {
	.tune0_rval = FIELD_PREP_CONST(MSDC_PAD_TUNE_DATWRDLY, 16) |
		      FIELD_PREP_CONST(MSDC_PAD_TUNE_DATRDDLY, 16) |
		      FIELD_PREP_CONST(MSDC_PAD_TUNE_CMDRDLY, 16) |
		      FIELD_PREP_CONST(MSDC_PAD_TUNE_CMDRRDLY, 4) |
		      FIELD_PREP_CONST(MSDC_PAD_TUNE_CLKTDLY, 10),
	.rddly0_rval = etc etc etc :-)
};

...then, check if .tune_para is present: if it is, apply the static settings,
otherwise, don't.

Cheers,
Angelo

>   };
>   
>   static const struct mtk_mmc_compatible mt7622_compat = {
> @@ -1090,7 +1094,12 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>   	 * mmc_select_hs400() will drop to 50Mhz and High speed mode,
>   	 * tune result of hs200/200Mhz is not suitable for 50Mhz
>   	 */
> -	if (mmc->actual_clock <= 52000000) {
> +	if (host->dev_comp->mips_mt762x &&
> +	    mmc->actual_clock > 25000000) {
> +		sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> +		sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_DSPL);
> +		sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_W_DSPL);
> +	} else if (mmc->actual_clock <= 52000000) {
>   		writel(host->def_tune_para.iocon, host->base + MSDC_IOCON);
>   		if (host->top_base) {
>   			writel(host->def_tune_para.emmc_top_control,
> @@ -2028,6 +2037,13 @@ static void msdc_init_hw(struct msdc_host *host)
>   				     MSDC_PAD_TUNE_RXDLYSEL);
>   	}
>   
> +	if (host->dev_comp->mips_mt762x) {
> +		/* Set default tuning parameters */
> +		writel(0x84101010, host->base + tune_reg);
> +		writel(0x10101010, host->base + MSDC_DAT_RDDLY0);
> +		writel(0x10101010, host->base + MSDC_DAT_RDDLY1);
> +	}
> +
>   	if (mmc->caps2 & MMC_CAP2_NO_SDIO) {
>   		sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIO);
>   		sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);