All of lore.kernel.org
 help / color / mirror / Atom feed
* Increasing stabilization time in sunxi_mmc_core_init
@ 2022-07-21 11:03 Da Xue
  2022-07-21 11:28 ` Andre Przywara
  0 siblings, 1 reply; 10+ messages in thread
From: Da Xue @ 2022-07-21 11:03 UTC (permalink / raw)
  To: linux-sunxi; +Cc: u-boot

Hi,

Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5). u-boot
gets stuck in SPL with this message for SD/eMMC respectively.

Trying to boot from MMC1 or Trying to boot from MMC2

I tested about 20 MicroSD cards from different brands and some were
happy and some were not. Increasing the udelay to 8-10ms in
drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to fix
the issue for the MicroSD cards.

Author: Da Xue <da@libre.computer>
Date:   Wed Jul 20 19:11:55 2022 -0400

    sunxi: raise stabilization time for mmc from 1ms to 8ms

diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
index 1bb7b6d0e9..499e057725 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(8000);

        return 0;
 }

I don't know the implications of this change so I am seeking feedback.
Are other boards having this issue as well or is it specific to our
hardware?

Best,
Da

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 11:03 Increasing stabilization time in sunxi_mmc_core_init Da Xue
@ 2022-07-21 11:28 ` Andre Przywara
  2022-07-21 15:14   ` Jernej Škrabec
  0 siblings, 1 reply; 10+ messages in thread
From: Andre Przywara @ 2022-07-21 11:28 UTC (permalink / raw)
  To: Da Xue, linux-sunxi; +Cc: u-boot

On 21/07/2022 12:03, Da Xue wrote:

Hi Da,

> Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5). u-boot
> gets stuck in SPL with this message for SD/eMMC respectively.
> 
> Trying to boot from MMC1 or Trying to boot from MMC2
> 
> I tested about 20 MicroSD cards from different brands and some were
> happy and some were not. Increasing the udelay to 8-10ms in
> drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to fix
> the issue for the MicroSD cards.

That's interesting, thanks for the report. I don't remember hearing of 
issues with MMC before, at least not in the SPL.
It's a bit odd that waiting after the *controller* reset should affect 
SD cards, and 1ms seems plenty for just the reset.
I just checked and at least the SOFT_RESET and FIFO_RESET bits are self 
clearing. Can you try to use wait_for_bit_le32() to wait for those parts 
to finish? See sun8i_emac_eth_start() for an example.

And since you mentioned it's card related: can you check whether the 
delay is actually needed somewhere else, later? At some point where we 
wait to the card to response, for instance?

I am not against taking this patch, if it fixes problems for you, but 
just want to avoid that it papers over other issues.

Cheers,
Andre

> 
> Author: Da Xue <da@libre.computer>
> Date:   Wed Jul 20 19:11:55 2022 -0400
> 
>      sunxi: raise stabilization time for mmc from 1ms to 8ms
> 
> diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> index 1bb7b6d0e9..499e057725 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(8000);
> 
>          return 0;
>   }
> 
> I don't know the implications of this change so I am seeking feedback.
> Are other boards having this issue as well or is it specific to our
> hardware?
> 
> Best,
> Da
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 11:28 ` Andre Przywara
@ 2022-07-21 15:14   ` Jernej Škrabec
  2022-07-21 19:56     ` Da Xue
  0 siblings, 1 reply; 10+ messages in thread
From: Jernej Škrabec @ 2022-07-21 15:14 UTC (permalink / raw)
  To: Da Xue, linux-sunxi, Andre Przywara; +Cc: u-boot

Hi!

Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara napisal(a):
> On 21/07/2022 12:03, Da Xue wrote:
> 
> Hi Da,
> 
> > Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5). u-boot
> > gets stuck in SPL with this message for SD/eMMC respectively.
> > 
> > Trying to boot from MMC1 or Trying to boot from MMC2
> > 
> > I tested about 20 MicroSD cards from different brands and some were
> > happy and some were not. Increasing the udelay to 8-10ms in
> > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to fix
> > the issue for the MicroSD cards.
> 
> That's interesting, thanks for the report. I don't remember hearing of
> issues with MMC before, at least not in the SPL.

I certainly experienced this issue on board in question. I vaguely remember 
asking about this issue on IRC. I also tried all sorts of tweaks, but it never 
occured to me that mmc reset timeout would be too short.

Da, how did you find this?

I only test one other H5 board occasionally, namely OrangePi PC2, but I never 
observed such issue there. I always needed about 5 attempts to boot ALL-H3-CC-
H5 board, but once it's cold booted, warm boots always succeed.

Best regards,
Jernej

> It's a bit odd that waiting after the *controller* reset should affect
> SD cards, and 1ms seems plenty for just the reset.
> I just checked and at least the SOFT_RESET and FIFO_RESET bits are self
> clearing. Can you try to use wait_for_bit_le32() to wait for those parts
> to finish? See sun8i_emac_eth_start() for an example.
> 
> And since you mentioned it's card related: can you check whether the
> delay is actually needed somewhere else, later? At some point where we
> wait to the card to response, for instance?
> 
> I am not against taking this patch, if it fixes problems for you, but
> just want to avoid that it papers over other issues.
> 
> Cheers,
> Andre
> 
> > Author: Da Xue <da@libre.computer>
> > Date:   Wed Jul 20 19:11:55 2022 -0400
> > 
> >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > 
> > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > index 1bb7b6d0e9..499e057725 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(8000);
> > 
> >          return 0;
> >   
> >   }
> > 
> > I don't know the implications of this change so I am seeking feedback.
> > Are other boards having this issue as well or is it specific to our
> > hardware?
> > 
> > Best,
> > Da





