* [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called
@ 2021-10-25 7:01 Chunfeng Yun
2021-10-25 7:01 ` [PATCH v4 2/3] usb: xhci-mtk: enable wake-up interrupt " Chunfeng Yun
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Chunfeng Yun @ 2021-10-25 7:01 UTC (permalink / raw)
To: Rafael J . Wysocki
Cc: Len Brown, Pavel Machek, Greg Kroah-Hartman, Mathias Nyman,
Chunfeng Yun, Matthias Brugger, linux-pm, linux-kernel,
linux-usb, linux-arm-kernel, linux-mediatek, Rafael J . Wysocki
When the dedicated wake IRQ is level trigger, and it uses the
device's low-power status as the wakeup source, that means if the
device is not in low-power state, the wake IRQ will be triggered
if enabled; For this case, need enable the wake IRQ after running
the device's ->runtime_suspend() which make it enter low-power state.
e.g.
Assume the wake IRQ is a low level trigger type, and the wakeup
signal comes from the low-power status of the device.
The wakeup signal is low level at running time (0), and becomes
high level when the device enters low-power state (runtime_suspend
(1) is called), a wakeup event at (2) make the device exit low-power
state, then the wakeup signal also becomes low level.
------------------
| ^ ^|
---------------- | | --------------
|<---(0)--->|<--(1)--| (3) (2) (4)
if enable the wake IRQ before running runtime_suspend during (0),
a wake IRQ will arise, it causes resume immediately;
it works if enable wake IRQ ( e.g. at (3) or (4)) after running
->runtime_suspend().
This patch introduces a new status WAKE_IRQ_DEDICATED_REVERSE to
optionally support enabling wake IRQ after running ->runtime_suspend().
Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v4: changes according to Rafael's suggestions
1. rename new flag as WAKE_IRQ_DEDICATED_REVERSE;
2. add __dev_pm_set_dedicated_wake_irq() with flag parameter, then
rebuild dev_pm_set_dedicated_wake_irq() and add new api
dev_pm_set_dedicated_wake_irq_reverse();
3. rename the new added parameter as cond_disable in
dev_pm_disable_wake_irq_check(), and also simplify its flow;
4. modify some comments
v3: add new status suggested by Rafael
v2: add more commit message
Use the falling edge trigger interrupt suggested by Ikjoon [1], it
works well at firstly when only use this related wakeup source, but
encounter issues if use other wakeup sources to wakeup platform as
below steps:
1. use another wakeup source to wake up the suspended system;
2. the consumer's resume() will be called, and exits sleep state;
3. the consumer's wakeup signal will fall into low level, due to
currently the wakeup irq is disabled, the wake-irq is pending;
4. the consumer tries to enter runtime suspend, but there is a
pending wakeup irq, so will resume again, this will repeat
endlessly.
Send out the patch again for further discussion.
[1]: https://patchwork.kernel.org/patch/12190407
---
drivers/base/power/power.h | 7 ++-
drivers/base/power/runtime.c | 6 ++-
drivers/base/power/wakeirq.c | 101 +++++++++++++++++++++++++++--------
include/linux/pm_wakeirq.h | 9 +++-
4 files changed, 96 insertions(+), 27 deletions(-)
diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index 54292cdd7808..0eb7f02b3ad5 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -25,8 +25,10 @@ extern u64 pm_runtime_active_time(struct device *dev);
#define WAKE_IRQ_DEDICATED_ALLOCATED BIT(0)
#define WAKE_IRQ_DEDICATED_MANAGED BIT(1)
+#define WAKE_IRQ_DEDICATED_REVERSE BIT(2)
#define WAKE_IRQ_DEDICATED_MASK (WAKE_IRQ_DEDICATED_ALLOCATED | \
- WAKE_IRQ_DEDICATED_MANAGED)
+ WAKE_IRQ_DEDICATED_MANAGED | \
+ WAKE_IRQ_DEDICATED_REVERSE)
struct wake_irq {
struct device *dev;
@@ -39,7 +41,8 @@ extern void dev_pm_arm_wake_irq(struct wake_irq *wirq);
extern void dev_pm_disarm_wake_irq(struct wake_irq *wirq);
extern void dev_pm_enable_wake_irq_check(struct device *dev,
bool can_change_status);
-extern void dev_pm_disable_wake_irq_check(struct device *dev);
+extern void dev_pm_disable_wake_irq_check(struct device *dev, bool cond_disable);
+extern void dev_pm_enable_wake_irq_complete(struct device *dev);
#ifdef CONFIG_PM_SLEEP
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index ec94049442b9..d504cd4ab3cb 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -645,6 +645,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
if (retval)
goto fail;
+ dev_pm_enable_wake_irq_complete(dev);
+
no_callback:
__update_runtime_status(dev, RPM_SUSPENDED);
pm_runtime_deactivate_timer(dev);
@@ -690,7 +692,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
return retval;
fail:
- dev_pm_disable_wake_irq_check(dev);
+ dev_pm_disable_wake_irq_check(dev, true);
__update_runtime_status(dev, RPM_ACTIVE);
dev->power.deferred_resume = false;
wake_up_all(&dev->power.wait_queue);
@@ -873,7 +875,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
callback = RPM_GET_CALLBACK(dev, runtime_resume);
- dev_pm_disable_wake_irq_check(dev);
+ dev_pm_disable_wake_irq_check(dev, false);
retval = rpm_callback(callback, dev);
if (retval) {
__update_runtime_status(dev, RPM_SUSPENDED);
diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
index b91a3a9bf9f6..76c163a89104 100644
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -142,24 +142,7 @@ static irqreturn_t handle_threaded_wake_irq(int irq, void *_wirq)
return IRQ_HANDLED;
}
-/**
- * dev_pm_set_dedicated_wake_irq - Request a dedicated wake-up interrupt
- * @dev: Device entry
- * @irq: Device wake-up interrupt
- *
- * Unless your hardware has separate wake-up interrupts in addition
- * to the device IO interrupts, you don't need this.
- *
- * Sets up a threaded interrupt handler for a device that has
- * a dedicated wake-up interrupt in addition to the device IO
- * interrupt.
- *
- * The interrupt starts disabled, and needs to be managed for
- * the device by the bus code or the device driver using
- * dev_pm_enable_wake_irq() and dev_pm_disable_wake_irq()
- * functions.
- */
-int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
+static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned int flag)
{
struct wake_irq *wirq;
int err;
@@ -197,7 +180,7 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
if (err)
goto err_free_irq;
- wirq->status = WAKE_IRQ_DEDICATED_ALLOCATED;
+ wirq->status = WAKE_IRQ_DEDICATED_ALLOCATED | flag;
return err;
@@ -210,8 +193,57 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
return err;
}
+
+
+/**
+ * dev_pm_set_dedicated_wake_irq - Request a dedicated wake-up interrupt
+ * @dev: Device entry
+ * @irq: Device wake-up interrupt
+ *
+ * Unless your hardware has separate wake-up interrupts in addition
+ * to the device IO interrupts, you don't need this.
+ *
+ * Sets up a threaded interrupt handler for a device that has
+ * a dedicated wake-up interrupt in addition to the device IO
+ * interrupt.
+ *
+ * The interrupt starts disabled, and needs to be managed for
+ * the device by the bus code or the device driver using
+ * dev_pm_enable_wake_irq*() and dev_pm_disable_wake_irq*()
+ * functions.
+ */
+int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
+{
+ return __dev_pm_set_dedicated_wake_irq(dev, irq, 0);
+}
EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq);
+/**
+ * dev_pm_set_dedicated_wake_irq_reverse - Request a dedicated wake-up interrupt
+ * with setting flag WAKE_IRQ_DEDICATED_REVERSE
+ * @dev: Device entry
+ * @irq: Device wake-up interrupt
+ *
+ * Unless your hardware has separate wake-up interrupts in addition
+ * to the device IO interrupts, you don't need this.
+ *
+ * Sets up a threaded interrupt handler for a device that has a dedicated
+ * wake-up interrupt in addition to the device IO interrupt. It sets
+ * the status of WAKE_IRQ_DEDICATED_REVERSE to tell rpm_suspend()
+ * to enable dedicated wake-up interrupt after running the
+ * ->runtime_suspend()
+ *
+ * The interrupt starts disabled, and needs to be managed for
+ * the device by the bus code or the device driver using
+ * dev_pm_enable_wake_irq*() and dev_pm_disable_wake_irq*()
+ * functions.
+ */
+int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq)
+{
+ return __dev_pm_set_dedicated_wake_irq(dev, irq, WAKE_IRQ_DEDICATED_REVERSE);
+}
+EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq_reverse);
+
/**
* dev_pm_enable_wake_irq - Enable device wake-up interrupt
* @dev: Device
@@ -282,27 +314,54 @@ void dev_pm_enable_wake_irq_check(struct device *dev,
return;
enable:
- enable_irq(wirq->irq);
+ if (!can_change_status || !(wirq->status & WAKE_IRQ_DEDICATED_REVERSE))
+ enable_irq(wirq->irq);
}
/**
* dev_pm_disable_wake_irq_check - Checks and disables wake-up interrupt
* @dev: Device
+ * @cond_disable: if set, also check WAKE_IRQ_DEDICATED_REVERSE
*
* Disables wake-up interrupt conditionally based on status.
* Should be only called from rpm_suspend() and rpm_resume() path.
*/
-void dev_pm_disable_wake_irq_check(struct device *dev)
+void dev_pm_disable_wake_irq_check(struct device *dev, bool cond_disable)
{
struct wake_irq *wirq = dev->power.wakeirq;
if (!wirq || !(wirq->status & WAKE_IRQ_DEDICATED_MASK))
return;
+ if (cond_disable && (wirq->status & WAKE_IRQ_DEDICATED_REVERSE))
+ return;
+
if (wirq->status & WAKE_IRQ_DEDICATED_MANAGED)
disable_irq_nosync(wirq->irq);
}
+/**
+ * dev_pm_enable_wake_irq_complete - enable wake IRQ not enabled before
+ * @dev: Device using the wake IRQ
+ *
+ * Enable wake IRQ conditionally based on status, mainly used if want to
+ * enable wake IRQ after running ->runtime_suspend() which depends on
+ * WAKE_IRQ_DEDICATED_REVERSE.
+ *
+ * Should be only called from rpm_suspend() path.
+ */
+void dev_pm_enable_wake_irq_complete(struct device *dev)
+{
+ struct wake_irq *wirq = dev->power.wakeirq;
+
+ if (!wirq || !(wirq->status & WAKE_IRQ_DEDICATED_MASK))
+ return;
+
+ if (wirq->status & WAKE_IRQ_DEDICATED_MANAGED &&
+ wirq->status & WAKE_IRQ_DEDICATED_REVERSE)
+ enable_irq(wirq->irq);
+}
+
/**
* dev_pm_arm_wake_irq - Arm device wake-up
* @wirq: Device wake-up interrupt
diff --git a/include/linux/pm_wakeirq.h b/include/linux/pm_wakeirq.h
index cd5b62db9084..e63a63aa47a3 100644
--- a/include/linux/pm_wakeirq.h
+++ b/include/linux/pm_wakeirq.h
@@ -17,8 +17,8 @@
#ifdef CONFIG_PM
extern int dev_pm_set_wake_irq(struct device *dev, int irq);
-extern int dev_pm_set_dedicated_wake_irq(struct device *dev,
- int irq);
+extern int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq);
+extern int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq);
extern void dev_pm_clear_wake_irq(struct device *dev);
extern void dev_pm_enable_wake_irq(struct device *dev);
extern void dev_pm_disable_wake_irq(struct device *dev);
@@ -35,6 +35,11 @@ static inline int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
return 0;
}
+static inline int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq)
+{
+ return 0;
+}
+
static inline void dev_pm_clear_wake_irq(struct device *dev)
{
}
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/3] usb: xhci-mtk: enable wake-up interrupt after runtime_suspend called
2021-10-25 7:01 [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called Chunfeng Yun
@ 2021-10-25 7:01 ` Chunfeng Yun
2021-10-25 7:01 ` [PATCH v4 3/3] usb: mtu3: " Chunfeng Yun
2021-10-26 15:39 ` [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq " Rafael J. Wysocki
2 siblings, 0 replies; 7+ messages in thread
From: Chunfeng Yun @ 2021-10-25 7:01 UTC (permalink / raw)
To: Rafael J . Wysocki
Cc: Len Brown, Pavel Machek, Greg Kroah-Hartman, Mathias Nyman,
Chunfeng Yun, Matthias Brugger, linux-pm, linux-kernel,
linux-usb, linux-arm-kernel, linux-mediatek
Use new function dev_pm_set_dedicated_wake_irq_reverse() to request
dedicated wake-up interrupt, due to we want to enable the wake IRQ
after running ->runtime_suspend().
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v4: use the new api dev_pm_set_dedicated_wake_irq_reverse()
v3: new patch
---
drivers/usb/host/xhci-mtk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index c53f6f276d5c..58a0eae4f41b 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -602,7 +602,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
goto dealloc_usb2_hcd;
if (wakeup_irq > 0) {
- ret = dev_pm_set_dedicated_wake_irq(dev, wakeup_irq);
+ ret = dev_pm_set_dedicated_wake_irq_reverse(dev, wakeup_irq);
if (ret) {
dev_err(dev, "set wakeup irq %d failed\n", wakeup_irq);
goto dealloc_usb3_hcd;
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 3/3] usb: mtu3: enable wake-up interrupt after runtime_suspend called
2021-10-25 7:01 [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called Chunfeng Yun
2021-10-25 7:01 ` [PATCH v4 2/3] usb: xhci-mtk: enable wake-up interrupt " Chunfeng Yun
@ 2021-10-25 7:01 ` Chunfeng Yun
2021-10-26 15:39 ` [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq " Rafael J. Wysocki
2 siblings, 0 replies; 7+ messages in thread
From: Chunfeng Yun @ 2021-10-25 7:01 UTC (permalink / raw)
To: Rafael J . Wysocki
Cc: Len Brown, Pavel Machek, Greg Kroah-Hartman, Mathias Nyman,
Chunfeng Yun, Matthias Brugger, linux-pm, linux-kernel,
linux-usb, linux-arm-kernel, linux-mediatek
Use the new API dev_pm_set_dedicated_wake_irq_reverse() to request
dedicated wake-up interrupt, due to we want to enable the wake IRQ
after running ->runtime_suspend().
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v4: new patch
---
drivers/usb/mtu3/mtu3_plat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
index f13531022f4a..4309ed939178 100644
--- a/drivers/usb/mtu3/mtu3_plat.c
+++ b/drivers/usb/mtu3/mtu3_plat.c
@@ -337,7 +337,7 @@ static int mtu3_probe(struct platform_device *pdev)
goto comm_init_err;
if (ssusb->wakeup_irq > 0) {
- ret = dev_pm_set_dedicated_wake_irq(dev, ssusb->wakeup_irq);
+ ret = dev_pm_set_dedicated_wake_irq_reverse(dev, ssusb->wakeup_irq);
if (ret) {
dev_err(dev, "failed to set wakeup irq %d\n", ssusb->wakeup_irq);
goto comm_exit;
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called
2021-10-25 7:01 [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called Chunfeng Yun
2021-10-25 7:01 ` [PATCH v4 2/3] usb: xhci-mtk: enable wake-up interrupt " Chunfeng Yun
2021-10-25 7:01 ` [PATCH v4 3/3] usb: mtu3: " Chunfeng Yun
@ 2021-10-26 15:39 ` Rafael J. Wysocki
2021-10-27 9:29 ` Chunfeng Yun
2021-10-27 9:33 ` Chunfeng Yun
2 siblings, 2 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-10-26 15:39 UTC (permalink / raw)
To: Chunfeng Yun
Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
Mathias Nyman, Matthias Brugger, Linux PM,
Linux Kernel Mailing List,
open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:,
Linux ARM, moderated list:ARM/Mediatek SoC...,
Rafael J . Wysocki
On Mon, Oct 25, 2021 at 9:02 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> When the dedicated wake IRQ is level trigger, and it uses the
> device's low-power status as the wakeup source, that means if the
> device is not in low-power state, the wake IRQ will be triggered
> if enabled; For this case, need enable the wake IRQ after running
> the device's ->runtime_suspend() which make it enter low-power state.
>
> e.g.
> Assume the wake IRQ is a low level trigger type, and the wakeup
> signal comes from the low-power status of the device.
> The wakeup signal is low level at running time (0), and becomes
> high level when the device enters low-power state (runtime_suspend
> (1) is called), a wakeup event at (2) make the device exit low-power
> state, then the wakeup signal also becomes low level.
>
> ------------------
> | ^ ^|
> ---------------- | | --------------
> |<---(0)--->|<--(1)--| (3) (2) (4)
>
> if enable the wake IRQ before running runtime_suspend during (0),
> a wake IRQ will arise, it causes resume immediately;
> it works if enable wake IRQ ( e.g. at (3) or (4)) after running
> ->runtime_suspend().
>
> This patch introduces a new status WAKE_IRQ_DEDICATED_REVERSE to
> optionally support enabling wake IRQ after running ->runtime_suspend().
>
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
I don't really have anything to add regarding the code.
The kerneldoc comments could be improved, but I can take care of this
when applying the patch.
Please collect ACKs for the remaining 2 patches in the series and I
will pick up all three.
Thanks!
> ---
> v4: changes according to Rafael's suggestions
> 1. rename new flag as WAKE_IRQ_DEDICATED_REVERSE;
> 2. add __dev_pm_set_dedicated_wake_irq() with flag parameter, then
> rebuild dev_pm_set_dedicated_wake_irq() and add new api
> dev_pm_set_dedicated_wake_irq_reverse();
> 3. rename the new added parameter as cond_disable in
> dev_pm_disable_wake_irq_check(), and also simplify its flow;
> 4. modify some comments
>
> v3: add new status suggested by Rafael
>
> v2: add more commit message
>
> Use the falling edge trigger interrupt suggested by Ikjoon [1], it
> works well at firstly when only use this related wakeup source, but
> encounter issues if use other wakeup sources to wakeup platform as
> below steps:
> 1. use another wakeup source to wake up the suspended system;
> 2. the consumer's resume() will be called, and exits sleep state;
> 3. the consumer's wakeup signal will fall into low level, due to
> currently the wakeup irq is disabled, the wake-irq is pending;
> 4. the consumer tries to enter runtime suspend, but there is a
> pending wakeup irq, so will resume again, this will repeat
> endlessly.
>
> Send out the patch again for further discussion.
>
> [1]: https://patchwork.kernel.org/patch/12190407
>
> ---
> drivers/base/power/power.h | 7 ++-
> drivers/base/power/runtime.c | 6 ++-
> drivers/base/power/wakeirq.c | 101 +++++++++++++++++++++++++++--------
> include/linux/pm_wakeirq.h | 9 +++-
> 4 files changed, 96 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 54292cdd7808..0eb7f02b3ad5 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -25,8 +25,10 @@ extern u64 pm_runtime_active_time(struct device *dev);
>
> #define WAKE_IRQ_DEDICATED_ALLOCATED BIT(0)
> #define WAKE_IRQ_DEDICATED_MANAGED BIT(1)
> +#define WAKE_IRQ_DEDICATED_REVERSE BIT(2)
> #define WAKE_IRQ_DEDICATED_MASK (WAKE_IRQ_DEDICATED_ALLOCATED | \
> - WAKE_IRQ_DEDICATED_MANAGED)
> + WAKE_IRQ_DEDICATED_MANAGED | \
> + WAKE_IRQ_DEDICATED_REVERSE)
>
> struct wake_irq {
> struct device *dev;
> @@ -39,7 +41,8 @@ extern void dev_pm_arm_wake_irq(struct wake_irq *wirq);
> extern void dev_pm_disarm_wake_irq(struct wake_irq *wirq);
> extern void dev_pm_enable_wake_irq_check(struct device *dev,
> bool can_change_status);
> -extern void dev_pm_disable_wake_irq_check(struct device *dev);
> +extern void dev_pm_disable_wake_irq_check(struct device *dev, bool cond_disable);
> +extern void dev_pm_enable_wake_irq_complete(struct device *dev);
>
> #ifdef CONFIG_PM_SLEEP
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index ec94049442b9..d504cd4ab3cb 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -645,6 +645,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> if (retval)
> goto fail;
>
> + dev_pm_enable_wake_irq_complete(dev);
> +
> no_callback:
> __update_runtime_status(dev, RPM_SUSPENDED);
> pm_runtime_deactivate_timer(dev);
> @@ -690,7 +692,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> return retval;
>
> fail:
> - dev_pm_disable_wake_irq_check(dev);
> + dev_pm_disable_wake_irq_check(dev, true);
> __update_runtime_status(dev, RPM_ACTIVE);
> dev->power.deferred_resume = false;
> wake_up_all(&dev->power.wait_queue);
> @@ -873,7 +875,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
>
> callback = RPM_GET_CALLBACK(dev, runtime_resume);
>
> - dev_pm_disable_wake_irq_check(dev);
> + dev_pm_disable_wake_irq_check(dev, false);
> retval = rpm_callback(callback, dev);
> if (retval) {
> __update_runtime_status(dev, RPM_SUSPENDED);
> diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
> index b91a3a9bf9f6..76c163a89104 100644
> --- a/drivers/base/power/wakeirq.c
> +++ b/drivers/base/power/wakeirq.c
> @@ -142,24 +142,7 @@ static irqreturn_t handle_threaded_wake_irq(int irq, void *_wirq)
> return IRQ_HANDLED;
> }
>
> -/**
> - * dev_pm_set_dedicated_wake_irq - Request a dedicated wake-up interrupt
> - * @dev: Device entry
> - * @irq: Device wake-up interrupt
> - *
> - * Unless your hardware has separate wake-up interrupts in addition
> - * to the device IO interrupts, you don't need this.
> - *
> - * Sets up a threaded interrupt handler for a device that has
> - * a dedicated wake-up interrupt in addition to the device IO
> - * interrupt.
> - *
> - * The interrupt starts disabled, and needs to be managed for
> - * the device by the bus code or the device driver using
> - * dev_pm_enable_wake_irq() and dev_pm_disable_wake_irq()
> - * functions.
> - */
> -int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
> +static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned int flag)
> {
> struct wake_irq *wirq;
> int err;
> @@ -197,7 +180,7 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
> if (err)
> goto err_free_irq;
>
> - wirq->status = WAKE_IRQ_DEDICATED_ALLOCATED;
> + wirq->status = WAKE_IRQ_DEDICATED_ALLOCATED | flag;
>
> return err;
>
> @@ -210,8 +193,57 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
>
> return err;
> }
> +
> +
> +/**
> + * dev_pm_set_dedicated_wake_irq - Request a dedicated wake-up interrupt
> + * @dev: Device entry
> + * @irq: Device wake-up interrupt
> + *
> + * Unless your hardware has separate wake-up interrupts in addition
> + * to the device IO interrupts, you don't need this.
> + *
> + * Sets up a threaded interrupt handler for a device that has
> + * a dedicated wake-up interrupt in addition to the device IO
> + * interrupt.
> + *
> + * The interrupt starts disabled, and needs to be managed for
> + * the device by the bus code or the device driver using
> + * dev_pm_enable_wake_irq*() and dev_pm_disable_wake_irq*()
> + * functions.
> + */
> +int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
> +{
> + return __dev_pm_set_dedicated_wake_irq(dev, irq, 0);
> +}
> EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq);
>
> +/**
> + * dev_pm_set_dedicated_wake_irq_reverse - Request a dedicated wake-up interrupt
> + * with setting flag WAKE_IRQ_DEDICATED_REVERSE
> + * @dev: Device entry
> + * @irq: Device wake-up interrupt
> + *
> + * Unless your hardware has separate wake-up interrupts in addition
> + * to the device IO interrupts, you don't need this.
> + *
> + * Sets up a threaded interrupt handler for a device that has a dedicated
> + * wake-up interrupt in addition to the device IO interrupt. It sets
> + * the status of WAKE_IRQ_DEDICATED_REVERSE to tell rpm_suspend()
> + * to enable dedicated wake-up interrupt after running the
> + * ->runtime_suspend()
> + *
> + * The interrupt starts disabled, and needs to be managed for
> + * the device by the bus code or the device driver using
> + * dev_pm_enable_wake_irq*() and dev_pm_disable_wake_irq*()
> + * functions.
> + */
> +int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq)
> +{
> + return __dev_pm_set_dedicated_wake_irq(dev, irq, WAKE_IRQ_DEDICATED_REVERSE);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq_reverse);
> +
> /**
> * dev_pm_enable_wake_irq - Enable device wake-up interrupt
> * @dev: Device
> @@ -282,27 +314,54 @@ void dev_pm_enable_wake_irq_check(struct device *dev,
> return;
>
> enable:
> - enable_irq(wirq->irq);
> + if (!can_change_status || !(wirq->status & WAKE_IRQ_DEDICATED_REVERSE))
> + enable_irq(wirq->irq);
> }
>
> /**
> * dev_pm_disable_wake_irq_check - Checks and disables wake-up interrupt
> * @dev: Device
> + * @cond_disable: if set, also check WAKE_IRQ_DEDICATED_REVERSE
> *
> * Disables wake-up interrupt conditionally based on status.
> * Should be only called from rpm_suspend() and rpm_resume() path.
> */
> -void dev_pm_disable_wake_irq_check(struct device *dev)
> +void dev_pm_disable_wake_irq_check(struct device *dev, bool cond_disable)
> {
> struct wake_irq *wirq = dev->power.wakeirq;
>
> if (!wirq || !(wirq->status & WAKE_IRQ_DEDICATED_MASK))
> return;
>
> + if (cond_disable && (wirq->status & WAKE_IRQ_DEDICATED_REVERSE))
> + return;
> +
> if (wirq->status & WAKE_IRQ_DEDICATED_MANAGED)
> disable_irq_nosync(wirq->irq);
> }
>
> +/**
> + * dev_pm_enable_wake_irq_complete - enable wake IRQ not enabled before
> + * @dev: Device using the wake IRQ
> + *
> + * Enable wake IRQ conditionally based on status, mainly used if want to
> + * enable wake IRQ after running ->runtime_suspend() which depends on
> + * WAKE_IRQ_DEDICATED_REVERSE.
> + *
> + * Should be only called from rpm_suspend() path.
> + */
> +void dev_pm_enable_wake_irq_complete(struct device *dev)
> +{
> + struct wake_irq *wirq = dev->power.wakeirq;
> +
> + if (!wirq || !(wirq->status & WAKE_IRQ_DEDICATED_MASK))
> + return;
> +
> + if (wirq->status & WAKE_IRQ_DEDICATED_MANAGED &&
> + wirq->status & WAKE_IRQ_DEDICATED_REVERSE)
> + enable_irq(wirq->irq);
> +}
> +
> /**
> * dev_pm_arm_wake_irq - Arm device wake-up
> * @wirq: Device wake-up interrupt
> diff --git a/include/linux/pm_wakeirq.h b/include/linux/pm_wakeirq.h
> index cd5b62db9084..e63a63aa47a3 100644
> --- a/include/linux/pm_wakeirq.h
> +++ b/include/linux/pm_wakeirq.h
> @@ -17,8 +17,8 @@
> #ifdef CONFIG_PM
>
> extern int dev_pm_set_wake_irq(struct device *dev, int irq);
> -extern int dev_pm_set_dedicated_wake_irq(struct device *dev,
> - int irq);
> +extern int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq);
> +extern int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq);
> extern void dev_pm_clear_wake_irq(struct device *dev);
> extern void dev_pm_enable_wake_irq(struct device *dev);
> extern void dev_pm_disable_wake_irq(struct device *dev);
> @@ -35,6 +35,11 @@ static inline int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
> return 0;
> }
>
> +static inline int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq)
> +{
> + return 0;
> +}
> +
> static inline void dev_pm_clear_wake_irq(struct device *dev)
> {
> }
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called
2021-10-26 15:39 ` [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq " Rafael J. Wysocki
@ 2021-10-27 9:29 ` Chunfeng Yun
2021-10-27 9:33 ` Chunfeng Yun
1 sibling, 0 replies; 7+ messages in thread
From: Chunfeng Yun @ 2021-10-27 9:29 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Len Brown, Pavel Machek, Greg Kroah-Hartman, Mathias Nyman,
Matthias Brugger, Linux PM, Linux Kernel Mailing List,
open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:,
Linux ARM, moderated list:ARM/Mediatek SoC...,
Rafael J . Wysocki
On Tue, 2021-10-26 at 17:39 +0200, Rafael J. Wysocki wrote:
> On Mon, Oct 25, 2021 at 9:02 AM Chunfeng Yun <
> chunfeng.yun@mediatek.com> wrote:
> >
> > When the dedicated wake IRQ is level trigger, and it uses the
> > device's low-power status as the wakeup source, that means if the
> > device is not in low-power state, the wake IRQ will be triggered
> > if enabled; For this case, need enable the wake IRQ after running
> > the device's ->runtime_suspend() which make it enter low-power
> > state.
> >
> > e.g.
> > Assume the wake IRQ is a low level trigger type, and the wakeup
> > signal comes from the low-power status of the device.
> > The wakeup signal is low level at running time (0), and becomes
> > high level when the device enters low-power state (runtime_suspend
> > (1) is called), a wakeup event at (2) make the device exit low-
> > power
> > state, then the wakeup signal also becomes low level.
> >
> > ------------------
> > | ^ ^|
> > ---------------- | | --------------
> > |<---(0)--->|<--(1)--| (3) (2) (4)
> >
> > if enable the wake IRQ before running runtime_suspend during (0),
> > a wake IRQ will arise, it causes resume immediately;
> > it works if enable wake IRQ ( e.g. at (3) or (4)) after running
> > ->runtime_suspend().
> >
> > This patch introduces a new status WAKE_IRQ_DEDICATED_REVERSE to
> > optionally support enabling wake IRQ after running
> > ->runtime_suspend().
> >
> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>
> I don't really have anything to add regarding the code.
>
> The kerneldoc comments could be improved, but I can take care of this
> when applying the patch.
Ok, thanks
>
> Please collect ACKs for the remaining 2 patches in the series and I
> will pick up all three.
The files (mtu3_plat.c, xhci-mtk.c) modified by other 2 patches are
maintained by myself, please pick up them, thanks a lot.
>
> Thanks!
>
> > ---
> > v4: changes according to Rafael's suggestions
> > 1. rename new flag as WAKE_IRQ_DEDICATED_REVERSE;
> > 2. add __dev_pm_set_dedicated_wake_irq() with flag parameter,
> > then
> > rebuild dev_pm_set_dedicated_wake_irq() and add new api
> > dev_pm_set_dedicated_wake_irq_reverse();
> > 3. rename the new added parameter as cond_disable in
> > dev_pm_disable_wake_irq_check(), and also simplify its flow;
> > 4. modify some comments
> >
> > v3: add new status suggested by Rafael
> >
> > v2: add more commit message
> >
> > Use the falling edge trigger interrupt suggested by Ikjoon [1],
> > it
> > works well at firstly when only use this related wakeup source, but
> > encounter issues if use other wakeup sources to wakeup platform as
> > below steps:
> > 1. use another wakeup source to wake up the suspended system;
> > 2. the consumer's resume() will be called, and exits sleep state;
> > 3. the consumer's wakeup signal will fall into low level, due to
> > currently the wakeup irq is disabled, the wake-irq is pending;
> > 4. the consumer tries to enter runtime suspend, but there is a
> > pending wakeup irq, so will resume again, this will repeat
> > endlessly.
> >
> > Send out the patch again for further discussion.
> >
> > [1]: https://patchwork.kernel.org/patch/12190407
> >
> > ---
> > drivers/base/power/power.h | 7 ++-
> > drivers/base/power/runtime.c | 6 ++-
> > drivers/base/power/wakeirq.c | 101 +++++++++++++++++++++++++++--
> > ------
> > include/linux/pm_wakeirq.h | 9 +++-
> > 4 files changed, 96 insertions(+), 27 deletions(-)
[...]
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called
2021-10-26 15:39 ` [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq " Rafael J. Wysocki
2021-10-27 9:29 ` Chunfeng Yun
@ 2021-10-27 9:33 ` Chunfeng Yun
2021-10-27 19:00 ` Rafael J. Wysocki
1 sibling, 1 reply; 7+ messages in thread
From: Chunfeng Yun @ 2021-10-27 9:33 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Len Brown, Pavel Machek, Greg Kroah-Hartman, Mathias Nyman,
Matthias Brugger, Linux PM, Linux Kernel Mailing List,
open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:,
Linux ARM, moderated list:ARM/Mediatek SoC...,
Rafael J . Wysocki
On Tue, 2021-10-26 at 17:39 +0200, Rafael J. Wysocki wrote:
> On Mon, Oct 25, 2021 at 9:02 AM Chunfeng Yun <
> chunfeng.yun@mediatek.com> wrote:
> >
> > When the dedicated wake IRQ is level trigger, and it uses the
> > device's low-power status as the wakeup source, that means if the
> > device is not in low-power state, the wake IRQ will be triggered
> > if enabled; For this case, need enable the wake IRQ after running
> > the device's ->runtime_suspend() which make it enter low-power
> > state.
> >
> > e.g.
> > Assume the wake IRQ is a low level trigger type, and the wakeup
> > signal comes from the low-power status of the device.
> > The wakeup signal is low level at running time (0), and becomes
> > high level when the device enters low-power state (runtime_suspend
> > (1) is called), a wakeup event at (2) make the device exit low-
> > power
> > state, then the wakeup signal also becomes low level.
> >
> > ------------------
> > | ^ ^|
> > ---------------- | | --------------
> > |<---(0)--->|<--(1)--| (3) (2) (4)
> >
> > if enable the wake IRQ before running runtime_suspend during (0),
> > a wake IRQ will arise, it causes resume immediately;
> > it works if enable wake IRQ ( e.g. at (3) or (4)) after running
> > ->runtime_suspend().
> >
> > This patch introduces a new status WAKE_IRQ_DEDICATED_REVERSE to
> > optionally support enabling wake IRQ after running
> > ->runtime_suspend().
> >
> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>
> I don't really have anything to add regarding the code.
>
> The kerneldoc comments could be improved, but I can take care of this
> when applying the patch.
Ok, thanks
>
> Please collect ACKs for the remaining 2 patches in the series and I
> will pick up all three.
The files (mtu3_plat.c, xhci-mtk.c) modified by other 2 patches are
maintained by myself, please pick up them, thanks a lot.
>
> Thanks!
>
> > ---
> > v4: changes according to Rafael's suggestions
> > 1. rename new flag as WAKE_IRQ_DEDICATED_REVERSE;
> > 2. add __dev_pm_set_dedicated_wake_irq() with flag parameter,
> > then
> > rebuild dev_pm_set_dedicated_wake_irq() and add new api
> > dev_pm_set_dedicated_wake_irq_reverse();
> > 3. rename the new added parameter as cond_disable in
> > dev_pm_disable_wake_irq_check(), and also simplify its flow;
> > 4. modify some comments
> >
> > v3: add new status suggested by Rafael
> >
> > v2: add more commit message
> >
> > Use the falling edge trigger interrupt suggested by Ikjoon [1],
> > it
> > works well at firstly when only use this related wakeup source, but
> > encounter issues if use other wakeup sources to wakeup platform as
> > below steps:
> > 1. use another wakeup source to wake up the suspended system;
> > 2. the consumer's resume() will be called, and exits sleep state;
> > 3. the consumer's wakeup signal will fall into low level, due to
> > currently the wakeup irq is disabled, the wake-irq is pending;
> > 4. the consumer tries to enter runtime suspend, but there is a
> > pending wakeup irq, so will resume again, this will repeat
> > endlessly.
> >
> > Send out the patch again for further discussion.
> >
> > [1]: https://patchwork.kernel.org/patch/12190407
> >
> > ---
> > drivers/base/power/power.h | 7 ++-
> > drivers/base/power/runtime.c | 6 ++-
> > drivers/base/power/wakeirq.c | 101 +++++++++++++++++++++++++++--
> > ------
> > include/linux/pm_wakeirq.h | 9 +++-
> > 4 files changed, 96 insertions(+), 27 deletions(-)
[...]
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called
2021-10-27 9:33 ` Chunfeng Yun
@ 2021-10-27 19:00 ` Rafael J. Wysocki
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-10-27 19:00 UTC (permalink / raw)
To: Chunfeng Yun
Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
Mathias Nyman, Matthias Brugger, Linux PM,
Linux Kernel Mailing List,
open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:,
Linux ARM, moderated list:ARM/Mediatek SoC...,
Rafael J . Wysocki
On Wed, Oct 27, 2021 at 11:34 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Tue, 2021-10-26 at 17:39 +0200, Rafael J. Wysocki wrote:
> > On Mon, Oct 25, 2021 at 9:02 AM Chunfeng Yun <
> > chunfeng.yun@mediatek.com> wrote:
> > >
> > > When the dedicated wake IRQ is level trigger, and it uses the
> > > device's low-power status as the wakeup source, that means if the
> > > device is not in low-power state, the wake IRQ will be triggered
> > > if enabled; For this case, need enable the wake IRQ after running
> > > the device's ->runtime_suspend() which make it enter low-power
> > > state.
> > >
> > > e.g.
> > > Assume the wake IRQ is a low level trigger type, and the wakeup
> > > signal comes from the low-power status of the device.
> > > The wakeup signal is low level at running time (0), and becomes
> > > high level when the device enters low-power state (runtime_suspend
> > > (1) is called), a wakeup event at (2) make the device exit low-
> > > power
> > > state, then the wakeup signal also becomes low level.
> > >
> > > ------------------
> > > | ^ ^|
> > > ---------------- | | --------------
> > > |<---(0)--->|<--(1)--| (3) (2) (4)
> > >
> > > if enable the wake IRQ before running runtime_suspend during (0),
> > > a wake IRQ will arise, it causes resume immediately;
> > > it works if enable wake IRQ ( e.g. at (3) or (4)) after running
> > > ->runtime_suspend().
> > >
> > > This patch introduces a new status WAKE_IRQ_DEDICATED_REVERSE to
> > > optionally support enabling wake IRQ after running
> > > ->runtime_suspend().
> > >
> > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >
> > I don't really have anything to add regarding the code.
> >
> > The kerneldoc comments could be improved, but I can take care of this
> > when applying the patch.
> Ok, thanks
> >
> > Please collect ACKs for the remaining 2 patches in the series and I
> > will pick up all three.
> The files (mtu3_plat.c, xhci-mtk.c) modified by other 2 patches are
> maintained by myself, please pick up them, thanks a lot.
OK, applied as 5.16 material, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-11-13 7:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 7:01 [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq after runtime_suspend called Chunfeng Yun
2021-10-25 7:01 ` [PATCH v4 2/3] usb: xhci-mtk: enable wake-up interrupt " Chunfeng Yun
2021-10-25 7:01 ` [PATCH v4 3/3] usb: mtu3: " Chunfeng Yun
2021-10-26 15:39 ` [PATCH v4 1/3] PM / wakeirq: support enabling wake-up irq " Rafael J. Wysocki
2021-10-27 9:29 ` Chunfeng Yun
2021-10-27 9:33 ` Chunfeng Yun
2021-10-27 19:00 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).