Re: [PATCH v2 0/5] Wakeup GPIO support for SDM845 SoC

[Lina Iyer]

Please ignore this series. The series is incorrectly marked as v2. I am
resending it as v1.

On Fri, Aug 17 2018 at 10:39 -0600, Lina Iyer wrote:
> Hi,
>
> Changes in v1:
> 	- Avoid GPIO-PDC map in .c file
> 	- Trigger GPIO by writing to the hardware
> 	- Hooked up to suspend/resume callbacks
> 	- Dropped PDC DT bindings (see dependencies)
>
> Dependencies:
> 	https://lkml.org/lkml/2018/8/17/137
> 	https://lkml.org/lkml/2018/8/15/289
>
> This is an attempt at a solution to enable wake up from suspend and deep idle
> using GPIO as a wakeup source. The 845 uses a new interrupt controller (PDC)
> that lies in the always-on domain and can sense interrupts that are routed to
> it, when the GIC is powered off. It would then wakeup the GIC and replay the
> interrupt which would then be relayed to the AP. The PDC interrupt controller
> driver is merged upstream [1],[2]. The following set of patches extends the
> wakeup capability to GPIOs using the PDC. The TLMM pinctrl driver for the SoC
> available at [3].
>
> The complexity with the solution stems from the fact that only a selected few
> GPIO lines are routed to the PDC in addition the TLMMs. They are also from
> different banks on the pinctrl and the TLMM summary line is not routed to the
> PDC. Hence the PDC cannot be considered as parent of the TLMM irqchip (or can
> we ?). This is what it looks like -
>
>   [ PIN ] -----[ TLMM ]---------------> [ GIC ] ---> [ CPU ]
>      |                  <summary IRQ>       ^
>      |                                      |
>      ----------------------------------> [ PDC ]
>                <wakeup IRQs>
>
> I had a brief discussion with Linus on this and the idea implemented is based
> on his suggestion.
>
> When an IRQ (let's call this latent IRQ) for a GPIO is requested, the
> ->irq_request_resources() is used by the TLMM driver to request a PDC pin. The
> PDC pin associated with the GPIO is read from a  static map available in the
> pinctrl-sdm845.c. (I think there should be a better location than a static map,
> more on that later). Knowing the PDC pin from the map, we could look up the DT
> bindings and request the PDC interrupt with the same trigger mask as the
> interrupt requested. The ->set_type and ->set_wake are also trapped to set the
> PDC IRQ's polarity and enable it when the latent IRQ is requested. When the PDC
> detects the interrupt at suspend, it wakes up the GIC and replays the wakeup
> IRQ. The GPIO handler function for the latent IRQ is invoked in turn.
>
> Please review these patches and your inputs would be greatly appreciated and
> (kindly) let me know if I have committed any blunders with this approach. There
> is definitely opportunity to improve the location of the static GPIO-PDC pin
> map. We could possibly put it as an data argument in the interrupts definition
> of the PDC or with interrupt names. Also, I am still sorting out some issues
> with the IRQ handling part of these patches. And I am unsure of how to set the
> polarity of the PDC pin without locking, since we are not in hierarchy with the
> PDC interrupt controller. Again, your inputs on these would be greatly helpful.
>
> Thanks,
> Lina
>
> [1]. drivers/irqchip/qcom-pdc.c
> [2]. Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> [3]. drivers/pinctrl/qcom/pinctrl-msm.c
>
> Lina Iyer (5):
>  drivers: pinctrl: qcom: add wakeup capability to GPIO
>  dt-bindings: pinctrl: add wakeup capable GPIOs for SDM845
>  drivers: pinctrl: msm: enable PDC interrupt only during suspend
>  drivers: pinctrl: qcom: sdm845: support GPIO wakeup from suspend
>  arm64: dts: qcom: add wake up interrupts for GPIOs for SDM845
>
> .../bindings/pinctrl/qcom,sdm845-pinctrl.txt  |  58 ++++++-
> arch/arm64/boot/dts/qcom/sdm845.dtsi          |  57 ++++++-
> drivers/pinctrl/qcom/pinctrl-msm.c            | 155 ++++++++++++++++++
> drivers/pinctrl/qcom/pinctrl-msm.h            |   3 +
> drivers/pinctrl/qcom/pinctrl-sdm845.c         |   6 +
> 5 files changed, 275 insertions(+), 4 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project