^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 15:14   ` Jernej Škrabec
@ 2022-07-21 19:56     ` Da Xue
  2022-07-21 20:05       ` Jernej Škrabec
  0 siblings, 1 reply; 10+ messages in thread
From: Da Xue @ 2022-07-21 19:56 UTC (permalink / raw)
  To: Jernej Škrabec; +Cc: linux-sunxi, Andre Przywara, u-boot

On Thu, Jul 21, 2022 at 11:14 AM Jernej Škrabec
<jernej.skrabec@gmail.com> wrote:
>
> Hi!
>
> Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara napisal(a):
> > On 21/07/2022 12:03, Da Xue wrote:
> >
> > Hi Da,
> >
> > > Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5). u-boot
> > > gets stuck in SPL with this message for SD/eMMC respectively.
> > >
> > > Trying to boot from MMC1 or Trying to boot from MMC2
> > >
> > > I tested about 20 MicroSD cards from different brands and some were
> > > happy and some were not. Increasing the udelay to 8-10ms in
> > > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to fix
> > > the issue for the MicroSD cards.
> >
> > That's interesting, thanks for the report. I don't remember hearing of
> > issues with MMC before, at least not in the SPL.
>
> I certainly experienced this issue on board in question. I vaguely remember
> asking about this issue on IRC. I also tried all sorts of tweaks, but it never
> occured to me that mmc reset timeout would be too short.
>
> Da, how did you find this?

Someone on the Armbian forum posted that they had the same problem
with eMMC so I got suspicious. I scoped the MicroSD clock line and
realized the frequency goes high and then drops to very low as if it
never found the card.
I had a hunch it was a stabilization delay and got lucky.

>
> I only test one other H5 board occasionally, namely OrangePi PC2, but I never
> observed such issue there. I always needed about 5 attempts to boot ALL-H3-CC-
> H5 board, but once it's cold booted, warm boots always succeed.

I had run into this too so it didn't make any sense. I tried 5ms and
it helped on some cards but not others.
I know the Orange Pis do not have the series resistor for ESD
protection of the SD GPIOs but that shouldn't affect this.
So...who knows?

>
> Best regards,
> Jernej
>
> > It's a bit odd that waiting after the *controller* reset should affect
> > SD cards, and 1ms seems plenty for just the reset.
> > I just checked and at least the SOFT_RESET and FIFO_RESET bits are self
> > clearing. Can you try to use wait_for_bit_le32() to wait for those parts
> > to finish? See sun8i_emac_eth_start() for an example.

I tested some more and here's the 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
Given this, I don't think it's an issue with the bit set delays. Might
need more than 10ms even. I didn't change the udelay in probe.

> >
> > And since you mentioned it's card related: can you check whether the
> > delay is actually needed somewhere else, later? At some point where we
> > wait to the card to response, for instance?
> >
> > I am not against taking this patch, if it fixes problems for you, but
> > just want to avoid that it papers over other issues.
> >
> > Cheers,
> > Andre
> >
> > > Author: Da Xue <da@libre.computer>
> > > Date:   Wed Jul 20 19:11:55 2022 -0400
> > >
> > >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > >
> > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > > index 1bb7b6d0e9..499e057725 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(8000);

This might need to be even higher. Like 20ms.

> > >
> > >          return 0;
> > >
> > >   }
> > >
> > > I don't know the implications of this change so I am seeking feedback.
> > > Are other boards having this issue as well or is it specific to our
> > > hardware?
> > >
> > > Best,
> > > Da
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 19:56     ` Da Xue
@ 2022-07-21 20:05       ` Jernej Škrabec
  2022-07-21 20:33         ` Da Xue
  0 siblings, 1 reply; 10+ messages in thread
From: Jernej Škrabec @ 2022-07-21 20:05 UTC (permalink / raw)
  To: Da Xue; +Cc: linux-sunxi, Andre Przywara, u-boot

Dne četrtek, 21. julij 2022 ob 21:56:35 CEST je Da Xue napisal(a):
> On Thu, Jul 21, 2022 at 11:14 AM Jernej Škrabec
> 
> <jernej.skrabec@gmail.com> wrote:
> > Hi!
> > 
> > Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara napisal(a):
> > > On 21/07/2022 12:03, Da Xue wrote:
> > > 
> > > Hi Da,
> > > 
> > > > Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5). u-boot
> > > > gets stuck in SPL with this message for SD/eMMC respectively.
> > > > 
> > > > Trying to boot from MMC1 or Trying to boot from MMC2
> > > > 
> > > > I tested about 20 MicroSD cards from different brands and some were
> > > > happy and some were not. Increasing the udelay to 8-10ms in
> > > > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to fix
> > > > the issue for the MicroSD cards.
> > > 
> > > That's interesting, thanks for the report. I don't remember hearing of
> > > issues with MMC before, at least not in the SPL.
> > 
> > I certainly experienced this issue on board in question. I vaguely
> > remember
> > asking about this issue on IRC. I also tried all sorts of tweaks, but it
> > never occured to me that mmc reset timeout would be too short.
> > 
> > Da, how did you find this?
> 
> Someone on the Armbian forum posted that they had the same problem
> with eMMC so I got suspicious. I scoped the MicroSD clock line and
> realized the frequency goes high and then drops to very low as if it
> never found the card.
> I had a hunch it was a stabilization delay and got lucky.
> 
> > I only test one other H5 board occasionally, namely OrangePi PC2, but I
> > never observed such issue there. I always needed about 5 attempts to boot
> > ALL-H3-CC- H5 board, but once it's cold booted, warm boots always
> > succeed.
> 
> I had run into this too so it didn't make any sense. I tried 5ms and
> it helped on some cards but not others.
> I know the Orange Pis do not have the series resistor for ESD
> protection of the SD GPIOs but that shouldn't affect this.
> So...who knows?
> 
> > Best regards,
> > Jernej
> > 
> > > It's a bit odd that waiting after the *controller* reset should affect
> > > SD cards, and 1ms seems plenty for just the reset.
> > > I just checked and at least the SOFT_RESET and FIFO_RESET bits are self
> > > clearing. Can you try to use wait_for_bit_le32() to wait for those parts
> > > to finish? See sun8i_emac_eth_start() for an example.
> 
> I tested some more and here's the 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
> Given this, I don't think it's an issue with the bit set delays. Might
> need more than 10ms even. I didn't change the udelay in probe.

