Skip to contentSkip to conversation

irqchip: qcom: pdc: Introduce irq_set_wake call

PATCH Applied Maulik Shah 6 participants 17 messages Sep 28, 2020 – Oct 11, 2020

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

v6 cover irqchip: qcom: pdc: Introduce irq_set_wake call

Changes in v6:
- Update commit message more descriptive in v5 patch 1
- Symmetrically enable/disable wakeirqs during suspend/resume in v5 patch 3
- Include Acked-by and Reviewed-by tags from v5 series

Changes in v5:
- Update commit subject in v4 patch 1
- Add more details to commit message in v4 patch 2
- Add change to enable wake irqs during suspend using new flag in irqchip
- Use this in PDC and qcom pinctrl driver to enable wakeirqs on suspend
- Make for loop more readable and add more details in commit in v4 patch 7

Changes in v4:
- Drop "Remove irq_disable callback from msmgpio irqchip" patch from v3
- Introduce irq_suspend_one() and irq_resume_one() callbacks
- Use the new callbacks to unmask wake interrupts during suspend
- Reset only pdc interrupts that are mapped in DTSI

Changes in v3:
- Drop gpiolib change (v2 patch 1) since its already in linux-next
- Add Acked-by Linus Walleij for v2 patch 2 and v2 patch 3.
- Address Stephen's comment to on v2 patch 3
- Address Stephen's comment to change variable to static on v2 patch 4.
- Add a new change to use return value from .irq_set_wake callback
- Add a new change to reset PDC irq enable bank during init time

Changes in v2:
- Fix compiler error on gpiolib patch

This series adds support to lazy disable pdc interrupt.

Some drivers using gpio interrupts want to configure gpio for wakeup using enable_irq_wake() but during suspend entry disables irq and expects system to resume when interrupt occurs. In the driver resume call interrupt is re-enabled and removes wakeup capability using disable_irq_wake() one such example is cros ec driver.

With [1] in documentation saying "An irq can be disabled with disable_irq() and still wake the system as long as the irq has wake enabled".

The PDC IRQs are currently "unlazy disabled" (disable here means that it will be masked in PDC & GIC HW GICD_ISENABLER, the moment driver invokes disable_irq()) such IRQs can not wakeup from low power modes like suspend to RAM since the driver chosen to disable this.

During suspend entry, no one re-enable/unmask in HW, even if its marked for wakeup.

One solutions thought to address this problem was...During suspend entry at last point, irq chip driver re-enable/unmask IRQs in HW that are marked for wakeup. This was attemped in [2].

This series adds alternate solution to [2] by "lazy disable" IRQs in HW. The genirq takes care of lazy disable in case if irqchip did not implement irq_disable callback. Below is high level steps on how this works out..

a. During driver's disable_irq() call, IRQ will be marked disabled in SW b. IRQ will still be enabled(read unmasked in HW)
c. The device then enters low power mode like suspend to RAM d. The HW detects unmasked IRQs and wakesup the CPU
e. During resume after local_irq_enable() CPU goes to handle the wake IRQ f. Generic handler comes to know that IRQ is disabled in SW
g. Generic handler marks IRQ as pending and now invokes mask callback h. IRQ gets disabled/masked in HW now
i. When driver invokes enable_irq() the SW pending IRQ leads IRQ's handler j. enable_irq() will again enable/unmask in HW

[1] https://www.spinics.net/lists/kernel/msg3398294.html
[2] https://patchwork.kernel.org/patch/11466021/

Maulik Shah (6):
pinctrl: qcom: Set IRQCHIP_SET_TYPE_MASKED and IRQCHIP_MASK_ON_SUSPEND
flags
pinctrl: qcom: Use return value from irq_set_wake() call genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag
pinctrl: qcom: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag
irqchip: qcom-pdc: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag
irqchip: qcom-pdc: Reset PDC interrupts during init

 drivers/irqchip/qcom-pdc.c         | 14 +++++++++--
 drivers/pinctrl/qcom/pinctrl-msm.c | 11 +++++----
 include/linux/irq.h                | 49 +++++++++++++++++++++++---------------
 kernel/irq/debugfs.c               |  3 +++
 kernel/irq/pm.c                    | 34 ++++++++++++++++++++++----
 5 files changed, 81 insertions(+), 30 deletions(-)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#1 lore raw

PATCH v6 1/6 pinctrl: qcom: Set IRQCHIP_SET_TYPE_MASKED and IRQCHIP_MASK_ON_SUSPEND flags

Both IRQCHIP_SET_TYPE_MASKED and IRQCHIP_MASK_ON_SUSPEND flags are already set for msmgpio's parent PDC irqchip but GPIO interrupts do not get masked during suspend or during setting irq type since genirq checks irqchip flag of msmgpio irqchip which forwards these calls to its parent PDC irqchip.

Add irqchip specific flags for msmgpio irqchip to mask non wakeirqs during suspend and mask before setting irq type. Masking before changing type make sures any spurious interrupt is not detected during this operation.

Fixes e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/pinctrl/qcom/pinctrl-msm.c +2 -0
index a2567e7..1c23f5c 100644
@@ -1243,6 +1243,8 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
 	pctrl->irq_chip.irq_set_affinity = msm_gpio_irq_set_affinity;
 	pctrl->irq_chip.irq_set_vcpu_affinity = msm_gpio_irq_set_vcpu_affinity;
