From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCH omap-fixes] OMAP2/3: GPIO: remove recursion in IRQ wakeup path Date: Fri, 20 Feb 2009 13:21:12 -0800 Message-ID: <499F1EC8.1030508@deeprootsystems.com> References: <1233167723-12026-1-git-send-email-khilman@deeprootsystems.com> <200902082039.32157.david-b@pacbell.net> <877i3xaksm.fsf@deeprootsystems.com> <20090220194636.GZ7414@atomide.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mu-out-0910.google.com ([209.85.134.184]:45636 "EHLO mu-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752289AbZBTVVT (ORCPT ); Fri, 20 Feb 2009 16:21:19 -0500 Received: by mu-out-0910.google.com with SMTP id i10so761795mue.1 for ; Fri, 20 Feb 2009 13:21:17 -0800 (PST) In-Reply-To: <20090220194636.GZ7414@atomide.com> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Tony Lindgren Cc: David Brownell , linux-omap@vger.kernel.org Tony Lindgren wrote: > * Kevin Hilman [090210 16:42]: >> David Brownell writes: >> >>> On Wednesday 28 January 2009, Kevin Hilman wrote: >>>> Now that the generic IRQ and GPIO frameworks are used for enabling and >>>> disabling GPIO IRQ wakeup sources, there is no longer a need to call >>>> [enable|disable]_irq_wake() in the low-level code. Doing so results >>>> in recursive calls to [enable|disable]_irq_wake(). >>> Could you clarify what actually made that requirement go away? >>> >>> The recursion was introduced -- using the generic IRQ framework! -- as >>> a simple way to ensure the parent IRQ was properly wake-enabled. Is >>> the issue that something changed, so that something else wake-enables >>> the parent? >>> >> My description was not very descriptive... sorry... >> >> From what I can understand here, I don't see the point in >> wake-enabling the parent IRQ since there is no wakeup glue for the >> bank IRQs, thus these calls are actually failing and causing >> 'unbalanced IRQ disable' noise in the generic IRQ layer. >> >> Here is what is happening. Consider the gpio-keys driver. Upon >> suspend, it enables the IRQ wake for its GPIO. The OMAP GPIO code >> then calls enable_wake_irq() for the parent irq (the GPIO bank IRQ). >> This call is actually failing because the bank IRQ has no 'set_wake' >> method. Because of this failure, the generic IRQ code doesn't >> actually do anything, and sets the 'disable_depth' to zero for the >> bank IRQ. >> >> Then, upon resume, the resume path disables IRQ wakeups for the GPIO. >> The OMAP GPIO code then tries to call disable_irq_wake() for the bank >> IRQ and you get noisy 'unbalanced IRQ disable' warnings from the >> generic IRQ layere because of the attempt to disable the IRQ wake of >> an IRQ that was never enabled. >> >> So the options that I see are: >> >> 1) fix the OMAP GPIO code to check the return code of the parent enable, or >> 2) drop the parent enable/disable >> >> I prefer (1) since those calls will always fail AFAICT. >> >> Does that make any sense? >> >> If you're OK with (1), I will re-issue the patch with a better discription. > > Ignoring this for now, please let me know if you want me to queue this > for omap-upstream with the updates mentioned above. OK. I'll re-send with updated description if above is OK with Dave. Kevin