public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ambx1@neo.rr.com
To: Linus Torvalds <torvalds@osdl.org>
Cc: Dave Airlie <airlied@gmail.com>, Pavel Machek <pavel@ucw.cz>,
	Hugh Dickins <hugh@veritas.com>, Andrew Morton <akpm@osdl.org>,
	Dominik Brodowski <linux@dominikbrodowski.net>,
	Daniel Ritz <daniel.ritz@gmx.ch>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Len Brown <len.brown@intel.com>
Subject: Re: revert yenta free_irq on suspend
Date: Sun, 31 Jul 2005 23:03:25 -0400	[thread overview]
Message-ID: <2e55d42e7427.2e74272e55d4@columbus.rr.com> (raw)

----- Original Message -----
From: Linus Torvalds <torvalds@osdl.org>
Date: Sunday, July 31, 2005 9:07 pm
Subject: Re: revert yenta free_irq on suspend

> 
> 
> On Mon, 1 Aug 2005, Dave Airlie wrote:
> > 
> > You said earlier we only should fix drivers that need fixing, but 
> they> all need fixing
> 
> I think you're still talking from a theoretical standpoing, while 
> all my 
> arguments are practical.
> 
> In _practice_, I hope that
> 
> (a) we don't see that very much (ie the people for whom things 
> work 
>     already are a strong argument that this is less of a problem in
>     practice than people try to make it appear)

They may not always be triggered, but they are real bugs.  We can't
ensure stable PM until they are fixed.  If they weren't a real issue,
then calling free_irq() in pcmcia probably wouldn't have broken the
sound driver we saw in this case.

> 
> (b) drivers, _especially_ on notebooks, are already able to handle 
> an 
>     incoming interrupt with the device in D3 state and returning 0xff
>     for all reads.
> 
>     In particular, this is exactly the same thing that you get on 
> a 
>     surprise device removal too.
> 
> iow it really _really_ shouldn't matter that a shared interrupt 
> comes in
> after (or before) a device has gone to sleep. Because a driver that 
> can'thandle that schenario is buggy for totally unrelated reasons, 
> and doing a 
> "free_irq()/request_irq()" pair at suspend time is _not_ the solution!

I understand this.  But calling free_irq()/request_irq() still seems
to make sense.  It's the cleanest and most straightforward approach.
It's the easiest way we can ensure there will not be race conditions.
An interrupt could be triggered during the device's power transition
when it's in between on and off.  More importantly, we need this change
for runtime power management anyway.

> 
> > This has nothing to do with the issues with highlevel PM interfaces
> > for shutting down hardware, this is do with fixing the drivers in 
> the> kernel currently and what the correct way to do it is without 
> breaking> someone elses hardware....
> 
> ... and I don't think this has _anything_ to do with 
> free_irq/request_irq, 
> and everything to do with the fact that we can try to make at least 
> the 
> common drivers "hardened" for unexpected interrupts coming in when 
> the hw 
> might not be ready for them.

We either need to change every driver to free irqs or "harden" each
of them.  Freeing irqs obviously seems easier.  I propose we make
this change in -mm in one pass to avoid bugs like this.

Also, as I said earlier, the better we support OSPM initiated power
management, the more likely APM will break.  This may be technically
unavoidable on some isolated boxes without quirks.  I agree with
Pavel that "do nothing" may make sense, but it seems some devices
may still need to be disabled by the OS.  As a real world example,
we currently can't turn off cardbus bridges because it breaks APM
on a couple of older laptops.

Thanks,
Adam


             reply	other threads:[~2005-08-01  3:03 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-01  3:03 ambx1 [this message]
2005-08-01  4:53 ` revert yenta free_irq on suspend Linus Torvalds
2005-08-01  8:49 ` Benjamin Herrenschmidt
2005-08-02 10:56   ` Pavel Machek
2005-08-03 11:42     ` Benjamin Herrenschmidt
  -- strict thread matches above, loose matches on Subject: below --
2005-07-31 20:34 ambx1
2005-07-31 21:20 ` Pavel Machek
2005-08-01  8:56   ` Benjamin Herrenschmidt
2005-07-31 22:55 ` Linus Torvalds
2005-07-31 23:05   ` Pavel Machek
2005-07-31 23:24     ` Linus Torvalds
2005-07-31 23:27       ` Pavel Machek
2005-07-31 23:44         ` Linus Torvalds
2005-07-31 23:59           ` Dave Airlie
2005-08-01  0:19             ` Linus Torvalds
2005-08-01  0:44               ` Dave Airlie
2005-08-01  1:07                 ` Linus Torvalds
2005-08-01  7:15                   ` Pavel Machek
2005-08-01  7:01               ` Sanjoy Mahajan
2005-08-01  7:25           ` Pavel Machek
2005-07-31 23:10   ` Dave Airlie
2005-08-01  1:59 ` Shaohua Li
2005-08-01  2:06   ` Andrew Morton
2005-08-01  2:22     ` Shaohua Li
2005-08-01  7:19     ` Pavel Machek
2005-08-01 21:38     ` Rafael J. Wysocki
2005-07-31  5:03 Brown, Len
2005-07-31  5:31 ` Linus Torvalds
2005-07-31  9:49 ` Rafael J. Wysocki
2005-07-31 22:27 ` Dave Jones
2005-08-01  0:00   ` Andreas Steinmetz
2005-08-01  0:06     ` Dave Jones
2005-08-01  0:09       ` Andreas Steinmetz
2005-08-03  9:23   ` Pavel Machek
2005-08-01  8:51 ` Matthew Garrett
2005-07-30 19:10 Hugh Dickins
2005-07-30 20:03 ` Russell King
2005-07-30 20:36   ` Linus Torvalds
2005-07-30 20:54     ` Russell King
2005-07-30 21:10       ` Linus Torvalds
2005-07-30 21:30         ` Russell King
2005-07-30 22:28         ` Rafael J. Wysocki
2005-07-31  4:49           ` Linus Torvalds
2005-08-01  9:06         ` Benjamin Herrenschmidt
2005-07-30 21:20       ` Rafael J. Wysocki
2005-07-30 20:34 ` Linus Torvalds
2005-07-31 13:29   ` Pavel Machek
2005-07-31 15:53     ` Linus Torvalds
2005-07-31 17:09       ` Linus Torvalds
2005-07-30 20:49 ` Rafael J. Wysocki
2005-07-30 21:08   ` Daniel Ritz
2005-07-30 21:32   ` Hugh Dickins
2005-07-30 22:00     ` Rafael J. Wysocki
2005-07-30 22:24       ` Hugh Dickins
2005-07-30 23:09         ` Rafael J. Wysocki
2005-07-31 20:15           ` Rafael J. Wysocki
2005-08-01 20:34             ` Hugh Dickins
2005-08-01 21:54               ` Rafael J. Wysocki

Reply instructions:

You may reply publicly 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=2e55d42e7427.2e74272e55d4@columbus.rr.com \
    --to=ambx1@neo.rr.com \
    --cc=airlied@gmail.com \
    --cc=akpm@osdl.org \
    --cc=daniel.ritz@gmx.ch \
    --cc=hugh@veritas.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@dominikbrodowski.net \
    --cc=pavel@ucw.cz \
    --cc=torvalds@osdl.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox