RE: [PATCH] OMAP2/3 Avoid GPIO pending irq status been set after irq_disable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] 
> Sent: 2009年6月6日 5:35
> To: Wang Sawsd-A24013
> Cc: linux-omap@xxxxxxxxxxxxxxx; nm@xxxxxx; Mike Chan
> Subject: Re: [PATCH] OMAP2/3 Avoid GPIO pending irq status 
> been set after irq_disable
> 
> "Wang Sawsd-A24013" <cqwang@xxxxxxxxxxxx> writes:
> 
> >> Could you try this patch with your TS GPIO configured as 
> >> level-triggered?
> >> 
> >
> > I tried the below patch, it can solve the issue also.
> >
> 
> Thanks for testing.
> 
> This whole time, you're probably wondering... "why doesn't Kevin just
> add the disable and enable hooks and be done with it."  I don't think
> I've explained myself there yet so let me try to explain so I don't
> come across as refusing your original idea for no good reason other
> than being stubborn.
> 
> We basically have the option of doing it in my proposed patch, which
> adds additional overhead to the mask/unmask hooks or we can just add
> disable/enable hooks as you did in your original patch.
> 
> The reason I prefer not to add disable/enable hooks is to take
> advantage of the lazy disable feature.  With lazy disable, by waiting
> to actually disable, it allows us to handle potentially lost
> interrupts and this is especially important for wakeup interrupts.  An
> interesting example this is interrupts that are lazy disabled during
> suspend/resume.
> 
> For example, if a wakeup interrupt happens between the drivers
> disable_irq(), what you want is for this interrupt to cancel suspend.
> If you you implement a ->disable hook, and thus mask interrupts in HW
> immediately, you will loose this interrupt.  With lazy disable, the
> drivers ISR will not be called, but genirq will see this interrupt and
> set IRQ_PENDING.  Late in the suspend path, IRQs that are IRQ_WAKEUP
> and IRQ_PENDING will cancel suspend (see
> kernel/irq/pm.c:check_wakeup_irqs())
> 
> Another possible scenario is an interrupt that happens between HW
> resume and the drivers enable_irq() hook.  With lazy disable, this
> interrupt will be IRQ_PENDING when enable_irq() gets called and be
> triggered/handled immedately.
> 
> Hope that helps explain my stubborness, ;)
> 
> Of course this doesn't mean it's an absolute decision.  As always, I'm
> certainly open to being pursuaded that there are other better reasons
> for doing it differently.
> 
> Kevin
> 

Kevin, I am totally ok with your change. :-)

Why I  didn't modify mask/unmask is only because I won't to impact the generic
IRQ handler with overhead, but as you said, lazy irq disable is good
To not lose interrupt before suspend, we should benefit from it as much as possible.

Your change is pretty good, more important is I am glad to see the issue
Is understood by others and we found a solution for it, I care nothing about
Whether it is addressed by using my original patch or not. :-)

Thanks,
Chunqiu
--
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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux