sdhci runtime_pm implementation

From: Yunpeng Gao <[email protected]>

Follow the kernel runtime PM framework, enable runtime PM support of the sdhci host controller with pci interface.

Note that this patch implements runtime_pm but now actually detects activity.
It relies on higher level (childrens) to do actual waking up Activity detection is put in a following patch

This patch also does not enable wakeups, so card insertion will not work if runtime_pm is activated.
This is also dealt in a following patch

Original version from: Yunpeng Gao <[email protected]> with modifications by: Pierre Tardy <[email protected]>

Signed-off-by Pierre Tardy <[email protected]>

---
 drivers/mmc/host/sdhci-pci.c |  120 +++++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci.c     |   31 +++++++++++
 drivers/mmc/host/sdhci.h     |    5 ++
 3 files changed, 155 insertions(+), 1 deletions(-)

drivers/mmc/host/sdhci-pci.c +1 -0

index 2f8d468..f167a55 100644
@@ -18,6 +18,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
 #include <linux/device.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/mmc/host.h>
 

drivers/mmc/host/sdhci-pci.c +36 -1

+
 	slots = PCI_SLOT_INFO_SLOTS(slots) + 1;
 	dev_dbg(&pdev->dev, "found %d slot(s)\n", slots);
 	if (slots == 0)

@@ -1086,7 +1089,7 @@ static int __devinit sdhci_pci_probe(struct pci_dev *pdev,
 
 		chip->slots[i] = slot;
 	}
-
+	pm_runtime_put_noidle(&pdev->dev);
 	return 0;
 
 free:

@@ -1114,8 +1117,120 @@ static void __devexit sdhci_pci_remove(struct pci_dev *pdev)
 	}
 
 	pci_disable_device(pdev);
+	pm_runtime_get_noresume(&pdev->dev);
+
+}
+
+#ifdef CONFIG_PM_RUNTIME
+
+static int sdhci_pci_runtime_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+	struct sdhci_pci_chip *chip;
+	struct sdhci_pci_slot *slot;
+	int i, ret;
+	mmc_pm_flag_t pm_flags = 0;
+	pm_message_t state;
+
+	chip = pci_get_drvdata(pdev);
+	if (!chip)
+		return 0;
+
+	for (i = 0; i < chip->num_slots; i++) {
+		slot = chip->slots[i];
+		if (!slot)
+			continue;
+
+		ret = sdhci_runtime_suspend(slot->host);
+
+		if (ret) {
+			for (i--; i >= 0; i--)
+				sdhci_runtime_resume(chip->slots[i]->host);
+			return ret;
+		}
+
+		pm_flags |= slot->host->mmc->pm_flags;
+	}

drivers/mmc/host/sdhci-pci.c +5 -0

+
+	return 0;
 }
 
+static int sdhci_pci_runtime_resume(struct device *dev)
+{
+	struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);

drivers/mmc/host/sdhci-pci.c +18 -0

+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	if (chip->fixes && chip->fixes->resume) {
+		ret = chip->fixes->resume(chip);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < chip->num_slots; i++) {
+		slot = chip->slots[i];
+		if (!slot)
+			continue;
+
+		ret = sdhci_runtime_resume(slot->host);
+		if (ret)
+			return ret;

From: Yunpeng Gao <[email protected]>

Follow the kernel runtime PM framework, enable runtime PM support of the sdhci host controller with pci interface.

Note that this patch implements runtime_pm but now actually detects activity.
It relies on higher level (childrens) to do actual waking up Activity detection is put in a following patch

This patch also does not enable wakeups, so card insertion will not work if runtime_pm is activated.
This is also dealt in a following patch

Original version from: Yunpeng Gao <[email protected]> with modifications by: Pierre Tardy <[email protected]>

Signed-off-by Pierre Tardy <[email protected]>

---
 drivers/mmc/host/sdhci-pci.c |  120 +++++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci.c     |   31 +++++++++++
 drivers/mmc/host/sdhci.h     |    5 ++
 3 files changed, 155 insertions(+), 1 deletions(-)

drivers/mmc/host/sdhci-pci.c +1 -0

index 2f8d468..f167a55 100644
@@ -18,6 +18,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
 #include <linux/device.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/mmc/host.h>
 

drivers/mmc/host/sdhci-pci.c +36 -1

+
 	slots = PCI_SLOT_INFO_SLOTS(slots) + 1;
 	dev_dbg(&pdev->dev, "found %d slot(s)\n", slots);
 	if (slots == 0)

@@ -1086,7 +1089,7 @@ static int __devinit sdhci_pci_probe(struct pci_dev *pdev,
 
 		chip->slots[i] = slot;
 	}
-
+	pm_runtime_put_noidle(&pdev->dev);
 	return 0;
 
 free:

@@ -1114,8 +1117,120 @@ static void __devexit sdhci_pci_remove(struct pci_dev *pdev)
 	}
 
 	pci_disable_device(pdev);
+	pm_runtime_get_noresume(&pdev->dev);
+
+}
+
+#ifdef CONFIG_PM_RUNTIME
+
+static int sdhci_pci_runtime_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+	struct sdhci_pci_chip *chip;
+	struct sdhci_pci_slot *slot;
+	int i, ret;
+	mmc_pm_flag_t pm_flags = 0;
+	pm_message_t state;
+
+	chip = pci_get_drvdata(pdev);
+	if (!chip)
+		return 0;
+
+	for (i = 0; i < chip->num_slots; i++) {
+		slot = chip->slots[i];
+		if (!slot)
+			continue;
+
+		ret = sdhci_runtime_suspend(slot->host);
+
+		if (ret) {
+			for (i--; i >= 0; i--)
+				sdhci_runtime_resume(chip->slots[i]->host);
+			return ret;
+		}
+
+		pm_flags |= slot->host->mmc->pm_flags;
+	}

drivers/mmc/host/sdhci-pci.c +5 -0

+
+	return 0;
 }
 
+static int sdhci_pci_runtime_resume(struct device *dev)
+{
+	struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);

drivers/mmc/host/sdhci-pci.c +18 -0

+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	if (chip->fixes && chip->fixes->resume) {
+		ret = chip->fixes->resume(chip);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < chip->num_slots; i++) {
+		slot = chip->slots[i];
+		if (!slot)
+			continue;
+
+		ret = sdhci_runtime_resume(slot->host);
+		if (ret)
+			return ret;

From Linus Walleij <[email protected]>

From Linus Walleij <[email protected]>

When sdhci is runtime_suspended, it can still receive a wake-up because of card insertion.
The wake-up will then be signaled by an interrupt which cannot be serviced immediatly, as the device is powered off, and register are not accessibles.

We cannot call pm_runtime_get_sync() in irq context as on some architecture (e.g. intel_mid), runtime_resume can lead to too much latency, due to e.g. PLL warm-up and such aggressive power gating.

We temporarly disable the interrupt, and trigger a runtime_resume of the device The interrupt will be re-enabled when resume is finished.

Signed-off-by Pierre Tardy <[email protected]>

---
 drivers/mmc/host/sdhci-pci.c |    8 +-------
 drivers/mmc/host/sdhci.c     |   30 +++++++++++++++++++++++++++++-
 include/linux/mmc/sdhci.h    |    2 ++
 scripts/checkpatch.pl        |    6 +++---
 4 files changed, 35 insertions(+), 11 deletions(-)

drivers/mmc/host/sdhci.c +0 -0

index 735c3f7..85a1956 100644
@@ -1599,6 +1599,21 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 	u32 intmask;
 	int cardint = 0;
 

drivers/mmc/host/sdhci.c +16 -1

+		return IRQ_NONE;
+	}
 	spin_lock(&host->lock);
 
 	intmask = sdhci_readl(host, SDHCI_INT_STATUS);

@@ -1747,7 +1762,12 @@ EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
 
 int sdhci_runtime_suspend(struct sdhci_host *host)
 {
-	/* nothing to do yet */
+	u8 val;
+	/* make sure we wake up on sdcard insert/remove enabled */
+	val = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE;
+	if (host->mmc->pm_flags & MMC_PM_WAKE_SDIO_IRQ)
+		val |= SDHCI_WAKE_ON_INT;
+	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
 	return 0;
 }
 

