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 (4):
gpio: gpiolib: Allow GPIO IRQs to lazy disable
pinctrl: qcom: Remove irq_disable callback from msmgpio irqchip
pinctrl: qcom: Add msmgpio irqchip flags
irqchip: qcom-pdc: Introduce irq_set_wake call
drivers/gpio/gpiolib.c | 59 ++++++++++++++++++++++++--------------
drivers/irqchip/qcom-pdc.c | 33 ++++++++++-----------
drivers/pinctrl/qcom/pinctrl-msm.c | 15 ++--------
include/linux/gpio/driver.h | 13 +++++++++
4 files changed, 70 insertions(+), 50 deletions(-)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Remove irq_disable callback to allow lazy disable for pdc interrupts.
Add irq_set_wake callback that unmask interrupt in HW when drivers
mark interrupt for wakeup. Interrupt will be cleared in HW during
lazy disable if its not marked for wakeup.
Signed-off-by: Maulik Shah <[email protected]>
---
drivers/irqchip/qcom-pdc.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 6ae9e1f..f7c0662 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -36,6 +36,7 @@ struct pdc_pin_region {
u32 cnt;
};
+DECLARE_BITMAP(pdc_wake_irqs, PDC_MAX_IRQS);
static DEFINE_RAW_SPINLOCK(pdc_lock);
static void __iomem *pdc_base;
static struct pdc_pin_region *pdc_region;
@@ -87,22 +88,20 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
raw_spin_unlock(&pdc_lock);
}
-static void qcom_pdc_gic_disable(struct irq_data *d)
+static int qcom_pdc_gic_set_wake(struct irq_data *d, unsigned int on)
{
if (d->hwirq == GPIO_NO_WAKE_IRQ)
- return;
-
- pdc_enable_intr(d, false);
- irq_chip_disable_parent(d);
-}
+ return 0;
-static void qcom_pdc_gic_enable(struct irq_data *d)
-{
- if (d->hwirq == GPIO_NO_WAKE_IRQ)
- return;
+ if (on) {
+ pdc_enable_intr(d, true);
+ irq_chip_enable_parent(d);
+ set_bit(d->hwirq, pdc_wake_irqs);
+ } else {
+ clear_bit(d->hwirq, pdc_wake_irqs);
+ }
- pdc_enable_intr(d, true);
- irq_chip_enable_parent(d);
+ return irq_chip_set_wake_parent(d, on);
}
static void qcom_pdc_gic_mask(struct irq_data *d)
@@ -110,6 +109,9 @@ static void qcom_pdc_gic_mask(struct irq_data *d)
if (d->hwirq == GPIO_NO_WAKE_IRQ)
return;
+ if (!test_bit(d->hwirq, pdc_wake_irqs))
+ pdc_enable_intr(d, false);
+
irq_chip_mask_parent(d);
}
@@ -118,6 +120,7 @@ static void qcom_pdc_gic_unmask(struct irq_data *d)
if (d->hwirq == GPIO_NO_WAKE_IRQ)
return;
+ pdc_enable_intr(d, true);
irq_chip_unmask_parent(d);
}
@@ -197,15 +200,13 @@ static struct irq_chip qcom_pdc_gic_chip = {
.irq_eoi = irq_chip_eoi_parent,
.irq_mask = qcom_pdc_gic_mask,
.irq_unmask = qcom_pdc_gic_unmask,
- .irq_disable = qcom_pdc_gic_disable,
- .irq_enable = qcom_pdc_gic_enable,
.irq_get_irqchip_state = qcom_pdc_gic_get_irqchip_state,
.irq_set_irqchip_state = qcom_pdc_gic_set_irqchip_state,
.irq_retrigger = irq_chip_retrigger_hierarchy,
.irq_set_type = qcom_pdc_gic_set_type,
+ .irq_set_wake = qcom_pdc_gic_set_wake,
.flags = IRQCHIP_MASK_ON_SUSPEND |
- IRQCHIP_SET_TYPE_MASKED |
- IRQCHIP_SKIP_SET_WAKE,
+ IRQCHIP_SET_TYPE_MASKED,
.irq_set_vcpu_affinity = irq_chip_set_vcpu_affinity_parent,
.irq_set_affinity = irq_chip_set_affinity_parent,
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
The gpio can be marked for wakeup and drivers can invoke disable_irq()
during suspend, in such cases unlazy approach will also disable at HW
and such gpios will not wakeup device from suspend to RAM.
Remove irq_disable callback to allow gpio interrupts to lazy disabled.
The gpio interrupts will get disabled during irq_mask callback.
Signed-off-by: Maulik Shah <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 83b7d64..2419023 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -815,18 +815,6 @@ static void msm_gpio_irq_enable(struct irq_data *d)
msm_gpio_irq_clear_unmask(d, true);
}
-static void msm_gpio_irq_disable(struct irq_data *d)
-{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
-
- if (d->parent_data)
- irq_chip_disable_parent(d);
-
- if (!test_bit(d->hwirq, pctrl->skip_wake_irqs))
- msm_gpio_irq_mask(d);
-}
-
static void msm_gpio_irq_unmask(struct irq_data *d)
{
msm_gpio_irq_clear_unmask(d, false);
@@ -1146,7 +1134,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
pctrl->irq_chip.name = "msmgpio";
pctrl->irq_chip.irq_enable = msm_gpio_irq_enable;
- pctrl->irq_chip.irq_disable = msm_gpio_irq_disable;
pctrl->irq_chip.irq_mask = msm_gpio_irq_mask;
pctrl->irq_chip.irq_unmask = msm_gpio_irq_unmask;
pctrl->irq_chip.irq_ack = msm_gpio_irq_ack;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
On Fri, May 22, 2020 at 3:20 PM Maulik Shah <[email protected]> wrote:
> pinctrl: qcom: Remove irq_disable callback from msmgpio irqchip
> pinctrl: qcom: Add msmgpio irqchip flags
For these two:
Acked-by: Linus Walleij <[email protected]>
so the irqchip maintainers can merge them.
But you ideally also need Björn's ACKs.
Yours,
Linus Walleij