LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Marc Zyngier <marc.zyngier@arm.com>
To: Lina Iyer <ilina@codeaurora.org>
Cc: swboyd@chromium.org, evgreen@chromium.org,
	linus.walleij@linaro.org, bjorn.andersson@linaro.org,
	rplsssn@codeaurora.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, rnayak@codeaurora.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH RESEND RFC 1/4] drivers: pinctrl: qcom: add wakeup capability to GPIO
Date: Thu, 02 Aug 2018 07:08:45 +0100
Message-ID: <86wot9wb9u.wl-marc.zyngier@arm.com> (raw)
In-Reply-To: <20180801194538.GA6422@codeaurora.org>

Hi Lina,

On Wed, 01 Aug 2018 20:45:38 +0100,
Lina Iyer <ilina@codeaurora.org> wrote:
> 
> Thanks for the feedback, Marc.
> 
> On Wed, Aug 01 2018 at 00:31 -0600, Marc Zyngier wrote:
> > On Wed, 01 Aug 2018 03:00:18 +0100,
> > Lina Iyer <ilina@codeaurora.org> wrote:
> >> 
> >> +static irqreturn_t wake_irq_gpio_handler(int irq, void *data)
> >> +{
> >> +	struct irq_data *irqd = data;
> >> +	struct irq_desc *desc = irq_data_to_desc(irqd);
> >> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> >> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> >> +	int irq_pin = irq_find_mapping(gc->irq.domain, irqd->hwirq);
> >> +
> >> +	chained_irq_enter(chip, desc);
> >> +	generic_handle_irq(irq_pin);
> >> +	chained_irq_exit(chip, desc);
> > 
> > That's crazy. I'm not even commenting on the irq handler vs chained
> > irqchip thing, but directly calling into a completely different part
> > of the irq hierarchy makes me feel nauseous,
> > 
> I know the sentiment; I am not too happy about it myself. Explanation
> below.
> 
> > Why isn't the interrupt still pending at the pinctrl level? Looking at
> > the diagram in the cover letter, I'd have hoped that the signal routed
> > to the PDC would wakeup the GIC, but that by virtue of being *also*
> > wired to the TLMM, the interrupt would be handled via the normal path.
> > 
> The short answer: TLMM is not active to sense a wakeup interrupt.

I get that bit. See below for the bit I don't get.

> When we enter system sleep mode, the TLMM and the GIC are powered off
> and the PDC is the only powered-on interrupt controller. The IRQs
> configured at the PDC are the only ones capable of waking the system.
> Upon sensing the interrupt, the PDC intiates a system wakeup and replays
> the interrupt to the GIC. The GIC relays that to AP. Unfortunately, the
> interrupts from the GPIO do not trigger the TLMM summary line. Therefore
> this handler needs to figure out what GPIO caused the wakeup and notify
> the corresponding driver.

That's most bizarre. What causes the TLMM output line not to reflect
the fact that an input is asserted? I understand that it has been
turned off, but surely the PDC wakes it up at the same time as the
GIC, and it should realise that there is something pending...

> 
> > Why isn't that the case? And if that's because the HW is broken and
> > doesn't buffer edge interrupts, why can't you use the resend mechanism
> > instead?
> > 
> The PDC hardware can replay the interrupts accurately. However, it will
> replay only the pin and its not the TLMM summary IRQ. The handler here,
> needs to notify the driver that the wakeup interrupt happened and needs
> to take action. If I could trip the summary IRQ in this handler that
> would work too. Can it be done?

So the TLMM has indeed noticed the interrupt and you can read the TLMM
status registers to find out what fired? The question remains: why
isn't it generating any output if it knows something as fired? The
output line is level, right? What you describe would make sense if it
was generating an edge, but that'd really be a terrible design...

As for tripping the summary interrupt, see check_irq_resend(). This
will only work if this summary interrupt is edge.

Thanks,

	M.

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

  parent reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-01  2:00 [PATCH RESEND RFC 0/4] Wakeup GPIO support for SDM845 SoC Lina Iyer
2018-08-01  2:00 ` [PATCH RESEND RFC 1/4] drivers: pinctrl: qcom: add wakeup capability to GPIO Lina Iyer
2018-08-01  6:31   ` Marc Zyngier
2018-08-01 19:45     ` Lina Iyer
2018-08-01 22:36       ` Bjorn Andersson
2018-08-02  2:10         ` Lina Iyer
2018-08-02  6:08       ` Marc Zyngier [this message]
2018-08-02  6:51         ` Lina Iyer
2018-08-02  7:27           ` Marc Zyngier
2018-08-02 12:58             ` Lina Iyer
2018-08-08  6:05               ` Stephen Boyd
2018-08-08  6:26                 ` Marc Zyngier
2018-08-09 17:30                   ` Stephen Boyd
2018-08-10  7:45                     ` Marc Zyngier
2018-08-10 15:06                       ` Stephen Boyd
2018-08-10 16:15                         ` Lina Iyer
2018-08-01  2:00 ` [PATCH RESEND RFC 2/4] drivers: pinctrl: qcom: add wakeup gpio map for sdm845 Lina Iyer
2018-08-01  8:42   ` Marc Zyngier
2018-08-01 20:04     ` Lina Iyer
2018-08-13 19:41       ` Lina Iyer
2018-08-14  9:26         ` Marc Zyngier
2018-08-01  2:00 ` [PATCH RESEND RFC 3/4] arm64: dts: msm: add PDC device bindings " Lina Iyer
2018-08-01  2:00 ` [PATCH RESEND RFC 4/4] arm64: dts: qcom: add wake up interrupts for GPIOs Lina Iyer

Reply instructions:

You may reply publically 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=86wot9wb9u.wl-marc.zyngier@arm.com \
    --to=marc.zyngier@arm.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=evgreen@chromium.org \
    --cc=ilina@codeaurora.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rnayak@codeaurora.org \
    --cc=rplsssn@codeaurora.org \
    --cc=swboyd@chromium.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox