From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751995AbaGaUM7 (ORCPT ); Thu, 31 Jul 2014 16:12:59 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:39409 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750868AbaGaUM5 (ORCPT ); Thu, 31 Jul 2014 16:12:57 -0400 Date: Thu, 31 Jul 2014 16:12:55 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: "Rafael J. Wysocki" cc: Thomas Gleixner , Peter Zijlstra , , Linux PM list , Dmitry Torokhov Subject: Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interrupts In-Reply-To: <2140085.bSS0bVjyxJ@vostro.rjw.lan> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 31 Jul 2014, Rafael J. Wysocki wrote: > And before we enter the wakeup handling slippery slope, let me make a note > that this problem is bothering me quite a bit at the moment. In my opinion > we need to address it somehow regardless of the wakeup issues and I'm not sure > if failing __setup_irq() when there's a mismatch (that is, there are existing > actions for the given irq_desc and their IRQF_NO_SUSPEND settings are not > consistent with the new one) is the right way to do that, because it may make > things behave a bit randomly (it will always fail the second guy, but that need > not be the one who's requested IRQF_NO_SUSPEND and it depends on the ordering > between them). Pardon me for sticking my nose into the middle of the conversation, but here's what it looks like to me: The entire no_irq phase of suspend/resume is starting to seem like a mistake. We should never have done it. Alternatively, it might be okay to disable _all_ interrupts during the no_irq phase provided they are then _all_ enabled again before entering the sysdev and platform-specific parts of suspend (or the final freeze). As I understand it, the idea behind the no_irq phase was to make life easier for drivers. They wouldn't have to worry about strange things cropping up while they're in the middle of powering down their devices or afterward. Well, guess what? It turns out that they do have to worry about it after all. Timers can still fire during suspend transitions, and if an IRQ line is shared with a wakeup source then it won't be disabled. The fact is, drivers should not rely on disabled interrupts to prevent untimely interrupt requests or wakeups. They should configure their devices not to generate any interrupt requests at all, apart from wakeup requests. And their interrupt handlers shouldn't mind being invoked for a shared IRQ, even after their devices are turned off. Any driver that leaves its device capable of generating non-wakeup interrupt requests during suspend, and relies on interrupts being globally disabled to avoid problems, is most likely broken. Yes, it may be acceptable in cases where the IRQ line isn't shared, but it's still a bad design. Alan Stern From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alan Stern Subject: Re: [PATCH 1/3] irq / PM: New driver interface for wakeup interrupts Date: Thu, 31 Jul 2014 16:12:55 -0400 (EDT) Message-ID: References: <2140085.bSS0bVjyxJ@vostro.rjw.lan> Mime-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Return-path: Received: from iolanthe.rowland.org ([192.131.102.54]:39410 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750945AbaGaUM5 (ORCPT ); Thu, 31 Jul 2014 16:12:57 -0400 In-Reply-To: <2140085.bSS0bVjyxJ@vostro.rjw.lan> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: "Rafael J. Wysocki" Cc: Thomas Gleixner , Peter Zijlstra , linux-kernel@vger.kernel.org, Linux PM list , Dmitry Torokhov On Thu, 31 Jul 2014, Rafael J. Wysocki wrote: > And before we enter the wakeup handling slippery slope, let me make a note > that this problem is bothering me quite a bit at the moment. In my opinion > we need to address it somehow regardless of the wakeup issues and I'm not sure > if failing __setup_irq() when there's a mismatch (that is, there are existing > actions for the given irq_desc and their IRQF_NO_SUSPEND settings are not > consistent with the new one) is the right way to do that, because it may make > things behave a bit randomly (it will always fail the second guy, but that need > not be the one who's requested IRQF_NO_SUSPEND and it depends on the ordering > between them). Pardon me for sticking my nose into the middle of the conversation, but here's what it looks like to me: The entire no_irq phase of suspend/resume is starting to seem like a mistake. We should never have done it. Alternatively, it might be okay to disable _all_ interrupts during the no_irq phase provided they are then _all_ enabled again before entering the sysdev and platform-specific parts of suspend (or the final freeze). As I understand it, the idea behind the no_irq phase was to make life easier for drivers. They wouldn't have to worry about strange things cropping up while they're in the middle of powering down their devices or afterward. Well, guess what? It turns out that they do have to worry about it after all. Timers can still fire during suspend transitions, and if an IRQ line is shared with a wakeup source then it won't be disabled. The fact is, drivers should not rely on disabled interrupts to prevent untimely interrupt requests or wakeups. They should configure their devices not to generate any interrupt requests at all, apart from wakeup requests. And their interrupt handlers shouldn't mind being invoked for a shared IRQ, even after their devices are turned off. Any driver that leaves its device capable of generating non-wakeup interrupt requests during suspend, and relies on interrupts being globally disabled to avoid problems, is most likely broken. Yes, it may be acceptable in cases where the IRQ line isn't shared, but it's still a bad design. Alan Stern