Idea here is that we wouldn't need to determine the appropriate delay, but 
instead, wait_for_bit_le32() would monitor reset bit and continue only after 
reset bit would clear itself. Hopefully that happens after everything is 
stable.

Best regards,
Jernej

> 
> > > And since you mentioned it's card related: can you check whether the
> > > delay is actually needed somewhere else, later? At some point where we
> > > wait to the card to response, for instance?
> > > 
> > > I am not against taking this patch, if it fixes problems for you, but
> > > just want to avoid that it papers over other issues.
> > > 
> > > Cheers,
> > > Andre
> > > 
> > > > Author: Da Xue <da@libre.computer>
> > > > Date:   Wed Jul 20 19:11:55 2022 -0400
> > > > 
> > > >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > > > 
> > > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > > > index 1bb7b6d0e9..499e057725 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(8000);
> 
> This might need to be even higher. Like 20ms.
> 
> > > >          return 0;
> > > >   
> > > >   }
> > > > 
> > > > I don't know the implications of this change so I am seeking feedback.
> > > > Are other boards having this issue as well or is it specific to our
> > > > hardware?
> > > > 
> > > > Best,
> > > > Da



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 20:05       ` Jernej Škrabec
@ 2022-07-21 20:33         ` Da Xue
  2022-07-21 20:49           ` Jernej Škrabec
  0 siblings, 1 reply; 10+ messages in thread
From: Da Xue @ 2022-07-21 20:33 UTC (permalink / raw)
  To: Jernej Škrabec; +Cc: linux-sunxi, Andre Przywara, u-boot

On Thu, Jul 21, 2022 at 4:05 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Dne četrtek, 21. julij 2022 ob 21:56:35 CEST je Da Xue napisal(a):
> > On Thu, Jul 21, 2022 at 11:14 AM Jernej Škrabec
> >
> > <jernej.skrabec@gmail.com> wrote:
> > > Hi!
> > >
> > > Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara napisal(a):
> > > > On 21/07/2022 12:03, Da Xue wrote:
> > > >
> > > > Hi Da,
> > > >
> > > > > Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5). u-boot
> > > > > gets stuck in SPL with this message for SD/eMMC respectively.
> > > > >
> > > > > Trying to boot from MMC1 or Trying to boot from MMC2
> > > > >
> > > > > I tested about 20 MicroSD cards from different brands and some were
> > > > > happy and some were not. Increasing the udelay to 8-10ms in
> > > > > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to fix
> > > > > the issue for the MicroSD cards.
> > > >
> > > > That's interesting, thanks for the report. I don't remember hearing of
> > > > issues with MMC before, at least not in the SPL.
> > >
> > > I certainly experienced this issue on board in question. I vaguely
> > > remember
> > > asking about this issue on IRC. I also tried all sorts of tweaks, but it
> > > never occured to me that mmc reset timeout would be too short.
> > >
> > > Da, how did you find this?
> >
> > Someone on the Armbian forum posted that they had the same problem
> > with eMMC so I got suspicious. I scoped the MicroSD clock line and
> > realized the frequency goes high and then drops to very low as if it
> > never found the card.
> > I had a hunch it was a stabilization delay and got lucky.
> >
> > > I only test one other H5 board occasionally, namely OrangePi PC2, but I
> > > never observed such issue there. I always needed about 5 attempts to boot
> > > ALL-H3-CC- H5 board, but once it's cold booted, warm boots always
> > > succeed.
> >
> > I had run into this too so it didn't make any sense. I tried 5ms and
> > it helped on some cards but not others.
> > I know the Orange Pis do not have the series resistor for ESD
> > protection of the SD GPIOs but that shouldn't affect this.
> > So...who knows?
> >
> > > Best regards,
> > > Jernej
> > >
> > > > It's a bit odd that waiting after the *controller* reset should affect
> > > > SD cards, and 1ms seems plenty for just the reset.
> > > > I just checked and at least the SOFT_RESET and FIFO_RESET bits are self
> > > > clearing. Can you try to use wait_for_bit_le32() to wait for those parts
> > > > to finish? See sun8i_emac_eth_start() for an example.
> >
> > I tested some more and here's the 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
> > Given this, I don't think it's an issue with the bit set delays. Might
> > need more than 10ms even. I didn't change the udelay in probe.
>
> Idea here is that we wouldn't need to determine the appropriate delay, but
> instead, wait_for_bit_le32() would monitor reset bit and continue only after
> reset bit would clear itself. Hopefully that happens after everything is
> stable.

I changed it to 50ms delay

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

and that seems to work. Shall I send the patch?

>
> Best regards,
> Jernej
>
> >
> > > > And since you mentioned it's card related: can you check whether the
> > > > delay is actually needed somewhere else, later? At some point where we
> > > > wait to the card to response, for instance?
> > > >
> > > > I am not against taking this patch, if it fixes problems for you, but
> > > > just want to avoid that it papers over other issues.
> > > >
> > > > Cheers,
> > > > Andre
> > > >
> > > > > Author: Da Xue <da@libre.computer>
> > > > > Date:   Wed Jul 20 19:11:55 2022 -0400
> > > > >
> > > > >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > > > >
> > > > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > > > > index 1bb7b6d0e9..499e057725 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(8000);
> >
> > This might need to be even higher. Like 20ms.
> >
> > > > >          return 0;
> > > > >
> > > > >   }
> > > > >
> > > > > I don't know the implications of this change so I am seeking feedback.
> > > > > Are other boards having this issue as well or is it specific to our
> > > > > hardware?
> > > > >
> > > > > Best,
> > > > > Da
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 20:33         ` Da Xue
@ 2022-07-21 20:49           ` Jernej Škrabec
  2022-07-21 20:58             ` Da Xue
  0 siblings, 1 reply; 10+ messages in thread
