[PATCH] mmc: sdio: change to use system_freezable_wq for sdio_irq_work

[PATCH] mmc: sdio: change to use system_freezable_wq for sdio_irq_work

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

Change to use system_freezable_wq because it can make sure the queue
becomes frozen when userspace becomes frozen during system PM.

Find this issue on i.MX7D-sdb board with broadcom sdio wifi.
When system resume, sometimes this wifi meet the tuning fail issue.
All tuning command get command timeout error. This is because
sdio_irq_work on system_wq was executed before the broadcom
wifi driver resume. Due to broadcom wifi driver set the wifi in
Sleep sate in system suspend, need to set the wifi to Wake state
first. So need to make sure wifi driver resume first, then do the
sdio_irq_work.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/core/sdio.c     | 2 +-
 drivers/mmc/core/sdio_irq.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 26cabd53ddc5..269ae63d09eb 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -1015,7 +1015,7 @@ static int mmc_sdio_resume(struct mmc_host *host)
 		if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD))
 			wake_up_process(host->sdio_irq_thread);
 		else if (host->caps & MMC_CAP_SDIO_IRQ)
-			queue_delayed_work(system_wq, &host->sdio_irq_work, 0);
+			queue_delayed_work(system_freezable_wq, &host->sdio_irq_work, 0);
 	}
 
 out:
diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
index 900871073bd7..9d7ec48bd963 100644
--- a/drivers/mmc/core/sdio_irq.c
+++ b/drivers/mmc/core/sdio_irq.c
@@ -132,7 +132,7 @@ void sdio_irq_work(struct work_struct *work)
 void sdio_signal_irq(struct mmc_host *host)
 {
 	host->sdio_irq_pending = true;
-	queue_delayed_work(system_wq, &host->sdio_irq_work, 0);
+	queue_delayed_work(system_freezable_wq, &host->sdio_irq_work, 0);
 }
 EXPORT_SYMBOL_GPL(sdio_signal_irq);
 
-- 
2.17.1

Re: [PATCH] mmc: sdio: change to use system_freezable_wq for sdio_irq_work

[Ulf Hansson]

On Thu, 6 Aug 2020 at 10:23, <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> Change to use system_freezable_wq because it can make sure the queue
> becomes frozen when userspace becomes frozen during system PM.
>
> Find this issue on i.MX7D-sdb board with broadcom sdio wifi.
> When system resume, sometimes this wifi meet the tuning fail issue.
> All tuning command get command timeout error. This is because
> sdio_irq_work on system_wq was executed before the broadcom
> wifi driver resume. Due to broadcom wifi driver set the wifi in
> Sleep sate in system suspend, need to set the wifi to Wake state
> first. So need to make sure wifi driver resume first, then do the
> sdio_irq_work.

Moving to the system_freezable_wq doesn't work. Simply because it's
too early in the system suspend phase to freeze the queue for SDIO
irqs.

I guess you figured that out, so you posted a v2? Moving to review that now..

Kind regards
Uffe

>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mmc/core/sdio.c     | 2 +-
>  drivers/mmc/core/sdio_irq.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 26cabd53ddc5..269ae63d09eb 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -1015,7 +1015,7 @@ static int mmc_sdio_resume(struct mmc_host *host)
>                 if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD))
>                         wake_up_process(host->sdio_irq_thread);
>                 else if (host->caps & MMC_CAP_SDIO_IRQ)
> -                       queue_delayed_work(system_wq, &host->sdio_irq_work, 0);
> +                       queue_delayed_work(system_freezable_wq, &host->sdio_irq_work, 0);
>         }
>
>  out:
> diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
> index 900871073bd7..9d7ec48bd963 100644
> --- a/drivers/mmc/core/sdio_irq.c
> +++ b/drivers/mmc/core/sdio_irq.c
> @@ -132,7 +132,7 @@ void sdio_irq_work(struct work_struct *work)
>  void sdio_signal_irq(struct mmc_host *host)
>  {
>         host->sdio_irq_pending = true;
> -       queue_delayed_work(system_wq, &host->sdio_irq_work, 0);
> +       queue_delayed_work(system_freezable_wq, &host->sdio_irq_work, 0);
>  }
>  EXPORT_SYMBOL_GPL(sdio_signal_irq);
>
> --
> 2.17.1
>

RE: [PATCH] mmc: sdio: change to use system_freezable_wq for sdio_irq_work

[Bough Chen]

> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: 2020年8月21日 15:50
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>; linux-mmc@vger.kernel.org;
> dl-linux-imx <linux-imx@nxp.com>; Matthias Kaehlcke <mka@chromium.org>;
> Doug Anderson <dianders@chromium.org>
> Subject: Re: [PATCH] mmc: sdio: change to use system_freezable_wq for
> sdio_irq_work
> 
> On Thu, 6 Aug 2020 at 10:23, <haibo.chen@nxp.com> wrote:
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > Change to use system_freezable_wq because it can make sure the queue
> > becomes frozen when userspace becomes frozen during system PM.
> >
> > Find this issue on i.MX7D-sdb board with broadcom sdio wifi.
> > When system resume, sometimes this wifi meet the tuning fail issue.
> > All tuning command get command timeout error. This is because
> > sdio_irq_work on system_wq was executed before the broadcom wifi
> > driver resume. Due to broadcom wifi driver set the wifi in Sleep sate
> > in system suspend, need to set the wifi to Wake state first. So need
> > to make sure wifi driver resume first, then do the sdio_irq_work.
> 
> Moving to the system_freezable_wq doesn't work. Simply because it's too early
> in the system suspend phase to freeze the queue for SDIO irqs.
> 
> I guess you figured that out, so you posted a v2? Moving to review that now..

Hi Ulf,

For the V1 patch, it can fix the issue on imx7d board with Broadcom wifi. But this patch will cause issue when switch to use Marvell 8989 sdio wifi. 

Best Regards
Haibo Chen
> 
> Kind regards
> Uffe
> 
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/mmc/core/sdio.c     | 2 +-
> >  drivers/mmc/core/sdio_irq.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
> > 26cabd53ddc5..269ae63d09eb 100644
> > --- a/drivers/mmc/core/sdio.c
> > +++ b/drivers/mmc/core/sdio.c
> > @@ -1015,7 +1015,7 @@ static int mmc_sdio_resume(struct mmc_host
> *host)
> >                 if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD))
> >                         wake_up_process(host->sdio_irq_thread);
> >                 else if (host->caps & MMC_CAP_SDIO_IRQ)
> > -                       queue_delayed_work(system_wq,
> &host->sdio_irq_work, 0);
> > +                       queue_delayed_work(system_freezable_wq,
> > + &host->sdio_irq_work, 0);
> >         }
> >
> >  out:
> > diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
> > index 900871073bd7..9d7ec48bd963 100644
> > --- a/drivers/mmc/core/sdio_irq.c
> > +++ b/drivers/mmc/core/sdio_irq.c
> > @@ -132,7 +132,7 @@ void sdio_irq_work(struct work_struct *work)  void
> > sdio_signal_irq(struct mmc_host *host)  {
> >         host->sdio_irq_pending = true;
> > -       queue_delayed_work(system_wq, &host->sdio_irq_work, 0);
> > +       queue_delayed_work(system_freezable_wq,
> &host->sdio_irq_work,
> > + 0);
> >  }
> >  EXPORT_SYMBOL_GPL(sdio_signal_irq);
> >
> > --
> > 2.17.1
> >