linux-kernel - Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120906175128.35fa076a@notabene.brown>
Date:	Thu, 6 Sep 2012 17:51:28 +1000
From:	NeilBrown <neilb@...e.de>
To:	"Shilimkar, Santosh" <santosh.shilimkar@...com>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	Tarun Kanti DebBarma <tarun.kanti@...com>,
	Kevin Hilman <khilman@...com>,
	Tony Lindgren <tony@...mide.com>, Benoit <b-cousson@...com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Felipe Balbi <balbi@...com>, linux-omap@...r.kernel.org,
	lkml <linux-kernel@...r.kernel.org>,
	Jon Hunter <jon-hunter@...com>
Subject: Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.

On Thu, 6 Sep 2012 12:57:46 +0530 "Shilimkar, Santosh"
<santosh.shilimkar@...com> wrote:

> On Thu, Sep 6, 2012 at 12:32 PM, NeilBrown <neilb@...e.de> wrote:
> > On Thu, 6 Sep 2012 11:18:09 +0530 "Shilimkar, Santosh"
> > <santosh.shilimkar@...com> wrote:
> >
> >> On Thu, Sep 6, 2012 at 8:35 AM, NeilBrown <neilb@...e.de> wrote:
> >> > On Mon, 3 Sep 2012 22:59:06 -0700 "Shilimkar, Santosh"
> >> > <santosh.shilimkar@...com> wrote:
> >
> >> >> After thinking bit more on this, the problem seems to be coming
> >> >> mainly because the gpio device is runtime suspended bit early than
> >> >> it should be. Similar issue seen with i2c driver as well. The i2c issue
> >> >> was discussed with Rafael at LPC last week. The idea is to move
> >> >> the pm_runtime_enable/disable() calls entirely up to the
> >> >> _late/_early stage of device suspend/resume.
> >> >> Will update this thread once I have further update.
> >> >
> >> > This won't be late enough.  IRQCHIP_MASK_ON_SUSPEND takes effect after all
> >> > the _late callbacks have been called.
> >> > I, too, spoke to Rafael about this in San Diego.  He seemed to agree with me
> >> > that the interrupt needs to be masked in the ->suspend callback.  any later
> >> > is too late.
> >> >
> >> Thanks for information about your discussion. Will wait for the patch then.
> >>
> >> Regards
> >> santosh
> >
> > I already sent a patch - that is what started this thread :-)
> >
> > I include it below.
> > You said "The patch doesn't seems to be correct" but didn't expand on why.
> > Do you still think it is not correct?  I wouldn't be surprised if there is
> > some case that it doesn't handle quite right, but it seems right to me.
> >
> Sorry I though you were talking about moving the "Checking wakeup interrupts"
> bit early as discussed on the follow up of alternate suggested patch to make
> use of  IRQCHIP_MASK_ON_SUSPEND.
> 
> I think we need to fix the issue seen with ' IRQCHIP_MASK_ON_SUSPEND'
> patch. That is at least the expected way to manage suspend and wakeup
> irq masks for a IRQCHIP.

That is what I thought at first too.  But when talking to Rafael he said that
IRQCHIP_MASK_ON_SUSPEND was intended mainly for clock interrupts.  For other
less fundamental interrupts, doing the mask/unmask in suspend/resume
callbacks is sufficient and simpler... and actually works.

IRQCHIP_MASK_ON_SUSPEND is currently used by precisely two drivers:

   arch/arm/mach-omap2/omap-wakeupgen.c
and 
   drivers/mfd/pm8xxx-irq.c

which suggests that it isn't widely used and quite possibly doesn't actually
work in general.
The pm8xxx-irq doesn't seem to do runtime pm, so maybe it manages to work for
that reason.
The omap-wakeupgen code is beyond my current understanding, but it seems like
it might be the sort of special case that IRQCHIP_MASK_ON_SUSPEND is intended
for...

Maybe we need to start a new thread and pester Rafael or Thomas Gleixner
to either explain what is intended for this case, or to fix 
IRQCHIP_MASK_ON_SUSPEND so that it can be used in general.

NeilBrown

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)