[PATCH RESEND 0/9] mmc: sdhci-pci: Respect PM flags when enabling card detect GPIO IRQ wakeup

[PATCH RESEND 0/9] mmc: sdhci-pci: Respect PM flags when enabling card detect GPIO IRQ wakeup

[Adrian Hunter]

Hi

Commit 03dbaa04a2e5 ("mmc: slot-gpio: Add support to enable irq wake on
cd_irq") enabled wakeup irrespective of the host controller's PM flags.
However, users also want to control it from sysfs power/wakeup attribute.
That means the driver needs to check the PM flags before enabling it in the
suspend callback.  Patch 9 adds support for that in sdhci-pci, which is the
only driver presently using the MMC_CAP_CD_WAKE flag.  Patches 1 - 7 tidy
up aspects of sdhci and sdhci-pci wakeup handling, and patch 8 adds a
helper function to make it easy for drivers.

There are 2 contentious aspects to this patch set:

1) An existing problem with the SDIO API which does not let the host
controller driver know that the SDIO function driver has requested SDIO
card interrupt wakeup until the suspend callback - which results in the
host controller driver having to enable or disable wakeup in the suspend
callback.  Fixing the SDIO API is a separate issue IMHO.

2) In order to use the sysfs power/wakeup attribute, the driver must set the
device as wake capable even when it is really a GPIO that wakes the system.


Adrian Hunter (9):
      mmc: sdhci-pci: Stop calling sdhci_enable_irq_wakeups()
      mmc: sdhci-pci: Use device wakeup capability to determine MMC_PM_WAKE_SDIO_IRQ capability
      mmc: sdhci: Stop exporting sdhci_enable_irq_wakeups()
      mmc: sdhci: Handle failure of enable_irq_wake()
      mmc: sdhci: Rework sdhci_enable_irq_wakeups()
      mmc: sdhci: Do not unnecessarily enable wakeup for card detect interrupt
      mmc: sdhci: Do not unnecessarily enable wakeup for SDIO card interrupt
      mmc: slot-gpio: Add a function to enable/disable card detect IRQ wakeup
      mmc: sdhci-pci: Respect PM flags when enabling card detect GPIO IRQ wakeup

 drivers/mmc/core/core.c           |  3 +-
 drivers/mmc/core/slot-gpio.c      | 25 ++++++++++++--
 drivers/mmc/host/sdhci-pci-core.c | 73 +++++++++++++++++++--------------------
 drivers/mmc/host/sdhci.c          | 67 ++++++++++++++++++++++++-----------
 drivers/mmc/host/sdhci.h          |  2 +-
 include/linux/mmc/slot-gpio.h     |  1 +
 6 files changed, 108 insertions(+), 63 deletions(-)


Regards
Adrian

[PATCH RESEND 1/9] mmc: sdhci-pci: Stop calling sdhci_enable_irq_wakeups()

[Adrian Hunter]

sdhci_enable_irq_wakeups() is already called by sdhci_suspend_host() so
sdhci-pci should not need to call it. However sdhci_suspend_host() only
calls it if wakeups are enabled, and sdhci-pci does not enable them until
after calling sdhci_suspend_host(). So move the calls to
sdhci_pci_init_wakeup() before calling sdhci_suspend_host(), and
stop calling sdhci_enable_irq_wakeups(). That results in some
simplification because sdhci_pci_suspend_host() and
__sdhci_pci_suspend_host() no longer need to be separate functions.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 58 ++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 37 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index c5b229b46314..1711b1815630 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -38,10 +38,29 @@
 static void sdhci_pci_hw_reset(struct sdhci_host *host);
 
 #ifdef CONFIG_PM_SLEEP
