* [PATCH] DRAFT: shdci: allows custom wakeup irqs for runtime PM
2016-03-25 16:05 [PATCH] sdhci: wakeup from runtime PM Ludovic Desroches
@ 2016-03-25 16:05 ` Ludovic Desroches
2016-03-25 16:46 ` kbuild test robot
` (3 more replies)
2016-03-25 16:05 ` [PATCH] mmc: sdhci-of-at91: allow the use of controller card detect as wake up Ludovic Desroches
2016-04-05 12:40 ` [PATCH] sdhci: wakeup from runtime PM Adrian Hunter
2 siblings, 4 replies; 12+ messages in thread
From: Ludovic Desroches @ 2016-03-25 16:05 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter
Cc: linux-kernel, linux-mmc, linux-pm, nicolas.ferre,
Ludovic Desroches
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
drivers/mmc/host/sdhci-of-at91.c | 36 ++++++++++--------------------------
drivers/mmc/host/sdhci.c | 12 +++++-------
drivers/mmc/host/sdhci.h | 2 +-
3 files changed, 16 insertions(+), 34 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index 2703aa9..d70bb7a 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -62,12 +62,14 @@ static int sdhci_at91_runtime_suspend(struct device *dev)
struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
int ret;
- ret = sdhci_runtime_suspend_host(host);
+ ret = sdhci_runtime_suspend_host(host, SDHCI_INT_CARD_INT | SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
clk_disable_unprepare(priv->gck);
- clk_disable_unprepare(priv->hclock);
+ //clk_disable_unprepare(priv->hclock);
clk_disable_unprepare(priv->mainck);
+ printk("sdhci_at91_runtime_suspend\n");
+
return ret;
}
@@ -78,17 +80,18 @@ static int sdhci_at91_runtime_resume(struct device *dev)
struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
int ret;
+ printk("sdhci_at91_runtime_resume\n");
ret = clk_prepare_enable(priv->mainck);
if (ret) {
dev_err(dev, "can't enable mainck\n");
return ret;
}
- ret = clk_prepare_enable(priv->hclock);
- if (ret) {
- dev_err(dev, "can't enable hclock\n");
- return ret;
- }
+ //ret = clk_prepare_enable(priv->hclock);
+ //if (ret) {
+ // dev_err(dev, "can't enable hclock\n");
+ // return ret;
+ //}
ret = clk_prepare_enable(priv->gck);
if (ret) {
@@ -205,25 +208,6 @@ static int sdhci_at91_probe(struct platform_device *pdev)
if (ret)
goto pm_runtime_disable;
- /*
- * When calling sdhci_runtime_suspend_host(), the sdhci layer makes
- * the assumption that all the clocks of the controller are disabled.
- * It means we can't get irq from it when it is runtime suspended.
- * For that reason, it is not planned to wake-up on a card detect irq
- * from the controller.
- * If we want to use runtime PM and to be able to wake-up on card
- * insertion, we have to use a GPIO for the card detection or we can
- * use polling. Be aware that using polling will resume/suspend the
- * controller between each attempt.
- * Disable SDHCI_QUIRK_BROKEN_CARD_DETECTION to be sure nobody tries
- * to enable polling via device tree with broken-cd property.
- */
- if (!(host->mmc->caps & MMC_CAP_NONREMOVABLE) &&
- IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc))) {
- host->mmc->caps |= MMC_CAP_NEEDS_POLL;
- host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
- }
-
pm_runtime_put_autosuspend(&pdev->dev);
return 0;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8670f16..619a64a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2467,11 +2467,6 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
spin_lock(&host->lock);
- if (host->runtime_suspended && !sdhci_sdio_irq_enabled(host)) {
- spin_unlock(&host->lock);
- return IRQ_NONE;
- }
-
intmask = sdhci_readl(host, SDHCI_INT_STATUS);
if (!intmask || intmask == 0xffffffff) {
result = IRQ_NONE;
@@ -2709,7 +2704,7 @@ static void sdhci_runtime_pm_bus_off(struct sdhci_host *host)
pm_runtime_put_noidle(host->mmc->parent);
}
-int sdhci_runtime_suspend_host(struct sdhci_host *host)
+int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs)
{
unsigned long flags;
@@ -2717,7 +2712,10 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host)
mmc_retune_needed(host->mmc);
spin_lock_irqsave(&host->lock, flags);
- host->ier &= SDHCI_INT_CARD_INT;
+ if (wakeup_irqs)
+ host->ier = wakeup_irqs;
+ else
+ host->ier &= SDHCI_INT_CARD_INT;
sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
spin_unlock_irqrestore(&host->lock, flags);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 3bd2803..b932e15 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -668,7 +668,7 @@ void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);
extern int sdhci_suspend_host(struct sdhci_host *host);
extern int sdhci_resume_host(struct sdhci_host *host);
extern void sdhci_enable_irq_wakeups(struct sdhci_host *host);
-extern int sdhci_runtime_suspend_host(struct sdhci_host *host);
+extern int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs);
extern int sdhci_runtime_resume_host(struct sdhci_host *host);
#endif
--
2.5.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] DRAFT: shdci: allows custom wakeup irqs for runtime PM
2016-03-25 16:05 ` [PATCH] DRAFT: shdci: allows custom wakeup irqs for " Ludovic Desroches
@ 2016-03-25 16:46 ` kbuild test robot
2016-03-25 16:50 ` kbuild test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-03-25 16:46 UTC (permalink / raw)
To: Ludovic Desroches
Cc: kbuild-all, ulf.hansson, adrian.hunter, linux-kernel, linux-mmc,
linux-pm, nicolas.ferre, Ludovic Desroches
[-- Attachment #1: Type: text/plain, Size: 2428 bytes --]
Hi Ludovic,
[auto build test ERROR on ulf.hansson-mmc/next]
[also build test ERROR on v4.5 next-20160324]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Ludovic-Desroches/DRAFT-shdci-allows-custom-wakeup-irqs-for-runtime-PM/20160326-000948
base: https://git.linaro.org/people/ulf.hansson/mmc next
config: x86_64-allyesdebian (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/mmc/host/sdhci-pci-core.c: In function 'sdhci_pci_runtime_suspend':
>> drivers/mmc/host/sdhci-pci-core.c:1497:9: error: too few arguments to function 'sdhci_runtime_suspend_host'
ret = sdhci_runtime_suspend_host(slot->host);
^
In file included from drivers/mmc/host/sdhci-pci-core.c:31:0:
drivers/mmc/host/sdhci.h:671:12: note: declared here
extern int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs);
^
vim +/sdhci_runtime_suspend_host +1497 drivers/mmc/host/sdhci-pci-core.c
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1491
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1492 for (i = 0; i < chip->num_slots; i++) {
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1493 slot = chip->slots[i];
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1494 if (!slot)
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1495 continue;
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1496
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 @1497 ret = sdhci_runtime_suspend_host(slot->host);
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1498
b678b91f drivers/mmc/host/sdhci-pci.c Axel Lin 2011-12-03 1499 if (ret)
b678b91f drivers/mmc/host/sdhci-pci.c Axel Lin 2011-12-03 1500 goto err_pci_runtime_suspend;
:::::: The code at line 1497 was first introduced by commit
:::::: 66fd8ad5100b5003046aa744a4f12fa31bb831f9 mmc: sdhci-pci: add runtime pm support
:::::: TO: Adrian Hunter <adrian.hunter@intel.com>
:::::: CC: Chris Ball <cjb@laptop.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 36639 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] DRAFT: shdci: allows custom wakeup irqs for runtime PM
2016-03-25 16:05 ` [PATCH] DRAFT: shdci: allows custom wakeup irqs for " Ludovic Desroches
2016-03-25 16:46 ` kbuild test robot
@ 2016-03-25 16:50 ` kbuild test robot
2016-03-25 17:12 ` kbuild test robot
2016-03-25 17:27 ` kbuild test robot
3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-03-25 16:50 UTC (permalink / raw)
To: Ludovic Desroches
Cc: kbuild-all, ulf.hansson, adrian.hunter, linux-kernel, linux-mmc,
linux-pm, nicolas.ferre, Ludovic Desroches
[-- Attachment #1: Type: text/plain, Size: 2378 bytes --]
Hi Ludovic,
[auto build test ERROR on ulf.hansson-mmc/next]
[also build test ERROR on v4.5 next-20160324]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Ludovic-Desroches/DRAFT-shdci-allows-custom-wakeup-irqs-for-runtime-PM/20160326-000948
base: https://git.linaro.org/people/ulf.hansson/mmc next
config: x86_64-rhel (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
drivers/mmc/host/sdhci-acpi.c: In function 'sdhci_acpi_runtime_suspend':
>> drivers/mmc/host/sdhci-acpi.c:466:9: error: too few arguments to function 'sdhci_runtime_suspend_host'
return sdhci_runtime_suspend_host(c->host);
^
In file included from drivers/mmc/host/sdhci-acpi.c:44:0:
drivers/mmc/host/sdhci.h:671:12: note: declared here
extern int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs);
^
>> drivers/mmc/host/sdhci-acpi.c:467:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +/sdhci_runtime_suspend_host +466 drivers/mmc/host/sdhci-acpi.c
162d6f98 Rafael J. Wysocki 2014-12-05 460 #ifdef CONFIG_PM
c4e05037 Adrian Hunter 2012-11-23 461
c4e05037 Adrian Hunter 2012-11-23 462 static int sdhci_acpi_runtime_suspend(struct device *dev)
c4e05037 Adrian Hunter 2012-11-23 463 {
c4e05037 Adrian Hunter 2012-11-23 464 struct sdhci_acpi_host *c = dev_get_drvdata(dev);
c4e05037 Adrian Hunter 2012-11-23 465
c4e05037 Adrian Hunter 2012-11-23 @466 return sdhci_runtime_suspend_host(c->host);
c4e05037 Adrian Hunter 2012-11-23 @467 }
c4e05037 Adrian Hunter 2012-11-23 468
c4e05037 Adrian Hunter 2012-11-23 469 static int sdhci_acpi_runtime_resume(struct device *dev)
c4e05037 Adrian Hunter 2012-11-23 470 {
:::::: The code at line 466 was first introduced by commit
:::::: c4e050376c69bb9d67895842665264df2a2004d9 mmc: sdhci-acpi: add SDHCI ACPI driver
:::::: TO: Adrian Hunter <adrian.hunter@intel.com>
:::::: CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 36091 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] DRAFT: shdci: allows custom wakeup irqs for runtime PM
2016-03-25 16:05 ` [PATCH] DRAFT: shdci: allows custom wakeup irqs for " Ludovic Desroches
2016-03-25 16:46 ` kbuild test robot
2016-03-25 16:50 ` kbuild test robot
@ 2016-03-25 17:12 ` kbuild test robot
2016-03-25 17:27 ` kbuild test robot
3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-03-25 17:12 UTC (permalink / raw)
To: Ludovic Desroches
Cc: kbuild-all, ulf.hansson, adrian.hunter, linux-kernel, linux-mmc,
linux-pm, nicolas.ferre, Ludovic Desroches
Hi Ludovic,
[auto build test WARNING on ulf.hansson-mmc/next]
[also build test WARNING on v4.5 next-20160324]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Ludovic-Desroches/DRAFT-shdci-allows-custom-wakeup-irqs-for-runtime-PM/20160326-000948
base: https://git.linaro.org/people/ulf.hansson/mmc next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
include/linux/compiler.h:228:8: sparse: attribute 'no_sanitize_address': unknown attribute
>> drivers/mmc/host/sdhci-pci-core.c:1497:49: sparse: not enough arguments for function sdhci_runtime_suspend_host
drivers/mmc/host/sdhci-pci-core.c: In function 'sdhci_pci_runtime_suspend':
drivers/mmc/host/sdhci-pci-core.c:1497:9: error: too few arguments to function 'sdhci_runtime_suspend_host'
ret = sdhci_runtime_suspend_host(slot->host);
^
In file included from drivers/mmc/host/sdhci-pci-core.c:31:0:
drivers/mmc/host/sdhci.h:671:12: note: declared here
extern int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs);
^
--
include/linux/compiler.h:228:8: sparse: attribute 'no_sanitize_address': unknown attribute
>> drivers/mmc/host/sdhci-acpi.c:466:42: sparse: not enough arguments for function sdhci_runtime_suspend_host
drivers/mmc/host/sdhci-acpi.c: In function 'sdhci_acpi_runtime_suspend':
drivers/mmc/host/sdhci-acpi.c:466:9: error: too few arguments to function 'sdhci_runtime_suspend_host'
return sdhci_runtime_suspend_host(c->host);
^
In file included from drivers/mmc/host/sdhci-acpi.c:44:0:
drivers/mmc/host/sdhci.h:671:12: note: declared here
extern int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs);
^
drivers/mmc/host/sdhci-acpi.c:467:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
--
include/linux/compiler.h:228:8: sparse: attribute 'no_sanitize_address': unknown attribute
>> drivers/mmc/host/sdhci-pxav3.c:539:41: sparse: not enough arguments for function sdhci_runtime_suspend_host
drivers/mmc/host/sdhci-pxav3.c: In function 'sdhci_pxav3_runtime_suspend':
drivers/mmc/host/sdhci-pxav3.c:539:8: error: too few arguments to function 'sdhci_runtime_suspend_host'
ret = sdhci_runtime_suspend_host(host);
^
In file included from drivers/mmc/host/sdhci-pxav3.c:39:0:
drivers/mmc/host/sdhci.h:671:12: note: declared here
extern int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs);
^
vim +1497 drivers/mmc/host/sdhci-pci-core.c
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1481 static int sdhci_pci_runtime_suspend(struct device *dev)
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1482 {
923a231c drivers/mmc/host/sdhci-pci-core.c Geliang Tang 2015-12-27 1483 struct pci_dev *pdev = to_pci_dev(dev);
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1484 struct sdhci_pci_chip *chip;
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1485 struct sdhci_pci_slot *slot;
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1486 int i, ret;
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1487
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1488 chip = pci_get_drvdata(pdev);
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1489 if (!chip)
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1490 return 0;
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1491
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1492 for (i = 0; i < chip->num_slots; i++) {
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1493 slot = chip->slots[i];
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1494 if (!slot)
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1495 continue;
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1496
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 @1497 ret = sdhci_runtime_suspend_host(slot->host);
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1498
b678b91f drivers/mmc/host/sdhci-pci.c Axel Lin 2011-12-03 1499 if (ret)
b678b91f drivers/mmc/host/sdhci-pci.c Axel Lin 2011-12-03 1500 goto err_pci_runtime_suspend;
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1501 }
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1502
66fd8ad5 drivers/mmc/host/sdhci-pci.c Adrian Hunter 2011-10-03 1503 if (chip->fixes && chip->fixes->suspend) {
29495aa0 drivers/mmc/host/sdhci-pci.c Manuel Lauss 2011-11-03 1504 ret = chip->fixes->suspend(chip);
b678b91f drivers/mmc/host/sdhci-pci.c Axel Lin 2011-12-03 1505 if (ret)
:::::: The code at line 1497 was first introduced by commit
:::::: 66fd8ad5100b5003046aa744a4f12fa31bb831f9 mmc: sdhci-pci: add runtime pm support
:::::: TO: Adrian Hunter <adrian.hunter@intel.com>
:::::: CC: Chris Ball <cjb@laptop.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] DRAFT: shdci: allows custom wakeup irqs for runtime PM
2016-03-25 16:05 ` [PATCH] DRAFT: shdci: allows custom wakeup irqs for " Ludovic Desroches
` (2 preceding siblings ...)
2016-03-25 17:12 ` kbuild test robot
@ 2016-03-25 17:27 ` kbuild test robot
3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-03-25 17:27 UTC (permalink / raw)
To: Ludovic Desroches
Cc: kbuild-all, ulf.hansson, adrian.hunter, linux-kernel, linux-mmc,
linux-pm, nicolas.ferre, Ludovic Desroches
[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]
Hi Ludovic,
[auto build test ERROR on ulf.hansson-mmc/next]
[also build test ERROR on v4.5 next-20160324]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Ludovic-Desroches/DRAFT-shdci-allows-custom-wakeup-irqs-for-runtime-PM/20160326-000948
base: https://git.linaro.org/people/ulf.hansson/mmc next
config: i386-allmodconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/mmc/host/sdhci-pxav3.c: In function 'sdhci_pxav3_runtime_suspend':
>> drivers/mmc/host/sdhci-pxav3.c:539:8: error: too few arguments to function 'sdhci_runtime_suspend_host'
ret = sdhci_runtime_suspend_host(host);
^
In file included from drivers/mmc/host/sdhci-pxav3.c:39:0:
drivers/mmc/host/sdhci.h:671:12: note: declared here
extern int sdhci_runtime_suspend_host(struct sdhci_host *host, u32 wakeup_irqs);
^
vim +/sdhci_runtime_suspend_host +539 drivers/mmc/host/sdhci-pxav3.c
bb691ae4 Kevin Liu 2013-02-01 533 {
bb691ae4 Kevin Liu 2013-02-01 534 struct sdhci_host *host = dev_get_drvdata(dev);
bb691ae4 Kevin Liu 2013-02-01 535 struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
f599da40 Jisheng Zhang 2016-02-16 536 struct sdhci_pxa *pxa = sdhci_pltfm_priv(pltfm_host);
3bb10f60 Jisheng Zhang 2015-01-23 537 int ret;
bb691ae4 Kevin Liu 2013-02-01 538
3bb10f60 Jisheng Zhang 2015-01-23 @539 ret = sdhci_runtime_suspend_host(host);
3bb10f60 Jisheng Zhang 2015-01-23 540 if (ret)
3bb10f60 Jisheng Zhang 2015-01-23 541 return ret;
bb691ae4 Kevin Liu 2013-02-01 542
:::::: The code at line 539 was first introduced by commit
:::::: 3bb10f60933e84abfe2be69f60b3486f9b96348b mmc: sdhci-pxav3: fix race between runtime pm and irq
:::::: TO: Jisheng Zhang <jszhang@marvell.com>
:::::: CC: Ulf Hansson <ulf.hansson@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 53462 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] mmc: sdhci-of-at91: allow the use of controller card detect as wake up
2016-03-25 16:05 [PATCH] sdhci: wakeup from runtime PM Ludovic Desroches
2016-03-25 16:05 ` [PATCH] DRAFT: shdci: allows custom wakeup irqs for " Ludovic Desroches
@ 2016-03-25 16:05 ` Ludovic Desroches
2016-03-25 17:11 ` kbuild test robot
2016-03-25 17:11 ` [PATCH] mmc: sdhci-of-at91: fix semicolon.cocci warnings kbuild test robot
2016-04-05 12:40 ` [PATCH] sdhci: wakeup from runtime PM Adrian Hunter
2 siblings, 2 replies; 12+ messages in thread
From: Ludovic Desroches @ 2016-03-25 16:05 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter
Cc: linux-kernel, linux-mmc, linux-pm, nicolas.ferre,
Ludovic Desroches
At the moment, there are only two ways to wake up from runtime suspend state:
- using a gpio for the card detection which is not a good solution since
you still need the card detect pin of the controller.
- using polling which is also not a good solution since it will
resume/suspend the controller between each attempt.
The sdhci layer makes the assumption that all the clocks are disabled
when suspending the controller. For that reason, irqs are rejected if
runtime_suspended is true.
This patch duplicates the sdhci_runtime_suspend_host() but doesn't set
runtime_suspended and doesn't disable the interface clock if there is a
removable device and no gpio for card detection.
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
drivers/mmc/host/sdhci-of-at91.c | 71 +++++++++++++++++++++++++---------------
1 file changed, 45 insertions(+), 26 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index 2703aa9..6683f2d 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -35,6 +35,7 @@ struct sdhci_at91_priv {
struct clk *hclock;
struct clk *gck;
struct clk *mainck;
+ bool disable_interface_clk;
};
static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
@@ -55,20 +56,47 @@ static const struct of_device_id sdhci_at91_dt_match[] = {
};
#ifdef CONFIG_PM
+/*
+ * This function is the very same as sdhci_runtime_suspend_host but if the
+ * device is removable and we only have the controller card detect signal then
+ * we won't disable all the clocks. The interface clock will be needed to get
+ * card event interrupt.
+ * When calling sdhci_runtime_suspend_host(), the sdhci layer makes the
+ * assumption that all the clocks of the controller are disabled. If
+ * runtime_suspended is set, irqs will be rejected. That's why it won't be set
+ * if we want to wake up on card event.
+ */
static int sdhci_at91_runtime_suspend(struct device *dev)
{
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
- int ret;
+ unsigned long flags;
+
+ mmc_retune_timer_stop(host->mmc);
+ mmc_retune_needed(host->mmc);
+
+ spin_lock_irqsave(&host->lock, flags);
+ host->ier &= SDHCI_INT_CARD_INT;
+ if (!priv->disable_interface_clk)
+ host->ier |= SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE;
+ sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+ sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+ spin_unlock_irqrestore(&host->lock, flags);
- ret = sdhci_runtime_suspend_host(host);
+ synchronize_hardirq(host->irq);
+
+ if (priv->disable_interface_clk) {
+ spin_lock_irqsave(&host->lock, flags);
+ host->runtime_suspended = true;
+ spin_unlock_irqrestore(&host->lock, flags);
+ clk_disable_unprepare(priv->hclock);
+ };
clk_disable_unprepare(priv->gck);
- clk_disable_unprepare(priv->hclock);
clk_disable_unprepare(priv->mainck);
- return ret;
+ return 0;
}
static int sdhci_at91_runtime_resume(struct device *dev)
@@ -84,18 +112,20 @@ static int sdhci_at91_runtime_resume(struct device *dev)
return ret;
}
- ret = clk_prepare_enable(priv->hclock);
- if (ret) {
- dev_err(dev, "can't enable hclock\n");
- return ret;
- }
-
ret = clk_prepare_enable(priv->gck);
if (ret) {
dev_err(dev, "can't enable gck\n");
return ret;
}
+ if (priv->disable_interface_clk) {
+ ret = clk_prepare_enable(priv->hclock);
+ if (ret) {
+ dev_err(dev, "can't enable hclock\n");
+ return ret;
+ }
+ }
+
return sdhci_runtime_resume_host(host);
}
#endif /* CONFIG_PM */
@@ -206,23 +236,12 @@ static int sdhci_at91_probe(struct platform_device *pdev)
goto pm_runtime_disable;
/*
- * When calling sdhci_runtime_suspend_host(), the sdhci layer makes
- * the assumption that all the clocks of the controller are disabled.
- * It means we can't get irq from it when it is runtime suspended.
- * For that reason, it is not planned to wake-up on a card detect irq
- * from the controller.
- * If we want to use runtime PM and to be able to wake-up on card
- * insertion, we have to use a GPIO for the card detection or we can
- * use polling. Be aware that using polling will resume/suspend the
- * controller between each attempt.
- * Disable SDHCI_QUIRK_BROKEN_CARD_DETECTION to be sure nobody tries
- * to enable polling via device tree with broken-cd property.
+ * If the device is non removable or if there is a gpio for the card
+ * detection, all the clocks can be disabled in runtime suspend state.
*/
- if (!(host->mmc->caps & MMC_CAP_NONREMOVABLE) &&
- IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc))) {
- host->mmc->caps |= MMC_CAP_NEEDS_POLL;
- host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
- }
+ priv->disable_interface_clk = (host->mmc->caps & MMC_CAP_NONREMOVABLE)
+ || !IS_ERR_VALUE(mmc_gpio_get_cd(host->mmc));
+
pm_runtime_put_autosuspend(&pdev->dev);
--
2.5.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] mmc: sdhci-of-at91: allow the use of controller card detect as wake up
2016-03-25 16:05 ` [PATCH] mmc: sdhci-of-at91: allow the use of controller card detect as wake up Ludovic Desroches
@ 2016-03-25 17:11 ` kbuild test robot
2016-03-25 17:11 ` [PATCH] mmc: sdhci-of-at91: fix semicolon.cocci warnings kbuild test robot
1 sibling, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-03-25 17:11 UTC (permalink / raw)
To: Ludovic Desroches
Cc: kbuild-all, ulf.hansson, adrian.hunter, linux-kernel, linux-mmc,
linux-pm, nicolas.ferre, Ludovic Desroches
Hi Ludovic,
[auto build test WARNING on ulf.hansson-mmc/next]
[also build test WARNING on next-20160324]
[cannot apply to v4.5]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Ludovic-Desroches/mmc-sdhci-of-at91-allow-the-use-of-controller-card-detect-as-wake-up/20160326-001143
base: https://git.linaro.org/people/ulf.hansson/mmc next
coccinelle warnings: (new ones prefixed by >>)
>> drivers/mmc/host/sdhci-of-at91.c:94:2-3: Unneeded semicolon
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] mmc: sdhci-of-at91: fix semicolon.cocci warnings
2016-03-25 16:05 ` [PATCH] mmc: sdhci-of-at91: allow the use of controller card detect as wake up Ludovic Desroches
2016-03-25 17:11 ` kbuild test robot
@ 2016-03-25 17:11 ` kbuild test robot
1 sibling, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-03-25 17:11 UTC (permalink / raw)
To: Ludovic Desroches
Cc: kbuild-all, ulf.hansson, adrian.hunter, linux-kernel, linux-mmc,
linux-pm, nicolas.ferre, Ludovic Desroches
drivers/mmc/host/sdhci-of-at91.c:94:2-3: Unneeded semicolon
Remove unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
CC: Ludovic Desroches <ludovic.desroches@atmel.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
sdhci-of-at91.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -91,7 +91,7 @@ static int sdhci_at91_runtime_suspend(st
host->runtime_suspended = true;
spin_unlock_irqrestore(&host->lock, flags);
clk_disable_unprepare(priv->hclock);
- };
+ }
clk_disable_unprepare(priv->gck);
clk_disable_unprepare(priv->mainck);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] sdhci: wakeup from runtime PM
2016-03-25 16:05 [PATCH] sdhci: wakeup from runtime PM Ludovic Desroches
2016-03-25 16:05 ` [PATCH] DRAFT: shdci: allows custom wakeup irqs for " Ludovic Desroches
2016-03-25 16:05 ` [PATCH] mmc: sdhci-of-at91: allow the use of controller card detect as wake up Ludovic Desroches
@ 2016-04-05 12:40 ` Adrian Hunter
2016-04-07 9:11 ` Ulf Hansson
2 siblings, 1 reply; 12+ messages in thread
From: Adrian Hunter @ 2016-04-05 12:40 UTC (permalink / raw)
To: Ludovic Desroches, ulf.hansson
Cc: linux-kernel, linux-mmc, linux-pm, nicolas.ferre
On 25/03/16 18:05, Ludovic Desroches wrote:
> Hi,
>
> When not using the SDHCI controller, it is logical to save power by suspending
> it. The issue is that SDHCI assumes that the controller is completely disabled.
> It means the only way to wake up on a card event is to have a gpio for the card
> detection (so two pins for the same signal). A possible workaround is to use
> polling but the controller will be resumed/suspended between each attempts.
>
> We have already discussed a long time about this and it seems we don't agree.
> In my opinion, even if I can't disable all clocks, I should use runtime PM
> to save some power.
>
> I propose two patches, one which is a draft to try to solve it at sdhci level
> and one at sdhci-of-at91 level.
>
> Concerning the first one, I don't understand why we need to reject irqs if
> runtime_suspended is true.
The interrupt handler might be called because the interrupt is shared i.e.
the interrupt is for a different device. In that case the host controller
might be off and the registers inaccessible. In that case we cannot even
look at the interrupt register to determine if we were expecting the interrupt.
> Only SDHCI_INT_CARD_INT irq is enabled so why we
> should have other IRQs than this one?
In the case of SDIO Card interrupt, it is delivered via the host controller,
so we have to assume the registers are accessible if we are
runtime-suspended with the SDIO IRQ enabled.
>
> Since you were not in favour of allowing to wakeup on SDHCI_INT_CARD_INSERT or
> SDHCI_INT_CARD_REMOVE, I assume you won't take it so I
> solved my issue only by modifying my driver.
I don't mind allowing card detect interrupts while runtime suspended, but we
need a flag so that:
- runtime suspend leaves the insert/remove interrupts enabled
- irq handler knows it can access registers
- irq thread handler knows to runtime resume before doing anything else
But it seems like you need to persuade Ulf first.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] sdhci: wakeup from runtime PM
2016-04-05 12:40 ` [PATCH] sdhci: wakeup from runtime PM Adrian Hunter
@ 2016-04-07 9:11 ` Ulf Hansson
2016-04-07 15:12 ` Ludovic Desroches
0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2016-04-07 9:11 UTC (permalink / raw)
To: Ludovic Desroches, Adrian Hunter
Cc: linux-kernel@vger.kernel.org, linux-mmc, linux-pm@vger.kernel.org,
Nicolas Ferre
On 5 April 2016 at 14:40, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 25/03/16 18:05, Ludovic Desroches wrote:
>> Hi,
>>
>> When not using the SDHCI controller, it is logical to save power by suspending
>> it. The issue is that SDHCI assumes that the controller is completely disabled.
>> It means the only way to wake up on a card event is to have a gpio for the card
>> detection (so two pins for the same signal). A possible workaround is to use
>> polling but the controller will be resumed/suspended between each attempts.
>>
>> We have already discussed a long time about this and it seems we don't agree.
>> In my opinion, even if I can't disable all clocks, I should use runtime PM
>> to save some power.
>>
>> I propose two patches, one which is a draft to try to solve it at sdhci level
>> and one at sdhci-of-at91 level.
>>
>> Concerning the first one, I don't understand why we need to reject irqs if
>> runtime_suspended is true.
>
> The interrupt handler might be called because the interrupt is shared i.e.
> the interrupt is for a different device. In that case the host controller
> might be off and the registers inaccessible. In that case we cannot even
> look at the interrupt register to determine if we were expecting the interrupt.
>
>> Only SDHCI_INT_CARD_INT irq is enabled so why we
>> should have other IRQs than this one?
>
> In the case of SDIO Card interrupt, it is delivered via the host controller,
> so we have to assume the registers are accessible if we are
> runtime-suspended with the SDIO IRQ enabled.
>
>>
>> Since you were not in favour of allowing to wakeup on SDHCI_INT_CARD_INSERT or
>> SDHCI_INT_CARD_REMOVE, I assume you won't take it so I
>> solved my issue only by modifying my driver.
>
> I don't mind allowing card detect interrupts while runtime suspended, but we
> need a flag so that:
> - runtime suspend leaves the insert/remove interrupts enabled
> - irq handler knows it can access registers
To me, this seem like the wrong way of how to configure wake-ups for
these kind of devices.
I don't think the regular IRQs shall be enabled and the driver
shouldn't assume the registers are accessible without first runtime
resuming the device.
> - irq thread handler knows to runtime resume before doing anything else
>
> But it seems like you need to persuade Ulf first.
For a more thorough explanation to why I don't like, please have a
look at my comment for another related thread on the mmc list.
http://www.spinics.net/lists/linux-mmc/msg36132.html
Kind regards
Uffe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] sdhci: wakeup from runtime PM
2016-04-07 9:11 ` Ulf Hansson
@ 2016-04-07 15:12 ` Ludovic Desroches
0 siblings, 0 replies; 12+ messages in thread
From: Ludovic Desroches @ 2016-04-07 15:12 UTC (permalink / raw)
To: Ulf Hansson
Cc: Ludovic Desroches, Adrian Hunter, linux-kernel@vger.kernel.org,
linux-mmc, linux-pm@vger.kernel.org, Nicolas Ferre
On Thu, Apr 07, 2016 at 11:11:08AM +0200, Ulf Hansson wrote:
> On 5 April 2016 at 14:40, Adrian Hunter <adrian.hunter@intel.com> wrote:
> > On 25/03/16 18:05, Ludovic Desroches wrote:
> >> Hi,
> >>
> >> When not using the SDHCI controller, it is logical to save power by suspending
> >> it. The issue is that SDHCI assumes that the controller is completely disabled.
> >> It means the only way to wake up on a card event is to have a gpio for the card
> >> detection (so two pins for the same signal). A possible workaround is to use
> >> polling but the controller will be resumed/suspended between each attempts.
> >>
> >> We have already discussed a long time about this and it seems we don't agree.
> >> In my opinion, even if I can't disable all clocks, I should use runtime PM
> >> to save some power.
> >>
> >> I propose two patches, one which is a draft to try to solve it at sdhci level
> >> and one at sdhci-of-at91 level.
> >>
> >> Concerning the first one, I don't understand why we need to reject irqs if
> >> runtime_suspended is true.
> >
> > The interrupt handler might be called because the interrupt is shared i.e.
> > the interrupt is for a different device. In that case the host controller
> > might be off and the registers inaccessible. In that case we cannot even
> > look at the interrupt register to determine if we were expecting the interrupt.
Ok, I have missed we can get a shared irq.
> >
> >> Only SDHCI_INT_CARD_INT irq is enabled so why we
> >> should have other IRQs than this one?
> >
> > In the case of SDIO Card interrupt, it is delivered via the host controller,
> > so we have to assume the registers are accessible if we are
> > runtime-suspended with the SDIO IRQ enabled.
> >
> >>
> >> Since you were not in favour of allowing to wakeup on SDHCI_INT_CARD_INSERT or
> >> SDHCI_INT_CARD_REMOVE, I assume you won't take it so I
> >> solved my issue only by modifying my driver.
> >
> > I don't mind allowing card detect interrupts while runtime suspended, but we
> > need a flag so that:
> > - runtime suspend leaves the insert/remove interrupts enabled
> > - irq handler knows it can access registers
>
> To me, this seem like the wrong way of how to configure wake-ups for
> these kind of devices.
We are definitely not in agreement on this point. It means I have to
rely on polling for card detection. I did it but I had in mind it would
be a temporary workaround while waiting to find a better one.
>
> I don't think the regular IRQs shall be enabled and the driver
> shouldn't assume the registers are accessible without first runtime
> resuming the device.
>
> > - irq thread handler knows to runtime resume before doing anything else
> >
> > But it seems like you need to persuade Ulf first.
>
It will be a difficult task :)
> For a more thorough explanation to why I don't like, please have a
> look at my comment for another related thread on the mmc list.
> http://www.spinics.net/lists/linux-mmc/msg36132.html
>
To facilitate discussion, I copy a part of your explanation here:
> My interpretation of how runtime PM should be deployed, it's s that a
> device shall not process data in the runtime suspend state. To do
> that, it first needs to be runtime resumed. I think this conforms to
> what the runtime PM documentation also recommends in section 2, Device
> Runtime PM Callbacks.
So simply invoking the resume callback when the card detect irq is
received should be in accordance with the PM documentation.
> Moreover I think it's an SoC specific detail whether a driver *can*
> access a device's registers in the runtime suspend state (thus it
> shouldn't do it). Especially, devices may reside in a PM domain, which
> could enter a retention state, because all its devices are runtime
> suspended. Typically for an ARM SoC is that devices looses their
> register's context in such retention state.
Maybe it's a SoC specific detail but that doesn't mean that this detail
should not be taken into account. The PM documentation mentions 'the PM
core regards the device as suspended, which need not mean that it has been
put into a low power state'. It sounds like we can access to the
registers, there is no requirement to disable all the clocks.
I am not arguing for SD card detection wake-up itself but for saving power
when I don't use my SDHCI controller. I boot Linux without a SD card,
the controller is runtime suspended, it seems obvious that the controller
should be resumed when I insert a card. I really want to use runtime PM
because the controller should be most of the time suspended excepting if the
rootfs is on the SD card.
Am I the only one to find that the constraint is too big to use runtime PM?
Regards
Ludovic
^ permalink raw reply [flat|nested] 12+ messages in thread