On Sat, May 23, 2009 at 8:47 AM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: >> >> >>> > What do you do if the interrupt triggers right after your driver has >> >> >>> > returned from its late suspend hook? >> >> >>> >> >> >>> If it's a wakeup IRQ, I assume you want it to prevent suspend. >> >> >>> >> >> >>> But I don't see how that can happen in the current code. IIUC, by the >> >> >>> time your late suspend hook is run, your device IRQ is already >> >> >>> disabled, so it won't trigger an interrupt that will be caught by >> >> >>> check_wakeup_irqs() anyways. >> >> >> >> >> >> My understanding of __disable_irq() was that it didn't actually disable the >> >> >> IRQ at the hardware level, allowing the CPU to actually receive the interrupt >> >> >> and acknowledge it, but preventing the device driver for receiving it. Does >> >> >> it work differently on the affected systems? >> >> > >> >> > Hi, Rafael. >> >> > Sorry for bring the old issue but please let me ask you about >> >> > suspend_device_irqs() function. >> >> > >> >> > __disable_irq() disables the IRQ at the hardware level in the >> >> > following irq_chips >> >> > >> >> > i8259A_chip >> >> > i8259_pic >> >> > i8259A_chip >> >> > bfin_internal_irqchip >> >> > crisv10_irq_type >> >> > crisv32_irq_type >> >> > h8300irq_chip >> >> > m_irq_chip >> >> > mn10300_cpu_pic_level >> >> > xtensa_irq_chip >> >> > iop13xx_msi_chip >> >> > msi_irq >> >> > >> >> > Because these irq_chips mask interrupts in 'disable' hook. >> >> > >> >> > Thus, your suspend_device_irqs() function disables all IRQs at the >> >> > hardware level on all architectures which use irq_chips listed above >> >> > in suspend state. >> >> > Is this really what you wanted? >> >> > >> >> > If interrupt can wake up the system from suspend in some architectures >> >> > and if disable_irq_wake is not supported in these architectures, I >> >> > wonder if suspend_device_irqs() don't allow waking up by interrupt. >> >> > >> >> > Regards, >> >> > Kyuwon >> >> > >> >> >> >> I saw resume_device_irqs() is invoked after arch_suspend_enable_irqs() >> >> in your resume code. >> >> So in this gap between resume_device_irqs() and >> >> arch_suspend_enable_irqs(), a few interrupts would be discarded. >> >> i.e, a few data would be lost. >> >> >> >> If keypad wake up the system, first key pressed information would be lost. >> >> If I2C, USB, SPI, UART wake up the system, first a few data would be lost. >> >> >> >> Did you also consider this issue? >> > >> > I think it would happen anyway with the old code, wouldn't it? >> >> That's not quite right. >> >> For example, let's assume a keypad device is alive in suspend/resume >> state to wake up the system. Before arch_suspend_enable_irqs(), none >> of keypad irqs is dropped. It is just pending. >> >> But in your code, a few irqs are discarded due to your resume_device_irqs(). > > You can't say for sure they are discarded, but there's a small window in which > they can be discarded. The question is whether it does cause problems in > practice. I tested it on my S3C6410(NCP) Board with MAX7359 keypad which is in the Dmitry's Input repo. > Anyway, IMO the device that caused a wake-up event should be deactivated before > arch_suspend_enable_irqs() and remain inactive until its driver is actually > ready to handle interrupts generated by it. Some devices such as Bluetooth chip, keypad(including gpio-key), Modem(Telephony, Wlan) chip and etc, should be activated while the system suspend/resume state. Because these devices may communicate other systems and should receive external wake-up events. Thanks Kyuwon _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
