Hi Lina, On Fri, Aug 24, 2018 at 02:01:55PM -0600, Lina Iyer wrote: > During suspend the system may power down some of the system rails. As a > result, the TLMM hw block may not be operational anymore and wakeup > capable GPIOs will not be detected. The PDC however will be operational > and the GPIOs that are routed to the PDC as IRQs can wake the system up. > > To avoid being interrupted twice (for TLMM and once for PDC IRQ) when a > GPIO trips, use TLMM for active and switch to PDC for suspend. When > entering suspend, disable the TLMM wakeup interrupt and instead enable > the PDC IRQ and revert upon resume. > > Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx> > --- > Changes in v2: > - Fix PDC IRQ max port, 126 is the max supported in h/w > - Use PDC hwirq in bitmap, linux numbers could be large > - Setup DISABLE_UNLAZY for both TLMM and PDC IRQs > --- > drivers/pinctrl/qcom/pinctrl-msm.c | 70 +++++++++++++++++++++++++++++- > drivers/pinctrl/qcom/pinctrl-msm.h | 3 ++ > 2 files changed, 72 insertions(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > index b675ea56a4ff..a880cefbd248 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > @@ -37,6 +37,7 @@ > #include "../pinctrl-utils.h" > > #define MAX_NR_GPIO 300 > +#define MAX_PDC_HWIRQ 126 > #define PS_HOLD_OFFSET 0x820 > > /** > @@ -51,6 +52,7 @@ > * @enabled_irqs: Bitmap of currently enabled irqs. > * @dual_edge_irqs: Bitmap of irqs that need sw emulated dual edge > * detection. > + * @pdc_hwirqs: Bitmap of wakeup capable irqs. > * @soc; Reference to soc_data of platform specific data. > * @regs: Base address for the TLMM register map. > */ > @@ -68,11 +70,15 @@ struct msm_pinctrl { > > DECLARE_BITMAP(dual_edge_irqs, MAX_NR_GPIO); > DECLARE_BITMAP(enabled_irqs, MAX_NR_GPIO); > + DECLARE_BITMAP(pdc_hwirqs, MAX_PDC_HWIRQ); > > const struct msm_pinctrl_soc_data *soc; > void __iomem *regs; > + struct irq_domain *pdc_irq_domain; > }; > > +static bool in_suspend; > + > static int msm_get_groups_count(struct pinctrl_dev *pctldev) > { > struct msm_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > @@ -787,8 +793,13 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on) > > raw_spin_lock_irqsave(&pctrl->lock, flags); > > - if (pdc_irqd) > + if (pdc_irqd && !in_suspend) { > irq_set_irq_wake(pdc_irqd->irq, on); > + if (on) > + set_bit(pdc_irqd->hwirq, pctrl->pdc_hwirqs); > + else > + clear_bit(pdc_irqd->hwirq, pctrl->pdc_hwirqs); > + } > > irq_set_irq_wake(pctrl->irq, on); > > @@ -919,7 +930,12 @@ static int msm_gpio_pdc_pin_request(struct irq_data *d) > } > > irq_set_handler_data(d->irq, irq_get_irq_data(irq)); > + irq_set_handler_data(irq, d); > + irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY); > + irq_set_status_flags(d->irq, IRQ_DISABLE_UNLAZY); > disable_irq(irq); > + if (!pctrl->pdc_irq_domain) > + pctrl->pdc_irq_domain = irq_get_irq_data(irq)->domain; > > return 0; > } > @@ -1069,6 +1085,58 @@ static void msm_pinctrl_setup_pm_reset(struct msm_pinctrl *pctrl) > } > } > > +int __maybe_unused msm_pinctrl_suspend_late(struct device *dev) > +{ > + struct msm_pinctrl *pctrl = dev_get_drvdata(dev); > + struct irq_data *irqd; > + unsigned int irq; > + int i; > + > + in_suspend = true; > + for_each_set_bit(i, pctrl->pdc_hwirqs, MAX_PDC_HWIRQ) { > + irq = irq_find_mapping(pctrl->pdc_irq_domain, i); > + irqd = irq_get_handler_data(irq); > + /* > + * We don't know if the TLMM will be functional > + * or not, during the suspend. If its functional, > + * we do not want duplicate interrupts from PDC. > + * Hence disable the GPIO IRQ and enable PDC IRQ. > + */ > + if (irqd_is_wakeup_set(irqd)) { > + disable_irq_wake(irqd->irq); > + disable_irq(irqd->irq); > + enable_irq(irq); > + } Would it make sense to limit this to edge triggered interrupts since the interrupt handler does nothing for level triggered ones? Cheers Matthias