+	pctrl->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND |
+				IRQCHIP_SET_TYPE_MASKED;
 
 	np = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
 	if (np) {
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
merge bot Applied
#2 lore raw

[tip: irq/core] pinctrl: qcom: Set IRQCHIP_SET_TYPE_MASKED and IRQCHIP_MASK_ON_SUSPEND flags

The following commit has been merged into the irq/core branch of tip:

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     c5f72aeb659eb2f809b9531d759651514d42aa3a
Gitweb:        https://git.kernel.org/tip/c5f72aeb659eb2f809b9531d759651514d42aa3a
Author:        Maulik Shah <[email protected]>
AuthorDate:    Mon, 28 Sep 2020 10:01:59 +05:30
Committer:     Marc Zyngier <[email protected]>
CommitterDate: Tue, 06 Oct 2020 11:23:13 +01:00

pinctrl: qcom: Set IRQCHIP_SET_TYPE_MASKED and IRQCHIP_MASK_ON_SUSPEND flags

Both IRQCHIP_SET_TYPE_MASKED and IRQCHIP_MASK_ON_SUSPEND flags are already set for msmgpio's parent PDC irqchip but GPIO interrupts do not get masked during suspend or during setting irq type since genirq checks irqchip flag of msmgpio irqchip which forwards these calls to its parent PDC irqchip.

Add irqchip specific flags for msmgpio irqchip to mask non wakeirqs during suspend and mask before setting irq type. Masking before changing type make sures any spurious interrupt is not detected during this operation.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/pinctrl/qcom/pinctrl-msm.c +2 -0
index a2567e7..1c23f5c 100644
@@ -1243,6 +1243,8 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
 	pctrl->irq_chip.irq_set_affinity = msm_gpio_irq_set_affinity;
 	pctrl->irq_chip.irq_set_vcpu_affinity = msm_gpio_irq_set_vcpu_affinity;
+	pctrl->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND |
+				IRQCHIP_SET_TYPE_MASKED;
 
 	np = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
 	if (np) {
#3 lore raw

PATCH v6 2/6 pinctrl: qcom: Use return value from irq_set_wake() call

In reply to Maulik Shah

msmgpio irqchip was not using return value of irq_set_irq_wake() callback since previously GIC-v3 irqchip neither had IRQCHIP_SKIP_SET_WAKE flag nor it implemented .irq_set_wake callback. This lead to irq_set_irq_wake() return error -ENXIO.

However from 'commit 4110b5cbb014 ("irqchip/gic-v3: Allow interrupt to be configured as wake-up sources")' GIC irqchip has IRQCHIP_SKIP_SET_WAKE flag.

Use return value from irq_set_irq_wake() and irq_chip_set_wake_parent() instead of always returning success.

Fixes e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/pinctrl/qcom/pinctrl-msm.c +3 -5
index 1c23f5c..1df2322 100644
@@ -1077,12 +1077,10 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 	 * when TLMM is powered on. To allow that, enable the GPIO
 	 * summary line to be wakeup capable at GIC.
 	 */
-	if (d->parent_data)
-		irq_chip_set_wake_parent(d, on);
-
-	irq_set_irq_wake(pctrl->irq, on);
+	if (d->parent_data && test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return irq_chip_set_wake_parent(d, on);
 
-	return 0;
+	return irq_set_irq_wake(pctrl->irq, on);
 }
 
 static int msm_gpio_irq_reqres(struct irq_data *d)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
merge bot Applied
#4 lore raw

[tip: irq/core] pinctrl: qcom: Use return value from irq_set_wake() call

The following commit has been merged into the irq/core branch of tip:

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     f41aaca593377a4fe3984459fd4539481263b4cd
Gitweb:        https://git.kernel.org/tip/f41aaca593377a4fe3984459fd4539481263b4cd
Author:        Maulik Shah <[email protected]>
AuthorDate:    Mon, 28 Sep 2020 10:02:00 +05:30
Committer:     Marc Zyngier <[email protected]>
CommitterDate: Tue, 06 Oct 2020 11:23:14 +01:00

pinctrl: qcom: Use return value from irq_set_wake() call

msmgpio irqchip was not using return value of irq_set_irq_wake() callback since previously GIC-v3 irqchip neither had IRQCHIP_SKIP_SET_WAKE flag nor it implemented .irq_set_wake callback. This lead to irq_set_irq_wake() return error -ENXIO.

However from 'commit 4110b5cbb014 ("irqchip/gic-v3: Allow interrupt to be configured as wake-up sources")' GIC irqchip has IRQCHIP_SKIP_SET_WAKE flag.

Use return value from irq_set_irq_wake() and irq_chip_set_wake_parent() instead of always returning success.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/pinctrl/qcom/pinctrl-msm.c +3 -5
index 1c23f5c..1df2322 100644
@@ -1077,12 +1077,10 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 	 * when TLMM is powered on. To allow that, enable the GPIO
 	 * summary line to be wakeup capable at GIC.
 	 */
-	if (d->parent_data)
-		irq_chip_set_wake_parent(d, on);
-
-	irq_set_irq_wake(pctrl->irq, on);
+	if (d->parent_data && test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return irq_chip_set_wake_parent(d, on);
 
-	return 0;
+	return irq_set_irq_wake(pctrl->irq, on);
 }
 
 static int msm_gpio_irq_reqres(struct irq_data *d)
#5 lore raw

PATCH v6 3/6 genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

In reply to Maulik Shah

An interrupt that is disabled/masked but set for wakeup still needs to be able to wake up the system from sleep states like "suspend to RAM".

This change introduces IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag. If irqchip have this flag set then irq PM will enable/unmask irqs that are marked for wakeup but are in disabled state.

On resume such irqs will be restored back to disabled state.

---
 include/linux/irq.h  | 49 ++++++++++++++++++++++++++++++-------------------
 kernel/irq/debugfs.c |  3 +++
 kernel/irq/pm.c      | 34 ++++++++++++++++++++++++++++++----
 3 files changed, 63 insertions(+), 23 deletions(-)
include/linux/irq.h +30 -19
index 63b9d96..ccd39e2 100644
@@ -217,6 +217,8 @@ struct irq_data {
  *				  from actual interrupt context.
  * IRQD_AFFINITY_ON_ACTIVATE	- Affinity is set on activation. Don't call
  *				  irq_chip::irq_set_affinity() when deactivated.
+ * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
+ *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
  */
 enum {
 	IRQD_TRIGGER_MASK		= 0xf,
@@ -242,6 +244,7 @@ enum {
 	IRQD_MSI_NOMASK_QUIRK		= (1 << 27),
 	IRQD_HANDLE_ENFORCE_IRQCTX	= (1 << 28),
 	IRQD_AFFINITY_ON_ACTIVATE	= (1 << 29),
+	IRQD_IRQ_ENABLED_ON_SUSPEND	= (1 << 30),
 };
 
 #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
@@ -321,6 +324,11 @@ static inline bool irqd_is_handle_enforce_irqctx(struct irq_data *d)
 	return __irqd_to_state(d) & IRQD_HANDLE_ENFORCE_IRQCTX;
 }
 
+static inline bool irqd_is_enabled_on_suspend(struct irq_data *d)
+{
+	return __irqd_to_state(d) & IRQD_IRQ_ENABLED_ON_SUSPEND;
+}
+
 static inline bool irqd_is_wakeup_set(struct irq_data *d)
 {
 	return __irqd_to_state(d) & IRQD_WAKEUP_STATE;
@@ -547,27 +555,30 @@ struct irq_chip {
 /*
  * irq_chip specific flags
  *
- * IRQCHIP_SET_TYPE_MASKED:	Mask before calling chip.irq_set_type()
- * IRQCHIP_EOI_IF_HANDLED:	Only issue irq_eoi() when irq was handled
- * IRQCHIP_MASK_ON_SUSPEND:	Mask non wake irqs in the suspend path
- * IRQCHIP_ONOFFLINE_ENABLED:	Only call irq_on/off_line callbacks
- *				when irq enabled
- * IRQCHIP_SKIP_SET_WAKE:	Skip chip.irq_set_wake(), for this irq chip
- * IRQCHIP_ONESHOT_SAFE:	One shot does not require mask/unmask
- * IRQCHIP_EOI_THREADED:	Chip requires eoi() on unmask in threaded mode
- * IRQCHIP_SUPPORTS_LEVEL_MSI	Chip can provide two doorbells for Level MSIs
- * IRQCHIP_SUPPORTS_NMI:	Chip can deliver NMIs, only for root irqchips
+ * IRQCHIP_SET_TYPE_MASKED:           Mask before calling chip.irq_set_type()
+ * IRQCHIP_EOI_IF_HANDLED:            Only issue irq_eoi() when irq was handled
+ * IRQCHIP_MASK_ON_SUSPEND:           Mask non wake irqs in the suspend path
+ * IRQCHIP_ONOFFLINE_ENABLED:         Only call irq_on/off_line callbacks
+ *                                    when irq enabled
+ * IRQCHIP_SKIP_SET_WAKE:             Skip chip.irq_set_wake(), for this irq chip
+ * IRQCHIP_ONESHOT_SAFE:              One shot does not require mask/unmask
+ * IRQCHIP_EOI_THREADED:              Chip requires eoi() on unmask in threaded mode
+ * IRQCHIP_SUPPORTS_LEVEL_MSI:        Chip can provide two doorbells for Level MSIs
+ * IRQCHIP_SUPPORTS_NMI:              Chip can deliver NMIs, only for root irqchips
+ * IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND:  Invokes __enable_irq()/__disable_irq() for wake irqs
+ *                                    in the suspend path if they are in disabled state
  */
 enum {
-	IRQCHIP_SET_TYPE_MASKED		= (1 <<  0),
-	IRQCHIP_EOI_IF_HANDLED		= (1 <<  1),
-	IRQCHIP_MASK_ON_SUSPEND		= (1 <<  2),
-	IRQCHIP_ONOFFLINE_ENABLED	= (1 <<  3),
-	IRQCHIP_SKIP_SET_WAKE		= (1 <<  4),
-	IRQCHIP_ONESHOT_SAFE		= (1 <<  5),
-	IRQCHIP_EOI_THREADED		= (1 <<  6),
-	IRQCHIP_SUPPORTS_LEVEL_MSI	= (1 <<  7),
-	IRQCHIP_SUPPORTS_NMI		= (1 <<  8),
+	IRQCHIP_SET_TYPE_MASKED			= (1 <<  0),
+	IRQCHIP_EOI_IF_HANDLED			= (1 <<  1),
+	IRQCHIP_MASK_ON_SUSPEND			= (1 <<  2),
+	IRQCHIP_ONOFFLINE_ENABLED		= (1 <<  3),
+	IRQCHIP_SKIP_SET_WAKE			= (1 <<  4),
+	IRQCHIP_ONESHOT_SAFE			= (1 <<  5),
+	IRQCHIP_EOI_THREADED			= (1 <<  6),
+	IRQCHIP_SUPPORTS_LEVEL_MSI		= (1 <<  7),
+	IRQCHIP_SUPPORTS_NMI			= (1 <<  8),
+	IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND	= (1 <<  9),
 };
 
 #include <linux/irqdesc.h>
kernel/irq/debugfs.c +3 -0
index acabc0c..e4cff35 100644
@@ -57,6 +57,7 @@ static const struct irq_bit_descr irqchip_flags[] = {
 	BIT_MASK_DESCR(IRQCHIP_EOI_THREADED),
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
+	BIT_MASK_DESCR(IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND),
 };
 
 static void
@@ -125,6 +126,8 @@ static const struct irq_bit_descr irqdata_states[] = {
 	BIT_MASK_DESCR(IRQD_DEFAULT_TRIGGER_SET),
 
 	BIT_MASK_DESCR(IRQD_HANDLE_ENFORCE_IRQCTX),
+
+	BIT_MASK_DESCR(IRQD_IRQ_ENABLED_ON_SUSPEND),
 };
 
 static const struct irq_bit_descr irqdesc_states[] = {
kernel/irq/pm.c +30 -4
index c6c7e18..ce0adb2 100644
@@ -69,12 +69,26 @@ void irq_pm_remove_action(struct irq_desc *desc, struct irqaction *action)
 
 static bool suspend_device_irq(struct irq_desc *desc)
 {
+	unsigned long chipflags = irq_desc_get_chip(desc)->flags;
+	struct irq_data *irqd = &desc->irq_data;
+
 	if (!desc->action || irq_desc_is_chained(desc) ||
 	    desc->no_suspend_depth)
 		return false;
 
-	if (irqd_is_wakeup_set(&desc->irq_data)) {
-		irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED);
+	if (irqd_is_wakeup_set(irqd)) {
+		irqd_set(irqd, IRQD_WAKEUP_ARMED);
+
+		if ((chipflags & IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND) &&
+		     irqd_irq_disabled(irqd)) {
+			/*
+			 * Interrupt marked for wakeup is in disabled state.
+			 * Enable interrupt here to unmask/enable in irqchip
+			 * to be able to resume with such interrupts.
+			 */
+			__enable_irq(desc);
+			irqd_set(irqd, IRQD_IRQ_ENABLED_ON_SUSPEND);
+		}
 		/*
 		 * We return true here to force the caller to issue
 		 * synchronize_irq(). We need to make sure that the
@@ -93,7 +107,7 @@ static bool suspend_device_irq(struct irq_desc *desc)
 	 * chip level. The chip implementation indicates that with
 	 * IRQCHIP_MASK_ON_SUSPEND.
 	 */
-	if (irq_desc_get_chip(desc)->flags & IRQCHIP_MASK_ON_SUSPEND)
+	if (chipflags & IRQCHIP_MASK_ON_SUSPEND)
 		mask_irq(desc);
 	return true;
 }
@@ -137,7 +151,19 @@ EXPORT_SYMBOL_GPL(suspend_device_irqs);
 
 static void resume_irq(struct irq_desc *desc)
 {
-	irqd_clear(&desc->irq_data, IRQD_WAKEUP_ARMED);
+	struct irq_data *irqd = &desc->irq_data;
+
+	irqd_clear(irqd, IRQD_WAKEUP_ARMED);
+
+	if (irqd_is_enabled_on_suspend(irqd)) {
+		/*
+		 * Interrupt marked for wakeup was enabled during suspend
+		 * entry. Disable such interrupts to restore them back to
+		 * original state.
+		 */
+		__disable_irq(desc);
+		irqd_clear(irqd, IRQD_IRQ_ENABLED_ON_SUSPEND);
+	}
 
 	if (desc->istate & IRQS_SUSPENDED)
 		goto resume;
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Reviewed-by on v6 3/6
#6 lore raw

Re: [PATCH v6 3/6] genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

Hi,

Quoted 10 lines · Maulik Shah

An interrupt that is disabled/masked but set for wakeup still needs to be able to wake up the system from sleep states like "suspend to RAM".

This change introduces IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag. If irqchip have this flag set then irq PM will enable/unmask irqs that are marked for wakeup but are in disabled state.

On resume such irqs will be restored back to disabled state.

Quoted 7 lines
---
 include/linux/irq.h  | 49 ++++++++++++++++++++++++++++++-------------------
 kernel/irq/debugfs.c |  3 +++
 kernel/irq/pm.c      | 34 ++++++++++++++++++++++++++++++----
 3 files changed, 63 insertions(+), 23 deletions(-)

I will freely admit not being an expert on this code / knowing all the subtle edge conditions, but this seems reasonable to me.

Reviewed-by on v6 3/6
#7 lore raw

Re: [PATCH v6 3/6] genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

In reply to Maulik Shah
Quoted 5 lines · Maulik Shah

An interrupt that is disabled/masked but set for wakeup still needs to be able to wake up the system from sleep states like "suspend to RAM".

This change introduces IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag. If irqchip

s/This change introduces/Introduce/

git grep 'This patch' Documentation/process/

Quoted 7 lines

have this flag set then irq PM will enable/unmask irqs that are marked for wakeup but are in disabled state.

On resume such irqs will be restored back to disabled state.

I assume Marc will pick that lot up. So:

merge bot Applied
#8 lore raw

[tip: irq/core] genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

In reply to Maulik Shah
The following commit has been merged into the irq/core branch of tip:

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     90428a8eb4947f9c7c905a178f3520dc7e2ee6d2
Gitweb:        https://git.kernel.org/tip/90428a8eb4947f9c7c905a178f3520dc7e2ee6d2
Author:        Maulik Shah <[email protected]>
AuthorDate:    Mon, 28 Sep 2020 10:02:01 +05:30
Committer:     Marc Zyngier <[email protected]>
CommitterDate: Tue, 06 Oct 2020 11:23:41 +01:00

genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

An interrupt that is disabled/masked but set for wakeup may still need to be able to wake up the system from sleep states like "suspend to RAM".

To that effect, introduce the IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag. If the irqchip have this flag set, the irq PM code will enable/unmask the irqs that are marked for wakeup, but that are in a disabled state.

On resume, such irqs will be restored back to their disabled state.

---
 include/linux/irq.h  | 49 ++++++++++++++++++++++++++-----------------
 kernel/irq/debugfs.c |  3 +++-
 kernel/irq/pm.c      | 34 ++++++++++++++++++++++++++----
 3 files changed, 63 insertions(+), 23 deletions(-)
include/linux/irq.h +30 -19
index 1b7f4df..a8b84b8 100644
@@ -215,6 +215,8 @@ struct irq_data {
  *				  from actual interrupt context.
  * IRQD_AFFINITY_ON_ACTIVATE	- Affinity is set on activation. Don't call
  *				  irq_chip::irq_set_affinity() when deactivated.
+ * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
+ *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
  */
 enum {
 	IRQD_TRIGGER_MASK		= 0xf,
@@ -240,6 +242,7 @@ enum {
 	IRQD_MSI_NOMASK_QUIRK		= (1 << 27),
 	IRQD_HANDLE_ENFORCE_IRQCTX	= (1 << 28),
 	IRQD_AFFINITY_ON_ACTIVATE	= (1 << 29),
+	IRQD_IRQ_ENABLED_ON_SUSPEND	= (1 << 30),
 };
 
 #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
@@ -319,6 +322,11 @@ static inline bool irqd_is_handle_enforce_irqctx(struct irq_data *d)
 	return __irqd_to_state(d) & IRQD_HANDLE_ENFORCE_IRQCTX;
 }
 
+static inline bool irqd_is_enabled_on_suspend(struct irq_data *d)
+{
+	return __irqd_to_state(d) & IRQD_IRQ_ENABLED_ON_SUSPEND;
+}
+
 static inline bool irqd_is_wakeup_set(struct irq_data *d)
 {
 	return __irqd_to_state(d) & IRQD_WAKEUP_STATE;
@@ -545,27 +553,30 @@ struct irq_chip {
 /*
  * irq_chip specific flags
  *
- * IRQCHIP_SET_TYPE_MASKED:	Mask before calling chip.irq_set_type()
- * IRQCHIP_EOI_IF_HANDLED:	Only issue irq_eoi() when irq was handled
- * IRQCHIP_MASK_ON_SUSPEND:	Mask non wake irqs in the suspend path
- * IRQCHIP_ONOFFLINE_ENABLED:	Only call irq_on/off_line callbacks
- *				when irq enabled
- * IRQCHIP_SKIP_SET_WAKE:	Skip chip.irq_set_wake(), for this irq chip
- * IRQCHIP_ONESHOT_SAFE:	One shot does not require mask/unmask
- * IRQCHIP_EOI_THREADED:	Chip requires eoi() on unmask in threaded mode
- * IRQCHIP_SUPPORTS_LEVEL_MSI	Chip can provide two doorbells for Level MSIs
- * IRQCHIP_SUPPORTS_NMI:	Chip can deliver NMIs, only for root irqchips
+ * IRQCHIP_SET_TYPE_MASKED:           Mask before calling chip.irq_set_type()
+ * IRQCHIP_EOI_IF_HANDLED:            Only issue irq_eoi() when irq was handled
+ * IRQCHIP_MASK_ON_SUSPEND:           Mask non wake irqs in the suspend path
+ * IRQCHIP_ONOFFLINE_ENABLED:         Only call irq_on/off_line callbacks
+ *                                    when irq enabled
+ * IRQCHIP_SKIP_SET_WAKE:             Skip chip.irq_set_wake(), for this irq chip
+ * IRQCHIP_ONESHOT_SAFE:              One shot does not require mask/unmask
+ * IRQCHIP_EOI_THREADED:              Chip requires eoi() on unmask in threaded mode
+ * IRQCHIP_SUPPORTS_LEVEL_MSI:        Chip can provide two doorbells for Level MSIs
+ * IRQCHIP_SUPPORTS_NMI:              Chip can deliver NMIs, only for root irqchips
+ * IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND:  Invokes __enable_irq()/__disable_irq() for wake irqs
+ *                                    in the suspend path if they are in disabled state
  */
 enum {
-	IRQCHIP_SET_TYPE_MASKED		= (1 <<  0),
-	IRQCHIP_EOI_IF_HANDLED		= (1 <<  1),
-	IRQCHIP_MASK_ON_SUSPEND		= (1 <<  2),
-	IRQCHIP_ONOFFLINE_ENABLED	= (1 <<  3),
-	IRQCHIP_SKIP_SET_WAKE		= (1 <<  4),
-	IRQCHIP_ONESHOT_SAFE		= (1 <<  5),
-	IRQCHIP_EOI_THREADED		= (1 <<  6),
-	IRQCHIP_SUPPORTS_LEVEL_MSI	= (1 <<  7),
-	IRQCHIP_SUPPORTS_NMI		= (1 <<  8),
+	IRQCHIP_SET_TYPE_MASKED			= (1 <<  0),
+	IRQCHIP_EOI_IF_HANDLED			= (1 <<  1),
+	IRQCHIP_MASK_ON_SUSPEND			= (1 <<  2),
+	IRQCHIP_ONOFFLINE_ENABLED		= (1 <<  3),
+	IRQCHIP_SKIP_SET_WAKE			= (1 <<  4),
+	IRQCHIP_ONESHOT_SAFE			= (1 <<  5),
+	IRQCHIP_EOI_THREADED			= (1 <<  6),
+	IRQCHIP_SUPPORTS_LEVEL_MSI		= (1 <<  7),
+	IRQCHIP_SUPPORTS_NMI			= (1 <<  8),
+	IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND	= (1 <<  9),
 };
 
 #include <linux/irqdesc.h>
kernel/irq/debugfs.c +3 -0
index b95ff5d..b6e1094 100644
@@ -57,6 +57,7 @@ static const struct irq_bit_descr irqchip_flags[] = {
 	BIT_MASK_DESCR(IRQCHIP_EOI_THREADED),
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
+	BIT_MASK_DESCR(IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND),
 };
 
 static void
@@ -125,6 +126,8 @@ static const struct irq_bit_descr irqdata_states[] = {
 	BIT_MASK_DESCR(IRQD_DEFAULT_TRIGGER_SET),
 
 	BIT_MASK_DESCR(IRQD_HANDLE_ENFORCE_IRQCTX),
+
+	BIT_MASK_DESCR(IRQD_IRQ_ENABLED_ON_SUSPEND),
 };
 
 static const struct irq_bit_descr irqdesc_states[] = {
kernel/irq/pm.c +30 -4
index c6c7e18..ce0adb2 100644
@@ -69,12 +69,26 @@ void irq_pm_remove_action(struct irq_desc *desc, struct irqaction *action)
 
 static bool suspend_device_irq(struct irq_desc *desc)
 {
+	unsigned long chipflags = irq_desc_get_chip(desc)->flags;
+	struct irq_data *irqd = &desc->irq_data;
+
 	if (!desc->action || irq_desc_is_chained(desc) ||
 	    desc->no_suspend_depth)
 		return false;
 
-	if (irqd_is_wakeup_set(&desc->irq_data)) {
-		irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED);
+	if (irqd_is_wakeup_set(irqd)) {
+		irqd_set(irqd, IRQD_WAKEUP_ARMED);
+
+		if ((chipflags & IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND) &&
+		     irqd_irq_disabled(irqd)) {
+			/*
+			 * Interrupt marked for wakeup is in disabled state.
+			 * Enable interrupt here to unmask/enable in irqchip
+			 * to be able to resume with such interrupts.
+			 */
+			__enable_irq(desc);
+			irqd_set(irqd, IRQD_IRQ_ENABLED_ON_SUSPEND);
+		}
 		/*
 		 * We return true here to force the caller to issue
 		 * synchronize_irq(). We need to make sure that the
@@ -93,7 +107,7 @@ static bool suspend_device_irq(struct irq_desc *desc)
 	 * chip level. The chip implementation indicates that with
 	 * IRQCHIP_MASK_ON_SUSPEND.
 	 */
-	if (irq_desc_get_chip(desc)->flags & IRQCHIP_MASK_ON_SUSPEND)
+	if (chipflags & IRQCHIP_MASK_ON_SUSPEND)
 		mask_irq(desc);
 	return true;
 }
@@ -137,7 +151,19 @@ EXPORT_SYMBOL_GPL(suspend_device_irqs);
 
 static void resume_irq(struct irq_desc *desc)
 {
-	irqd_clear(&desc->irq_data, IRQD_WAKEUP_ARMED);
+	struct irq_data *irqd = &desc->irq_data;
+
+	irqd_clear(irqd, IRQD_WAKEUP_ARMED);
+
+	if (irqd_is_enabled_on_suspend(irqd)) {
+		/*
+		 * Interrupt marked for wakeup was enabled during suspend
+		 * entry. Disable such interrupts to restore them back to
+		 * original state.
+		 */
+		__disable_irq(desc);
+		irqd_clear(irqd, IRQD_IRQ_ENABLED_ON_SUSPEND);
+	}
 
 	if (desc->istate & IRQS_SUSPENDED)
 		goto resume;
#9 lore raw

PATCH v6 4/6 pinctrl: qcom: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

In reply to Maulik Shah

Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag to enable/unmask the wakeirqs during suspend entry.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/pinctrl/qcom/pinctrl-msm.c +2 -1
index 1df2322..c4bcda9 100644
@@ -1242,7 +1242,8 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	pctrl->irq_chip.irq_set_affinity = msm_gpio_irq_set_affinity;
 	pctrl->irq_chip.irq_set_vcpu_affinity = msm_gpio_irq_set_vcpu_affinity;
 	pctrl->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND |
-				IRQCHIP_SET_TYPE_MASKED;
+				IRQCHIP_SET_TYPE_MASKED |
+				IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND;
 
 	np = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
 	if (np) {
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
merge bot Applied
#10 lore raw

[tip: irq/core] pinctrl: qcom: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

The following commit has been merged into the irq/core branch of tip:

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     dd87bd09822c294a3c7c4daf11f11a9f81222f80
Gitweb:        https://git.kernel.org/tip/dd87bd09822c294a3c7c4daf11f11a9f81222f80
Author:        Maulik Shah <[email protected]>
AuthorDate:    Mon, 28 Sep 2020 10:02:02 +05:30
Committer:     Marc Zyngier <[email protected]>
CommitterDate: Tue, 06 Oct 2020 11:26:34 +01:00

pinctrl: qcom: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag to enable/unmask the wakeirqs during suspend entry.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/pinctrl/qcom/pinctrl-msm.c +2 -1
index 1df2322..c4bcda9 100644
@@ -1242,7 +1242,8 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	pctrl->irq_chip.irq_set_affinity = msm_gpio_irq_set_affinity;
 	pctrl->irq_chip.irq_set_vcpu_affinity = msm_gpio_irq_set_vcpu_affinity;
 	pctrl->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND |
-				IRQCHIP_SET_TYPE_MASKED;
+				IRQCHIP_SET_TYPE_MASKED |
+				IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND;
 
 	np = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
 	if (np) {
#11 lore raw

PATCH v6 5/6 irqchip: qcom-pdc: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

In reply to Maulik Shah

Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag to enable/unmask the wakeirqs during suspend entry.

---
 drivers/irqchip/qcom-pdc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/irqchip/qcom-pdc.c +2 -1
index 6ae9e1f..acc0620 100644
@@ -205,7 +205,8 @@ static struct irq_chip qcom_pdc_gic_chip = {
 	.irq_set_type		= qcom_pdc_gic_set_type,
 	.flags			= IRQCHIP_MASK_ON_SUSPEND |
 				  IRQCHIP_SET_TYPE_MASKED |
-				  IRQCHIP_SKIP_SET_WAKE,
+				  IRQCHIP_SKIP_SET_WAKE |
+				  IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND,
 	.irq_set_vcpu_affinity	= irq_chip_set_vcpu_affinity_parent,
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 };
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
merge bot Applied
#12 lore raw

[tip: irq/core] irqchip/qcom-pdc: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

The following commit has been merged into the irq/core branch of tip:

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     299d7890792e75065b906f83fcb0ca92e5c8c072
Gitweb:        https://git.kernel.org/tip/299d7890792e75065b906f83fcb0ca92e5c8c072
Author:        Maulik Shah <[email protected]>
AuthorDate:    Mon, 28 Sep 2020 10:02:03 +05:30
Committer:     Marc Zyngier <[email protected]>
CommitterDate: Tue, 06 Oct 2020 11:26:34 +01:00

irqchip/qcom-pdc: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag

Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag to enable/unmask the wakeirqs during suspend entry.

---
 drivers/irqchip/qcom-pdc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/irqchip/qcom-pdc.c +2 -1
index 6ae9e1f..acc0620 100644
@@ -205,7 +205,8 @@ static struct irq_chip qcom_pdc_gic_chip = {
 	.irq_set_type		= qcom_pdc_gic_set_type,
 	.flags			= IRQCHIP_MASK_ON_SUSPEND |
 				  IRQCHIP_SET_TYPE_MASKED |
-				  IRQCHIP_SKIP_SET_WAKE,
+				  IRQCHIP_SKIP_SET_WAKE |
+				  IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND,
 	.irq_set_vcpu_affinity	= irq_chip_set_vcpu_affinity_parent,
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 };
#13 lore raw

PATCH v6 6/6 irqchip: qcom-pdc: Reset PDC interrupts during init

In reply to Maulik Shah

Kexec can directly boot into a new kernel without going to complete reboot. This can leave the previous kernel's configuration for PDC interrupts as is.

Clear previous kernel's configuration during init by setting interrupts in enable bank to zero. The IRQs specified in qcom,pdc-ranges property are the only ones that can be used by the new kernel so clear only those IRQs. The remaining ones may be in use by a different kernel and should not be set by new kernel.

---
 drivers/irqchip/qcom-pdc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/irqchip/qcom-pdc.c +10 -1
index acc0620..bd39e9d 100644
@@ -341,7 +341,8 @@ static const struct irq_domain_ops qcom_pdc_gpio_ops = {
 
 static int pdc_setup_pin_mapping(struct device_node *np)
 {
-	int ret, n;
+	int ret, n, i;
+	u32 irq_index, reg_index, val;
 
 	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
 	if (n <= 0 || n % 3)
@@ -370,6 +371,14 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 						 &pdc_region[n].cnt);
 		if (ret)
 			return ret;
+
+		for (i = 0; i < pdc_region[n].cnt; i++) {
+			reg_index = (i + pdc_region[n].pin_base) >> 5;
+			irq_index = (i + pdc_region[n].pin_base) & 0x1f;
+			val = pdc_reg_read(IRQ_ENABLE_BANK, reg_index);
+			val &= ~BIT(irq_index);
+			pdc_reg_write(IRQ_ENABLE_BANK, reg_index, val);
+		}
 	}
 
 	return 0;
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
merge bot Applied
#14 lore raw

[tip: irq/core] irqchip/qcom-pdc: Reset PDC interrupts during init

The following commit has been merged into the irq/core branch of tip:

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     d7bc63fa20b8a3b0d0645bed1887848c65c01529
Gitweb:        https://git.kernel.org/tip/d7bc63fa20b8a3b0d0645bed1887848c65c01529
Author:        Maulik Shah <[email protected]>
AuthorDate:    Mon, 28 Sep 2020 10:02:04 +05:30
Committer:     Marc Zyngier <[email protected]>
CommitterDate: Tue, 06 Oct 2020 11:26:41 +01:00

irqchip/qcom-pdc: Reset PDC interrupts during init

Kexec can directly boot into a new kernel without going to complete reboot. This can leave the previous kernel's configuration for PDC interrupts as is.

Clear previous kernel's configuration during init by setting interrupts in enable bank to zero. The IRQs specified in qcom,pdc-ranges property are the only ones that can be used by the new kernel so clear only those IRQs. The remaining ones may be in use by a different kernel and should not be set by new kernel.

---
 drivers/irqchip/qcom-pdc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/irqchip/qcom-pdc.c +10 -1
index acc0620..bd39e9d 100644
@@ -341,7 +341,8 @@ static const struct irq_domain_ops qcom_pdc_gpio_ops = {
 
 static int pdc_setup_pin_mapping(struct device_node *np)
 {
-	int ret, n;
+	int ret, n, i;
+	u32 irq_index, reg_index, val;
 
 	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
 	if (n <= 0 || n % 3)
@@ -370,6 +371,14 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 						 &pdc_region[n].cnt);
 		if (ret)
 			return ret;
+
+		for (i = 0; i < pdc_region[n].cnt; i++) {
+			reg_index = (i + pdc_region[n].pin_base) >> 5;
+			irq_index = (i + pdc_region[n].pin_base) & 0x1f;
+			val = pdc_reg_read(IRQ_ENABLE_BANK, reg_index);
+			val &= ~BIT(irq_index);
+			pdc_reg_write(IRQ_ENABLE_BANK, reg_index, val);
+		}
 	}
 
 	return 0;
Tested-by on the v6 series
#15 lore raw
In reply to Maulik Shah

Quoting Maulik Shah (2020-09-27 21:31:58)

Quoted 5 lines

Changes in v6:
- Update commit message more descriptive in v5 patch 1
- Symmetrically enable/disable wakeirqs during suspend/resume in v5 patch 3
- Include Acked-by and Reviewed-by tags from v5 series

Thanks. I tested this series and it is working for me.

Applied
#16 lore raw
In reply to Maulik Shah
Quoted 5 lines · Maulik Shah

Changes in v6:
- Update commit message more descriptive in v5 patch 1
- Symmetrically enable/disable wakeirqs during suspend/resume in v5 patch 3
- Include Acked-by and Reviewed-by tags from v5 series

Quoted 8 lines

Changes in v5:
- Update commit subject in v4 patch 1
- Add more details to commit message in v4 patch 2
- Add change to enable wake irqs during suspend using new flag in irqchip
- Use this in PDC and qcom pinctrl driver to enable wakeirqs on suspend
- Make for loop more readable and add more details in commit in v4 patch 7

[...]

Applied to irq/irqchip-next, thanks!

[1/6] pinctrl: qcom: Set IRQCHIP_SET_TYPE_MASKED and IRQCHIP_MASK_ON_SUSPEND flags

      commit: c5f72aeb659eb2f809b9531d759651514d42aa3a
[2/6] pinctrl: qcom: Use return value from irq_set_wake() call
      commit: f41aaca593377a4fe3984459fd4539481263b4cd
[3/6] genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag
      commit: 90428a8eb4947f9c7c905a178f3520dc7e2ee6d2
[4/6] pinctrl: qcom: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag
      commit: dd87bd09822c294a3c7c4daf11f11a9f81222f80
[5/6] irqchip/qcom-pdc: Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag
      commit: 299d7890792e75065b906f83fcb0ca92e5c8c072
[6/6] irqchip/qcom-pdc: Reset PDC interrupts during init
      commit: d7bc63fa20b8a3b0d0645bed1887848c65c01529

Cheers,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.