[U-Boot] [PATCH v2] mmc: fsl_esdhc: Avoid infinite loop in esdhc_send_cmd_common()
The following hang is observed on a Hummingboard 2 MicroSOM i2eX iMX6D - rev 1.3 with no eMMC populated on board:
U-Boot SPL 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000) Trying to boot from MMC1
U-Boot 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000)
CPU: Freescale i.MX6D rev1.5 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 33C Reset cause: POR Board: MX6 HummingBoard2 DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment
No panel detected: default to HDMI Display: HDMI (1024x768) In: serial Out: serial Err: serial ---> hangs
which is caused by the following infinite loop inside esdhc_send_cmd_common()
while (!(esdhc_read32(®s->irqstat) & flags)) ;
Instead of looping forever, provide an exit path so that a timeout error can be propagated in the case irqstat does not report any interrupts, which may happen when no eMMC is populated on board.
Reported-by: Ricardo Salveti rsalveti@rsalveti.net Signed-off-by: Fabio Estevam festevam@gmail.com --- Changes since v1: - Jump to the label out on error path (Baruch).
drivers/mmc/fsl_esdhc.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f..b8d0b00 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -396,6 +396,7 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint irqstat; u32 flags = IRQSTAT_CC | IRQSTAT_CTOE; struct fsl_esdhc *regs = priv->esdhc_regs; + unsigned long start;
#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111 if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION) @@ -453,8 +454,13 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, flags = IRQSTAT_BRR;
/* Wait for the command to complete */ - while (!(esdhc_read32(®s->irqstat) & flags)) - ; + start = get_timer(0); + while (!(esdhc_read32(®s->irqstat) & flags)) { + if (get_timer(start) > 1000) { + err = -ETIMEDOUT; + goto out; + } + }
irqstat = esdhc_read32(®s->irqstat);
On Mon, Nov 19, 2018 at 12:31 PM Fabio Estevam festevam@gmail.com wrote:
The following hang is observed on a Hummingboard 2 MicroSOM i2eX iMX6D - rev 1.3 with no eMMC populated on board:
U-Boot SPL 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000) Trying to boot from MMC1
U-Boot 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000)
CPU: Freescale i.MX6D rev1.5 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 33C Reset cause: POR Board: MX6 HummingBoard2 DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment
No panel detected: default to HDMI Display: HDMI (1024x768) In: serial Out: serial Err: serial ---> hangs
which is caused by the following infinite loop inside esdhc_send_cmd_common()
while (!(esdhc_read32(®s->irqstat) & flags)) ;Instead of looping forever, provide an exit path so that a timeout error can be propagated in the case irqstat does not report any interrupts, which may happen when no eMMC is populated on board.
Reported-by: Ricardo Salveti rsalveti@rsalveti.net Signed-off-by: Fabio Estevam festevam@gmail.com
Tested-by: Peter Robinson pbrobinson@gmail.com
Boots fine on my i.MX6 Quad hummingboard2 with SOM rev 1.5 which previously has the problem I reported back in 2018.09 series.
Peter
Changes since v1:
- Jump to the label out on error path (Baruch).
drivers/mmc/fsl_esdhc.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f..b8d0b00 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -396,6 +396,7 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint irqstat; u32 flags = IRQSTAT_CC | IRQSTAT_CTOE; struct fsl_esdhc *regs = priv->esdhc_regs;
unsigned long start;#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111 if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION) @@ -453,8 +454,13 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, flags = IRQSTAT_BRR;
/* Wait for the command to complete */
while (!(esdhc_read32(®s->irqstat) & flags));
start = get_timer(0);while (!(esdhc_read32(®s->irqstat) & flags)) {if (get_timer(start) > 1000) {err = -ETIMEDOUT;goto out;}} irqstat = esdhc_read32(®s->irqstat);-- 2.7.4
Hi Fabio,
On Mon, Nov 19, 2018 at 10:31 AM Fabio Estevam festevam@gmail.com wrote:
The following hang is observed on a Hummingboard 2 MicroSOM i2eX iMX6D - rev 1.3 with no eMMC populated on board:
U-Boot SPL 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000) Trying to boot from MMC1
U-Boot 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000)
CPU: Freescale i.MX6D rev1.5 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 33C Reset cause: POR Board: MX6 HummingBoard2 DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment
No panel detected: default to HDMI Display: HDMI (1024x768) In: serial Out: serial Err: serial ---> hangs
which is caused by the following infinite loop inside esdhc_send_cmd_common()
while (!(esdhc_read32(®s->irqstat) & flags)) ;Instead of looping forever, provide an exit path so that a timeout error can be propagated in the case irqstat does not report any interrupts, which may happen when no eMMC is populated on board.
Reported-by: Ricardo Salveti rsalveti@rsalveti.net Signed-off-by: Fabio Estevam festevam@gmail.com
Changes since v1:
- Jump to the label out on error path (Baruch).
drivers/mmc/fsl_esdhc.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f..b8d0b00 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -396,6 +396,7 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint irqstat; u32 flags = IRQSTAT_CC | IRQSTAT_CTOE; struct fsl_esdhc *regs = priv->esdhc_regs;
unsigned long start;#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111 if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION) @@ -453,8 +454,13 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, flags = IRQSTAT_BRR;
/* Wait for the command to complete */
while (!(esdhc_read32(®s->irqstat) & flags));
start = get_timer(0);while (!(esdhc_read32(®s->irqstat) & flags)) {if (get_timer(start) > 1000) {err = -ETIMEDOUT;goto out;}}
This fixes the hang I was having, but adds a significant boot delay because it needs to wait until it gets the timeout (1000). Is there a better timeout value to be used here?
Thanks,
On Mon, Nov 19, 2018 at 12:03 PM Ricardo Salveti rsalveti@rsalveti.net wrote:
Hi Fabio,
On Mon, Nov 19, 2018 at 10:31 AM Fabio Estevam festevam@gmail.com wrote:
The following hang is observed on a Hummingboard 2 MicroSOM i2eX iMX6D - rev 1.3 with no eMMC populated on board:
U-Boot SPL 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000) Trying to boot from MMC1
U-Boot 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000)
CPU: Freescale i.MX6D rev1.5 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 33C Reset cause: POR Board: MX6 HummingBoard2 DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment
No panel detected: default to HDMI Display: HDMI (1024x768) In: serial Out: serial Err: serial ---> hangs
which is caused by the following infinite loop inside esdhc_send_cmd_common()
while (!(esdhc_read32(®s->irqstat) & flags)) ;Instead of looping forever, provide an exit path so that a timeout error can be propagated in the case irqstat does not report any interrupts, which may happen when no eMMC is populated on board.
Reported-by: Ricardo Salveti rsalveti@rsalveti.net Signed-off-by: Fabio Estevam festevam@gmail.com
Changes since v1:
- Jump to the label out on error path (Baruch).
drivers/mmc/fsl_esdhc.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f..b8d0b00 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -396,6 +396,7 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint irqstat; u32 flags = IRQSTAT_CC | IRQSTAT_CTOE; struct fsl_esdhc *regs = priv->esdhc_regs;
unsigned long start;#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111 if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION) @@ -453,8 +454,13 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc, flags = IRQSTAT_BRR;
/* Wait for the command to complete */
while (!(esdhc_read32(®s->irqstat) & flags));
start = get_timer(0);while (!(esdhc_read32(®s->irqstat) & flags)) {if (get_timer(start) > 1000) {err = -ETIMEDOUT;goto out;}}This fixes the hang I was having, but adds a significant boot delay because it needs to wait until it gets the timeout (1000). Is there a better timeout value to be used here?
Looking a bit further, this ends up adding a significant boot delay because the mmc driver still tries to send another two additional commands, so it ends executing this timeout logic 3 times in total. Guess this can be improved at http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mmc/mmc.c;h=d6b9cdc99229d8... by either checking for mmc first or by simply just checking and handling the return code from mmc_send_if_cond, but this could be another patch.
Would still be nice to confirm if 1000 is the minimal timeout value to be used here.
Thanks,
Hi Ricardo,
On Mon, Nov 19, 2018 at 12:25 PM Ricardo Salveti rsalveti@rsalveti.net wrote:
Looking a bit further, this ends up adding a significant boot delay because the mmc driver still tries to send another two additional commands, so it ends executing this timeout logic 3 times in total. Guess this can be improved at http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mmc/mmc.c;h=d6b9cdc99229d8... by either checking for mmc first or by simply just checking and handling the return code from mmc_send_if_cond, but this could be another patch.
Yes, I think you are right.
Does only adding the error check below avoid the hang in your case?
/* Test for SD version 2 */ err = mmc_send_if_cond(mmc); if (err) return err;
Thanks
Hi Fabio,
On Mon, Nov 19, 2018 at 12:41:49PM -0200, Fabio Estevam wrote:
On Mon, Nov 19, 2018 at 12:25 PM Ricardo Salveti rsalveti@rsalveti.net wrote:
Looking a bit further, this ends up adding a significant boot delay because the mmc driver still tries to send another two additional commands, so it ends executing this timeout logic 3 times in total. Guess this can be improved at http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mmc/mmc.c;h=d6b9cdc99229d8... by either checking for mmc first or by simply just checking and handling the return code from mmc_send_if_cond, but this could be another patch.
Yes, I think you are right.
Does only adding the error check below avoid the hang in your case?
/* Test for SD version 2 */ err = mmc_send_if_cond(mmc); if (err) return err;
There used to be a similar check in the past. Commit f33cb34b3971dabe3 ("mmc: Remove return from mmc_init for non SD 2.0 compatible cards.") removed this check.
baruch
On Mon, Nov 19, 2018 at 12:42 PM Fabio Estevam festevam@gmail.com wrote:
Hi Ricardo,
On Mon, Nov 19, 2018 at 12:25 PM Ricardo Salveti rsalveti@rsalveti.net wrote:
Looking a bit further, this ends up adding a significant boot delay because the mmc driver still tries to send another two additional commands, so it ends executing this timeout logic 3 times in total. Guess this can be improved at http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mmc/mmc.c;h=d6b9cdc99229d8... by either checking for mmc first or by simply just checking and handling the return code from mmc_send_if_cond, but this could be another patch.
Yes, I think you are right.
Does only adding the error check below avoid the hang in your case?
/* Test for SD version 2 */ err = mmc_send_if_cond(mmc); if (err) return err;
You patch is still required as the hang I was getting was happening in the while loop itself. Adding this change on top of your patch would just reduce the delay as the code path would avoid another 2 extra commands (both returning timeout).
The only issue, as pointed out by Baruch, is that we already had a check that was ignoring the timeout here before, so I guess the current behavior is correct/expected.
+1 for your patch as it is a good fix anyway and unblocks the boot process on my som/board, even if it adds a longer boot delay.
Tested-by: Ricardo Salveti rsalveti@rsalveti.net
Thanks,
Hi Jaehoon,
On Mon, Nov 19, 2018 at 10:31 AM Fabio Estevam festevam@gmail.com wrote:
The following hang is observed on a Hummingboard 2 MicroSOM i2eX iMX6D - rev 1.3 with no eMMC populated on board:
U-Boot SPL 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000) Trying to boot from MMC1
U-Boot 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000)
CPU: Freescale i.MX6D rev1.5 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 33C Reset cause: POR Board: MX6 HummingBoard2 DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment
No panel detected: default to HDMI Display: HDMI (1024x768) In: serial Out: serial Err: serial ---> hangs
which is caused by the following infinite loop inside esdhc_send_cmd_common()
while (!(esdhc_read32(®s->irqstat) & flags)) ;Instead of looping forever, provide an exit path so that a timeout error can be propagated in the case irqstat does not report any interrupts, which may happen when no eMMC is populated on board.
Reported-by: Ricardo Salveti rsalveti@rsalveti.net Signed-off-by: Fabio Estevam festevam@gmail.com
Could this one be applied to 2019.01?
Hi Tom/Stefano,
On Sat, Dec 8, 2018 at 12:58 PM Fabio Estevam festevam@gmail.com wrote:
Hi Jaehoon,
On Mon, Nov 19, 2018 at 10:31 AM Fabio Estevam festevam@gmail.com wrote:
The following hang is observed on a Hummingboard 2 MicroSOM i2eX iMX6D - rev 1.3 with no eMMC populated on board:
U-Boot SPL 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000) Trying to boot from MMC1
U-Boot 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000)
CPU: Freescale i.MX6D rev1.5 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 33C Reset cause: POR Board: MX6 HummingBoard2 DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment
No panel detected: default to HDMI Display: HDMI (1024x768) In: serial Out: serial Err: serial ---> hangs
which is caused by the following infinite loop inside esdhc_send_cmd_common()
while (!(esdhc_read32(®s->irqstat) & flags)) ;Instead of looping forever, provide an exit path so that a timeout error can be propagated in the case irqstat does not report any interrupts, which may happen when no eMMC is populated on board.
Reported-by: Ricardo Salveti rsalveti@rsalveti.net Signed-off-by: Fabio Estevam festevam@gmail.com
Could this one be applied to 2019.01?
I haven't heard back from Jaehoon.
Can we have this one applied for 2019.01 in order to fix a regression?
Thanks
On Mon, Nov 19, 2018 at 10:31:53AM -0200, Fabio Estevam wrote:
The following hang is observed on a Hummingboard 2 MicroSOM i2eX iMX6D - rev 1.3 with no eMMC populated on board:
U-Boot SPL 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000) Trying to boot from MMC1
U-Boot 2018.11+gf6206f8587 (Nov 16 2018 - 00:56:34 +0000)
CPU: Freescale i.MX6D rev1.5 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 33C Reset cause: POR Board: MX6 HummingBoard2 DRAM: 1 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment
No panel detected: default to HDMI Display: HDMI (1024x768) In: serial Out: serial Err: serial
Applied to u-boot/master, thanks!
participants (5)
-
Baruch Siach -
Fabio Estevam -
Peter Robinson -
Ricardo Salveti -
Tom Rini