diff mbox series

mmc: core: Wait for command setting 'Power Off Notification' bit to complete

Message ID 20220115121447.641524-1-andrej.skvortzov@gmail.com (mailing list archive)
State New, archived
Headers show
Series mmc: core: Wait for command setting 'Power Off Notification' bit to complete | expand

Commit Message

Andrey Skvortsov Jan. 15, 2022, 12:14 p.m. UTC
SD card is allowed to signal busy on DAT0 up to 1s after the
CMD49. According to SD spec (version 6.0 section 5.8.1.3) first host
waits until busy of CMD49 is released and only then polls Power
Management Status register up to 1s until the card indicates ready to
power off.

Without waiting for busy before polling status register sometimes card
becomes unresponsive and system fails to suspend:

  [  205.907459] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
  [  206.421274] sunxi-mmc 1c0f000.mmc: data error, sending stop command
  [  206.421321] sunxi-mmc 1c0f000.mmc: send stop command failed
  [  206.421347] mmc0: error -110 reading status reg of PM func
  [  206.421366] PM: dpm_run_callback(): mmc_bus_suspend+0x0/0x74 returns -110
  [  206.421402] mmcblk mmc0:aaaa: PM: failed to suspend async: error -110
  [  206.437064] PM: Some devices failed to suspend, or early wake event detected

Tested with Sandisk Extreme PRO A2 64GB on Allwinner A64 system.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---
 drivers/mmc/core/sd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Ulf Hansson Jan. 18, 2022, 6:48 p.m. UTC | #1
On Sat, 15 Jan 2022 at 13:15, Andrey Skvortsov
<andrej.skvortzov@gmail.com> wrote:
>
> SD card is allowed to signal busy on DAT0 up to 1s after the
> CMD49. According to SD spec (version 6.0 section 5.8.1.3) first host
> waits until busy of CMD49 is released and only then polls Power
> Management Status register up to 1s until the card indicates ready to
> power off.
>
> Without waiting for busy before polling status register sometimes card
> becomes unresponsive and system fails to suspend:
>
>   [  205.907459] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>   [  206.421274] sunxi-mmc 1c0f000.mmc: data error, sending stop command
>   [  206.421321] sunxi-mmc 1c0f000.mmc: send stop command failed
>   [  206.421347] mmc0: error -110 reading status reg of PM func
>   [  206.421366] PM: dpm_run_callback(): mmc_bus_suspend+0x0/0x74 returns -110
>   [  206.421402] mmcblk mmc0:aaaa: PM: failed to suspend async: error -110
>   [  206.437064] PM: Some devices failed to suspend, or early wake event detected

Thanks for your patch!

I recall I was hesitating on adding another busy completion check for
this, but thought polling the status register for the power management
function should be sufficient.

>
> Tested with Sandisk Extreme PRO A2 64GB on Allwinner A64 system.

I will give this patch a try too, to make sure it still works on my
side. Assuming that works fine, I will queue this up for fixes and by
adding a fixes/stable tag.

>
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>

Kind regards
Uffe

