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 <il...@codeaurora.org>
> ---
> 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

Reply via email to