From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [regression, bisected] adb trackpad disappears after suspend to ram Date: Wed, 3 Jun 2009 22:00:54 +0200 Message-ID: <200906032200.55563.rjw@sisk.pl> References: <878wkl6vce.fsf@scholz.fias.uni-frankfurt.de> <200906031202.28916.rjw@sisk.pl> <877hzty0hm.fsf@scholz.fias.uni-frankfurt.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4073945655369946166==" Return-path: In-Reply-To: <877hzty0hm.fsf@scholz.fias.uni-frankfurt.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Mime-version: 1.0 Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org To: Jan Scholz Cc: Adrian Bunk , linux-kernel@vger.kernel.org, Johannes Berg , Ingo Molnar , pm list List-Id: linux-pm@vger.kernel.org --===============4073945655369946166== Content-Type: multipart/alternative; boundary="Boundary-00=_3ZtJKDr6lTvqp5q" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-00=_3ZtJKDr6lTvqp5q Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit On Wednesday 03 June 2009, Jan Scholz wrote: > "Rafael J. Wysocki" writes: > > > On Tuesday 02 June 2009, Jan Scholz wrote: > >> "Rafael J. Wysocki" writes: > >> > >> > On Monday 01 June 2009, Jan Scholz wrote: > >> >> "Rafael J. Wysocki" writes: > >> >> > >> >> > On Friday 29 May 2009, Jan Scholz wrote: > >> >> >> Benjamin Herrenschmidt writes: > >> >> >> > >> >> >> >> We are too late in the cycle to revert this commit and it > > really is needed to > >> >> >> >> fix a more serious issue. Nevertheless knowing that it caused > > the problem to > >> >> >> >> appear on your system is also important. > >> >> >> >> > >> >> >> >> Ben, do you have an idea what may be going on here? Does > > __disable_irq() mask > >> >> >> >> the interrupt on this platform? > >> >> >> > > >> >> >> > I suppose so :-) I'll have to check what's going on, it's not > >> >> >> > immediately clear to me. > >> >> >> > > >> >> >> > Cheers, > >> >> >> > Ben. > >> >> >> > > >> >> >> >> > >> >> >> >> I'd like to see a boot log, preferably containing a > > suspend-resume in which > >> >> >> >> the problem was reproduced. > >> >> >> > >> >> >> Here is a log from booting, through several suspend cycles, until > > the > >> >> >> trackpad disappeared. The first few suspends were done while X was > >> >> >> running but from the console. As you can tell from lines like > >> >> >> May 28 23:51:26 [kernel] adb devices: [2]: 2 c4 [3]: 3 1 [7]: 7 1f > >> >> >> the trackpad was still present. The last suspend was done from > > within X, > >> >> >> here the trackpad did not make it. > >> >> >> May 28 23:58:09 [kernel] adb devices: [2]: 2 c4 [7]: 7 1f > >> >> >> However, there have been cases, although not in this log, were the > >> >> >> trackpad has been alive even when suspending from within X. > >> >> > > >> >> > This means the problem is probably timing-related. > >> >> > > >> >> > Can you please try to comment out suspend_device_irqs() and > >> >> > resume_device_irqs() in drivers/base/power/main.c and see if that > > changes > >> >> > anything? It's not entirely safe (well, that's why the calls are > > there after > >> >> > all), but hopefully the box won't hang during this test. > >> >> > > >> >> > >> >> Tried that against v2.6.30-rc7 and it seems to fix the issue. I did > > ~10 > >> >> suspend-resume cycles from the console (which worked just like > > before) > >> >> and ~20 cycles from within X and the trackpad is still alive. > >> > > >> > So, it seems we lose and interrupt during resume and that confuses the > >> > ADB controller driver or something like this. Do you use the keyboard > > or > >> > the trackpad as a wake-up device? > >> > >> I do the wakeup by pressing keys on the keyboard. I'd try wakeup via the > >> trackpad's button, but I have no idea how to activate that. > >> > >> > Please additionally try to go back to the original code, put > >> > 'sleepy_trackpad = 1' at the beginning of do_adb_reset_bus() in > >> > drivers/macintosh/adb.c and see if the problem is reproducible with > > that. > >> > >> Tried this, but it does not help. > >> > >> What I haven't noticed before is, that even if the trackpad already > >> disappeared, an additional suspend-resume cycle from the console brought > >> it back to life every time. Sometimes this worked from within X as well, > >> just not with such a high probability as from the console. > >> However, this is independent from 'sleepy_trackpad = 1'. > > > > OK, the patch is below, but I haven't had the opportunity to test it yet. > > > > Can you please check if it helps, on top of 2.6.30-rc8? > > > > Best, > > Rafael > > > > --- > > kernel/irq/manage.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > Index: linux-2.6/kernel/irq/manage.c > > =================================================================== > > --- linux-2.6.orig/kernel/irq/manage.c > > +++ linux-2.6/kernel/irq/manage.c > > @@ -187,9 +187,9 @@ static inline int setup_affinity(unsigne > > void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend) > > { > > if (suspend) { > > - if (!desc->action || (desc->action->flags & IRQF_TIMER)) > > - return; > > - desc->status |= IRQ_SUSPENDED; > > + if (desc->action && !(desc->action->flags & IRQF_TIMER)) > > + desc->status |= IRQ_SUSPENDED | IRQ_DISABLED; > > + return; > > } > > > > if (!desc->depth++) { > > @@ -250,8 +250,12 @@ EXPORT_SYMBOL(disable_irq); > > > > void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume) > > { > > - if (resume) > > + if (resume) { > > desc->status &= ~IRQ_SUSPENDED; > > + if (!desc->depth) > > + desc->status &= ~IRQ_DISABLED; > > + return; > > + } > > > > switch (desc->depth) { > > case 0: > > Hi Rafael, > > I tried your patch, but it makes my box crash on wakeup. It even reset > the hwclock which normally only happens with completely empty battery > and without AC. Ouch, sorry. In that case I have no idea why it breaks. Johannes also reported a problem with MacBook that was most probably introduced by the suspend reordering changes, but that one related to hibernation. Best, Rafael --Boundary-00=_3ZtJKDr6lTvqp5q Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: 7bit
On Wednesday 03 June 2009, Jan Scholz wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>
> > On Tuesday 02 June 2009, Jan Scholz wrote:
> >> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> >>
> >> > On Monday 01 June 2009, Jan Scholz wrote:
> >> >> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> >> >>
> >> >> > On Friday 29 May 2009, Jan Scholz wrote:
> >> >> >> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> >> >> >>
> >> >> >> >> We are too late in the cycle to revert this commit and it
> > really is needed to
> >> >> >> >> fix a more serious issue. Nevertheless knowing that it caused
> > the problem to
> >> >> >> >> appear on your system is also important.
> >> >> >> >>
> >> >> >> >> Ben, do you have an idea what may be going on here? Does
> > __disable_irq() mask
> >> >> >> >> the interrupt on this platform?
> >> >> >> >
> >> >> >> > I suppose so :-) I'll have to check what's going on, it's not
> >> >> >> > immediately clear to me.
> >> >> >> >
> >> >> >> > Cheers,
> >> >> >> > Ben.
> >> >> >> >
> >> >> >> >>
> >> >> >> >> I'd like to see a boot log, preferably containing a
> > suspend-resume in which
> >> >> >> >> the problem was reproduced.
> >> >> >>
> >> >> >> Here is a log from booting, through several suspend cycles, until
> > the
> >> >> >> trackpad disappeared. The first few suspends were done while X was
> >> >> >> running but from the console. As you can tell from lines like
> >> >> >> May 28 23:51:26 [kernel] adb devices: [2]: 2 c4 [3]: 3 1 [7]: 7 1f
> >> >> >> the trackpad was still present. The last suspend was done from
> > within X,
> >> >> >> here the trackpad did not make it.
> >> >> >> May 28 23:58:09 [kernel] adb devices: [2]: 2 c4 [7]: 7 1f
> >> >> >> However, there have been cases, although not in this log, were the
> >> >> >> trackpad has been alive even when suspending from within X.
> >> >> >
> >> >> > This means the problem is probably timing-related.
> >> >> >
> >> >> > Can you please try to comment out suspend_device_irqs() and
> >> >> > resume_device_irqs() in drivers/base/power/main.c and see if that
> > changes
> >> >> > anything? It's not entirely safe (well, that's why the calls are
> > there after
> >> >> > all), but hopefully the box won't hang during this test.
> >> >> >
> >> >>
> >> >> Tried that against v2.6.30-rc7 and it seems to fix the issue. I did
> > ~10
> >> >> suspend-resume cycles from the console (which worked just like
> > before)
> >> >> and ~20 cycles from within X and the trackpad is still alive.
> >> >
> >> > So, it seems we lose and interrupt during resume and that confuses the
> >> > ADB controller driver or something like this. Do you use the keyboard
> > or
> >> > the trackpad as a wake-up device?
> >>
> >> I do the wakeup by pressing keys on the keyboard. I'd try wakeup via the
> >> trackpad's button, but I have no idea how to activate that.
> >>
> >> > Please additionally try to go back to the original code, put
> >> > 'sleepy_trackpad = 1' at the beginning of do_adb_reset_bus() in
> >> > drivers/macintosh/adb.c and see if the problem is reproducible with
> > that.
> >>
> >> Tried this, but it does not help.
> >>
> >> What I haven't noticed before is, that even if the trackpad already
> >> disappeared, an additional suspend-resume cycle from the console brought
> >> it back to life every time. Sometimes this worked from within X as well,
> >> just not with such a high probability as from the console.
> >> However, this is independent from 'sleepy_trackpad = 1'.
> >
> > OK, the patch is below, but I haven't had the opportunity to test it yet.
> >
> > Can you please check if it helps, on top of 2.6.30-rc8?
> >
> > Best,
> > Rafael
> >
> > ---
> > kernel/irq/manage.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > Index: linux-2.6/kernel/irq/manage.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/irq/manage.c
> > +++ linux-2.6/kernel/irq/manage.c
> > @@ -187,9 +187,9 @@ static inline int setup_affinity(unsigne
> > void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
> > {
> > if (suspend) {
> > - if (!desc->action || (desc->action->flags & IRQF_TIMER))
> > - return;
> > - desc->status |= IRQ_SUSPENDED;
> > + if (desc->action && !(desc->action->flags & IRQF_TIMER))
> > + desc->status |= IRQ_SUSPENDED | IRQ_DISABLED;
> > + return;
> > }
> >
> > if (!desc->depth++) {
> > @@ -250,8 +250,12 @@ EXPORT_SYMBOL(disable_irq);
> >
> > void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume)
> > {
> > - if (resume)
> > + if (resume) {
> > desc->status &= ~IRQ_SUSPENDED;
> > + if (!desc->depth)
> > + desc->status &= ~IRQ_DISABLED;
> > + return;
> > + }
> >
> > switch (desc->depth) {
> > case 0:
>
> Hi Rafael,
>
> I tried your patch, but it makes my box crash on wakeup. It even reset
> the hwclock which normally only happens with completely empty battery
> and without AC.


Ouch, sorry.


In that case I have no idea why it breaks.


Johannes also reported a problem with MacBook that was most probably introduced
by the suspend reordering changes, but that one related to hibernation.


Best,
Rafael


--Boundary-00=_3ZtJKDr6lTvqp5q-- --===============4073945655369946166== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --===============4073945655369946166==--