[PATCH 0/3] mmc: mmci: stm32: silent data corruption and HS200 fixes

[PATCH 0/3] mmc: mmci: stm32: silent data corruption and HS200 fixes

[Mateusz Nowicki]

The eMMC of the STM32MP257F-DK in HS200 returned stale data and lost
writes without any error. Patch 1 makes data CRC errors visible to the
core again, patches 2 and 3 remove the causes seen on the DK.

Tested on the STM32MP257F-DK (eMMC in HS200 at 100 and 150 MHz) with a
v7.3-rc6 based kernel whose mmci files match this series. Build tested
on master. Not tested on STM32MP1x.

Mateusz Nowicki (3):
  mmc: mmci: do not count a data CRC error as a complete transfer
  mmc: mmci: stm32: do not use NEGEDGE in HS200 mode
  mmc: mmci: stm32: lock the STM32MP25 delay block again before tuning

 drivers/mmc/host/mmci.c             |  5 +++--
 drivers/mmc/host/mmci_stm32_sdmmc.c | 18 ++++++++++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)


base-commit: 36aad21d50d1af3f6954bb0c988257a908202477
-- 
2.56.0

[PATCH 1/3] mmc: mmci: do not count a data CRC error as a complete transfer

[Mateusz Nowicki]

With datacnt_useless (STM32 variants) success is 0 on a data error and
a CRC error subtracts one from it. bytes_xfered becomes 0xfffffe00 and
mmc_blk completes the whole request as successful, although the
controller stopped at the failing block. Reads return stale data and
writes are lost without any error.

Tested on the STM32MP257F-DK eMMC in HS200 with a forced bad tuning:
without this change 5 of 14 reads returned zeros, with it all 10 reads
were correct.

Fixes: b79220b3e0af ("mmc: mmci: add variant property to not read datacnt")
Cc: stable@vger.kernel.org
Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
---
 drivers/mmc/host/mmci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 416bdb184ed4..10eb0439fc52 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1422,8 +1422,9 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		dev_dbg(mmc_dev(host->mmc), "MCI ERROR IRQ, status 0x%08x at 0x%08x\n",
 			status_err, success);
 		if (status_err & MCI_DATACRCFAIL) {
-			/* Last block was not successful */
-			success -= 1;
+			/* Last block was not successful, nothing counted when DATACNT is useless */
+			if (success)
+				success -= 1;
 			data->error = -EILSEQ;
 		} else if (status_err & MCI_DATATIMEOUT) {
 			data->error = -ETIMEDOUT;
-- 
2.56.0

[PATCH 2/3] mmc: mmci: stm32: do not use NEGEDGE in HS200 mode

[Mateusz Nowicki]

RM0457 marks NEGEDGE as not applicable in HS200. With a clock divider it
corrupts multi-block writes: on the STM32MP257F-DK (st,neg-edge, 200 MHz
kernel clock, HS200 at 100 MHz) written data read back different. The
same writes are correct without NEGEDGE.

Fixes: 36240ef8665b ("mmc: mmci: add hs200 support for stm32 sdmmc")
Cc: stable@vger.kernel.org
Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
---
RM0457 describes HS200 only without a divider. If HS200 through a
divider should not be used at all, the alternatives for the DK are:

- Drop mmc-hs200-1_8v from the DK device tree. DDR52 is tested and only
  about 5% slower than HS200 at 100 MHz (read/write 78/66 MiB/s against
  82/68 MiB/s).
- Drop st,neg-edge from the DK device tree. This also changes DDR52,
  which was not tested without NEGEDGE.
- Set the SDMMC2 kernel clock to 150 MHz in the firmware (OP-TEE). HS200
  then runs without a divider (tested, 119/86 MiB/s).

Which way is preferred?

 drivers/mmc/host/mmci_stm32_sdmmc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 9dc51859c2e5..d74efdff6ac2 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -342,6 +342,10 @@ static void mmci_sdmmc_set_clkreg(struct mmci_host *host, unsigned int desired)
 	clk |= host->clk_reg_add;
 	clk |= ddr;
 
+	/* NEGEDGE is not applicable in HS200 (RM0457), with a divider it corrupts writes */
+	if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200)
+		clk &= ~MCI_STM32_CLK_NEGEDGE;
+
 	if (host->mmc->ios.timing >= MMC_TIMING_UHS_SDR50)
 		clk |= MCI_STM32_CLK_BUSSPEED;
 
-- 
2.56.0

Re: [PATCH 2/3] mmc: mmci: stm32: do not use NEGEDGE in HS200 mode

[Christophe KERELLO]

Hello Mateusz,

On 10/31/26 13:07, Mateusz Nowicki wrote:
> RM0457 marks NEGEDGE as not applicable in HS200. With a clock divider it

No, it is not what it is written in the RM. If clkdiv is equal to 0, 
NEGEDGE has no effect.
NEGEDGE has an effect only when clkdiv > 0, so it can be used with HS200.

On ST downstream Linux Kernel, you can find following patch not already 
upstream:
mmc: mmci: stm32: use the delay block above 100 MHz on stm32mp25 SoC 
(Commit ca3b1cf)
By applying previous downstream patch + patch 3 of this patchset + 
using  original slew rate that was upstream on
this board (slew rate set to 2 for the clock IO, 1 for the others IOs), 
HS200 should run on a ST MP25 DK board at 100 MHz.

This patch breaks also MP13 SoC that is supporting HS200.

Regards,
Christophe Kerello.

> corrupts multi-block writes: on the STM32MP257F-DK (st,neg-edge, 200 MHz
> kernel clock, HS200 at 100 MHz) written data read back different. The
> same writes are correct without NEGEDGE.
>
> Fixes: 36240ef8665b ("mmc: mmci: add hs200 support for stm32 sdmmc")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
> ---
> RM0457 describes HS200 only without a divider. If HS200 through a
> divider should not be used at all, the alternatives for the DK are:
>
> - Drop mmc-hs200-1_8v from the DK device tree. DDR52 is tested and only
>    about 5% slower than HS200 at 100 MHz (read/write 78/66 MiB/s against
>    82/68 MiB/s).
> - Drop st,neg-edge from the DK device tree. This also changes DDR52,
>    which was not tested without NEGEDGE.
> - Set the SDMMC2 kernel clock to 150 MHz in the firmware (OP-TEE). HS200
>    then runs without a divider (tested, 119/86 MiB/s).
>
> Which way is preferred?
>
>   drivers/mmc/host/mmci_stm32_sdmmc.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
> index 9dc51859c2e5..d74efdff6ac2 100644
> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
> @@ -342,6 +342,10 @@ static void mmci_sdmmc_set_clkreg(struct mmci_host *host, unsigned int desired)
>   	clk |= host->clk_reg_add;
>   	clk |= ddr;
>   
> +	/* NEGEDGE is not applicable in HS200 (RM0457), with a divider it corrupts writes */
> +	if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200)
> +		clk &= ~MCI_STM32_CLK_NEGEDGE;
> +
>   	if (host->mmc->ios.timing >= MMC_TIMING_UHS_SDR50)
>   		clk |= MCI_STM32_CLK_BUSSPEED;
>   

[PATCH 3/3] mmc: mmci: stm32: lock the STM32MP25 delay block again before tuning

[Mateusz Nowicki]

Once locked, the DLL of the delay block follows voltage and temperature
but not a change of the clock frequency. RM0457 requires the DLL to be
disabled and locked again after a frequency switch.

sdmmc_dlyb_mp25_enable() only sets EN, so a DLL left enabled by an
earlier tuning keeps its old lock. On the STM32MP257F-DK eMMC, switching
from DDR52 to HS200 then made the tuning pick a wrong phase in 9 of 18
tries. With this change the phase was correct in all 12 tries.

Clear EN and wait for LOCK to drop before enabling the DLL again.