> ---
>  drivers/mmc/core/sd.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index e223275bbad1..842b886bdd4e 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -66,7 +66,7 @@ static const unsigned int sd_au_size[] = {
>                 __res & __mask;                                         \
>         })
>
> -#define SD_POWEROFF_NOTIFY_TIMEOUT_MS 2000
> +#define SD_POWEROFF_NOTIFY_TIMEOUT_MS 1000
>  #define SD_WRITE_EXTR_SINGLE_TIMEOUT_MS 1000
>
>  struct sd_busy_data {
> @@ -1663,6 +1663,13 @@ static int sd_poweroff_notify(struct mmc_card *card)
>                 goto out;
>         }
>
> +       /* Find out when the command is completed. */
> +       err = mmc_poll_for_busy(card, SD_POWEROFF_NOTIFY_TIMEOUT_MS, false,
> +                               MMC_BUSY_EXTR_SINGLE);
> +
> +       if (err)
> +               goto out;
> +
>         cb_data.card = card;
>         cb_data.reg_buf = reg_buf;
>         err = __mmc_poll_for_busy(card->host, SD_POWEROFF_NOTIFY_TIMEOUT_MS,
> --
> 2.34.1
>
Andrey Skvortsov Jan. 19, 2022, 6:49 a.m. UTC | #2
Hi, Ulf

On 22-01-18 19:48, Ulf Hansson wrote:
> On Sat, 15 Jan 2022 at 13:15, Andrey Skvortsov
> <andrej.skvortzov@gmail.com> wrote:
> >
> > SD card is allowed to signal busy on DAT0 up to 1s after the
> > CMD49. According to SD spec (version 6.0 section 5.8.1.3) first host
> > waits until busy of CMD49 is released and only then polls Power
> > Management Status register up to 1s until the card indicates ready to
> > power off.
> >
> > Without waiting for busy before polling status register sometimes card
> > becomes unresponsive and system fails to suspend:
> >
> >   [  205.907459] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> >   [  206.421274] sunxi-mmc 1c0f000.mmc: data error, sending stop command
> >   [  206.421321] sunxi-mmc 1c0f000.mmc: send stop command failed
> >   [  206.421347] mmc0: error -110 reading status reg of PM func
> >   [  206.421366] PM: dpm_run_callback(): mmc_bus_suspend+0x0/0x74 returns -110
> >   [  206.421402] mmcblk mmc0:aaaa: PM: failed to suspend async: error -110
> >   [  206.437064] PM: Some devices failed to suspend, or early wake event detected
> 
> Thanks for your patch!
> 
> I recall I was hesitating on adding another busy completion check for
> this, but thought polling the status register for the power management
> function should be sufficient.
> 
> >
> > Tested with Sandisk Extreme PRO A2 64GB on Allwinner A64 system.
> 
> I will give this patch a try too, to make sure it still works on my
> side. Assuming that works fine, I will queue this up for fixes and by
> adding a fixes/stable tag.

Thank you! More testing would be certainly great, because only one of my SD-cards
supports "Power Notification".
Ulf Hansson Jan. 21, 2022, 2:32 p.m. UTC | #3
On Sat, 15 Jan 2022 at 13:15, Andrey Skvortsov
<andrej.skvortzov@gmail.com> wrote:
>
> SD card is allowed to signal busy on DAT0 up to 1s after the
> CMD49. According to SD spec (version 6.0 section 5.8.1.3) first host
> waits until busy of CMD49 is released and only then polls Power
> Management Status register up to 1s until the card indicates ready to
> power off.
>
> Without waiting for busy before polling status register sometimes card
> becomes unresponsive and system fails to suspend:
>
>   [  205.907459] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>   [  206.421274] sunxi-mmc 1c0f000.mmc: data error, sending stop command
>   [  206.421321] sunxi-mmc 1c0f000.mmc: send stop command failed
>   [  206.421347] mmc0: error -110 reading status reg of PM func
>   [  206.421366] PM: dpm_run_callback(): mmc_bus_suspend+0x0/0x74 returns -110
>   [  206.421402] mmcblk mmc0:aaaa: PM: failed to suspend async: error -110
>   [  206.437064] PM: Some devices failed to suspend, or early wake event detected
>
> Tested with Sandisk Extreme PRO A2 64GB on Allwinner A64 system.
>
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>

I did some tests at my side too, using a Qcom platform and it worked
fine. However, I also used the same type of card as you, as I didn't
have any other try with at this moment.

So, applied for fixes and by adding a fixes/stable tag, thanks!

I also did a minor amendment to the patch, see below.

Kind regards
Uffe

> ---
>  drivers/mmc/core/sd.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index e223275bbad1..842b886bdd4e 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -66,7 +66,7 @@ static const unsigned int sd_au_size[] = {
>                 __res & __mask;                                         \
>         })
>
> -#define SD_POWEROFF_NOTIFY_TIMEOUT_MS 2000
> +#define SD_POWEROFF_NOTIFY_TIMEOUT_MS 1000
>  #define SD_WRITE_EXTR_SINGLE_TIMEOUT_MS 1000
>
>  struct sd_busy_data {
> @@ -1663,6 +1663,13 @@ static int sd_poweroff_notify(struct mmc_card *card)
>                 goto out;
>         }
>
> +       /* Find out when the command is completed. */
> +       err = mmc_poll_for_busy(card, SD_POWEROFF_NOTIFY_TIMEOUT_MS, false,

/s/SD_POWEROFF_NOTIFY_TIMEOUT_MS/SD_WRITE_EXTR_SINGLE_TIMEOUT_MS

> +                               MMC_BUSY_EXTR_SINGLE);
> +
> +       if (err)
> +               goto out;
> +
>         cb_data.card = card;
>         cb_data.reg_buf = reg_buf;
>         err = __mmc_poll_for_busy(card->host, SD_POWEROFF_NOTIFY_TIMEOUT_MS,
> --
> 2.34.1
>

Kind regards
Uffe
Andrey Skvortsov Jan. 21, 2022, 4:01 p.m. UTC | #4
On 22-01-21 15:32, Ulf Hansson wrote:
> On Sat, 15 Jan 2022 at 13:15, Andrey Skvortsov
> <andrej.skvortzov@gmail.com> wrote:
> >
> > SD card is allowed to signal busy on DAT0 up to 1s after the
> > CMD49. According to SD spec (version 6.0 section 5.8.1.3) first host
> > waits until busy of CMD49 is released and only then polls Power
> > Management Status register up to 1s until the card indicates ready to
> > power off.
> >
> > Without waiting for busy before polling status register sometimes card
> > becomes unresponsive and system fails to suspend:
> >
> >   [  205.907459] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> >   [  206.421274] sunxi-mmc 1c0f000.mmc: data error, sending stop command
> >   [  206.421321] sunxi-mmc 1c0f000.mmc: send stop command failed
> >   [  206.421347] mmc0: error -110 reading status reg of PM func
> >   [  206.421366] PM: dpm_run_callback(): mmc_bus_suspend+0x0/0x74 returns -110
> >   [  206.421402] mmcblk mmc0:aaaa: PM: failed to suspend async: error -110
> >   [  206.437064] PM: Some devices failed to suspend, or early wake event detected
> >
> > Tested with Sandisk Extreme PRO A2 64GB on Allwinner A64 system.
> >
> > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> 
> I did some tests at my side too, using a Qcom platform and it worked
> fine. However, I also used the same type of card as you, as I didn't
> have any other try with at this moment.

> So, applied for fixes and by adding a fixes/stable tag, thanks!
Thank you very much.
There is one another test report, that this patch fixes suspend with
KLEVV CRAS 128GB (K128GUSD6U3-CA). [1]

1. https://gitlab.com/mobian1/issues/-/issues/389#note_816636364

> I also did a minor amendment to the patch, see below.
> 
> Kind regards
> Uffe
diff mbox series

Patch

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index e223275bbad1..842b886bdd4e 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -66,7 +66,7 @@  static const unsigned int sd_au_size[] = {
 		__res & __mask;						\
 	})
 
-#define SD_POWEROFF_NOTIFY_TIMEOUT_MS 2000
+#define SD_POWEROFF_NOTIFY_TIMEOUT_MS 1000
 #define SD_WRITE_EXTR_SINGLE_TIMEOUT_MS 1000
 
 struct sd_busy_data {
@@ -1663,6 +1663,13 @@  static int sd_poweroff_notify(struct mmc_card *card)
 		goto out;
 	}
 
+	/* Find out when the command is completed. */
+	err = mmc_poll_for_busy(card, SD_POWEROFF_NOTIFY_TIMEOUT_MS, false,
+				MMC_BUSY_EXTR_SINGLE);
+
+	if (err)
+		goto out;
+
 	cb_data.card = card;
 	cb_data.reg_buf = reg_buf;
 	err = __mmc_poll_for_busy(card->host, SD_POWEROFF_NOTIFY_TIMEOUT_MS,