[PATCH 0/4] mci: stm32_sdmmc2: timeout and bus-width bug fixes

[PATCH 0/4] mci: stm32_sdmmc2: timeout and bus-width bug fixes

[Ahmad Fatoum]

Hi,

would be great if the first 4 patches could go into master.

Cheers,
Ahmad

Ahmad Fatoum (5):
  mci: stm32_sdmmc2: use correct card power cycle sequence
  mci: stm32_sdmmc2: don't reset before retrying failed operations
  mci: core: don't test 4-bit-buswidth support if MMC host lacks
    capability
  mci: stm32_sdmmc2: compare ios ->bus_width with correct constants
  mci: stm32_sdmmc2: fix typo in debug string

 drivers/mci/mci-core.c     |  3 +++
 drivers/mci/stm32_sdmmc2.c | 42 +++++++++++++++-----------------------
 2 files changed, 20 insertions(+), 25 deletions(-)

-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/5] mci: stm32_sdmmc2: use correct card power cycle sequence

[Ahmad Fatoum]

The stm32mp157c RM0436 reference manual lists following sequence for
resetting the SDMMC:

1. Reset using the RCC.SDMMCxRST register bit
  -> We do this via reset_control_assert

2. Disable Vcc power to the card
  -> We don't do this, because the rail often has other 3v3 peripherals
     hanging off it

3. Set the SDMMC to power-cycle state
  -> We do this only when the MCI core sets the frequency to zero, not
     on init!

4. reenable Vcc power to the card after a >1ms
  -> We don't do this, because we don't disable Vcc

5. Wait >1ms for the power ramp period, then put card into power-off
  -> We do this in stm32_sdmmc2_pwron

6. Wait >1ms, then put card into power-on state
  -> We do this in stm32_sdmmc2_pwron

7. Wait 74 SDMMC_CK cycles before sending the first command to the card
  -> We don't have an explicit delay between mci_startup_{sd,mmc} and
     mci_set_blocklen, so a low max-speed may run afoul of the timing
     constraint

Fix 3 & 7. 3 is the main one and doing it fixes an issue of timeouts
occurring, when having barebox in a reboot loop or the SD-Card inside
a USB-SD-Mux.

Fixes: ce99d0c86b32ec ("mci: add support for stm32mp sd/mmc controller")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/stm32_sdmmc2.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c
index 1a41c34d24e2..eba079f1799b 100644
--- a/drivers/mci/stm32_sdmmc2.c
+++ b/drivers/mci/stm32_sdmmc2.c
@@ -185,31 +185,31 @@ struct stm32_sdmmc2_priv {
 
 #define to_mci_host(mci)	container_of(mci, struct stm32_sdmmc2_priv, mci)
 
-/*
- * Reset the SDMMC with the RCC.SDMMCxRST register bit.
- * This will reset the SDMMC to the reset state and the CPSM and DPSM
- * to the Idle state. SDMMC is disabled, Signals Hiz.
- */
 static int stm32_sdmmc2_reset(struct mci_host *mci, struct device_d *mci_dev)
 {
 	struct stm32_sdmmc2_priv *priv = to_mci_host(mci);
 
+	/*
+	 * Reset the SDMMC with the RCC.SDMMCxRST register bit.
+	 * This will reset the SDMMC to the reset state and the CPSM and DPSM
+	 * to the Idle state. SDMMC is disabled, Signals Hiz.
+	 */
 	reset_control_assert(priv->reset_ctl);
 	udelay(2);
 	reset_control_deassert(priv->reset_ctl);
 
-	/* init the needed SDMMC register after reset */
-	writel(priv->pwr_reg_msk, priv->base + SDMMC_POWER);
+	/*
+	 * Set the SDMMC in power-cycle state.
+	 * This will make that the SDMMC_D[7:0],
+	 * SDMMC_CMD and SDMMC_CK are driven low, to prevent the card from being
+	 * supplied through the signal lines.
+	 */
+	writel(SDMMC_POWER_PWRCTRL_CYCLE | priv->pwr_reg_msk,
+	       priv->base + SDMMC_POWER);
 
 	return 0;
 }
 
-/*
- * Set the SDMMC in power-cycle state.
- * This will make that the SDMMC_D[7:0],
- * SDMMC_CMD and SDMMC_CK are driven low, to prevent the card from being
- * supplied through the signal lines.
- */
 static void stm32_sdmmc2_pwrcycle(struct stm32_sdmmc2_priv *priv)
 {
 	if ((readl(priv->base + SDMMC_POWER) & SDMMC_POWER_PWRCTRL_MASK) ==
@@ -217,8 +217,6 @@ static void stm32_sdmmc2_pwrcycle(struct stm32_sdmmc2_priv *priv)
 		return;
 
 	stm32_sdmmc2_reset(&priv->mci, priv->dev);
-	writel(SDMMC_POWER_PWRCTRL_CYCLE | priv->pwr_reg_msk,
-	       priv->base + SDMMC_POWER);
 }
 
 /*
@@ -256,6 +254,7 @@ static void stm32_sdmmc2_pwron(struct stm32_sdmmc2_priv *priv)
 	       priv->base + SDMMC_POWER);
 
 	/* during the first 74 SDMMC_CK cycles the SDMMC is still disabled. */
+	udelay(DIV_ROUND_UP(74 * USEC_PER_SEC, priv->mci.clock));
 }
 
 static void stm32_sdmmc2_start_data(struct stm32_sdmmc2_priv *priv,
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/5] mci: stm32_sdmmc2: don't reset before retrying failed operations

[Ahmad Fatoum]

The original U-Boot code doesn't do this and it wipes the configured
settings like clock rate, ensuring that we won't ever recover from the
error. Drop it.

Fixes: ce99d0c86b32ec ("mci: add support for stm32mp sd/mmc controller")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/stm32_sdmmc2.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c
index eba079f1799b..7f22a90668e4 100644
--- a/drivers/mci/stm32_sdmmc2.c
+++ b/drivers/mci/stm32_sdmmc2.c
@@ -534,8 +534,6 @@ retry_cmd:
 		dev_warn(priv->dev, "%s: cmd %d failed, retrying ...\n",
 			 __func__, cmd->cmdidx);
 
-		stm32_sdmmc2_pwrcycle(priv);
-		stm32_sdmmc2_pwron(priv);
 		retry--;
 
 		goto retry_cmd;
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/5] mci: core: don't test 4-bit-buswidth support if MMC host lacks capability

