On Sat, May 23, 2009 at 3:16 AM, Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx> wrote: > Kim Kyuwon <chammoru@xxxxxxxxx> writes: > >> On Fri, May 22, 2009 at 11:54 PM, Kevin Hilman >> <khilman@xxxxxxxxxxxxxxxxxxx> wrote: >>> Kim Kyuwon <q1.kim@xxxxxxxxxxx> writes: >>> >>>> Kevin Hilman wrote: >>>>> This reverts commit 5461af5af5c6a7fee78978aafe720541bf3a2f55. >>>>> >>>>> Adding a disable hook to the irq_chip is not the way to fix the >>>>> problem being addressed by this patch. Instead, we need to fix >>>>> support for [enable|disable]_irq_wake(). >>>> >>>> Agree with you if we can use disable_irq_wake for MPU Interrupt with >>>> not masking the IRQ. Can you explain how we can fix support for >>>> disable_irq_wake() for omap_irq_chip? >>> >>> Yes. The PRCM has a wake-enable per device bit that can be set (see >>> PM_WKEN_<pwrdm>) to control device wakeup enables. >> >> PM_WKEN_<pwrdm> is not entirely matched to each MPU interrupt. > > Correct. This bit disables the module from generating any wakeup > event to the PRCM. > >> If you want to use disable_irq_wake() with enabling PRCM_Interrupt, >> you should disable all PM_WKEN_<pwrdm> bits. >> And how can you make support of disable_irq_wake() for other MPU interrupts? > > To control the ability of a module to wake the MPU directly, we would > need to use the PM_MPUGRPSEL_<pwrdm> regs. PM_MPUGRPSEL_<pwrdm> regs can't disable irq wakeup, it is just related wake-up dependencies. I tested with this code: prm_write_mod_reg(0, WKUP_MOD, OMAP3430_PM_MPUGRPSEL); prm_write_mod_reg(0, MPU_MOD, OMAP3430_PM_MPUGRPSEL); prm_write_mod_reg(0, CORE_MOD, OMAP3430_PM_MPUGRPSEL); prm_write_mod_reg(0, OMAP3430_PER_MOD, OMAP3430_PM_MPUGRPSEL); But USB interrupt have continuously woken up the system. I checked with my omap wakeup source driver and wake-up source was shown below # cat /sys/power/omap_resume_irq 92 >>> But the implemenation of that should not hold up this revert because >>> this patch breaks *all* wakeups since the PRCM interrupt itself is >>> disabled in the suspend path. >> >> Yes, I saw the same problem. This is caused by resume_device_irqs() >> recently added by Rafael not by my patch. > > The point is, with your patch applied, *all* OMAP wakeups are broken > upstream. > > You're right, your patch didn't cause the broken wakeup problem by > itself, but your patch on top of Rafael's in combination with the new > lazy-disable support which are both already in mainline breaks *all* > OMAP wakeup capabilities. Therefore it should be reverted and the > OMAP specific IRQ wake support fixed. > > I am working on fixing the OMAP IRQ wake support, but I do not want > that to hold up this series getting upstream. > >> I'm asking him that he made a right decision. > > Yes, we had a long discussion on that on linux-pm and I came to the > conclusion that while his change was probably premature, it's the > right decision and platforms with wakeup capabilities should > use/fix their set_irq_wakeup() functionality. > >>> As a workaround for your USB problem that this patch was initially >>> intended to fix you could manually disable USB OTG wakeups like this: >>> >>> wken = prm_read_mod_reg(CORE_MOD, PM_WKEN); >>> wken &= ~OMAP3430_EN_HSOTGUSB; >>> prm_write_mod_reg(wken, CORE_MOD, PM_WKEN); I also confirmed this code can't help disabling interrupt wakeup. >> Did you checked this masking prevent waking up from the interrupt of USB HOST? > > No I did not test, nor was I able to reproduce your original problem > since the description wasn't that clear to me. > > This will disable the USB OTG controller module from generating > wakeups for any reason. If disabling the device wakeup in PM_WKEN is > still resutling in interrupts, then the powerdomain with that device > is most likely not in retention/off. > > I do know that disabling PM_WKEN for UARTs in the same powerdomain as > USB OTG (CORE) will stop the UART from waking, and thus from > generating interrupts, as long as the powerdomain has actually > transitioned to retention/off. > > Re: USB HOST. The problem you reported in your original patch was > that you were waking from IRQ 92, which is the USB OTG interrupt. If > your problem is with USBHOST, that is in a different powerdomain. > > Kevin > > Kevin, I'm terribly sorry to hold up your work. I will not disturb you anymore in relation to this revert patch. However, please keep in mind that adding a disable hook to the irq_chip was the best way to fix the problem addressed by my original patch. Regards, Kyuwon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