From: Jernej Škrabec @ 2022-07-21 20:49 UTC (permalink / raw)
  To: Da Xue; +Cc: linux-sunxi, Andre Przywara, u-boot

Dne četrtek, 21. julij 2022 ob 22:33:09 CEST je Da Xue napisal(a):
> On Thu, Jul 21, 2022 at 4:05 PM Jernej Škrabec <jernej.skrabec@gmail.com> 
wrote:
> > Dne četrtek, 21. julij 2022 ob 21:56:35 CEST je Da Xue napisal(a):
> > > On Thu, Jul 21, 2022 at 11:14 AM Jernej Škrabec
> > > 
> > > <jernej.skrabec@gmail.com> wrote:
> > > > Hi!
> > > > 
> > > > Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara 
napisal(a):
> > > > > On 21/07/2022 12:03, Da Xue wrote:
> > > > > 
> > > > > Hi Da,
> > > > > 
> > > > > > Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5).
> > > > > > u-boot
> > > > > > gets stuck in SPL with this message for SD/eMMC respectively.
> > > > > > 
> > > > > > Trying to boot from MMC1 or Trying to boot from MMC2
> > > > > > 
> > > > > > I tested about 20 MicroSD cards from different brands and some
> > > > > > were
> > > > > > happy and some were not. Increasing the udelay to 8-10ms in
> > > > > > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to
> > > > > > fix
> > > > > > the issue for the MicroSD cards.
> > > > > 
> > > > > That's interesting, thanks for the report. I don't remember hearing
> > > > > of
> > > > > issues with MMC before, at least not in the SPL.
> > > > 
> > > > I certainly experienced this issue on board in question. I vaguely
> > > > remember
> > > > asking about this issue on IRC. I also tried all sorts of tweaks, but
> > > > it
> > > > never occured to me that mmc reset timeout would be too short.
> > > > 
> > > > Da, how did you find this?
> > > 
> > > Someone on the Armbian forum posted that they had the same problem
> > > with eMMC so I got suspicious. I scoped the MicroSD clock line and
> > > realized the frequency goes high and then drops to very low as if it
> > > never found the card.
> > > I had a hunch it was a stabilization delay and got lucky.
> > > 
> > > > I only test one other H5 board occasionally, namely OrangePi PC2, but
> > > > I
> > > > never observed such issue there. I always needed about 5 attempts to
> > > > boot
> > > > ALL-H3-CC- H5 board, but once it's cold booted, warm boots always
> > > > succeed.
> > > 
> > > I had run into this too so it didn't make any sense. I tried 5ms and
> > > it helped on some cards but not others.
> > > I know the Orange Pis do not have the series resistor for ESD
> > > protection of the SD GPIOs but that shouldn't affect this.
> > > So...who knows?
> > > 
> > > > Best regards,
> > > > Jernej
> > > > 
> > > > > It's a bit odd that waiting after the *controller* reset should
> > > > > affect
> > > > > SD cards, and 1ms seems plenty for just the reset.
> > > > > I just checked and at least the SOFT_RESET and FIFO_RESET bits are
> > > > > self
> > > > > clearing. Can you try to use wait_for_bit_le32() to wait for those
> > > > > parts
> > > > > to finish? See sun8i_emac_eth_start() for an example.
> > > 
> > > I tested some more and here's the 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
> > > Given this, I don't think it's an issue with the bit set delays. Might
> > > need more than 10ms even. I didn't change the udelay in probe.
> > 
> > Idea here is that we wouldn't need to determine the appropriate delay, but
> > instead, wait_for_bit_le32() would monitor reset bit and continue only
> > after reset bit would clear itself. Hopefully that happens after
> > everything is stable.
> 
> I changed it to 50ms delay
> 
> writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> if (wait_for_bit_le32( &priv->reg->gctrl,
> SUNXI_MMC_GCTRL_RESET, false, 50, false)) {
> printf("%s: Timeout\n", __func__);
> return ret;
> }
> 
> and that seems to work. Shall I send the patch?

Certainly, that's major improvement. Just 1 small nitpick - you don't need to 
introduce ret variable, just return error -ETIMEDOUT directly.

Maybe timeout can be raised even to 100 ms, now that it continues as soon as 
possible. Better to be on the completely safe side. But I'll leave that 
decision to you and Andre.

Best regards,
Jernej

