| Message ID | 1598113021-4149-1-git-send-email-mkshah@codeaurora.org |
|---|---|
| Headers | show |
| Series | irqchip: qcom: pdc: Introduce irq_set_wake call | expand |
Quoting Maulik Shah (2020-08-22 09:17:01) > 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. Presumably they're not in use anymore if the kernel has been kexeced and replaced this one? > > Suggested-by: Stephen Boyd <swboyd@chromium.org> > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Maulik Shah (2020-08-22 09:16:59) > Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag to enable/unmask the > wakeirqs during suspend entry. > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org> maybe just squash this with the other patch in this area?
Hi, On Sat, Aug 22, 2020 at 9:17 AM Maulik Shah <mkshah@codeaurora.org> wrote: > > Set IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag to enable/unmask the > wakeirqs during suspend entry. > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > --- > drivers/pinctrl/qcom/pinctrl-msm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Douglas Anderson <dianders@chromium.org>
Hi, On Sat, Aug 22, 2020 at 9:18 AM Maulik Shah <mkshah@codeaurora.org> wrote: > > 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. > > Suggested-by: Stephen Boyd <swboyd@chromium.org> > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > --- > drivers/irqchip/qcom-pdc.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) Reviewed-by: Douglas Anderson <dianders@chromium.org>
On Sat 22 Aug 16:16 UTC 2020, Maulik Shah wrote: > Add irqchip specific flags for msmgpio irqchip to mask non wakeirqs during > suspend and mask before setting irq type. > > Masking before changing type should make sure any spurious interrupt is not > detected during this operation. > This seems like two different problems and both descriptions are thin on details imho. If you're respinning the series I would appreciate if you improved this. Otherwise Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy") > Acked-by: Linus Walleij <linus.walleij@linaro.org> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > --- > drivers/pinctrl/qcom/pinctrl-msm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > index a2567e7..1c23f5c 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > @@ -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) { > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
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 | 41 ++++++++++++++++++++------------------ kernel/irq/debugfs.c | 1 + kernel/irq/pm.c | 7 ++++++- 5 files changed, 47 insertions(+), 27 deletions(-)