Re: [PATCH 01/13] Revert "ARM: OMAP: Mask interrupts when disabling interrupts, v2"

[Kevin Hilman <khilman@deeprootsystems.com>]

Kim Kyuwon <chammoru@gmail.com> writes:

> On Fri, May 22, 2009 at 11:54 PM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> Kim Kyuwon <q1.kim@samsung.com> 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.

>> 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);
>
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html