> 
> > Best regards,
> > Jernej
> > 
> > > > > And since you mentioned it's card related: can you check whether the
> > > > > delay is actually needed somewhere else, later? At some point where
> > > > > we
> > > > > wait to the card to response, for instance?
> > > > > 
> > > > > I am not against taking this patch, if it fixes problems for you,
> > > > > but
> > > > > just want to avoid that it papers over other issues.
> > > > > 
> > > > > Cheers,
> > > > > Andre
> > > > > 
> > > > > > Author: Da Xue <da@libre.computer>
> > > > > > Date:   Wed Jul 20 19:11:55 2022 -0400
> > > > > > 
> > > > > >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > > > > > 
> > > > > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > > > > > index 1bb7b6d0e9..499e057725 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(8000);
> > > 
> > > This might need to be even higher. Like 20ms.
> > > 
> > > > > >          return 0;
> > > > > >   
> > > > > >   }
> > > > > > 
> > > > > > I don't know the implications of this change so I am seeking
> > > > > > feedback.
> > > > > > Are other boards having this issue as well or is it specific to
> > > > > > our
> > > > > > hardware?
> > > > > > 
> > > > > > Best,
> > > > > > Da



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 20:49           ` Jernej Škrabec
@ 2022-07-21 20:58             ` Da Xue
  2022-07-21 21:23               ` Da Xue
  0 siblings, 1 reply; 10+ messages in thread
From: Da Xue @ 2022-07-21 20:58 UTC (permalink / raw)
  To: Jernej Škrabec; +Cc: linux-sunxi, Andre Przywara, u-boot

On Thu, Jul 21, 2022 at 4:49 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Dne četrtek, 21. julij 2022 ob 22:33:09 CEST je Da Xue napisal(a):
> > On Thu, Jul 21, 2022 at 4:05 PM Jernej Škrabec <jernej.skrabec@gmail.com>
> wrote:
> > > Dne četrtek, 21. julij 2022 ob 21:56:35 CEST je Da Xue napisal(a):
> > > > On Thu, Jul 21, 2022 at 11:14 AM Jernej Škrabec
> > > >
> > > > <jernej.skrabec@gmail.com> wrote:
> > > > > Hi!
> > > > >
> > > > > Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara
> napisal(a):
> > > > > > On 21/07/2022 12:03, Da Xue wrote:
> > > > > >
> > > > > > Hi Da,
> > > > > >
> > > > > > > Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5).
> > > > > > > u-boot
> > > > > > > gets stuck in SPL with this message for SD/eMMC respectively.
> > > > > > >
> > > > > > > Trying to boot from MMC1 or Trying to boot from MMC2
> > > > > > >
> > > > > > > I tested about 20 MicroSD cards from different brands and some
> > > > > > > were
> > > > > > > happy and some were not. Increasing the udelay to 8-10ms in
> > > > > > > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to
> > > > > > > fix
> > > > > > > the issue for the MicroSD cards.
> > > > > >
> > > > > > That's interesting, thanks for the report. I don't remember hearing
> > > > > > of
> > > > > > issues with MMC before, at least not in the SPL.
> > > > >
> > > > > I certainly experienced this issue on board in question. I vaguely
> > > > > remember
> > > > > asking about this issue on IRC. I also tried all sorts of tweaks, but
> > > > > it
> > > > > never occured to me that mmc reset timeout would be too short.
> > > > >
> > > > > Da, how did you find this?
> > > >
> > > > Someone on the Armbian forum posted that they had the same problem
> > > > with eMMC so I got suspicious. I scoped the MicroSD clock line and
> > > > realized the frequency goes high and then drops to very low as if it
> > > > never found the card.
> > > > I had a hunch it was a stabilization delay and got lucky.
> > > >
> > > > > I only test one other H5 board occasionally, namely OrangePi PC2, but
> > > > > I
> > > > > never observed such issue there. I always needed about 5 attempts to
> > > > > boot
> > > > > ALL-H3-CC- H5 board, but once it's cold booted, warm boots always
> > > > > succeed.
> > > >
> > > > I had run into this too so it didn't make any sense. I tried 5ms and
> > > > it helped on some cards but not others.
> > > > I know the Orange Pis do not have the series resistor for ESD
> > > > protection of the SD GPIOs but that shouldn't affect this.
> > > > So...who knows?
> > > >
> > > > > Best regards,
> > > > > Jernej
> > > > >
> > > > > > It's a bit odd that waiting after the *controller* reset should
> > > > > > affect
> > > > > > SD cards, and 1ms seems plenty for just the reset.
> > > > > > I just checked and at least the SOFT_RESET and FIFO_RESET bits are
> > > > > > self
> > > > > > clearing. Can you try to use wait_for_bit_le32() to wait for those
> > > > > > parts
> > > > > > to finish? See sun8i_emac_eth_start() for an example.
> > > >
> > > > I tested some more and here's the 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
> > > > Given this, I don't think it's an issue with the bit set delays. Might
> > > > need more than 10ms even. I didn't change the udelay in probe.
> > >
> > > Idea here is that we wouldn't need to determine the appropriate delay, but
> > > instead, wait_for_bit_le32() would monitor reset bit and continue only
> > > after reset bit would clear itself. Hopefully that happens after
> > > everything is stable.
> >
> > I changed it to 50ms delay
> >
> > writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> > if (wait_for_bit_le32( &priv->reg->gctrl,
> > SUNXI_MMC_GCTRL_RESET, false, 50, false)) {
> > printf("%s: Timeout\n", __func__);
> > return ret;
> > }
> >
> > and that seems to work. Shall I send the patch?
>
> Certainly, that's major improvement. Just 1 small nitpick - you don't need to
> introduce ret variable, just return error -ETIMEDOUT directly.

OK

>
> Maybe timeout can be raised even to 100 ms, now that it continues as soon as
> possible. Better to be on the completely safe side. But I'll leave that
> decision to you and Andre.

I'll retest everything and check about changing the probe reset as
well. If everything works, I'll submit the patch.

>
> Best regards,
> Jernej
>
> >
> > > Best regards,
> > > Jernej
> > >
> > > > > > And since you mentioned it's card related: can you check whether the
> > > > > > delay is actually needed somewhere else, later? At some point where
> > > > > > we
> > > > > > wait to the card to response, for instance?
> > > > > >
> > > > > > I am not against taking this patch, if it fixes problems for you,
> > > > > > but
> > > > > > just want to avoid that it papers over other issues.
> > > > > >
> > > > > > Cheers,
> > > > > > Andre
> > > > > >
> > > > > > > Author: Da Xue <da@libre.computer>
> > > > > > > Date:   Wed Jul 20 19:11:55 2022 -0400
> > > > > > >
> > > > > > >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > > > > > >
> > > > > > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > > > > > > index 1bb7b6d0e9..499e057725 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(8000);
> > > >
> > > > This might need to be even higher. Like 20ms.
> > > >
> > > > > > >          return 0;
> > > > > > >
> > > > > > >   }
> > > > > > >
> > > > > > > I don't know the implications of this change so I am seeking
> > > > > > > feedback.
> > > > > > > Are other boards having this issue as well or is it specific to
> > > > > > > our
> > > > > > > hardware?
> > > > > > >
> > > > > > > Best,
> > > > > > > Da
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 20:58             ` Da Xue
@ 2022-07-21 21:23               ` Da Xue
  2022-07-21 21:30                 ` Jernej Škrabec
  0 siblings, 1 reply; 10+ messages in thread
