Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
To:     Lina Iyer <ilina@codeaurora.org>
From:   Stephen Boyd <swboyd@chromium.org>
In-Reply-To: <20181011002958.2597-2-ilina@codeaurora.org>
Cc:     linux-kernel@vger.kernel.org, evgreen@chromium.org,
        marc.zyngier@arm.com
References: <20181011002958.2597-1-ilina@codeaurora.org>
 <20181011002958.2597-2-ilina@codeaurora.org>
Message-ID: <154096954339.98144.12348474096990107321@swboyd.mtv.corp.google.com>
User-Agent: alot/0.7
Subject: Re: [PATCH RFC 1/1] drivers: pinctrl: qcom: add wakeup capability to GPIO
Date:   Wed, 31 Oct 2018 00:05:43 -0700
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Lina,

Quoting Lina Iyer (2018-10-10 17:29:58)
> QCOM SoC's that have Power Domain Controller (PDC) chip in the always-on
> domain can wakeup the SoC, when interrupts and GPIOs are routed to its
> interrupt controller. Only select GPIOs that are deemed wakeup capable
> are routed to specific PDC pins. During low power state, the pinmux
> interrupt controller may be non-functional but the PDC would be. The PDC
> can detect the wakeup GPIO is triggered and bring the TLMM to an
> operational state.
> =

> Interrupts that are level triggered will be detected at the TLMM when
> the controller becomes operational. Edge interrupts however need to be
> replayed again.
> =

> Request the corresponding PDC IRQ, when the GPIO is requested as an IRQ,
> but keep it disabled. During suspend, we can enable the PDC IRQ instead
> of the GPIO IRQ, which may or not be detected.
> =

> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---

I spoke with Marc about this at ELCE last week and I think we came up
with a plan for how to make this all work while keeping the TLMM driver
largely unaware of what's happening. One important point that we need to
keep in mind is that we don't want the interrupt to appear twice at the
GIC for the same TLMM irq, once in TLMM and once in PDC. That situation
would be quite bad and confuse things.

So the plan is as follows:

 1) Split PDC irqdomain into two domains. One for GIC interrupts and one
    for TLMM interrupts

 2) Support hierarchical irq domains in the TLMM driver and/or gpiolib

 3) Stack the irq controllers into a hierarchy so that GIC is the parent
    of PDC and PDC is the parent of TLMM while also leaving the TLMM
    summary IRQ chained directly from the GIC irqdomain

 4) Mask a TLMM irq in the TLMM driver and unmask the PDC irq in the PDC
    driver when an irq_set_wake() call is made on a TLMM gpio irq that
    PDC can monitor (and vice versa)

 5) Have the PDC driver be the only driver that knows if a TLMM pin is
    wakeup capable or not expressed in some DT table

Implementing #4 will require changing TLMM irqchip to call
irq_chip_set_wake_parent() for the .irq_set_wake op and then checking
the return value to figure out if PDC is monitoring the pin or not and
doing the mask/unmask dance. Furthermore, implementing #2 will be a bit
of work that Thierry has already started for Nvidia Tegra pinctrl
drivers.

The important part of #4 is that the irq can only happen in TLMM or in
PDC, and not in both places because of how we mask and unmask in
different drivers. The TLMM summary irq line can't be triggered when the
PDC is monitoring the line.

This also limits the usage of PDC to pins that are really waking up the
system from some sort of sleep mode. Either the device driver is trying
to wakeup from suspend so it's setting irq wake in the driver suspend
path, or it's runtime suspending a device and wanting to wakeup from
that device suspend state so it's again setting irq wake in runtime
suspend. Either way, in "normal" active modes the irq path will be
through the TLMM summary irq line and not through PDC. I don't know if
a similar design would work for pre-PDC qcom hardware where the MPM is
handled by RPM software.

And finally, thinking about this after writing this mail I'm a little
concerned that doing any sort of mask/unmask or irq movement from TLMM
to PDC at runtime will miss edges for edge triggered interrupts. Is that
also your concern? How is this handled with MPM? We would leave the TLMM
interrupt enabled in that case and then have to replay the edge in
software when resuming from suspend or idle paths but otherwise ignore
the level type interrupts that MPM sees? I suppose MPM never gets an
edge if TLMM gets the edge so this just works.

I'm worried that when we're moving the gpio irq line from the summary
GIC SPI line to the PDC's GIC SPI line we'll get an edge and then miss
it in TLMM and be too late unmasking it in PDC or worse see it in PDC
and TLMM because we unmask in PDC before masking in TLMM. So we may need
to change #4 up above to always allocate the irq from PDC and somehow
communicate that the irq is wakeup capable in PDC back to the TLMM
driver so TLMM knows to keep the irq masked in the hardware forever.
That way it can't cause the summary irq line to trigger in addition to
the PDC one. Given that we have allocation hooks with domain hierarchy
it may be easy enough to remove TLMM irqs from the summary irq domain
when they can be allocated from the parent PDC domain.