lkml.org 
[lkml]   [2014]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V3 3/3] mfd: palmas: Add support for optional wakeup
On Fri, 19 Sep 2014, Nishanth Menon wrote:
> On 08:37-20140919, Thomas Gleixner wrote:
> > The other omap drivers using this have the same issue ... And of
> > course they are subtly different.
> >
> > The uart one handles the actual device interrupt, which is violating
> > the general rule of possible interrupt reentrancy in the pm-runtime
> > case if the two interrupts are affine to two different cores. Yes,
> > it's protected by a lock and works by chance ....
> >
> > The mmc one issues a disable_irq_nosync() in the wakeup irq handler
> > itself.
> >
> > WHY does one driver need that and the other does not? You are not even
> > able to come up with a common scheme for OMAP. I don't want to see the
> > mess others are going to create when this stuff becomes more used.
> >
> > Thanks,
> >
> > tglx
>
> I think I understand your concern - I request Tony to comment about
> this. I mean, I can try and hook things like uart in other drivers
> (like https://patchwork.kernel.org/patch/4759171/ ), but w.r.t overall
> generic usage guideline wise, I would prefer Tony to comment.

No, the uart and that i2c thing are just wrong. Assume the following

device irq affine to cpu0
wakeup irq affine to cpu1

CPU 0 CPU 1

runtime suspend

enable_wake(wakeup irq);

wakeup interrupt is raised device interrupt is raised

dev_handler(device) dev_handler(device)

It might work due to locking, but it is nevertheless wrong. Interrupt
handlers for devices are guaranteed not to be reentrant. And this
brilliant stuff simply violates that guarantee. So, no. It's wrong
even if it happens to work by chance.

Thanks,

tglx


\
 
 \ /
  Last update: 2014-09-19 20:01    [W:0.073 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site