linux-kernel - Re: [PATCH] mmc: mtk-sd: optimize register settings for hs400(es) mode

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: <f7f414fc-1794-4e73-9b63-68df933a3244@collabora.com>
Date: Thu, 23 Jan 2025 09:44:01 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Andy-ld Lu <andy-ld.lu@...iatek.com>, ulf.hansson@...aro.org,
 matthias.bgg@...il.com, wenbin.mei@...iatek.com
Cc: linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] mmc: mtk-sd: optimize register settings for hs400(es)
 mode

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@...iatek.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