Fixes: b5c3eb385797 ("mmc: mmci: stm32: add delay block support for STM32MP25")
Cc: stable@vger.kernel.org
Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
---
 drivers/mmc/host/mmci_stm32_sdmmc.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index d74efdff6ac2..2310dba75c35 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -541,11 +541,21 @@ static int sdmmc_dlyb_mp15_prepare(struct mmci_host *host)
 static int sdmmc_dlyb_mp25_enable(struct sdmmc_dlyb *dlyb)
 {
 	u32 cr, sr;
+	int ret;
 
+	/*
+	 * The DLL tracks voltage and temperature but not a frequency switch
+	 * (RM0457), start the lock sequence again on the current clock
+	 */
 	cr = readl_relaxed(dlyb->base + SYSCFG_DLYBSD_CR);
-	cr |= DLYBSD_CR_EN;
+	writel_relaxed(cr & ~DLYBSD_CR_EN, dlyb->base + SYSCFG_DLYBSD_CR);
+	ret = readl_relaxed_poll_timeout(dlyb->base + SYSCFG_DLYBSD_SR,
+					 sr, !(sr & DLYBSD_SR_LOCK), 1,
+					 DLYBSD_TIMEOUT_1S_IN_US);
+	if (ret)
+		return ret;
 
-	writel_relaxed(cr, dlyb->base + SYSCFG_DLYBSD_CR);
+	writel_relaxed(cr | DLYBSD_CR_EN, dlyb->base + SYSCFG_DLYBSD_CR);
 
 	return readl_relaxed_poll_timeout(dlyb->base + SYSCFG_DLYBSD_SR,
 					   sr, sr & DLYBSD_SR_LOCK, 1,
-- 
2.56.0

Re: [PATCH 3/3] mmc: mmci: stm32: lock the STM32MP25 delay block again before tuning

[Christophe KERELLO]

Hello Mateusz,

This patch is similar with the one that we have on ST downstream.
mmc: mmci: stm32: release the delay block before enabling it (Commit 
01d3cac)

If it is the case and if you are going to upstream ST downstream source 
code,
do not forget to keep ST signed-off author, please.

Regards,
Christophe Kerello.

On 10/31/26 13:07, Mateusz Nowicki wrote:
> Once locked, the DLL of the delay block follows voltage and temperature
> but not a change of the clock frequency. RM0457 requires the DLL to be
> disabled and locked again after a frequency switch.
>
> sdmmc_dlyb_mp25_enable() only sets EN, so a DLL left enabled by an
> earlier tuning keeps its old lock. On the STM32MP257F-DK eMMC, switching
> from DDR52 to HS200 then made the tuning pick a wrong phase in 9 of 18
> tries. With this change the phase was correct in all 12 tries.
>
> Clear EN and wait for LOCK to drop before enabling the DLL again.
>
> Fixes: b5c3eb385797 ("mmc: mmci: stm32: add delay block support for STM32MP25")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
> ---
>   drivers/mmc/host/mmci_stm32_sdmmc.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
> index d74efdff6ac2..2310dba75c35 100644
> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
> @@ -541,11 +541,21 @@ static int sdmmc_dlyb_mp15_prepare(struct mmci_host *host)
>   static int sdmmc_dlyb_mp25_enable(struct sdmmc_dlyb *dlyb)
>   {
>   	u32 cr, sr;
> +	int ret;
>   
> +	/*
> +	 * The DLL tracks voltage and temperature but not a frequency switch
> +	 * (RM0457), start the lock sequence again on the current clock
> +	 */
>   	cr = readl_relaxed(dlyb->base + SYSCFG_DLYBSD_CR);
> -	cr |= DLYBSD_CR_EN;
> +	writel_relaxed(cr & ~DLYBSD_CR_EN, dlyb->base + SYSCFG_DLYBSD_CR);
> +	ret = readl_relaxed_poll_timeout(dlyb->base + SYSCFG_DLYBSD_SR,
> +					 sr, !(sr & DLYBSD_SR_LOCK), 1,
> +					 DLYBSD_TIMEOUT_1S_IN_US);
> +	if (ret)
> +		return ret;
>   
> -	writel_relaxed(cr, dlyb->base + SYSCFG_DLYBSD_CR);
> +	writel_relaxed(cr | DLYBSD_CR_EN, dlyb->base + SYSCFG_DLYBSD_CR);
>   
>   	return readl_relaxed_poll_timeout(dlyb->base + SYSCFG_DLYBSD_SR,
>   					   sr, sr & DLYBSD_SR_LOCK, 1,