-static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
+static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
+{
+	mmc_pm_flag_t pm_flags = 0;
+	int i;
+
+	for (i = 0; i < chip->num_slots; i++) {
+		struct sdhci_pci_slot *slot = chip->slots[i];
+
+		if (slot)
+			pm_flags |= slot->host->mmc->pm_flags;
+	}
+
+	return device_init_wakeup(&chip->pdev->dev,
+				  (pm_flags & MMC_PM_KEEP_POWER) &&
+				  (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
+}
+
+static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
 {
 	int i, ret;
 
+	sdhci_pci_init_wakeup(chip);
+
 	for (i = 0; i < chip->num_slots; i++) {
 		struct sdhci_pci_slot *slot = chip->slots[i];
 		struct sdhci_host *host;
@@ -57,9 +76,6 @@ static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
 		ret = sdhci_suspend_host(host);
 		if (ret)
 			goto err_pci_suspend;
-
-		if (host->mmc->pm_flags & MMC_PM_WAKE_SDIO_IRQ)
-			sdhci_enable_irq_wakeups(host);
 	}
 
 	return 0;
@@ -70,36 +86,6 @@ static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
 	return ret;
 }
 
-static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
-{
-	mmc_pm_flag_t pm_flags = 0;
-	int i;
-
-	for (i = 0; i < chip->num_slots; i++) {
-		struct sdhci_pci_slot *slot = chip->slots[i];
-
-		if (slot)
-			pm_flags |= slot->host->mmc->pm_flags;
-	}
-
-	return device_init_wakeup(&chip->pdev->dev,
-				  (pm_flags & MMC_PM_KEEP_POWER) &&
-				  (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
-}
-
-static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
-{
-	int ret;
-
-	ret = __sdhci_pci_suspend_host(chip);
-	if (ret)
-		return ret;
-
-	sdhci_pci_init_wakeup(chip);
-
-	return 0;
-}
-
 int sdhci_pci_resume_host(struct sdhci_pci_chip *chip)
 {
 	struct sdhci_pci_slot *slot;
@@ -1107,7 +1093,7 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip)
 {
 	int i, ret;
 
-	ret = __sdhci_pci_suspend_host(chip);
+	ret = sdhci_pci_suspend_host(chip);
 	if (ret)
 		return ret;
 
@@ -1117,8 +1103,6 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip)
 			jmicron_enable_mmc(chip->slots[i]->host, 0);
 	}
 
-	sdhci_pci_init_wakeup(chip);
-
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH RESEND 1/9] mmc: sdhci-pci: Stop calling sdhci_enable_irq_wakeups()

[Ulf Hansson]

On 9 January 2018 at 08:52, Adrian Hunter <adrian.hunter@intel.com> wrote:
> sdhci_enable_irq_wakeups() is already called by sdhci_suspend_host() so
> sdhci-pci should not need to call it. However sdhci_suspend_host() only
> calls it if wakeups are enabled, and sdhci-pci does not enable them until
> after calling sdhci_suspend_host(). So move the calls to
> sdhci_pci_init_wakeup() before calling sdhci_suspend_host(), and
> stop calling sdhci_enable_irq_wakeups(). That results in some
> simplification because sdhci_pci_suspend_host() and
> __sdhci_pci_suspend_host() no longer need to be separate functions.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 58 ++++++++++++++-------------------------
>  1 file changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index c5b229b46314..1711b1815630 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -38,10 +38,29 @@
>  static void sdhci_pci_hw_reset(struct sdhci_host *host);
>
>  #ifdef CONFIG_PM_SLEEP
> -static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> +static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
> +{
> +       mmc_pm_flag_t pm_flags = 0;
> +       int i;
> +
> +       for (i = 0; i < chip->num_slots; i++) {
> +               struct sdhci_pci_slot *slot = chip->slots[i];
> +
> +               if (slot)
> +                       pm_flags |= slot->host->mmc->pm_flags;
> +       }
> +
> +       return device_init_wakeup(&chip->pdev->dev,
> +                                 (pm_flags & MMC_PM_KEEP_POWER) &&
> +                                 (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
> +}
> +
> +static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
>  {
>         int i, ret;
>
> +       sdhci_pci_init_wakeup(chip);
> +
>         for (i = 0; i < chip->num_slots; i++) {
>                 struct sdhci_pci_slot *slot = chip->slots[i];
>                 struct sdhci_host *host;
> @@ -57,9 +76,6 @@ static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
>                 ret = sdhci_suspend_host(host);
>                 if (ret)
>                         goto err_pci_suspend;
> -
> -               if (host->mmc->pm_flags & MMC_PM_WAKE_SDIO_IRQ)
> -                       sdhci_enable_irq_wakeups(host);
>         }
>
>         return 0;
> @@ -70,36 +86,6 @@ static int __sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
>         return ret;
>  }
>
> -static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
> -{
> -       mmc_pm_flag_t pm_flags = 0;
> -       int i;
> -
> -       for (i = 0; i < chip->num_slots; i++) {
> -               struct sdhci_pci_slot *slot = chip->slots[i];
> -
> -               if (slot)
> -                       pm_flags |= slot->host->mmc->pm_flags;
> -       }
> -
> -       return device_init_wakeup(&chip->pdev->dev,
> -                                 (pm_flags & MMC_PM_KEEP_POWER) &&
> -                                 (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
> -}
> -
> -static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> -{
> -       int ret;
> -
> -       ret = __sdhci_pci_suspend_host(chip);
> -       if (ret)
> -               return ret;
> -
> -       sdhci_pci_init_wakeup(chip);
> -
> -       return 0;
> -}
> -
>  int sdhci_pci_resume_host(struct sdhci_pci_chip *chip)
>  {
>         struct sdhci_pci_slot *slot;
> @@ -1107,7 +1093,7 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip)
>  {
>         int i, ret;
>
> -       ret = __sdhci_pci_suspend_host(chip);
> +       ret = sdhci_pci_suspend_host(chip);
>         if (ret)
>                 return ret;
>
> @@ -1117,8 +1103,6 @@ static int jmicron_suspend(struct sdhci_pci_chip *chip)
>                         jmicron_enable_mmc(chip->slots[i]->host, 0);
>         }
>
> -       sdhci_pci_init_wakeup(chip);
> -
>         return 0;
>  }
>
> --
> 1.9.1
>

[PATCH RESEND 2/9] mmc: sdhci-pci: Use device wakeup capability to determine MMC_PM_WAKE_SDIO_IRQ capability

[Adrian Hunter]

PCI and ACPI determine if a device is wakeup capable, so use that to
determine the MMC_PM_WAKE_SDIO_IRQ capability correctly.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 1711b1815630..baedb2624e36 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -50,9 +50,9 @@ static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
 			pm_flags |= slot->host->mmc->pm_flags;
 	}
 
-	return device_init_wakeup(&chip->pdev->dev,
-				  (pm_flags & MMC_PM_KEEP_POWER) &&
-				  (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
+	return device_set_wakeup_enable(&chip->pdev->dev,
+					(pm_flags & MMC_PM_KEEP_POWER) &&
+					(pm_flags & MMC_PM_WAKE_SDIO_IRQ));
 }
 
 static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
@@ -1680,10 +1680,13 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 		}
 	}
 
-	host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ;
+	host->mmc->pm_caps = MMC_PM_KEEP_POWER;
 	host->mmc->slotno = slotno;
 	host->mmc->caps2 |= MMC_CAP2_NO_PRESCAN_POWERUP;
 
+	if (device_can_wakeup(&pdev->dev))
+		host->mmc->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+
 	if (slot->cd_idx >= 0) {
 		ret = mmc_gpiod_request_cd(host->mmc, NULL, slot->cd_idx,
 					   slot->cd_override_level, 0, NULL);
-- 
1.9.1

Re: [PATCH RESEND 2/9] mmc: sdhci-pci: Use device wakeup capability to determine MMC_PM_WAKE_SDIO_IRQ capability

[Ulf Hansson]

On 9 January 2018 at 08:52, Adrian Hunter <adrian.hunter@intel.com> wrote:
> PCI and ACPI determine if a device is wakeup capable, so use that to
> determine the MMC_PM_WAKE_SDIO_IRQ capability correctly.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 1711b1815630..baedb2624e36 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -50,9 +50,9 @@ static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
>                         pm_flags |= slot->host->mmc->pm_flags;
>         }
>
> -       return device_init_wakeup(&chip->pdev->dev,
> -                                 (pm_flags & MMC_PM_KEEP_POWER) &&
> -                                 (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
> +       return device_set_wakeup_enable(&chip->pdev->dev,
> +                                       (pm_flags & MMC_PM_KEEP_POWER) &&
> +                                       (pm_flags & MMC_PM_WAKE_SDIO_IRQ));
>  }
>
>  static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
> @@ -1680,10 +1680,13 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>                 }
>         }
>
> -       host->mmc->pm_caps = MMC_PM_KEEP_POWER | MMC_PM_WAKE_SDIO_IRQ;
> +       host->mmc->pm_caps = MMC_PM_KEEP_POWER;
>         host->mmc->slotno = slotno;
>         host->mmc->caps2 |= MMC_CAP2_NO_PRESCAN_POWERUP;
>
> +       if (device_can_wakeup(&pdev->dev))
> +               host->mmc->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
> +
>         if (slot->cd_idx >= 0) {
>                 ret = mmc_gpiod_request_cd(host->mmc, NULL, slot->cd_idx,
>                                            slot->cd_override_level, 0, NULL);
> --
> 1.9.1
>

[PATCH RESEND 3/9] mmc: sdhci: Stop exporting sdhci_enable_irq_wakeups()

[Adrian Hunter]

Now that it is not being used by any drivers, stop exporting it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 3 +--
 drivers/mmc/host/sdhci.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e9290a3439d5..8b2ccf7795ec 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2821,7 +2821,7 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
  * sdhci_disable_irq_wakeups() since it will be set by
  * sdhci_enable_card_detection() or sdhci_init().
  */
-void sdhci_enable_irq_wakeups(struct sdhci_host *host)
+static void sdhci_enable_irq_wakeups(struct sdhci_host *host)
 {
 	u8 val;
 	u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
@@ -2839,7 +2839,6 @@ void sdhci_enable_irq_wakeups(struct sdhci_host *host)
 	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
 	sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
 }
-EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
 
 static void sdhci_disable_irq_wakeups(struct sdhci_host *host)
 {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 54bc444c317f..7393b3a54772 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -718,7 +718,6 @@ int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 #ifdef CONFIG_PM
 int sdhci_suspend_host(struct sdhci_host *host);
 int sdhci_resume_host(struct sdhci_host *host);
-void sdhci_enable_irq_wakeups(struct sdhci_host *host);
 int sdhci_runtime_suspend_host(struct sdhci_host *host);
 int sdhci_runtime_resume_host(struct sdhci_host *host);
 #endif
-- 
1.9.1

Re: [PATCH RESEND 3/9] mmc: sdhci: Stop exporting sdhci_enable_irq_wakeups()

[Ulf Hansson]

On 9 January 2018 at 08:52, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Now that it is not being used by any drivers, stop exporting it.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci.c | 3 +--
>  drivers/mmc/host/sdhci.h | 1 -
>  2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index e9290a3439d5..8b2ccf7795ec 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2821,7 +2821,7 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
>   * sdhci_disable_irq_wakeups() since it will be set by
>   * sdhci_enable_card_detection() or sdhci_init().
>   */
> -void sdhci_enable_irq_wakeups(struct sdhci_host *host)
> +static void sdhci_enable_irq_wakeups(struct sdhci_host *host)
>  {
>         u8 val;
>         u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
> @@ -2839,7 +2839,6 @@ void sdhci_enable_irq_wakeups(struct sdhci_host *host)
>         sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
>         sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
>  }
> -EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
>
>  static void sdhci_disable_irq_wakeups(struct sdhci_host *host)
>  {
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 54bc444c317f..7393b3a54772 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -718,7 +718,6 @@ int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>  #ifdef CONFIG_PM
>  int sdhci_suspend_host(struct sdhci_host *host);
>  int sdhci_resume_host(struct sdhci_host *host);
> -void sdhci_enable_irq_wakeups(struct sdhci_host *host);
>  int sdhci_runtime_suspend_host(struct sdhci_host *host);
>  int sdhci_runtime_resume_host(struct sdhci_host *host);
>  #endif
> --
> 1.9.1
>

[PATCH RESEND 4/9] mmc: sdhci: Handle failure of enable_irq_wake()

[Adrian Hunter]

Now that sdhci_enable_irq_wakeups() is a local function, change it to
return whether the IRQ wakeup was successfully enabled. This is in
preparation for adding more conditions for whether IRQ wakeup is enabled.

Note it is assumed, for SDHCI devices, that suspend is more important than
wakeup, so we continue to suspend regardless.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 24 +++++++++++++++---------
 drivers/mmc/host/sdhci.h |  1 +
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8b2ccf7795ec..a8129d091207 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2821,7 +2821,7 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
  * sdhci_disable_irq_wakeups() since it will be set by
  * sdhci_enable_card_detection() or sdhci_init().
  */
-static void sdhci_enable_irq_wakeups(struct sdhci_host *host)
+static bool sdhci_enable_irq_wakeups(struct sdhci_host *host)
 {
 	u8 val;
 	u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
@@ -2838,6 +2838,10 @@ static void sdhci_enable_irq_wakeups(struct sdhci_host *host)
 	}
 	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
 	sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
+
+	host->irq_wake_enabled = !enable_irq_wake(host->irq);
+
+	return host->irq_wake_enabled;
 }
 
 static void sdhci_disable_irq_wakeups(struct sdhci_host *host)
@@ -2849,6 +2853,10 @@ static void sdhci_disable_irq_wakeups(struct sdhci_host *host)
 	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
 	val &= ~mask;
 	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
+
+	disable_irq_wake(host->irq);
+
+	host->irq_wake_enabled = false;
 }
 
 int sdhci_suspend_host(struct sdhci_host *host)
@@ -2857,15 +2865,14 @@ int sdhci_suspend_host(struct sdhci_host *host)
 
 	mmc_retune_timer_stop(host->mmc);
 
-	if (!device_may_wakeup(mmc_dev(host->mmc))) {
+	if (!device_may_wakeup(mmc_dev(host->mmc)) ||
+	    !sdhci_enable_irq_wakeups(host)) {
 		host->ier = 0;
 		sdhci_writel(host, 0, SDHCI_INT_ENABLE);
 		sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
 		free_irq(host->irq, host);
-	} else {
-		sdhci_enable_irq_wakeups(host);
-		enable_irq_wake(host->irq);
 	}
+
 	return 0;
 }
 
@@ -2893,15 +2900,14 @@ int sdhci_resume_host(struct sdhci_host *host)
 		mmiowb();
 	}
 
-	if (!device_may_wakeup(mmc_dev(host->mmc))) {
+	if (host->irq_wake_enabled) {
+		sdhci_disable_irq_wakeups(host);
+	} else {
 		ret = request_threaded_irq(host->irq, sdhci_irq,
 					   sdhci_thread_irq, IRQF_SHARED,
 					   mmc_hostname(host->mmc), host);
 		if (ret)
 			return ret;
-	} else {
-		sdhci_disable_irq_wakeups(host);
-		disable_irq_wake(host->irq);
 	}
 
 	sdhci_enable_card_detection(host);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7393b3a54772..afab26fd70e6 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -484,6 +484,7 @@ struct sdhci_host {
 	bool bus_on;		/* Bus power prevents runtime suspend */
 	bool preset_enabled;	/* Preset is enabled */
 	bool pending_reset;	/* Cmd/data reset is pending */
+	bool irq_wake_enabled;	/* IRQ wakeup is enabled */
 
 	struct mmc_request *mrqs_done[SDHCI_MAX_MRQS];	/* Requests done */
 	struct mmc_command *cmd;	/* Current command */
-- 
1.9.1

Re: [PATCH RESEND 4/9] mmc: sdhci: Handle failure of enable_irq_wake()

[Ulf Hansson]

On 9 January 2018 at 08:52, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Now that sdhci_enable_irq_wakeups() is a local function, change it to
> return whether the IRQ wakeup was successfully enabled. This is in
> preparation for adding more conditions for whether IRQ wakeup is enabled.
>
> Note it is assumed, for SDHCI devices, that suspend is more important than
> wakeup, so we continue to suspend regardless.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci.c | 24 +++++++++++++++---------
>  drivers/mmc/host/sdhci.h |  1 +
>  2 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 8b2ccf7795ec..a8129d091207 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2821,7 +2821,7 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
>   * sdhci_disable_irq_wakeups() since it will be set by
>   * sdhci_enable_card_detection() or sdhci_init().
>   */
> -static void sdhci_enable_irq_wakeups(struct sdhci_host *host)
> +static bool sdhci_enable_irq_wakeups(struct sdhci_host *host)
>  {
>         u8 val;
>         u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
> @@ -2838,6 +2838,10 @@ static void sdhci_enable_irq_wakeups(struct sdhci_host *host)
>         }
>         sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
>         sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
> +
> +       host->irq_wake_enabled = !enable_irq_wake(host->irq);
> +
> +       return host->irq_wake_enabled;
>  }
>
>  static void sdhci_disable_irq_wakeups(struct sdhci_host *host)
> @@ -2849,6 +2853,10 @@ static void sdhci_disable_irq_wakeups(struct sdhci_host *host)
>         val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>         val &= ~mask;
>         sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
> +
> +       disable_irq_wake(host->irq);
> +
> +       host->irq_wake_enabled = false;
>  }
>
>  int sdhci_suspend_host(struct sdhci_host *host)
> @@ -2857,15 +2865,14 @@ int sdhci_suspend_host(struct sdhci_host *host)
>
>         mmc_retune_timer_stop(host->mmc);
>
> -       if (!device_may_wakeup(mmc_dev(host->mmc))) {
> +       if (!device_may_wakeup(mmc_dev(host->mmc)) ||
> +           !sdhci_enable_irq_wakeups(host)) {
>                 host->ier = 0;
>                 sdhci_writel(host, 0, SDHCI_INT_ENABLE);
>                 sdhci_writel(host, 0, SDHCI_SIGNAL_ENABLE);
>                 free_irq(host->irq, host);
> -       } else {
> -               sdhci_enable_irq_wakeups(host);
> -               enable_irq_wake(host->irq);
>         }
> +
>         return 0;
>  }
>
> @@ -2893,15 +2900,14 @@ int sdhci_resume_host(struct sdhci_host *host)
>                 mmiowb();
>         }
>
> -       if (!device_may_wakeup(mmc_dev(host->mmc))) {
> +       if (host->irq_wake_enabled) {
> +               sdhci_disable_irq_wakeups(host);
> +       } else {
>                 ret = request_threaded_irq(host->irq, sdhci_irq,
>                                            sdhci_thread_irq, IRQF_SHARED,
>                                            mmc_hostname(host->mmc), host);
>                 if (ret)
>                         return ret;
> -       } else {
> -               sdhci_disable_irq_wakeups(host);
> -               disable_irq_wake(host->irq);
>         }
>
>         sdhci_enable_card_detection(host);
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 7393b3a54772..afab26fd70e6 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -484,6 +484,7 @@ struct sdhci_host {
>         bool bus_on;            /* Bus power prevents runtime suspend */
>         bool preset_enabled;    /* Preset is enabled */
>         bool pending_reset;     /* Cmd/data reset is pending */
> +       bool irq_wake_enabled;  /* IRQ wakeup is enabled */
>
>         struct mmc_request *mrqs_done[SDHCI_MAX_MRQS];  /* Requests done */
>         struct mmc_command *cmd;        /* Current command */
> --
> 1.9.1
>

[PATCH RESEND 5/9] mmc: sdhci: Rework sdhci_enable_irq_wakeups()

[Adrian Hunter]

In preparation for adding more conditions for whether IRQ wakeup is
enabled, rework sdhci_enable_irq_wakeups() so that needed bits are added
instead of adding them all and then removing the unneeded bits.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a8129d091207..dd922923019a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2823,20 +2823,25 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
  */
 static bool sdhci_enable_irq_wakeups(struct sdhci_host *host)
 {
+	u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE |
+		  SDHCI_WAKE_ON_INT;
+	u32 irq_val = 0;
+	u8 wake_val = 0;
 	u8 val;
-	u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
-			| SDHCI_WAKE_ON_INT;
-	u32 irq_val = SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
-		      SDHCI_INT_CARD_INT;
 
-	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
-	val |= mask ;
-	/* Avoid fake wake up */
-	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) {
-		val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
-		irq_val &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
+	if (!(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)) {
+		wake_val |= SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE;
+		irq_val |= SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE;
 	}
+
+	wake_val |= SDHCI_WAKE_ON_INT;
+	irq_val |= SDHCI_INT_CARD_INT;
+
+	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
+	val &= ~mask;
+	val |= wake_val;
 	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
+
 	sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
 
 	host->irq_wake_enabled = !enable_irq_wake(host->irq);
-- 
1.9.1

Re: [PATCH RESEND 5/9] mmc: sdhci: Rework sdhci_enable_irq_wakeups()

[Ulf Hansson]

On 9 January 2018 at 08:52, Adrian Hunter <adrian.hunter@intel.com> wrote:
> In preparation for adding more conditions for whether IRQ wakeup is
> enabled, rework sdhci_enable_irq_wakeups() so that needed bits are added
> instead of adding them all and then removing the unneeded bits.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a8129d091207..dd922923019a 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2823,20 +2823,25 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
>   */
>  static bool sdhci_enable_irq_wakeups(struct sdhci_host *host)
>  {
> +       u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE |
> +                 SDHCI_WAKE_ON_INT;
> +       u32 irq_val = 0;
> +       u8 wake_val = 0;
>         u8 val;
> -       u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
> -                       | SDHCI_WAKE_ON_INT;
> -       u32 irq_val = SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
> -                     SDHCI_INT_CARD_INT;
>
> -       val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
> -       val |= mask ;
> -       /* Avoid fake wake up */
> -       if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) {
> -               val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
> -               irq_val &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
> +       if (!(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)) {
> +               wake_val |= SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE;
> +               irq_val |= SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE;
>         }
> +
> +       wake_val |= SDHCI_WAKE_ON_INT;
> +       irq_val |= SDHCI_INT_CARD_INT;
> +
> +       val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
> +       val &= ~mask;
> +       val |= wake_val;
>         sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
> +
>         sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
>
>         host->irq_wake_enabled = !enable_irq_wake(host->irq);
> --
> 1.9.1
>

[PATCH RESEND 6/9] mmc: sdhci: Do not unnecessarily enable wakeup for card detect interrupt

[Adrian Hunter]

Do not unnecessarily enable card detect wakeup in the cases that the card
is not removable or a GPIO is used for card detect.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index dd922923019a..f16df253cc59 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2813,6 +2813,14 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
 \*****************************************************************************/
 
 #ifdef CONFIG_PM
+
+static bool sdhci_cd_irq_can_wakeup(struct sdhci_host *host)
+{
+	return mmc_card_is_removable(host->mmc) &&
+	       !(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
+	       host->mmc->slot.cd_irq < 0;
+}
+
 /*
  * To enable wakeup events, the corresponding events have to be enabled in
  * the Interrupt Status Enable register too. See 'Table 1-6: Wakeup Signal
@@ -2829,7 +2837,7 @@ static bool sdhci_enable_irq_wakeups(struct sdhci_host *host)
 	u8 wake_val = 0;
 	u8 val;
 
-	if (!(host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)) {
+	if (sdhci_cd_irq_can_wakeup(host)) {
 		wake_val |= SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE;
 		irq_val |= SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE;
 	}
-- 
1.9.1

[PATCH RESEND 7/9] mmc: sdhci: Do not unnecessarily enable wakeup for SDIO card interrupt

[Adrian Hunter]

Do not enable wakeup for SDIO card interrupt unless the SDIO function
driver has requested it which is indicated by mmc_card_wake_sdio_irq().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f16df253cc59..c82fd16dd14e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2842,8 +2842,13 @@ static bool sdhci_enable_irq_wakeups(struct sdhci_host *host)
 		irq_val |= SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE;
 	}
 
-	wake_val |= SDHCI_WAKE_ON_INT;
-	irq_val |= SDHCI_INT_CARD_INT;
+	if (mmc_card_wake_sdio_irq(host->mmc)) {
+		wake_val |= SDHCI_WAKE_ON_INT;
+		irq_val |= SDHCI_INT_CARD_INT;
+	}
+
+	if (!irq_val)
+		return false;
 
 	val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
 	val &= ~mask;
-- 
1.9.1

[PATCH RESEND 8/9] mmc: slot-gpio: Add a function to enable/disable card detect IRQ wakeup

[Adrian Hunter]

Commit 03dbaa04a2e5 ("mmc: slot-gpio: Add support to enable irq wake on
cd_irq") enabled wakeup irrespective of the host controller's PM flags.
However, users also want to control it from sysfs power/wakeup attribute.
That means the driver needs to check the PM flags before enabling it in the
suspend callback. Add helper function mmc_gpio_set_cd_wake() to make it
easy for drivers to do that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/core.c       |  3 +--
 drivers/mmc/core/slot-gpio.c  | 23 +++++++++++++++++++++++
 include/linux/mmc/slot-gpio.h |  1 +
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c0ba6d8823b7..7bed23c877de 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2655,8 +2655,7 @@ void mmc_start_host(struct mmc_host *host)
 void mmc_stop_host(struct mmc_host *host)
 {
 	if (host->slot.cd_irq >= 0) {
-		if (host->slot.cd_wake_enabled)
-			disable_irq_wake(host->slot.cd_irq);
+		mmc_gpio_set_cd_wake(host, false);
 		disable_irq(host->slot.cd_irq);
 	}
 
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index f7c6e0542de7..3a30535c2f31 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -154,6 +154,29 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);
 
+int mmc_gpio_set_cd_wake(struct mmc_host *host, bool on)
+{
+	int ret = 0;
+
+	if (!(host->caps & MMC_CAP_CD_WAKE) ||
+	    host->slot.cd_irq < 0 ||
+	    on == host->slot.cd_wake_enabled)
+		return 0;
+
+	if (on) {
+		if (device_may_wakeup(mmc_dev(host))) {
+			ret = enable_irq_wake(host->slot.cd_irq);
+			host->slot.cd_wake_enabled = !ret;
+		}
+	} else {
+		disable_irq_wake(host->slot.cd_irq);
+		host->slot.cd_wake_enabled = false;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(mmc_gpio_set_cd_wake);
+
 /* Register an alternate interrupt service routine for
  * the card-detect GPIO.
  */
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index 82f0d289f110..1ee9f15bf2c0 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -33,5 +33,6 @@ void mmc_gpio_set_cd_isr(struct mmc_host *host,
 			 irqreturn_t (*isr)(int irq, void *dev_id));
 void mmc_gpiod_request_cd_irq(struct mmc_host *host);
 bool mmc_can_gpio_cd(struct mmc_host *host);
+int mmc_gpio_set_cd_wake(struct mmc_host *host, bool on);
 
 #endif
-- 
1.9.1

[PATCH RESEND 9/9] mmc: sdhci-pci: Respect PM flags when enabling card detect GPIO IRQ wakeup

[Adrian Hunter]

Commit 03dbaa04a2e5 ("mmc: slot-gpio: Add support to enable irq wake on
cd_irq") enabled wakeup irrespective of the host controller's PM flags.
However, users also want to control it from sysfs power/wakeup attribute.
That means the driver needs to check the PM flags before enabling it in the
suspend callback. Add support for that in sdhci-pci, which is the only
driver presently using the MMC_CAP_CD_WAKE flag, and remove the enabling in
mmc_gpiod_request_cd_irq().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/slot-gpio.c      |  2 --
 drivers/mmc/host/sdhci-pci-core.c | 18 ++++++++++++++----
 drivers/mmc/host/sdhci.c          |  4 ++++
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 3a30535c2f31..74dd534905f7 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -149,8 +149,6 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 
 	if (irq < 0)
 		host->caps |= MMC_CAP_NEEDS_POLL;
-	else if ((host->caps & MMC_CAP_CD_WAKE) && !enable_irq_wake(irq))
-		host->slot.cd_wake_enabled = true;
 }
 EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);
 
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index baedb2624e36..fa233fc6c854 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -41,18 +41,25 @@
 static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
 {
 	mmc_pm_flag_t pm_flags = 0;
+	bool cap_cd_wake = false;
 	int i;
 
 	for (i = 0; i < chip->num_slots; i++) {
 		struct sdhci_pci_slot *slot = chip->slots[i];
 
-		if (slot)
+		if (slot) {
 			pm_flags |= slot->host->mmc->pm_flags;
+			if (slot->host->mmc->caps & MMC_CAP_CD_WAKE)
+				cap_cd_wake = true;
+		}
 	}
 
-	return device_set_wakeup_enable(&chip->pdev->dev,
-					(pm_flags & MMC_PM_KEEP_POWER) &&
-					(pm_flags & MMC_PM_WAKE_SDIO_IRQ));
+	if ((pm_flags & MMC_PM_KEEP_POWER) && (pm_flags & MMC_PM_WAKE_SDIO_IRQ))
+		return device_wakeup_enable(&chip->pdev->dev);
+	else if (!cap_cd_wake)
+		return device_wakeup_disable(&chip->pdev->dev);
+
+	return 0;
 }
 
 static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
@@ -1687,6 +1694,9 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 	if (device_can_wakeup(&pdev->dev))
 		host->mmc->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
 
+	if (host->mmc->caps & MMC_CAP_CD_WAKE)
+		device_init_wakeup(&pdev->dev, true);
+
 	if (slot->cd_idx >= 0) {
 		ret = mmc_gpiod_request_cd(host->mmc, NULL, slot->cd_idx,
 					   slot->cd_override_level, 0, NULL);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c82fd16dd14e..883b55077231 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2891,6 +2891,8 @@ int sdhci_suspend_host(struct sdhci_host *host)
 		free_irq(host->irq, host);
 	}
 
+	mmc_gpio_set_cd_wake(host->mmc, true);
+
 	return 0;
 }
 
@@ -2918,6 +2920,8 @@ int sdhci_resume_host(struct sdhci_host *host)
 		mmiowb();
 	}
 
+	mmc_gpio_set_cd_wake(host->mmc, false);
+
 	if (host->irq_wake_enabled) {
 		sdhci_disable_irq_wakeups(host);
 	} else {
-- 
1.9.1

Re: [PATCH RESEND 0/9] mmc: sdhci-pci: Respect PM flags when enabling card detect GPIO IRQ wakeup

[Adrian Hunter]

On 09/01/18 09:52, Adrian Hunter wrote:
> Hi
> 
> Commit 03dbaa04a2e5 ("mmc: slot-gpio: Add support to enable irq wake on
> cd_irq") enabled wakeup irrespective of the host controller's PM flags.
> However, users also want to control it from sysfs power/wakeup attribute.
> That means the driver needs to check the PM flags before enabling it in the
> suspend callback.  Patch 9 adds support for that in sdhci-pci, which is the
> only driver presently using the MMC_CAP_CD_WAKE flag.  Patches 1 - 7 tidy
> up aspects of sdhci and sdhci-pci wakeup handling, and patch 8 adds a
> helper function to make it easy for drivers.
> 
> There are 2 contentious aspects to this patch set:
> 
> 1) An existing problem with the SDIO API which does not let the host
> controller driver know that the SDIO function driver has requested SDIO
> card interrupt wakeup until the suspend callback - which results in the
> host controller driver having to enable or disable wakeup in the suspend
> callback.  Fixing the SDIO API is a separate issue IMHO.
> 
> 2) In order to use the sysfs power/wakeup attribute, the driver must set the
> device as wake capable even when it is really a GPIO that wakes the system.
> 

Given that we have more-or-less agreed to use sysfs power/wakeup attribute,
I don't see a problem with going ahead with these patches, and adjusting the
API later.  Thoughts?

Re: [PATCH RESEND 0/9] mmc: sdhci-pci: Respect PM flags when enabling card detect GPIO IRQ wakeup

[Ulf Hansson]

On 16 January 2018 at 08:43, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 09/01/18 09:52, Adrian Hunter wrote:
>> Hi
>>
>> Commit 03dbaa04a2e5 ("mmc: slot-gpio: Add support to enable irq wake on
>> cd_irq") enabled wakeup irrespective of the host controller's PM flags.
>> However, users also want to control it from sysfs power/wakeup attribute.
>> That means the driver needs to check the PM flags before enabling it in the
>> suspend callback.  Patch 9 adds support for that in sdhci-pci, which is the
>> only driver presently using the MMC_CAP_CD_WAKE flag.  Patches 1 - 7 tidy
>> up aspects of sdhci and sdhci-pci wakeup handling, and patch 8 adds a
>> helper function to make it easy for drivers.
>>
>> There are 2 contentious aspects to this patch set:
>>
>> 1) An existing problem with the SDIO API which does not let the host
>> controller driver know that the SDIO function driver has requested SDIO
>> card interrupt wakeup until the suspend callback - which results in the
>> host controller driver having to enable or disable wakeup in the suspend
>> callback.  Fixing the SDIO API is a separate issue IMHO.
>>
>> 2) In order to use the sysfs power/wakeup attribute, the driver must set the
>> device as wake capable even when it is really a GPIO that wakes the system.
>>
>
> Given that we have more-or-less agreed to use sysfs power/wakeup attribute,
> I don't see a problem with going ahead with these patches, and adjusting the
> API later.  Thoughts?

Let me have look again, I might be able to pick up at least some of
the changes in $subject series. No matter what, please re-post once
4.16 rc1 is out.

The reason for being conservative, is that I would prefer the
discussion [1] to conclude on how to handle out/in band wakeups, as
that would help to understand the final goal of $subject series.

Kind regards
Uffe

[1]
https://marc.info/?l=linux-pm&m=151600749514917&w=2

Re: [PATCH RESEND 0/9] mmc: sdhci-pci: Respect PM flags when enabling card detect GPIO IRQ wakeup

[Ulf Hansson]

On 16 January 2018 at 09:24, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 16 January 2018 at 08:43, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 09/01/18 09:52, Adrian Hunter wrote:
>>> Hi
>>>
>>> Commit 03dbaa04a2e5 ("mmc: slot-gpio: Add support to enable irq wake on
>>> cd_irq") enabled wakeup irrespective of the host controller's PM flags.
>>> However, users also want to control it from sysfs power/wakeup attribute.
>>> That means the driver needs to check the PM flags before enabling it in the
>>> suspend callback.  Patch 9 adds support for that in sdhci-pci, which is the
>>> only driver presently using the MMC_CAP_CD_WAKE flag.  Patches 1 - 7 tidy
>>> up aspects of sdhci and sdhci-pci wakeup handling, and patch 8 adds a
>>> helper function to make it easy for drivers.
>>>
>>> There are 2 contentious aspects to this patch set:
>>>
>>> 1) An existing problem with the SDIO API which does not let the host
>>> controller driver know that the SDIO function driver has requested SDIO
>>> card interrupt wakeup until the suspend callback - which results in the
>>> host controller driver having to enable or disable wakeup in the suspend
>>> callback.  Fixing the SDIO API is a separate issue IMHO.
>>>
>>> 2) In order to use the sysfs power/wakeup attribute, the driver must set the
>>> device as wake capable even when it is really a GPIO that wakes the system.
>>>
>>
>> Given that we have more-or-less agreed to use sysfs power/wakeup attribute,
>> I don't see a problem with going ahead with these patches, and adjusting the
>> API later.  Thoughts?
>
> Let me have look again, I might be able to pick up at least some of
> the changes in $subject series. No matter what, please re-post once
> 4.16 rc1 is out.

I have picked up patch 1 -> patch 5, the rest seems like something we
need to defer until we know how to deal with in/out-band wakeups.
However, please re-post them once 4.16rc1 is out.

[...]

Kind regards
Uffe