[PATCH] sunxi-mmc: increase stabilization delay from 1ms to 20ms

[PATCH] sunxi-mmc: increase stabilization delay from 1ms to 20ms

[Da Xue]

Some users experienced problems booting u-boot from SPL hanging here:

Trying to boot from MMC1 or Trying to boot from MMC2

This seems to occur with both MicroSD and eMMC modules on ALL-H3-CC.
Increasing the delay after mmc reset fixes these boot problems.
Some MicroSD cards are impacted more than others so it is possible that
MicroSD internals need time to stabilize. Below is some failure data.

sandisk ultra   64gb 9/20 with 1ms,  20/20 with 10ms
sandisk ultra   16gb 2/20 with 1ms,  20/20 with 10ms
sandisk extreme 16gb 6/20 with 10ms, 20/20 with 20ms

A quick comparison of schematics show series resistors for ESD
protection on the MicroSD GPIOs not present on all H3/H5 boards.
It is not known if this is related to the issue.

This patch adds a fixed 20ms delay to mmc init to mitigate the problem.
If boot time optimization is required and the platform does not require
the delay. The delay can be replaced with:

writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
if (wait_for_bit_le32( &priv->reg->gctrl,
SUNXI_MMC_GCTRL_RESET, false, 20, false)) {
printf("%s: Timeout\n", __func__);
return -ETIMEDOUT;
}

Signed-off-by: Da Xue <da@libre.computer>
---
 drivers/mmc/sunxi_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
index 1bb7b6d0e9..f7942b69ce 100644
--- a/drivers/mmc/sunxi_mmc.c
+++ b/drivers/mmc/sunxi_mmc.c
@@ -297,7 +297,7 @@ static int sunxi_mmc_core_init(struct mmc *mmc)

  /* Reset controller */
  writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
- udelay(1000);
+ udelay(20000);

  return 0;
 }
-- 
2.30.2

Re: [PATCH] sunxi-mmc: increase stabilization delay from 1ms to 20ms

[Andre Przywara]

On 21/07/2022 23:08, Da Xue wrote:

Hi,

> Some users experienced problems booting u-boot from SPL hanging here:
> 
> Trying to boot from MMC1 or Trying to boot from MMC2
> 
> This seems to occur with both MicroSD and eMMC modules on ALL-H3-CC.
> Increasing the delay after mmc reset fixes these boot problems.
> Some MicroSD cards are impacted more than others so it is possible that
> MicroSD internals need time to stabilize. Below is some failure data.
> 
> sandisk ultra   64gb 9/20 with 1ms,  20/20 with 10ms
> sandisk ultra   16gb 2/20 with 1ms,  20/20 with 10ms
> sandisk extreme 16gb 6/20 with 10ms, 20/20 with 20ms
> 
> A quick comparison of schematics show series resistors for ESD
> protection on the MicroSD GPIOs not present on all H3/H5 boards.
> It is not known if this is related to the issue.
> 
> This patch adds a fixed 20ms delay to mmc init to mitigate the problem.
> If boot time optimization is required and the platform does not require
> the delay. The delay can be replaced with:
> 
> writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> if (wait_for_bit_le32( &priv->reg->gctrl,
> SUNXI_MMC_GCTRL_RESET, false, 20, false)) {
> printf("%s: Timeout\n", __func__);
> return -ETIMEDOUT;
> }

So what about adding this for everyone (we should have it regardless) then?

And then have an extra Kconfig option to specify an extra delay, and 
define this only in your board defconfig? Because IIUC this is specific
to your board?

And as I mentioned: it looks odd to have this here and have it fixing 
your SD card problems:
- The soft reset should reset just the internal controller logic (MMC 
state machine and FIFOs), this shouldn't affect cards. IIUC, nothing 
should happen on the MMC *pins* because of this operation.
- Why isn't this a problem for U-Boot proper, and Linux, FWIW?

Cheers,
Andre

> 
> Signed-off-by: Da Xue <da@libre.computer>
> ---
>   drivers/mmc/sunxi_mmc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> index 1bb7b6d0e9..f7942b69ce 100644
> --- a/drivers/mmc/sunxi_mmc.c
> +++ b/drivers/mmc/sunxi_mmc.c
> @@ -297,7 +297,7 @@ static int sunxi_mmc_core_init(struct mmc *mmc)
> 
>    /* Reset controller */
>    writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> - udelay(1000);
> + udelay(20000);
> 
>    return 0;
>   }

Re: [PATCH] sunxi-mmc: increase stabilization delay from 1ms to 20ms

[Jernej Škrabec]

Dne petek, 22. julij 2022 ob 18:55:14 CEST je Andre Przywara napisal(a):
> On 21/07/2022 23:08, Da Xue wrote:
> 
> Hi,
> 
> > Some users experienced problems booting u-boot from SPL hanging here:
> > 
> > Trying to boot from MMC1 or Trying to boot from MMC2
> > 
> > This seems to occur with both MicroSD and eMMC modules on ALL-H3-CC.
> > Increasing the delay after mmc reset fixes these boot problems.
> > Some MicroSD cards are impacted more than others so it is possible that
> > MicroSD internals need time to stabilize. Below is some failure data.
> > 
> > sandisk ultra   64gb 9/20 with 1ms,  20/20 with 10ms
> > sandisk ultra   16gb 2/20 with 1ms,  20/20 with 10ms
> > sandisk extreme 16gb 6/20 with 10ms, 20/20 with 20ms
> > 
> > A quick comparison of schematics show series resistors for ESD
> > protection on the MicroSD GPIOs not present on all H3/H5 boards.
> > It is not known if this is related to the issue.
> > 
> > This patch adds a fixed 20ms delay to mmc init to mitigate the problem.
> > If boot time optimization is required and the platform does not require
> > the delay. The delay can be replaced with:
> > 
> > writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> > if (wait_for_bit_le32( &priv->reg->gctrl,
> > SUNXI_MMC_GCTRL_RESET, false, 20, false)) {
> > printf("%s: Timeout\n", __func__);
> > return -ETIMEDOUT;
> > }
> 
> So what about adding this for everyone (we should have it regardless) then?
> 
> And then have an extra Kconfig option to specify an extra delay, and
> define this only in your board defconfig? Because IIUC this is specific
> to your board?
> 
> And as I mentioned: it looks odd to have this here and have it fixing
> your SD card problems:
> - The soft reset should reset just the internal controller logic (MMC
> state machine and FIFOs), this shouldn't affect cards. IIUC, nothing
> should happen on the MMC *pins* because of this operation.
> - Why isn't this a problem for U-Boot proper, and Linux, FWIW?