From: Da Xue @ 2022-07-21 21:23 UTC (permalink / raw)
  To: Jernej Škrabec; +Cc: linux-sunxi, Andre Przywara, u-boot

On Thu, Jul 21, 2022 at 4:58 PM Da Xue <da@libre.computer> wrote:
>
> On Thu, Jul 21, 2022 at 4:49 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> >
> > Dne četrtek, 21. julij 2022 ob 22:33:09 CEST je Da Xue napisal(a):
> > > On Thu, Jul 21, 2022 at 4:05 PM Jernej Škrabec <jernej.skrabec@gmail.com>
> > wrote:
> > > > Dne četrtek, 21. julij 2022 ob 21:56:35 CEST je Da Xue napisal(a):
> > > > > On Thu, Jul 21, 2022 at 11:14 AM Jernej Škrabec
> > > > >
> > > > > <jernej.skrabec@gmail.com> wrote:
> > > > > > Hi!
> > > > > >
> > > > > > Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara
> > napisal(a):
> > > > > > > On 21/07/2022 12:03, Da Xue wrote:
> > > > > > >
> > > > > > > Hi Da,
> > > > > > >
> > > > > > > > Users were reporting non-boot on our H5 boards (ALL-H3-CC-H5).
> > > > > > > > u-boot
> > > > > > > > gets stuck in SPL with this message for SD/eMMC respectively.
> > > > > > > >
> > > > > > > > Trying to boot from MMC1 or Trying to boot from MMC2
> > > > > > > >
> > > > > > > > I tested about 20 MicroSD cards from different brands and some
> > > > > > > > were
> > > > > > > > happy and some were not. Increasing the udelay to 8-10ms in
> > > > > > > > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset seems to
> > > > > > > > fix
> > > > > > > > the issue for the MicroSD cards.
> > > > > > >
> > > > > > > That's interesting, thanks for the report. I don't remember hearing
> > > > > > > of
> > > > > > > issues with MMC before, at least not in the SPL.
> > > > > >
> > > > > > I certainly experienced this issue on board in question. I vaguely
> > > > > > remember
> > > > > > asking about this issue on IRC. I also tried all sorts of tweaks, but
> > > > > > it
> > > > > > never occured to me that mmc reset timeout would be too short.
> > > > > >
> > > > > > Da, how did you find this?
> > > > >
> > > > > Someone on the Armbian forum posted that they had the same problem
> > > > > with eMMC so I got suspicious. I scoped the MicroSD clock line and
> > > > > realized the frequency goes high and then drops to very low as if it
> > > > > never found the card.
> > > > > I had a hunch it was a stabilization delay and got lucky.
> > > > >
> > > > > > I only test one other H5 board occasionally, namely OrangePi PC2, but
> > > > > > I
> > > > > > never observed such issue there. I always needed about 5 attempts to
> > > > > > boot
> > > > > > ALL-H3-CC- H5 board, but once it's cold booted, warm boots always
> > > > > > succeed.
> > > > >
> > > > > I had run into this too so it didn't make any sense. I tried 5ms and
> > > > > it helped on some cards but not others.
> > > > > I know the Orange Pis do not have the series resistor for ESD
> > > > > protection of the SD GPIOs but that shouldn't affect this.
> > > > > So...who knows?
> > > > >
> > > > > > Best regards,
> > > > > > Jernej
> > > > > >
> > > > > > > It's a bit odd that waiting after the *controller* reset should
> > > > > > > affect
> > > > > > > SD cards, and 1ms seems plenty for just the reset.
> > > > > > > I just checked and at least the SOFT_RESET and FIFO_RESET bits are
> > > > > > > self
> > > > > > > clearing. Can you try to use wait_for_bit_le32() to wait for those
> > > > > > > parts
> > > > > > > to finish? See sun8i_emac_eth_start() for an example.

After more extensive testing with 5 MicroSD cards, 2 of them are still
inconsistent with waiting for SOFT_RESET and FIFO_RESET vs a hard 20ms
delay. I even tried putting a delay of 1ms after they cleared to no
avail.
I'm curious how larger 1TB MicroSD cards behave now but I don't have
any samples.

