linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Zyngier <marc.zyngier@arm.com>
To: Stephen Boyd <swboyd@chromium.org>
Cc: Lina Iyer <ilina@codeaurora.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	linux-tegra@vger.kernel.org,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 0/9] Implement wake event support on Tegra186 and later
Date: Tue, 09 Oct 2018 13:58:34 +0100	[thread overview]
Message-ID: <86pnwjtgxx.wl-marc.zyngier@arm.com> (raw)
In-Reply-To: <153898289355.119890.15326986475278938009@swboyd.mtv.corp.google.com>

On Mon, 08 Oct 2018 08:14:53 +0100,
Stephen Boyd <swboyd@chromium.org> wrote:
> 
> Quoting Lina Iyer (2018-09-25 10:16:05)
> > Thanks Linus, for bringing this to my attention.
> > 
> > Hi Thierry,
> > 
> > On Tue, Sep 25 2018 at 03:57 -0600, Thierry Reding wrote:
> > >On Tue, Sep 25, 2018 at 10:48:39AM +0200, Linus Walleij wrote:
> > >> Hi Thierry,
> > >>
> > >> thanks for working on the wakeup business!
> > >>
> > >> This patch gets me a bit confused on our different approaches
> > >> toward wakeups in the kernel, so I included Lina, Marc and Ulf
> > >> to see if we can get some common understanding.
> > >>
> > >> On Fri, Sep 21, 2018 at 12:25 PM Thierry Reding
> > >> <thierry.reding@gmail.com> wrote:
> > >>
> > >> > The following is a set of patches that allow certain interrupts to be
> > >> > used as wakeup sources on Tegra186 and later. To implement this, each
> > >> > of the GPIO controllers' IRQ domain needs to become hierarchical, and
> > >> > parented to the PMC domain. The PMC domain in turn implements a new
> > >> > IRQ domain that is a child to the GIC IRQ domain.
> > >> >
> > >> > The above ensures that the interrupt chip implementation of the PMC is
> > >> > called at the correct time. The ->irq_set_type() and ->irq_set_wake()
> > >> > implementations program the PMC wake registers in a way to enable the
> > >> > given interrupts as wakeup sources.
> > >> >
> > >> > This is based on a suggestion from Thomas Gleixner that resulted from
> > >> > the following thread:
> > >> >
> > >> >         https://lkml.org/lkml/2018/9/13/1042
> [...]
> > >
> > >Yes, there was some good discussion in that thread which helped me come
> > >up with this solution. I think it's pretty elegant because it allows all
> > >of this interaction to happen almost automatically via the existing
> > >infrastructure. I'm not sure the same could be applied to the PDC,
> > >though, because of the need to manually replay the interrupt. That's not
> > >something I think can be done with just the simple parent/child
> > >relationship that we use on Tegra.
> > >
> > I wasn't able to use the hierarchy because not all GPIOs and the summary
> > line are routed to the PDC. But I am exploring options of hierarchy as
> > well.
> > 
> 
> From reading this thread (and https://lkml.org/lkml/2018/9/17/756) it
> looks almost exactly the same. The only difference is that Nvidia Tegra
> does the replay in hardware whereas Qualcomm SDM845 decided to replay
> the irq in software. Either way, the gpio controller has two parent
> domains, one is wakeup capable (PDC or PMC) and the other is not (GIC)
> and some wakeup capable irqs only go through the PDC/PMC and then to the
> GIC (e.g. RTC) instead of through gpio first. And it sounds like not all
> gpios are wakeup capable in both designs.
> 
> The plan to have the gpio to wakeup capable irq map live in DT for the
> PMC sounds good too. That way, the wakeup domain alloc function can
> figure things out and redirect gpios by itself while the gpio controller
> doesn't need to do anything special besides ask for wakeup to be set and
> fail if the parent can't support it.
> 
> Can hierarchical irq domains entirely replace the chained irqchip code
> in gpiolib? That would be interesting.

I'm not convinced this is generally doable. Most GPIO blocks multiplex
the signalling on a single parent interrupt, meaning that although you
may be able to have a hierarchy extending to that point, it can't go
any further (at which point you're back into chained-irq land). It
doesn't mean it invalidates the above design, but it probably requires
a bit of flexibility.

I must admit having slightly lost track of the intricacies of the QC
design, but we already have a set of interrupt controllers whose sole
task is to generate wake-up events. They are well behaved though, in
the sense that they will regenerate edges that the QC HW drops on the
floor.

The main issue I can see is that the QC HW relies on some signal other
than the normal interrupt we can handle, and this completely breaks
the very notion of a hierarchy. You need some "side-band signalling"
which will re-inject the lost edges. That, on its own, is a bit of a
deal-breaker.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

      reply	other threads:[~2018-10-09 12:58 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-21 10:25 [PATCH 0/9] Implement wake event support on Tegra186 and later Thierry Reding
2018-09-21 10:25 ` [PATCH 1/9] dt-bindings: tegra186-pmc: Add interrupt controller properties Thierry Reding
2018-10-15 14:47   ` Rob Herring
2018-09-21 10:25 ` [PATCH 2/9] soc/tegra: pmc: Add Tegra194 support Thierry Reding
2018-09-21 10:25 ` [PATCH 3/9] soc/tegra: pmc: Add wake event support Thierry Reding
2018-09-21 10:35   ` Mikko Perttunen
2018-09-21 10:25 ` [PATCH 4/9] soc/tegra: pmc: Add initial Tegra186 wake events Thierry Reding
2018-09-21 10:25 ` [PATCH 5/9] soc/tegra: pmc: Add initial Tegra194 " Thierry Reding
2018-09-21 10:25 ` [PATCH 6/9] gpio: Add support for hierarchical IRQ domains Thierry Reding
2018-09-25  8:11   ` Linus Walleij
2018-09-25  9:33     ` Thierry Reding
2018-09-25 10:33       ` Linus Walleij
2018-09-25 11:17         ` Thierry Reding
2018-10-03  7:52           ` Linus Walleij
2018-09-21 10:25 ` [PATCH 7/9] dt-bindings: tegra186-gpio: Add wakeup parent support Thierry Reding
2018-09-21 10:37   ` Mikko Perttunen
2018-10-15 14:46   ` Rob Herring
2018-11-28 10:44     ` Thierry Reding
2018-09-21 10:25 ` [PATCH 8/9] gpio: tegra186: Rename flow variable to type Thierry Reding
2018-09-21 10:25 ` [PATCH 9/9] gpio: tegra186: Implement wake event support Thierry Reding
2018-09-25  8:48 ` [PATCH 0/9] Implement wake event support on Tegra186 and later Linus Walleij
2018-09-25  9:57   ` Thierry Reding
2018-09-25 17:16     ` Lina Iyer
2018-10-08  7:14       ` Stephen Boyd
2018-10-09 12:58         ` Marc Zyngier [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86pnwjtgxx.wl-marc.zyngier@arm.com \
    --to=marc.zyngier@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=ilina@codeaurora.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=swboyd@chromium.org \
    --cc=tglx@linutronix.de \
    --cc=thierry.reding@gmail.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).