As I said before, I have issue only at cold boot in SPL. Once I pass this 
point, it works all the time, even if rebooted. Why is that so it's unclear.

Thinking about this a bit, I have another question. How is it possible that 
BROM manages to read SD card just fine and loads SPL beforehand? Is it using 
lower speed? It also couldn't be power issue, since card must have been 
properly powered up by BROM...

Best regards,
Jernej

> 
> Cheers,
> Andre
> 
> > Signed-off-by: Da Xue <da@libre.computer>
> > ---
> > 
> >   drivers/mmc/sunxi_mmc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > index 1bb7b6d0e9..f7942b69ce 100644
> > --- a/drivers/mmc/sunxi_mmc.c
> > +++ b/drivers/mmc/sunxi_mmc.c
> > @@ -297,7 +297,7 @@ static int sunxi_mmc_core_init(struct mmc *mmc)
> > 
> >    /* Reset controller */
> >    writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> > 
> > - udelay(1000);
> > + udelay(20000);
> > 
> >    return 0;
> >   
> >   }

Re: [PATCH] sunxi-mmc: increase stabilization delay from 1ms to 20ms

[Da Xue]

On Fri, Jul 22, 2022 at 1:56 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Dne petek, 22. julij 2022 ob 18:55:14 CEST je Andre Przywara napisal(a):
> > On 21/07/2022 23:08, Da Xue wrote:
> >
> > Hi,
> >
> > > Some users experienced problems booting u-boot from SPL hanging here:
> > >
> > > Trying to boot from MMC1 or Trying to boot from MMC2
> > >
> > > This seems to occur with both MicroSD and eMMC modules on ALL-H3-CC.
> > > Increasing the delay after mmc reset fixes these boot problems.
> > > Some MicroSD cards are impacted more than others so it is possible that
> > > MicroSD internals need time to stabilize. Below is some failure data.
> > >
> > > sandisk ultra   64gb 9/20 with 1ms,  20/20 with 10ms
> > > sandisk ultra   16gb 2/20 with 1ms,  20/20 with 10ms
> > > sandisk extreme 16gb 6/20 with 10ms, 20/20 with 20ms
> > >
> > > A quick comparison of schematics show series resistors for ESD
> > > protection on the MicroSD GPIOs not present on all H3/H5 boards.
> > > It is not known if this is related to the issue.
> > >
> > > This patch adds a fixed 20ms delay to mmc init to mitigate the problem.
> > > If boot time optimization is required and the platform does not require
> > > the delay. The delay can be replaced with:
> > >
> > > writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> > > if (wait_for_bit_le32( &priv->reg->gctrl,
> > > SUNXI_MMC_GCTRL_RESET, false, 20, false)) {
> > > printf("%s: Timeout\n", __func__);
> > > return -ETIMEDOUT;
> > > }
> >
> > So what about adding this for everyone (we should have it regardless) then?

I think it would be good to add for everyone. This only happens in SPL
so the impact is limited in terms of time cost.

> >
> > And then have an extra Kconfig option to specify an extra delay, and
> > define this only in your board defconfig? Because IIUC this is specific
> > to your board?

I only have my boards to test. I'm sure there are other devices that have the
same issue. I don't think it's exclusive to our boards.

> >
> > And as I mentioned: it looks odd to have this here and have it fixing
> > your SD card problems:
> > - The soft reset should reset just the internal controller logic (MMC
> > state machine and FIFOs), this shouldn't affect cards. IIUC, nothing
> > should happen on the MMC *pins* because of this operation.
> > - Why isn't this a problem for U-Boot proper, and Linux, FWIW?
>
> As I said before, I have issue only at cold boot in SPL. Once I pass this
> point, it works all the time, even if rebooted. Why is that so it's unclear.
>
> Thinking about this a bit, I have another question. How is it possible that
> BROM manages to read SD card just fine and loads SPL beforehand? Is it using
> lower speed? It also couldn't be power issue, since card must have been
> properly powered up by BROM...

Is the bootrom using SPI mode on first boot? If that is the case, a transition
would take time and may explain the necessity of this.

>
> Best regards,
> Jernej
>
> >
> > Cheers,
> > Andre
> >
> > > Signed-off-by: Da Xue <da@libre.computer>
> > > ---
> > >
> > >   drivers/mmc/sunxi_mmc.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > > index 1bb7b6d0e9..f7942b69ce 100644
> > > --- a/drivers/mmc/sunxi_mmc.c
> > > +++ b/drivers/mmc/sunxi_mmc.c
> > > @@ -297,7 +297,7 @@ static int sunxi_mmc_core_init(struct mmc *mmc)
> > >
> > >    /* Reset controller */
> > >    writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> > >
> > > - udelay(1000);
> > > + udelay(20000);
> > >
> > >    return 0;
> > >
> > >   }
>
>
>
>