[Ahmad Fatoum]

We already check for MMC_CAP_8_BIT_DATA before trying if buswidth=8
yields legible data. For consistency, don't test whether buswidth=4
works if the MMC host doesn't indicated MMC_CAP_4_BIT_DATA.

Fixes: 68db956d ("mci: Fix 8 bit mmc cards")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/mci-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 67257bcd186c..dd163c3d1652 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1025,6 +1025,9 @@ static int mci_startup_mmc(struct mci *mci)
 
 	mci_set_clock(mci, mci->tran_speed);
 
+	if (!(host->host_caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)))
+		return 0;
+
 	/*
 	 * Unlike SD, MMC cards dont have a configuration register to notify
 	 * supported bus width. So bus test command should be run to identify
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/5] mci: stm32_sdmmc2: compare ios ->bus_width with correct constants

[Ahmad Fatoum]

MMC_BUS_WIDTH_4 != 4 && MMC_BUS_WIDTH_8 != 8.

This enables driver support for 4-bit SD-Cards and 8-bit MMCs,
which before was disabled because of CRC errors:

  ERROR: error SDMMC_STA_DCRCFAIL (0x81042) for cmd 18
  ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12

These happened because the card was correctly instructed to use the new
bus width, but the controller wasn't.

Cc: Oleksij Rempel <o.rempel@pengutronix.de>
Fixes: ce99d0c86b32ec ("mci: add support for stm32mp sd/mmc controller")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/stm32_sdmmc2.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c
index 7f22a90668e4..fdd0e7858f9e 100644
--- a/drivers/mci/stm32_sdmmc2.c
+++ b/drivers/mci/stm32_sdmmc2.c
@@ -574,9 +574,9 @@ static void stm32_sdmmc2_set_ios(struct mci_host *mci, struct mci_ios *ios)
 			clk = SDMMC_CLKCR_CLKDIV_MAX;
 	}
 
-	if (mci->bus_width == 4)
+	if (mci->bus_width == MMC_BUS_WIDTH_4)
 		clk |= SDMMC_CLKCR_WIDBUS_4;
-	if (mci->bus_width == 8)
+	if (mci->bus_width == MMC_BUS_WIDTH_8)
 		clk |= SDMMC_CLKCR_WIDBUS_8;
 
 	writel(clk | priv->clk_reg_msk | SDMMC_CLKCR_HWFC_EN,
@@ -631,11 +631,6 @@ static int stm32_sdmmc2_probe(struct amba_device *adev,
 
 	mci_of_parse(&priv->mci);
 
-	if (mci->host_caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)) {
-		dev_notice(dev, "Fixing bus-width to 1 due to driver limitation\n");
-		mci->host_caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
-	}
-
 	return mci_register(&priv->mci);
 
 priv_free:
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/5] mci: stm32_sdmmc2: fix typo in debug string

[Ahmad Fatoum]

s/bus_with/bus_width/

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/stm32_sdmmc2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/stm32_sdmmc2.c b/drivers/mci/stm32_sdmmc2.c
index fdd0e7858f9e..44f7e6239949 100644
--- a/drivers/mci/stm32_sdmmc2.c
+++ b/drivers/mci/stm32_sdmmc2.c
@@ -552,7 +552,7 @@ static void stm32_sdmmc2_set_ios(struct mci_host *mci, struct mci_ios *ios)
 	u32 sys_clock = clk_get_rate(priv->clk);
 	u32 clk = 0;
 
-	dev_dbg(priv->dev, "%s: bus_with = %d, clock = %d\n", __func__,
+	dev_dbg(priv->dev, "%s: bus_width = %d, clock = %d\n", __func__,
 		mci->bus_width, mci->clock);
 
 	if (mci->clock)
-- 
2.25.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/4] mci: stm32_sdmmc2: timeout and bus-width bug fixes

[Sascha Hauer]

On Thu, Jan 30, 2020 at 06:04:44PM +0100, Ahmad Fatoum wrote:
> Hi,
> 
> would be great if the first 4 patches could go into master.

Did that, thanks

Sascha

> 
> Cheers,
> Ahmad
> 
> Ahmad Fatoum (5):
>   mci: stm32_sdmmc2: use correct card power cycle sequence
>   mci: stm32_sdmmc2: don't reset before retrying failed operations
>   mci: core: don't test 4-bit-buswidth support if MMC host lacks
>     capability
>   mci: stm32_sdmmc2: compare ios ->bus_width with correct constants
>   mci: stm32_sdmmc2: fix typo in debug string
> 
>  drivers/mci/mci-core.c     |  3 +++
>  drivers/mci/stm32_sdmmc2.c | 42 +++++++++++++++-----------------------
>  2 files changed, 20 insertions(+), 25 deletions(-)
> 
> -- 
> 2.25.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox