Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Marc Zyngier <marc.zyngier@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Lina Iyer <ilina@codeaurora.org>
Subject: Re: [PATCH v2] genirq: Respect IRQCHIP_SKIP_SET_WAKE in irq_chip_set_wake_parent()
Date: Tue, 26 Mar 2019 09:58:54 -0700
Message-ID: <155361953432.20095.14563397546308928190@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <f53f4556-72f1-051b-0dbe-f1789521233e@arm.com>

Quoting Marc Zyngier (2019-03-26 04:11:56)
> Hi Stephen,
> 
> On 25/03/2019 18:10, Stephen Boyd wrote:
> > This function returns an error if a child irqchip calls
> > irq_chip_set_wake_parent() but its parent irqchip has the
> > IRQCHIP_SKIP_SET_WAKE flag set. Let's return 0 for success here instead
> > because there isn't anything to do.
> > 
> > This keeps the behavior consistent with how set_irq_wake_real() is
> > implemented. That function returns 0 when the irqchip has the
> > IRQCHIP_SKIP_SET_WAKE flag set. It doesn't attempt to walk the chain of
> > parents and set irq wake on any chips that don't have the flag set
> > either. If the intent is to call the .irq_set_wake() callback of the
> > parent irqchip, then we expect irqchip implementations to omit the
> > IRQCHIP_SKIP_SET_WAKE flag and implement an .irq_set_wake() function
> > that calls irq_chip_set_wake_parent().
> > 
> > This fixes a problem on my Qualcomm sdm845 device where I can't set wake
> > on any GPIO interrupts after I apply work in progress wakeup irq patches
> > to the GPIO driver. The chain of chips looks like this:
> > 
> >  ARM GIC (skip) -> QCOM PDC (skip) -> QCOM GPIO
> 
> nit: the parenting chain is actually built the other way around (we
> don't express the 'child' relationship). This doesn't change anything to
> the patch, but would make the reasoning a but easier to understand.

I take it you want the sentence below to say 'parent' instead of 'child'
then?

> 
> > 
> > The GPIO controller is a child of the QCOM PDC irqchip which is a child
> > of the ARM GIC irqchip. The QCOM PDC irqchip has the
> > IRQCHIP_SKIP_SET_WAKE flag set, and so does the grandparent ARM GIC.
> > 
> > The GPIO driver doesn't know if the parent needs to set wake or not, so
> > it unconditionally calls irq_chip_set_wake_parent() causing this
> > function to return a failure because the parent irqchip (PDC) doesn't
> > have the .irq_set_wake() callback set. Returning 0 instead makes
> > everything work and irqs from the GPIO controller can be configured for
> > wakeup.
> > 
> > Cc: Lina Iyer <ilina@codeaurora.org>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> 
> Fixes: 08b55e2a9208e ("genirq: Add irqchip_set_wake_parent")
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> 

I'm happy to resend with the commit text clarified more and the above
tags added.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

      reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-25 18:10 Stephen Boyd
2019-03-26 11:11 ` Marc Zyngier
2019-03-26 16:58   ` Stephen Boyd [this message]

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=155361953432.20095.14563397546308928190@swboyd.mtv.corp.google.com \
    --to=swboyd@chromium.org \
    --cc=ilina@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=tglx@linutronix.de \
    /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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

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

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


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