> > > > >
> > > > > I tested some more and here's the 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
> > > > > Given this, I don't think it's an issue with the bit set delays. Might
> > > > > need more than 10ms even. I didn't change the udelay in probe.
> > > >
> > > > Idea here is that we wouldn't need to determine the appropriate delay, but
> > > > instead, wait_for_bit_le32() would monitor reset bit and continue only
> > > > after reset bit would clear itself. Hopefully that happens after
> > > > everything is stable.
> > >
> > > I changed it to 50ms delay
> > >
> > > writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> > > if (wait_for_bit_le32( &priv->reg->gctrl,
> > > SUNXI_MMC_GCTRL_RESET, false, 50, false)) {
> > > printf("%s: Timeout\n", __func__);
> > > return ret;
> > > }
> > >
> > > and that seems to work. Shall I send the patch?
> >
> > Certainly, that's major improvement. Just 1 small nitpick - you don't need to
> > introduce ret variable, just return error -ETIMEDOUT directly.
>
> OK
>
> >
> > Maybe timeout can be raised even to 100 ms, now that it continues as soon as
> > possible. Better to be on the completely safe side. But I'll leave that
> > decision to you and Andre.
>
> I'll retest everything and check about changing the probe reset as
> well. If everything works, I'll submit the patch.
>
> >
> > Best regards,
> > Jernej
> >
> > >
> > > > Best regards,
> > > > Jernej
> > > >
> > > > > > > And since you mentioned it's card related: can you check whether the
> > > > > > > delay is actually needed somewhere else, later? At some point where
> > > > > > > we
> > > > > > > wait to the card to response, for instance?
> > > > > > >
> > > > > > > I am not against taking this patch, if it fixes problems for you,
> > > > > > > but
> > > > > > > just want to avoid that it papers over other issues.
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Andre
> > > > > > >
> > > > > > > > Author: Da Xue <da@libre.computer>
> > > > > > > > Date:   Wed Jul 20 19:11:55 2022 -0400
> > > > > > > >
> > > > > > > >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > > > > > > >
> > > > > > > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > > > > > > > index 1bb7b6d0e9..499e057725 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(8000);
> > > > >
> > > > > This might need to be even higher. Like 20ms.
> > > > >
> > > > > > > >          return 0;
> > > > > > > >
> > > > > > > >   }
> > > > > > > >
> > > > > > > > I don't know the implications of this change so I am seeking
> > > > > > > > feedback.
> > > > > > > > Are other boards having this issue as well or is it specific to
> > > > > > > > our
> > > > > > > > hardware?
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Da
> >
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Re: Re: Re: Increasing stabilization time in sunxi_mmc_core_init
  2022-07-21 21:23               ` Da Xue
@ 2022-07-21 21:30                 ` Jernej Škrabec
  0 siblings, 0 replies; 10+ messages in thread
From: Jernej Škrabec @ 2022-07-21 21:30 UTC (permalink / raw)
  To: Da Xue; +Cc: linux-sunxi, Andre Przywara, u-boot

Dne četrtek, 21. julij 2022 ob 23:23:28 CEST je Da Xue napisal(a):
> On Thu, Jul 21, 2022 at 4:58 PM Da Xue <da@libre.computer> wrote:
> > On Thu, Jul 21, 2022 at 4:49 PM Jernej Škrabec <jernej.skrabec@gmail.com> 
wrote:
> > > Dne četrtek, 21. julij 2022 ob 22:33:09 CEST je Da Xue napisal(a):
> > > > On Thu, Jul 21, 2022 at 4:05 PM Jernej Škrabec
> > > > <jernej.skrabec@gmail.com>
> > > 
> > > wrote:
> > > > > Dne četrtek, 21. julij 2022 ob 21:56:35 CEST je Da Xue napisal(a):
> > > > > > On Thu, Jul 21, 2022 at 11:14 AM Jernej Škrabec
> > > > > > 
> > > > > > <jernej.skrabec@gmail.com> wrote:
> > > > > > > Hi!
> > > > > > > 
> > > > > > > Dne četrtek, 21. julij 2022 ob 13:28:59 CEST je Andre Przywara
> > > 
> > > napisal(a):
> > > > > > > > On 21/07/2022 12:03, Da Xue wrote:
> > > > > > > > 
> > > > > > > > Hi Da,
> > > > > > > > 
> > > > > > > > > Users were reporting non-boot on our H5 boards
> > > > > > > > > (ALL-H3-CC-H5).
> > > > > > > > > u-boot
> > > > > > > > > gets stuck in SPL with this message for SD/eMMC
> > > > > > > > > respectively.
> > > > > > > > > 
> > > > > > > > > Trying to boot from MMC1 or Trying to boot from MMC2
> > > > > > > > > 
> > > > > > > > > I tested about 20 MicroSD cards from different brands and
> > > > > > > > > some
> > > > > > > > > were
> > > > > > > > > happy and some were not. Increasing the udelay to 8-10ms in
> > > > > > > > > drivers/mmc/sunxi_mmc.c sunxi_mmc_core_init after reset
> > > > > > > > > seems to
> > > > > > > > > fix
> > > > > > > > > the issue for the MicroSD cards.
> > > > > > > > 
> > > > > > > > That's interesting, thanks for the report. I don't remember
> > > > > > > > hearing
> > > > > > > > of
> > > > > > > > issues with MMC before, at least not in the SPL.
> > > > > > > 
> > > > > > > I certainly experienced this issue on board in question. I
> > > > > > > vaguely
> > > > > > > remember
> > > > > > > asking about this issue on IRC. I also tried all sorts of
> > > > > > > tweaks, but
> > > > > > > it
> > > > > > > never occured to me that mmc reset timeout would be too short.
> > > > > > > 
> > > > > > > Da, how did you find this?
> > > > > > 
> > > > > > Someone on the Armbian forum posted that they had the same problem
> > > > > > with eMMC so I got suspicious. I scoped the MicroSD clock line and
> > > > > > realized the frequency goes high and then drops to very low as if
> > > > > > it
> > > > > > never found the card.
> > > > > > I had a hunch it was a stabilization delay and got lucky.
> > > > > > 
> > > > > > > I only test one other H5 board occasionally, namely OrangePi
> > > > > > > PC2, but
> > > > > > > I
> > > > > > > never observed such issue there. I always needed about 5
> > > > > > > attempts to
> > > > > > > boot
> > > > > > > ALL-H3-CC- H5 board, but once it's cold booted, warm boots
> > > > > > > always
> > > > > > > succeed.
> > > > > > 
> > > > > > I had run into this too so it didn't make any sense. I tried 5ms
> > > > > > and
> > > > > > it helped on some cards but not others.
> > > > > > I know the Orange Pis do not have the series resistor for ESD
> > > > > > protection of the SD GPIOs but that shouldn't affect this.
> > > > > > So...who knows?
> > > > > > 
> > > > > > > Best regards,
> > > > > > > Jernej
> > > > > > > 
> > > > > > > > It's a bit odd that waiting after the *controller* reset
> > > > > > > > should
> > > > > > > > affect
> > > > > > > > SD cards, and 1ms seems plenty for just the reset.
> > > > > > > > I just checked and at least the SOFT_RESET and FIFO_RESET bits
> > > > > > > > are
> > > > > > > > self
> > > > > > > > clearing. Can you try to use wait_for_bit_le32() to wait for
> > > > > > > > those
> > > > > > > > parts
> > > > > > > > to finish? See sun8i_emac_eth_start() for an example.
> 
> After more extensive testing with 5 MicroSD cards, 2 of them are still
> inconsistent with waiting for SOFT_RESET and FIFO_RESET vs a hard 20ms
> delay. I even tried putting a delay of 1ms after they cleared to no
> avail.

Fixed delay it is, then. Just add a comment with explanation why. People 
looking for boot time optimization might lower it.

> I'm curious how larger 1TB MicroSD cards behave now but I don't have
> any samples.
> 
> > > > > > I tested some more and here's the 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
> > > > > > Given this, I don't think it's an issue with the bit set delays.
> > > > > > Might
> > > > > > need more than 10ms even. I didn't change the udelay in probe.
> > > > > 
> > > > > Idea here is that we wouldn't need to determine the appropriate
> > > > > delay, but
> > > > > instead, wait_for_bit_le32() would monitor reset bit and continue
> > > > > only
> > > > > after reset bit would clear itself. Hopefully that happens after
> > > > > everything is stable.
> > > > 
> > > > I changed it to 50ms delay
> > > > 
> > > > writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl);
> > > > if (wait_for_bit_le32( &priv->reg->gctrl,
> > > > SUNXI_MMC_GCTRL_RESET, false, 50, false)) {
> > > > printf("%s: Timeout\n", __func__);
> > > > return ret;
> > > > }
> > > > 
> > > > and that seems to work. Shall I send the patch?
> > > 
> > > Certainly, that's major improvement. Just 1 small nitpick - you don't
> > > need to introduce ret variable, just return error -ETIMEDOUT directly.
> > 
> > OK
> > 
> > > Maybe timeout can be raised even to 100 ms, now that it continues as
> > > soon as possible. Better to be on the completely safe side. But I'll
> > > leave that decision to you and Andre.
> > 
> > I'll retest everything and check about changing the probe reset as
> > well. If everything works, I'll submit the patch.
> > 
> > > Best regards,
> > > Jernej
> > > 
> > > > > Best regards,
> > > > > Jernej
> > > > > 
> > > > > > > > And since you mentioned it's card related: can you check
> > > > > > > > whether the
> > > > > > > > delay is actually needed somewhere else, later? At some point
> > > > > > > > where
> > > > > > > > we
> > > > > > > > wait to the card to response, for instance?
> > > > > > > > 
> > > > > > > > I am not against taking this patch, if it fixes problems for
> > > > > > > > you,
> > > > > > > > but
> > > > > > > > just want to avoid that it papers over other issues.
> > > > > > > > 
> > > > > > > > Cheers,
> > > > > > > > Andre
> > > > > > > > 
> > > > > > > > > Author: Da Xue <da@libre.computer>
> > > > > > > > > Date:   Wed Jul 20 19:11:55 2022 -0400
> > > > > > > > > 
> > > > > > > > >      sunxi: raise stabilization time for mmc from 1ms to 8ms
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/mmc/sunxi_mmc.c
> > > > > > > > > b/drivers/mmc/sunxi_mmc.c
> > > > > > > > > index 1bb7b6d0e9..499e057725 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(8000);
> > > > > > 
> > > > > > This might need to be even higher. Like 20ms.
> > > > > > 
> > > > > > > > >          return 0;
> > > > > > > > >   
> > > > > > > > >   }
> > > > > > > > > 
> > > > > > > > > I don't know the implications of this change so I am seeking
> > > > > > > > > feedback.
> > > > > > > > > Are other boards having this issue as well or is it specific
> > > > > > > > > to
> > > > > > > > > our
> > > > > > > > > hardware?
> > > > > > > > > 
> > > > > > > > > Best,
> > > > > > > > > Da



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-07-21 21:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21 11:03 Increasing stabilization time in sunxi_mmc_core_init Da Xue
2022-07-21 11:28 ` Andre Przywara
2022-07-21 15:14   ` Jernej Škrabec
2022-07-21 19:56     ` Da Xue
2022-07-21 20:05       ` Jernej Škrabec
2022-07-21 20:33         ` Da Xue
2022-07-21 20:49           ` Jernej Škrabec
2022-07-21 20:58             ` Da Xue
2022-07-21 21:23               ` Da Xue
2022-07-21 21:30                 ` Jernej Škrabec

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.