@@ -1768,6 +1788,14 @@ int sdhci_runtime_resume(struct sdhci_host *host)
 	sdhci_set_ios(host->mmc, &host->mmc->ios);
 	mmiowb();
 
+	if (host->waking_up) {
+		host->waking_up = 0;
+		/* Re-enable the irq now we are perfectly resumed.
+		 * Any yet unhandled interrupt (i.e. the wake) will retrigger
+		 * irq
+		 */
+		irq_enable(host->irq);
+	}
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sdhci_runtime_resume);

scripts/checkpatch.pl +2 -2

index e3c7fc0..17aeda3 100755
@@ -1325,8 +1325,8 @@ sub process {
 		$prefix = "$filename:$realline: " if ($emacs && $file);
 		$prefix = "$filename:$linenr: " if ($emacs && !$file);
 
-		$here = "#$linenr: " if (!$file);
-		$here = "#$realline: " if ($file);
+		$here = "" if (!$file);
+		$here = "" if ($file);
 
 		# extract the filename as it passes
 		if ($line =~ /^diff --git.*?(\S+)$/) {

When sdhci is runtime_suspended, it can still receive a wake-up because of card insertion.
The wake-up will then be signaled by an interrupt which cannot be serviced immediatly, as the device is powered off, and register are not accessibles.

We cannot call pm_runtime_get_sync() in irq context as on some architecture (e.g. intel_mid), runtime_resume can lead to too much latency, due to e.g. PLL warm-up and such aggressive power gating.

We temporarly disable the interrupt, and trigger a runtime_resume of the device The interrupt will be re-enabled when resume is finished.

Signed-off-by Pierre Tardy <[email protected]>

---
 drivers/mmc/host/sdhci-pci.c |    8 +-------
 drivers/mmc/host/sdhci.c     |   30 +++++++++++++++++++++++++++++-
 include/linux/mmc/sdhci.h    |    2 ++
 scripts/checkpatch.pl        |    6 +++---
 4 files changed, 35 insertions(+), 11 deletions(-)

drivers/mmc/host/sdhci.c +0 -0

index 735c3f7..85a1956 100644
@@ -1599,6 +1599,21 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 	u32 intmask;
 	int cardint = 0;
 

drivers/mmc/host/sdhci.c +16 -1

+		return IRQ_NONE;
+	}
 	spin_lock(&host->lock);
 
 	intmask = sdhci_readl(host, SDHCI_INT_STATUS);

@@ -1747,7 +1762,12 @@ EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
 
 int sdhci_runtime_suspend(struct sdhci_host *host)
 {
-	/* nothing to do yet */
+	u8 val;
+	/* make sure we wake up on sdcard insert/remove enabled */
+	val = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE;
+	if (host->mmc->pm_flags & MMC_PM_WAKE_SDIO_IRQ)
+		val |= SDHCI_WAKE_ON_INT;
+	sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
 	return 0;
 }
 

@@ -1768,6 +1788,14 @@ int sdhci_runtime_resume(struct sdhci_host *host)
 	sdhci_set_ios(host->mmc, &host->mmc->ios);
 	mmiowb();
 
+	if (host->waking_up) {
+		host->waking_up = 0;
+		/* Re-enable the irq now we are perfectly resumed.
+		 * Any yet unhandled interrupt (i.e. the wake) will retrigger
+		 * irq
+		 */
+		irq_enable(host->irq);
+	}
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sdhci_runtime_resume);

scripts/checkpatch.pl +2 -2

index e3c7fc0..17aeda3 100755
@@ -1325,8 +1325,8 @@ sub process {
 		$prefix = "$filename:$realline: " if ($emacs && $file);
 		$prefix = "$filename:$linenr: " if ($emacs && !$file);
 
-		$here = "#$linenr: " if (!$file);
-		$here = "#$realline: " if ($file);
+		$here = "" if (!$file);
+		$here = "" if ($file);
 
 		# extract the filename as it passes
 		if ($line =~ /^diff --git.*?(\S+)$/) {

Quoted 9 lines

+     if (pm_runtime_suspended()) {
+             host->waking_up = 1;
+             /* Note that we disable temporarly the interrupt until we do the
+              * resume. If we don't then we'll get constantly interrupted
+              * until we actually resume.
+              *
+              * as the irq is shared, this might not be very friendly to our
+              * irq sharers but the pm_runtime workqueue should really be
+              * called soon.

Instead of disabling the IRQ, would it be possible to tell the device to stop generating an interrupt request?

Quoted 3 lines

+              */
+             disable_irq_nosync(irq);
+             pm_runtime_get(host->mmc->parent);

Does this pm_runtime_get() have a corresponding pm_runtime_put()?  I didn't notice one anywhere.

Quoted 1 line

Surely this doesn't belong in the patch.

Quoted 9 lines

+     if (pm_runtime_suspended()) {
+             host->waking_up = 1;
+             /* Note that we disable temporarly the interrupt until we do the
+              * resume. If we don't then we'll get constantly interrupted
+              * until we actually resume.
+              *
+              * as the irq is shared, this might not be very friendly to our
+              * irq sharers but the pm_runtime workqueue should really be
+              * called soon.

Instead of disabling the IRQ, would it be possible to tell the device to stop generating an interrupt request?

Quoted 3 lines

+              */
+             disable_irq_nosync(irq);
+             pm_runtime_get(host->mmc->parent);

Does this pm_runtime_get() have a corresponding pm_runtime_put()?  I didn't notice one anywhere.

Quoted 1 line

Surely this doesn't belong in the patch.

Quoted 10 lines

Quoted 9 lines

+     if (pm_runtime_suspended()) {
+             host->waking_up = 1;
+             /* Note that we disable temporarly the interrupt until we do the
+              * resume. If we don't then we'll get constantly interrupted
+              * until we actually resume.
+              *
+              * as the irq is shared, this might not be very friendly to our
+              * irq sharers but the pm_runtime workqueue should really be
+              * called soon.

Quoted 2 lines

Instead of disabling the IRQ, would it be possible to tell the device to stop generating an interrupt request?

Well, the device is power gated, so any access to it will cause a bus timeout (depending of the platform)

Quoted 10 lines · Pierre Tardy

Quoted 10 lines

Quoted 9 lines

+     if (pm_runtime_suspended()) {
+             host->waking_up = 1;
+             /* Note that we disable temporarly the interrupt until we do the
+              * resume. If we don't then we'll get constantly interrupted
+              * until we actually resume.
+              *
+              * as the irq is shared, this might not be very friendly to our
+              * irq sharers but the pm_runtime workqueue should really be
+              * called soon.

Quoted 5 lines

Quoted 2 lines

Instead of disabling the IRQ, would it be possible to tell the device to stop generating an interrupt request?

Well, the device is power gated, so any access to it will cause a bus timeout (depending of the platform)

But could you gate the power back on and then turn off the interrupt request?

It seems like a foolish design to have an interrupt request that can't be turned off by an interrupt handler.

Quoted 10 lines · Pierre Tardy

Quoted 10 lines

Quoted 9 lines

+     if (pm_runtime_suspended()) {
+             host->waking_up = 1;
+             /* Note that we disable temporarly the interrupt until we do the
+              * resume. If we don't then we'll get constantly interrupted
+              * until we actually resume.
+              *
+              * as the irq is shared, this might not be very friendly to our
+              * irq sharers but the pm_runtime workqueue should really be
+              * called soon.

Quoted 5 lines

Quoted 2 lines

Instead of disabling the IRQ, would it be possible to tell the device to stop generating an interrupt request?

Well, the device is power gated, so any access to it will cause a bus timeout (depending of the platform)

But could you gate the power back on and then turn off the interrupt request?

It seems like a foolish design to have an interrupt request that can't be turned off by an interrupt handler.

Quoted 10 lines

Quoted 9 lines

+     if (pm_runtime_suspended()) {
+             host->waking_up = 1;
+             /* Note that we disable temporarly the interrupt until we do the
+              * resume. If we don't then we'll get constantly interrupted
+              * until we actually resume.
+              *
+              * as the irq is shared, this might not be very friendly to our
+              * irq sharers but the pm_runtime workqueue should really be
+              * called soon.

Quoted 2 lines

Instead of disabling the IRQ, would it be possible to tell the device to stop generating an interrupt request?

Well, the device is power gated, so any access to it will cause a bus timeout (depending of the platform)

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow
are not duplicate from what the core is doing.

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

I'm not sure if I'm doing it the good way.
We have the exact same issue for our not yet upstreamable usb-otg driver. Not sure if this cannot be implemented generically in core.

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow
are not duplicate from what the core is doing.

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

I'm not sure if I'm doing it the good way.
We have the exact same issue for our not yet upstreamable usb-otg driver. Not sure if this cannot be implemented generically in core.

Quoted 10 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

Quoted 2 lines

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Quoted 10 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

Quoted 2 lines

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Quoted 20 lines · Pierre Tardy

Quoted 19 lines · Alan Stern

Quoted 5 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

Quoted 7 lines

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Quoted 4 lines

Quoted 1 line

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

It depends on the platform.  On systems with ACPI, PCI PME generates an ACPI I/O event, which is handled by the ACPI and PM cores.  It does not invoke the device driver's interrupt handler.

Quoted 20 lines · Pierre Tardy

Quoted 19 lines · Alan Stern

Quoted 5 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

Quoted 7 lines

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Quoted 4 lines

Quoted 1 line

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

It depends on the platform.  On systems with ACPI, PCI PME generates an ACPI I/O event, which is handled by the ACPI and PM cores.  It does not invoke the device driver's interrupt handler.

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

It depends on the platform.  On systems with ACPI, PCI PME generates an ACPI I/O event, which is handled by the ACPI and PM cores.  It does not invoke the device driver's interrupt handler.

So, let's say, in the ACPI case, if the interrupt handler dont get called, how would the driver know that he got a sdcard insert event, and trigger a mmc_rescan() ?

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

It depends on the platform.  On systems with ACPI, PCI PME generates an ACPI I/O event, which is handled by the ACPI and PM cores.  It does not invoke the device driver's interrupt handler.

So, let's say, in the ACPI case, if the interrupt handler dont get called, how would the driver know that he got a sdcard insert event, and trigger a mmc_rescan() ?

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow
are not duplicate from what the core is doing.

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow
are not duplicate from what the core is doing.

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Quoted 16 lines · Pierre Tardy

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

Quoted 8 lines

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Your driver won't get those interrupts.

How it works is, basically, that when the device signals wakeup, it either causes a PME# signal to be raised (parallel PCI), or a PME Message to be sent upstream (PCIe).  In the first case it will cause a platform event (eg. ACPI GPE) to occur and the handle of that event will resume your device (using pm_request_resume()).  In the second case it will cause the PCIe root port handling the PME Message to generate an interrupt and the handler of that interrupt will resume your device.

In fact, there are a few drivers in the tree using this mechanism already (r8169, e1000e, i believe PCI HCDs too).

Quoted 16 lines · Pierre Tardy

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow are not duplicate from what the core is doing.

Quoted 8 lines

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Your driver won't get those interrupts.

How it works is, basically, that when the device signals wakeup, it either causes a PME# signal to be raised (parallel PCI), or a PME Message to be sent upstream (PCIe).  In the first case it will cause a platform event (eg. ACPI GPE) to occur and the handle of that event will resume your device (using pm_request_resume()).  In the second case it will cause the PCIe root port handling the PME Message to generate an interrupt and the handler of that interrupt will resume your device.

In fact, there are a few drivers in the tree using this mechanism already (r8169, e1000e, i believe PCI HCDs too).

Quoted 28 lines · Rafael J. Wysocki

Quoted 16 lines · Pierre Tardy

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow
are not duplicate from what the core is doing.

Quoted 8 lines

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Your driver won't get those interrupts.

Quoted 7 lines

How it works is, basically, that when the device signals wakeup, it either causes a PME# signal to be raised (parallel PCI), or a PME Message to be sent upstream (PCIe). In the first case it will cause a platform event (eg. ACPI GPE) to occur and the handle of that event will resume your device (using pm_request_resume()). In the second case it will cause the PCIe root port handling the PME Message to generate an interrupt and the handler of that interrupt will resume your device.

Thanks, that explain a lot how it works.
What I still dont understand is that the wake source I'll have (e.g. sd card insert) is still an interrupt source.
So it is supposed to generate interrupt until the interrupt source is acknowledged.
Are we supposed to mask that interrupt source while suspended to make sure we have either wake or interrupt?

Quoted 28 lines · Rafael J. Wysocki

Quoted 16 lines · Pierre Tardy

Quoted 15 lines · Alan Stern

Quoted 12 lines · Pierre Tardy

Please find sdhci runtime_pm implementation.

It uses clock gating fw as a tip to know when our chip is idle. It implements wake up from card insertion/removal.

This is RFC, please dont merge yet. I really would like to have deep review from PCI linux-pm guys.

Opens are:

1/ Not sure if the pci configs in the driver in rpm_suspend/resume flow
are not duplicate from what the core is doing.

Quoted 8 lines

Quoted 5 lines

There may be one or two small errors.

Quoted 1 line

2/ Wakeup from D3hot: I cannot find any driver that is implementing it in current upstream,

Other drivers do it, but they use PCI PME# instead of interrupts.

Could you please elaborate?
My understanding is that PCI PME will generate MSI, which translate in interrupt.

Your driver won't get those interrupts.

Quoted 7 lines

How it works is, basically, that when the device signals wakeup, it either causes a PME# signal to be raised (parallel PCI), or a PME Message to be sent upstream (PCIe). In the first case it will cause a platform event (eg. ACPI GPE) to occur and the handle of that event will resume your device (using pm_request_resume()). In the second case it will cause the PCIe root port handling the PME Message to generate an interrupt and the handler of that interrupt will resume your device.

Thanks, that explain a lot how it works.
What I still dont understand is that the wake source I'll have (e.g. sd card insert) is still an interrupt source.
So it is supposed to generate interrupt until the interrupt source is acknowledged.
Are we supposed to mask that interrupt source while suspended to make sure we have either wake or interrupt?

Quoted 7 lines

Quoted 7 lines

How it works is, basically, that when the device signals wakeup, it either causes a PME# signal to be raised (parallel PCI), or a PME Message to be sent upstream (PCIe). In the first case it will cause a platform event (eg. ACPI GPE) to occur and the handle of that event will resume your device (using pm_request_resume()). In the second case it will cause the PCIe root port handling the PME Message to generate an interrupt and the handler of that interrupt will resume your device.

Quoted 7 lines

Thanks, that explain a lot how it works.
What I still dont understand is that the wake source I'll have (e.g. sd card insert) is still an interrupt source.
So it is supposed to generate interrupt until the interrupt source is acknowledged.
Are we supposed to mask that interrupt source while suspended to make sure we have either wake or interrupt?

I think the interrupt should be masked while suspended and the driver's resume routine should unmask it.

Quoted 7 lines

Quoted 7 lines

How it works is, basically, that when the device signals wakeup, it either causes a PME# signal to be raised (parallel PCI), or a PME Message to be sent upstream (PCIe). In the first case it will cause a platform event (eg. ACPI GPE) to occur and the handle of that event will resume your device (using pm_request_resume()). In the second case it will cause the PCIe root port handling the PME Message to generate an interrupt and the handler of that interrupt will resume your device.

Quoted 7 lines

Thanks, that explain a lot how it works.
What I still dont understand is that the wake source I'll have (e.g. sd card insert) is still an interrupt source.
So it is supposed to generate interrupt until the interrupt source is acknowledged.
Are we supposed to mask that interrupt source while suspended to make sure we have either wake or interrupt?

I think the interrupt should be masked while suspended and the driver's resume routine should unmask it.